diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java index 79841eb718d6..2bbd51145886 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java @@ -268,6 +268,18 @@ Map getMetricsAttributes() { return attributes; } + /** + * @return a map of attributes to be included in operation-level spans + */ + Map getOperationAttributes() { + Map attributes = getMetricsAttributes(); + if (!Strings.isNullOrEmpty(destinationResourceId())) { + attributes.put( + ObservabilityAttributes.DESTINATION_RESOURCE_ID_ATTRIBUTE, destinationResourceId()); + } + return attributes; + } + /** * Merges this context with another context. The values in the other context take precedence. * diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracingTracer.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracingTracer.java index cb41da9ccbeb..dbdd517cfb92 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracingTracer.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracingTracer.java @@ -35,7 +35,10 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; +import io.opentelemetry.context.Context; import java.util.HashMap; import java.util.Map; import java.util.concurrent.CancellationException; @@ -51,15 +54,24 @@ class OpenTelemetryTracingTracer implements ApiTracer { private final Tracer tracer; private final Map attemptAttributes; private final String attemptSpanName; + private final String operationSpanName; private final ApiTracerContext apiTracerContext; + // Captures the active trace context from the calling thread at RPC initiation. + // This allows the operation span and attempt spans to link back to the caller's trace. + private final Context parentContext; + // Trace context containing the operationSpan, serving as the parent for attempt spans. + private final Context operationContext; + private @Nullable Span operationSpan; private @Nullable Span attemptSpan; @Override - public void injectTraceContext(java.util.Map carrier) { - if (attemptSpan != null) { - io.opentelemetry.context.Context context = - io.opentelemetry.context.Context.current().with(attemptSpan); - io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator.getInstance() + public void injectTraceContext(Map carrier) { + // Prefer the active attempt span so outgoing RPC wire context reflects the specific attempt; + // fall back to the overall operation span if no attempt is currently in-flight. + Span currentSpan = attemptSpan != null ? attemptSpan : operationSpan; + if (currentSpan != null) { + Context context = Context.current().with(currentSpan); + W3CTraceContextPropagator.getInstance() .inject( context, carrier, @@ -78,11 +90,7 @@ public void injectTraceContext(java.util.Map carrier) { * @param apiTracerContext the {@link ApiTracerContext} to use for recording spans */ OpenTelemetryTracingTracer(Tracer tracer, ApiTracerContext apiTracerContext) { - this.tracer = tracer; - this.apiTracerContext = apiTracerContext; - this.attemptSpanName = resolveAttemptSpanName(apiTracerContext); - this.attemptAttributes = new HashMap<>(); - buildAttributes(); + this(tracer, apiTracerContext, resolveAttemptSpanName(apiTracerContext)); } /** @@ -97,12 +105,53 @@ public void injectTraceContext(java.util.Map carrier) { OpenTelemetryTracingTracer( Tracer tracer, ApiTracerContext apiTracerContext, String attemptSpanName) { this.tracer = tracer; - this.attemptSpanName = attemptSpanName; this.apiTracerContext = apiTracerContext; + this.attemptSpanName = attemptSpanName; + this.operationSpanName = resolveOperationSpanName(attemptSpanName); this.attemptAttributes = new HashMap<>(); + this.parentContext = Context.current(); buildAttributes(); + this.operationSpan = startOperationSpan(); + this.operationContext = parentContext.with(this.operationSpan); + } + + /** + * Starts and initializes the operation-level client request span. + * + * @return the newly started {@link Span} for the overall operation + */ + private Span startOperationSpan() { + SpanBuilder operationSpanBuilder = tracer.spanBuilder(operationSpanName); + operationSpanBuilder.setSpanKind(SpanKind.INTERNAL); + operationSpanBuilder.setParent(parentContext); + operationSpanBuilder.setAllAttributes( + ObservabilityUtils.toOtelAttributes(this.apiTracerContext.getOperationAttributes())); + return operationSpanBuilder.startSpan(); + } + + /** + * Derives the operation-level span name from the attempt span name. + * + * @param attemptSpanName the attempt span name + * @return the operation span name + */ + private static String resolveOperationSpanName(String attemptSpanName) { + if (!Strings.isNullOrEmpty(attemptSpanName)) { + if (attemptSpanName.endsWith("/attempt")) { + String name = attemptSpanName.substring(0, attemptSpanName.length() - "/attempt".length()); + return name.isEmpty() ? "operation" : name; + } + return "attempt".equals(attemptSpanName) ? "operation" : attemptSpanName; + } + return "operation"; } + /** + * Resolves the canonical attempt-level span name based on transport and context. + * + * @param apiTracerContext the tracer context containing transport and method metadata + * @return the attempt span name + */ private static String resolveAttemptSpanName(ApiTracerContext apiTracerContext) { if (apiTracerContext.transport() == ApiTracerContext.Transport.GRPC) { // gRPC Uses the full method name as span name. @@ -118,6 +167,7 @@ private static String resolveAttemptSpanName(ApiTracerContext apiTracerContext) } } + /** Copies attempt-level attributes from the tracer context into the local attribute cache. */ private void buildAttributes() { this.attemptAttributes.putAll(this.apiTracerContext.getAttemptAttributes()); } @@ -142,19 +192,72 @@ public void attemptStarted(Object request, int attemptNumber) { // Attempt spans are of the CLIENT kind spanBuilder.setSpanKind(SpanKind.CLIENT); + // Link attempt span to operation context (parent operation span) + spanBuilder.setParent(operationContext); + // Pass the combined attributes to the new SpanBuilder method spanBuilder.setAllAttributes(ObservabilityUtils.toOtelAttributes(currentAttemptAttributes)); this.attemptSpan = spanBuilder.startSpan(); } + /** + * Signals that the overall logical operation succeeded. + * + *

Closes any remaining in-flight attempt span and ends the operation span. + */ + @Override + public void operationSucceeded() { + recordErrorAndEndOperation(null); + } + + /** + * Signals that the overall logical operation was cancelled. + * + *

Closes any remaining in-flight attempt span with a {@link CancellationException} and ends + * the operation span with an ERROR status. + */ + @Override + public void operationCancelled() { + recordErrorAndEndOperation(new CancellationException()); + } + + /** + * Signals that the overall logical operation failed permanently. + * + *

Closes any remaining in-flight attempt span with the provided error details and ends the + * operation span with an ERROR status. + * + * @param error the cause of the operation failure + */ + @Override + public void operationFailed(Throwable error) { + recordErrorAndEndOperation(error); + } + + /** + * Records error details and ends both the active attempt span and the operation span. + * + * @param error the exception associated with the operation failure, or {@code null} if successful + */ + private void recordErrorAndEndOperation(@Nullable Throwable error) { + if (attemptSpan != null) { + endSpan(attemptSpan, error); + attemptSpan = null; + } + if (operationSpan != null) { + endSpan(operationSpan, error); + operationSpan = null; + } + } + @Override public void attemptSucceeded() { recordErrorAndEndAttempt(null); } @Override - public void responseHeadersReceived(java.util.Map headers) { + public void responseHeadersReceived(Map headers) { if (attemptSpan == null) { return; } @@ -174,7 +277,7 @@ public void responseHeadersReceived(java.util.Map headers) { * @param headers the map of response headers. * @return the content length in bytes, or -1 if the header is missing or malformed. */ - private long extractContentLength(java.util.Map headers) { + private long extractContentLength(Map headers) { try { if (headers == null || headers.isEmpty()) return -1; // google-http-client HttpHeaders uses a case-insensitive map but we copy it for safety @@ -215,31 +318,40 @@ public void attemptPermanentFailure(Throwable error) { recordErrorAndEndAttempt(error); } + /** + * Records error details and ends the current attempt span. + * + * @param error the exception associated with the attempt failure, or {@code null} if successful + */ private void recordErrorAndEndAttempt(@Nullable Throwable error) { if (attemptSpan == null) { return; } + endSpan(attemptSpan, error); + attemptSpan = null; + } + + /** + * Attaches response status attributes and error messages to the span and ends it. + * + * @param span the span to finish + * @param error the exception that caused the span to end, or {@code null} if successful + */ + private void endSpan(Span span, @Nullable Throwable error) { Map responseAttributes = ObservabilityUtils.getResponseAttributes(error, this.apiTracerContext.transport()); if (!responseAttributes.isEmpty()) { - attemptSpan.setAllAttributes(ObservabilityUtils.toOtelAttributes(responseAttributes)); + span.setAllAttributes(ObservabilityUtils.toOtelAttributes(responseAttributes)); } - if (error != null && !Strings.isNullOrEmpty(error.getMessage())) { - attemptSpan.setAttribute( - ObservabilityAttributes.STATUS_MESSAGE_ATTRIBUTE, error.getMessage()); - } - - endAttempt(); - } - - private void endAttempt() { - if (attemptSpan == null) { - return; + if (error != null) { + span.setStatus(StatusCode.ERROR); + if (!Strings.isNullOrEmpty(error.getMessage())) { + span.setAttribute(ObservabilityAttributes.STATUS_MESSAGE_ATTRIBUTE, error.getMessage()); + } } - attemptSpan.end(); - attemptSpan = null; + span.end(); } @Override diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java index 9c0c01e54408..b9639a2e1293 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java @@ -529,4 +529,43 @@ void testDestinationResourceId_evaluatedEveryTime() { assertThat(result.destinationResourceId()).isEqualTo("extracted-id-2"); assertThat(counter[0]).isEqualTo(2); } + + @Test + void testGetOperationAttributes() { + ApiTracerContext context = + ApiTracerContext.newBuilder() + .setLibraryMetadata(LibraryMetadata.newBuilder().setRepository("test-repo").build()) + .setServerAddress("test-address") + .setServerPort(443) + .setServiceName("test-service") + .setTransport(ApiTracerContext.Transport.HTTP) + .setHttpMethod("POST") + .setHttpPathTemplate("/v1/{name}:access") + .setFullMethodName("google.test.v1.TestService/Access") + .setUrlDomain("test.googleapis.com") + .setDestinationResourceIdSupplier(() -> "projects/p/secrets/s") + .build(); + + Map attributes = context.getOperationAttributes(); + + assertThat(attributes) + .containsEntry(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE, "test-address"); + assertThat(attributes).containsEntry(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE, 443); + assertThat(attributes).containsEntry(ObservabilityAttributes.REPO_ATTRIBUTE, "test-repo"); + assertThat(attributes) + .containsEntry(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE, "test-service"); + assertThat(attributes).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "http"); + assertThat(attributes) + .containsEntry( + ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, "google.test.v1.TestService/Access"); + assertThat(attributes) + .containsEntry(ObservabilityAttributes.URL_DOMAIN_ATTRIBUTE, "test.googleapis.com"); + assertThat(attributes) + .containsEntry(ObservabilityAttributes.URL_TEMPLATE_ATTRIBUTE, "/v1/{name}:access"); + assertThat(attributes) + .containsEntry( + ObservabilityAttributes.DESTINATION_RESOURCE_ID_ATTRIBUTE, + "//test.googleapis.com/projects/p/secrets/s"); + assertThat(attributes).doesNotContainKey(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE); + } } diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTracingTracerFactoryTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTracingTracerFactoryTest.java index 3c78cef6dbd7..4a58a3b07bfd 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTracingTracerFactoryTest.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTracingTracerFactoryTest.java @@ -75,6 +75,7 @@ void setUp() { when(openTelemetry.getTracer(anyString())).thenReturn(tracer); when(tracer.spanBuilder(anyString())).thenReturn(spanBuilder); when(spanBuilder.setSpanKind(any())).thenReturn(spanBuilder); + when(spanBuilder.setParent(any())).thenReturn(spanBuilder); when(spanBuilder.setAllAttributes(any(Attributes.class))).thenReturn(spanBuilder); when(spanBuilder.startSpan()).thenReturn(span); @@ -228,7 +229,7 @@ void testNewTracer_withContext_grpc_usesFullMethodName() { tracerInstance.attemptStarted(null, 1); - verify(tracer).spanBuilder("google.cloud.v1.Service/Method"); + verify(tracer, atLeastOnce()).spanBuilder("google.cloud.v1.Service/Method"); } @ParameterizedTest @@ -255,7 +256,7 @@ void testNewTracer_withContext_http_usesHttpMethodAndPathTemplate( tracerInstance.attemptStarted(null, 1); - verify(tracer).spanBuilder(expectedSpanName); + verify(tracer, atLeastOnce()).spanBuilder(expectedSpanName); } @Test @@ -273,7 +274,7 @@ void testNewTracer_withContext_http_noHttpMethodOrPathTemplate_usesFullMethodNam tracerInstance.attemptStarted(null, 1); - verify(tracer).spanBuilder("google.cloud.v1.Service.Method"); + verify(tracer, atLeastOnce()).spanBuilder("google.cloud.v1.Service.Method"); } @Test @@ -309,7 +310,7 @@ void testNewTracer_mergesFactoryContext() { tracerInstance.attemptStarted(null, 1); ArgumentCaptor attributesCaptor = ArgumentCaptor.forClass(Attributes.class); - verify(spanBuilder).setAllAttributes(attributesCaptor.capture()); + verify(spanBuilder, atLeastOnce()).setAllAttributes(attributesCaptor.capture()); Attributes attributes = attributesCaptor.getValue(); assertThat(attributes.asMap()) diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTracingTracerTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTracingTracerTest.java index 33fa2efcc0da..34bf84da0221 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTracingTracerTest.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTracingTracerTest.java @@ -30,9 +30,11 @@ package com.google.api.gax.tracing; import static com.google.common.truth.Truth.assertThat; +import static io.opentelemetry.api.trace.StatusCode.ERROR; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -51,6 +53,7 @@ import io.opentelemetry.api.trace.Tracer; import java.net.ConnectException; import java.net.SocketTimeoutException; +import java.util.HashMap; import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -64,15 +67,32 @@ class OpenTelemetryTracingTracerTest { @Mock private Tracer tracer; @Mock private SpanBuilder spanBuilder; @Mock private Span span; + @Mock private SpanBuilder operationSpanBuilder; + @Mock private Span operationSpan; private OpenTelemetryTracingTracer openTelemetryTracingTracer; private static final String ATTEMPT_SPAN_NAME = "Service/Method/attempt"; @BeforeEach void setUp() { - when(tracer.spanBuilder(anyString())).thenReturn(spanBuilder); - when(spanBuilder.setSpanKind(any(SpanKind.class))).thenReturn(spanBuilder); - when(spanBuilder.setAllAttributes(any(Attributes.class))).thenReturn(spanBuilder); - when(spanBuilder.startSpan()).thenReturn(span); + lenient().when(tracer.spanBuilder(anyString())).thenReturn(spanBuilder); + lenient().when(spanBuilder.setSpanKind(any(SpanKind.class))).thenReturn(spanBuilder); + lenient().when(spanBuilder.setParent(any())).thenReturn(spanBuilder); + lenient().when(spanBuilder.setAllAttributes(any(Attributes.class))).thenReturn(spanBuilder); + lenient().when(spanBuilder.startSpan()).thenReturn(span); + + lenient() + .when(operationSpanBuilder.setSpanKind(any(SpanKind.class))) + .thenReturn(operationSpanBuilder); + lenient().when(operationSpanBuilder.setParent(any())).thenReturn(operationSpanBuilder); + lenient() + .when(operationSpanBuilder.setAllAttributes(any(Attributes.class))) + .thenReturn(operationSpanBuilder); + lenient().when(operationSpanBuilder.startSpan()).thenReturn(operationSpan); + lenient() + .when(operationSpan.storeInContext(any(io.opentelemetry.context.Context.class))) + .thenAnswer(invocation -> invocation.getArgument(0)); + lenient().when(tracer.spanBuilder("Service/Method")).thenReturn(operationSpanBuilder); + openTelemetryTracingTracer = new OpenTelemetryTracingTracer(tracer, ApiTracerContext.empty(), ATTEMPT_SPAN_NAME); } @@ -680,4 +700,71 @@ void testInjectTraceContext_addsHeaders() { assertThat(carrier.get("traceparent")).contains("00000000000000000000000000000001"); assertThat(carrier.get("traceparent")).contains("0000000000000002"); } + + @Test + void testAttemptStarted_setsParentToParentContext() { + openTelemetryTracingTracer.attemptStarted(new Object(), 1); + verify(spanBuilder).setParent(any(io.opentelemetry.context.Context.class)); + } + + @Test + void testOperationSucceeded_endsActiveAttemptSpan() { + openTelemetryTracingTracer.attemptStarted(new Object(), 1); + openTelemetryTracingTracer.operationSucceeded(); + + verify(span).end(); + verify(operationSpan).end(); + } + + @Test + void testOperationFailed_endsActiveAttemptSpanWithErrorAttributes() { + openTelemetryTracingTracer.attemptStarted(new Object(), 1); + openTelemetryTracingTracer.operationFailed(new RuntimeException("operation failed")); + + verify(span).setAttribute(ObservabilityAttributes.STATUS_MESSAGE_ATTRIBUTE, "operation failed"); + verify(span).end(); + verify(operationSpan).setStatus(ERROR); + verify(operationSpan).end(); + } + + @Test + void testOperationCancelled_endsActiveAttemptSpanWithCancellation() { + openTelemetryTracingTracer.attemptStarted(new Object(), 1); + openTelemetryTracingTracer.operationCancelled(); + + ArgumentCaptor attrsCaptor = ArgumentCaptor.forClass(Attributes.class); + verify(span).setAllAttributes(attrsCaptor.capture()); + verify(span).end(); + verify(operationSpan).setStatus(ERROR); + verify(operationSpan).end(); + + assertThat(attrsCaptor.getValue().asMap()) + .containsEntry( + AttributeKey.stringKey(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE), + "CANCELLED"); + } + + @Test + void testInjectTraceContext_withOperationSpanFallback() { + // Verifies that injectTraceContext() injects the operation span context into the carrier + // when between attempts so that context propagation doesn't drop trace state. + io.opentelemetry.api.trace.SpanContext mockSpanContext = + io.opentelemetry.api.trace.SpanContext.create( + "00000000000000000000000000000003", + "0000000000000004", + io.opentelemetry.api.trace.TraceFlags.getSampled(), + io.opentelemetry.api.trace.TraceState.getDefault()); + Span realSpan = Span.wrap(mockSpanContext); + when(operationSpanBuilder.startSpan()).thenReturn(realSpan); + + openTelemetryTracingTracer = + new OpenTelemetryTracingTracer(tracer, ApiTracerContext.empty(), ATTEMPT_SPAN_NAME); + + Map carrier = new HashMap<>(); + openTelemetryTracingTracer.injectTraceContext(carrier); + + assertThat(carrier).containsKey("traceparent"); + assertThat(carrier.get("traceparent")).contains("00000000000000000000000000000003"); + assertThat(carrier.get("traceparent")).contains("0000000000000004"); + } }