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

activate grpc.client span in ClientCall.request and ClientCall.halfClose #4640

Merged
merged 1 commit into from
Feb 2, 2023

Conversation

richardstartin
Copy link
Member

What Does This Do

Activates the client span in the request method, which ensures context propagation throughout the call.

Motivation

Better profiler integration for code hotspots.

Additional Notes

@richardstartin richardstartin requested a review from a team as a code owner February 2, 2023 14:35
@richardstartin richardstartin added inst: grpc gRPC instrumentation comp: profiling Profiling labels Feb 2, 2023
Copy link
Contributor

@PerfectSlayer PerfectSlayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@richardstartin richardstartin force-pushed the rgs/grpc-request-context branch from d4da0da to e9962a1 Compare February 2, 2023 14:55
@richardstartin richardstartin changed the title activate grpc.client span in ClientCall.request activate grpc.client span in ClientCall.request and ClientCall.halfClose Feb 2, 2023
named("request")
.and(isMethod())
.and(takesArguments(int.class))
.or(named("halfClose").and(takesArguments(0))),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the 2nd matcher also need isMethod() like the first one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, I don't think it actually matters but I'll add it in for the sake of being thorough

@richardstartin richardstartin force-pushed the rgs/grpc-request-context branch from d76b36a to df8e123 Compare February 2, 2023 17:01
@richardstartin richardstartin merged commit a7b10e2 into master Feb 2, 2023
@richardstartin richardstartin deleted the rgs/grpc-request-context branch February 2, 2023 17:51
@github-actions github-actions bot added this to the 1.7.0 milestone Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: profiling Profiling inst: grpc gRPC instrumentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants