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

Add server interceptor to end in javaagent because they are run in re… #1383

Merged

Conversation

anuraaga
Copy link
Contributor

…verse order.

/cc @asarkar This fixes the javaagent which also made the wrong assumption that server interceptors are run in order. But you may be able to get some inspiration from the unit test changes.

.intercept(new ServerInterceptor() {
@Override
<ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(ServerCall<ReqT, RespT> call, Metadata headers, ServerCallHandler<ReqT, RespT> next) {
def ctx = Context.current().withValue(key, "meow")
def oldCtx = ctx.attach()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was wrong in my previous change, naturally just like TracingServerInterceptor, any interceptor that modifies context must use Contexts.interceptCall to make sure it's propagated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Incidentally this is much easier when context is mutable :)

@trask trask merged commit 30e75c6 into open-telemetry:master Oct 14, 2020
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

4 participants