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

Don't mount context in gRPC instrumentation since gRPC automatically … #1343

Merged
merged 3 commits into from
Oct 9, 2020

Conversation

anuraaga
Copy link
Contributor

@anuraaga anuraaga commented Oct 8, 2020

…does it, better.

The client seems to mount the context automatically with no work, and on server we can use ServerCalls.intercept. This makes sure we don't clobber context modifications that happen downstream from our interceptors.

/cc @asarkar Thanks for bring this to our attention!

@anuraaga
Copy link
Contributor Author

anuraaga commented Oct 8, 2020

There's some workarounds for the bootstrap classpath, but it will become simpler after grpc context is gone.

@anuraaga
Copy link
Contributor Author

anuraaga commented Oct 8, 2020

Strange can't repro the test failures locally which is surprising. YOLO workaround seems to have worked though.

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

Nice 👍

@anuraaga anuraaga merged commit bb26c17 into open-telemetry:master Oct 9, 2020
@trask
Copy link
Member

trask commented Oct 9, 2020

Just a note, since I didn't understand this before chatting with @anuraaga, the goal is that piggybacking on the "real" gRPC Context should continue to work even after we migrate to OTel Context because @anuraaga is planning to add interop between OTel Context / gRPC Context interop into the agent at that time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants