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

gRPC AsyncServerInterceptor throws an exception when opentelemetry-javaagent is enabled #5937

Open
kabigon-sung opened this issue Oct 14, 2024 · 3 comments · May be fixed by #5938
Open

gRPC AsyncServerInterceptor throws an exception when opentelemetry-javaagent is enabled #5937

kabigon-sung opened this issue Oct 14, 2024 · 3 comments · May be fixed by #5938
Labels
Milestone

Comments

@kabigon-sung
Copy link

kabigon-sung commented Oct 14, 2024

Hi team,
I found that the AsyncServerInterceptor throws an exception when opentelemetry-javaagent is enabled.
Here is the exception and stacktrace:
截圖 2024-10-14 下午2 44 41

Armeria Version - 1.30.1

Possible root cause:
The default method interceptCall in AsyncServerInterceptor creates an instance of DeferredListener. The constructor of DeferredListener performs checking on the ServerCall<I, O> call parameter. The checking requires that the passed call should be a subclass of com.linecorp.armeria.internal.server.grpc.AbstractServerCall.
When opentelemetry-javagent is enabled, the original com.linecorp.armeria.server.grpc.UnaryServerCall object is replaced by io.opentelemetry.javaagent.shaded.instrumentation.grpc.v1_6.TracingServerInterceptor$TracingServerCall. Since TracingServerCall is not a subclass of AbstractServerCall , the constructor of DeferredListener throws an exception.

How to reproduce the problem

  1. Implement a AsyncServerInterceptor
public static class MyInterceptor implements AsyncServerInterceptor {
        @Override
        public <I, O> CompletableFuture<Listener<I>> asyncInterceptCall(ServerCall<I, O> call,
                                                                        Metadata headers,
                                                                        ServerCallHandler<I, O> next) {
            final Context context = Context.current();
            return CompletableFuture.supplyAsync(() -> {
                try {
                    return context.call(() -> next.startCall(call, headers));
                } catch (Exception e) {
                    throw new RuntimeException(e);
                }
            });
        }
    }
  1. Add the interceptor to the GrpcService
sb.service(GrpcService.builder()
                                        .intercept(new MyInterceptor()) // add the interceptor
                                        .addService(new MyHelloService())
                                        .build());
  1. Download the opentelemetry-javaagent from https://github.com/open-telemetry/opentelemetry-java-instrumentation.
  2. Run the server with opentelemetry-javaagent.
  3. Sending a gRCP request causes the error.
@ikhoon ikhoon added the defect label Oct 14, 2024
@jrhee17
Copy link
Contributor

jrhee17 commented Oct 14, 2024

Did a local check, and it seems like the delegate method cannot be found for some reason:

스크린샷 2024-10-14 오후 6 44 01

@jrhee17
Copy link
Contributor

jrhee17 commented Oct 14, 2024

I realized delegateMH doesn't work as expected 😅 We'll fix this from armeria-side in the next version 🙏

@jrhee17 jrhee17 added this to the 1.31.0 milestone Oct 14, 2024
@kabigon-sung
Copy link
Author

Hi @jrhee17 , I think you are right. Thanks for checking the issues.

ikhoon added a commit to ikhoon/armeria that referenced this issue Oct 14, 2024
Motivation:

There was a bug in `AsyncServerInterceptor` where a `ForwardingServerCall`
was not unwrapped properly via reflection. Because of that,
OpenTelemetry's `TracingServerCall` that wraps the existing
`ServerCall` using `ForwardingServerCall` was rejected by the following
condition.
https://github.com/line/armeria/blob/6dd5cd18dcdc32431316f272128287b2037249d3/grpc/src/main/java/com/linecorp/armeria/server/grpc/DeferredListener.java#L51-L52

Modifications:

- Use the correct reflection API to unsafely access
  `ForwardingServerCall.delegate()`.

Result:

- Fix a bug where `AsyncServerInterceptor` is incompatible with
  the OpenTelemetry gRPC agent.
- Closes line#5937
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants