feat(gax): Implement trace context extraction and injection interceptors#12310
feat(gax): Implement trace context extraction and injection interceptors#12310westarle wants to merge 1 commit intogoogleapis:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces OpenTelemetry trace propagation for both gRPC and HTTP/JSON callables. It adds new interceptors, GrpcTracePropagationInterceptor and HttpJsonTracePropagationInterceptor, which inject W3C trace context into request headers when OpenTelemetry is detected on the classpath. The PR also updates various callable implementations to wrap call executions within an ApiTracer.Scope and provides a concrete implementation of the inScope method in SpanTracer. Review feedback suggests adding debug-level logging when OpenTelemetry classes or methods are unavailable to improve observability during troubleshooting.
| // OpenTelemetry API is not available | ||
| isOpentelemetryAvailable = false; | ||
| } |
There was a problem hiding this comment.
Catching ClassNotFoundException and silently ignoring it can make debugging difficult if OpenTelemetry is expected to be available but isn't. Consider logging this exception at a debug level to provide more insight into why OpenTelemetry might not be found.
...gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcTracePropagationInterceptor.java
Outdated
Show resolved
Hide resolved
...-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonTracePropagationInterceptor.java
Outdated
Show resolved
Hide resolved
...-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonTracePropagationInterceptor.java
Outdated
Show resolved
Hide resolved
westarle
force-pushed
the
trace-propagation-interceptors
branch
from
7dfaa9f to
65a795f
Compare
| @Override | ||
| public void start(ClientCall.Listener<RespT> responseListener, Metadata headers) { | ||
| try { | ||
| TextMapPropagator propagator = W3CTraceContextPropagator.getInstance(); |
There was a problem hiding this comment.
I think we can extract this logic to a common place such as SpanTracer.injectTracerId(). Call it in GrpcClientCalls and HttpJsonClientCallImpl where we have access to both ApiTracer and request headers.
This avoids creating interceptors for both transports and reuse similar logics.
There was a problem hiding this comment.
It looks a lot better now!
westarle
force-pushed
the
trace-propagation-interceptors
branch
3 times, most recently
from
9eafa4f to
5017179
Compare
|
|
||
| @Override | ||
| public void injectTraceContext(java.util.Map<String, String> carrier) { | ||
| if (!isOpenTelemetryAvailable()) { |
There was a problem hiding this comment.
We can assume OpenTelemetry is available here now. There would be compilation errors if SpanTracerFactory is passed in but OpenTelemetry is not available.
| clientCall.start( | ||
| new ResponseObserverAdapter(), | ||
| HttpJsonMetadata.newBuilder().build().withHeaders(context.getExtraHeaders())); | ||
| HttpJsonClientCalls.getMetadataWithTraceContext(httpJsonContext)); |
There was a problem hiding this comment.
It looks good but we haven't fully implemented traces for Streaming calls yet. It may not be easy to test it.
| </parent> | ||
|
|
||
| <dependencies> | ||
| <dependency> |
There was a problem hiding this comment.
I don't think opentelemetry dependency is needed now since the logic is wrapped in SpanTracer. Same comment for the pom in httpjson.
westarle
force-pushed
the
trace-propagation-interceptors
branch
from
5017179 to
b020910
Compare
| @@ -196,6 +196,9 @@ default void requestSent() {} | |||
| default void batchRequestSent(long elementCount, long requestSize) {} | |||
| ; | |||
|
|
|||
| /** Extract the trace context from the tracer and add it to the given headers map. */ | |||
| default void injectTraceContext(java.util.Map<String, String> carrier) {} | |||
There was a problem hiding this comment.
We should implement this in CompositeTracer now that #12321 is merged.
2 participants
See main...westarle:google-cloud-java:trace-propagation-integration-test for the end-to-end integration test.