Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

binarylog: generalize binarylog's MethodLogger preparing for new observability features #5244

Merged
merged 1 commit into from Mar 21, 2022

Conversation

lidizheng
Copy link
Contributor

@lidizheng lidizheng commented Mar 15, 2022

As discussed offline, we want to utilize the pre-existing state-storing object MethodLogger for gRPC Observability. This object has the same life span as each RPC, and we can conveniently store extra context information about the RPC in it to generate better logs.

Child PR: #5196

RELEASE NOTES: N/A

@lidizheng
Copy link
Contributor Author

lidizheng commented Mar 16, 2022

@menghanl @dfawley PTAL.

@lidizheng
Copy link
Contributor Author

@menghanl PTAL. I have updated the interface as suggested. Now, the LOC in internal binarylog is reduced to +37-20 (from >100).

@menghanl menghanl added the Type: Internal Cleanup Refactors, etc label Mar 17, 2022
@menghanl menghanl added this to the 1.46 Release milestone Mar 17, 2022
@lidizheng
Copy link
Contributor Author

@dfawley PTAL.

@dfawley dfawley assigned lidizheng and unassigned dfawley Mar 21, 2022
@lidizheng
Copy link
Contributor Author

(Feel free to merge, I don't have write access)

@menghanl menghanl changed the title observability: generalize binarylog's MethodLogger binarylog: generalize binarylog's MethodLogger preparing for new observability features Mar 21, 2022
@menghanl menghanl merged commit 1ffd63d into grpc:master Mar 21, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants