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

Capture HTTP2 Headers (scheme, authority, path, method) gRPC client/server #109

Open
pavolloffay opened this issue Nov 13, 2020 · 2 comments

Comments

@pavolloffay
Copy link
Member

This will have to be done at the grpc-netty level.

If we could do it in such a way that it works with single instrumentation on grpc-okhttp grpc-netty-shaded it would be great.

@pavolloffay
Copy link
Member Author

pavolloffay commented Dec 8, 2020

#158 added support for capturing client and server HTTP2 headers. However the client instrumentation is not bullet proof - the headers are not captured for the first request if DelayedStream is used - maybe they won't be also captured if the channel status switches from READY to other states and back to ready.

/**
 * The current span in Utils.convertClientHeaders is DefaultSpan for the first request. The first
 * request uses io.grpc.internal.DelayedClientTransport and it is called from
 * io.grpc.stub.ClientCalls.blockingUnaryCall. Subsequent requests use
 * io.grpc.stub.ClientCalls.futureUnaryCall - see (1). The DelayedClientTransport calls network in a
 * separate branch after ClientCallImpl or gRPC interceptors. To propagate span to
 * Utils.convertClientHeaders it would have to be started in
 * io.grpc.stub.ClientCalls.blockingUnaryCall.
 *
 * <p>Span is not recording (Default) java.lang.Exception: Stack trace at
 * java.lang.Thread.dumpStack(Thread.java:1336) at
 * io.grpc.netty.Utils.convertClientHeaders(Utils.java:107) at
 * io.grpc.netty.NettyClientStream$Sink.writeHeaders(NettyClientStream.java:124) at
 * io.grpc.internal.AbstractClientStream.start(AbstractClientStream.java:132) at
 * io.grpc.internal.DelayedStream$4.run(DelayedStream.java:197) at
 * io.grpc.internal.DelayedStream.drainPendingCalls(DelayedStream.java:132) at
 * io.grpc.internal.DelayedStream.setStream(DelayedStream.java:101) at
 * io.grpc.internal.DelayedClientTransport$PendingStream.createRealStream(DelayedClientTransport.java:351)
 * at
 * io.grpc.internal.DelayedClientTransport$PendingStream.access$200(DelayedClientTransport.java:334)
 * at io.grpc.internal.DelayedClientTransport$5.run(DelayedClientTransport.java:293) at
 * io.grpc.stub.ClientCalls$ThreadlessExecutor.waitAndDrain(ClientCalls.java:575) (1) at
 *
 * <p>io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:120) at
 * org.hypertrace.example.GreeterGrpc$GreeterBlockingStub.sayHello(GreeterGrpc.java:172) at
 * io.opentelemetry.instrumentation.hypertrace.grpc.v1_5.GrpcInstrumentationTest.serverRequestBlocking(GrpcInstrumentationTest.java:150)
 *
 * <p>Span is recording java.lang.Exception: Stack trace at
 * java.lang.Thread.dumpStack(Thread.java:1336) at
 * io.grpc.netty.Utils.convertClientHeaders(Utils.java:107) at
 * io.grpc.netty.NettyClientStream$Sink.writeHeaders(NettyClientStream.java:124) at
 * io.grpc.internal.AbstractClientStream.start(AbstractClientStream.java:132) at
 * io.grpc.internal.ClientCallImpl.start(ClientCallImpl.java:225) at
 * io.grpc.ForwardingClientCall.start(ForwardingClientCall.java:32) at
 * io.opentelemetry.instrumentation.grpc.v1_5.client.TracingClientInterceptor$TracingClientCall.start(TracingClientInterceptor.java:102)
 * at io.grpc.stub.ClientCalls.startCall(ClientCalls.java:261) at
 * io.grpc.stub.ClientCalls.asyncUnaryRequestCall(ClientCalls.java:237) at
 * io.grpc.stub.ClientCalls.futureUnaryCall(ClientCalls.java:171) at
 *
 * <p>io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:117) at
 * org.hypertrace.example.GreeterGrpc$GreeterBlockingStub.sayHello(GreeterGrpc.java:172) at
 * io.opentelemetry.instrumentation.hypertrace.grpc.v1_5.GrpcInstrumentationTest.disabledInstrumentation_dynamicConfig(GrpcInstrumentationTest.java:182)

Fix could start span in io.grpc.stub.ClientCalls.startCall - however we would have to rewrite the whole gRPC instrumentation and do not reuse OTEL.

@pavolloffay
Copy link
Member Author

Asked in the gRPC repo grpc/grpc-java#7711

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

No branches or pull requests

1 participant