Skip to content

Commit bc5871d

Browse files
committed
fix(gax): manage OpenTelemetry scope in OpenTelemetryTracingTracer
Manage the lifecycle of the OpenTelemetry Scope in OpenTelemetryTracingTracer so that log records emitted during attempt lifecycle can access the active span context. Also order ApiTracerFactory before LoggingTracerFactory in CompositeTracerFactory to ensure span scope remains active during error logging. Fixes: b/499388983
1 parent cc4b980 commit bc5871d

3 files changed

Lines changed: 53 additions & 1 deletion

File tree

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ static ApiTracerFactory getApiTracerFactory(
300300
if (LoggingUtils.isLoggingEnabled()) {
301301
apiTracerFactory =
302302
new CompositeTracerFactory(
303-
ImmutableList.of(new LoggingTracerFactory(), apiTracerFactory));
303+
ImmutableList.of(apiTracerFactory, new LoggingTracerFactory()));
304304
}
305305

306306
if (apiTracerFactory.needsContext()) {

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracingTracer.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class OpenTelemetryTracingTracer implements ApiTracer {
5353
private final String attemptSpanName;
5454
private final ApiTracerContext apiTracerContext;
5555
private @Nullable Span attemptSpan;
56+
private io.opentelemetry.context.@Nullable Scope scope;
5657

5758
@Override
5859
public void injectTraceContext(java.util.Map<String, String> carrier) {
@@ -146,6 +147,7 @@ public void attemptStarted(Object request, int attemptNumber) {
146147
spanBuilder.setAllAttributes(ObservabilityUtils.toOtelAttributes(currentAttemptAttributes));
147148

148149
this.attemptSpan = spanBuilder.startSpan();
150+
this.scope = attemptSpan.makeCurrent();
149151
}
150152

151153
@Override
@@ -234,6 +236,10 @@ private void recordErrorAndEndAttempt(@Nullable Throwable error) {
234236
}
235237

236238
private void endAttempt() {
239+
if (scope != null) {
240+
scope.close();
241+
scope = null;
242+
}
237243
if (attemptSpan == null) {
238244
return;
239245
}

sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTracingTracerTest.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import static org.mockito.ArgumentMatchers.any;
3434
import static org.mockito.ArgumentMatchers.anyString;
3535
import static org.mockito.ArgumentMatchers.eq;
36+
import static org.mockito.Mockito.lenient;
3637
import static org.mockito.Mockito.never;
3738
import static org.mockito.Mockito.verify;
3839
import static org.mockito.Mockito.when;
@@ -49,6 +50,7 @@
4950
import io.opentelemetry.api.trace.SpanBuilder;
5051
import io.opentelemetry.api.trace.SpanKind;
5152
import io.opentelemetry.api.trace.Tracer;
53+
import io.opentelemetry.context.Scope;
5254
import java.net.ConnectException;
5355
import java.net.SocketTimeoutException;
5456
import java.util.Map;
@@ -64,6 +66,7 @@ class OpenTelemetryTracingTracerTest {
6466
@Mock private Tracer tracer;
6567
@Mock private SpanBuilder spanBuilder;
6668
@Mock private Span span;
69+
@Mock private Scope scope;
6770
private OpenTelemetryTracingTracer openTelemetryTracingTracer;
6871
private static final String ATTEMPT_SPAN_NAME = "Service/Method/attempt";
6972

@@ -73,6 +76,7 @@ void setUp() {
7376
when(spanBuilder.setSpanKind(any(SpanKind.class))).thenReturn(spanBuilder);
7477
when(spanBuilder.setAllAttributes(any(Attributes.class))).thenReturn(spanBuilder);
7578
when(spanBuilder.startSpan()).thenReturn(span);
79+
lenient().when(span.makeCurrent()).thenReturn(scope);
7680
openTelemetryTracingTracer =
7781
new OpenTelemetryTracingTracer(tracer, ApiTracerContext.empty(), ATTEMPT_SPAN_NAME);
7882
}
@@ -680,4 +684,46 @@ void testInjectTraceContext_addsHeaders() {
680684
assertThat(carrier.get("traceparent")).contains("00000000000000000000000000000001");
681685
assertThat(carrier.get("traceparent")).contains("0000000000000002");
682686
}
687+
688+
@Test
689+
void testAttemptStarted_makesSpanCurrent() {
690+
openTelemetryTracingTracer.attemptStarted(new Object(), 1);
691+
verify(span).makeCurrent();
692+
}
693+
694+
@Test
695+
void testAttemptEnded_closesScope_succeeded() {
696+
openTelemetryTracingTracer.attemptStarted(new Object(), 1);
697+
openTelemetryTracingTracer.attemptSucceeded();
698+
verify(scope).close();
699+
}
700+
701+
@Test
702+
void testAttemptEnded_closesScope_failedRetriesExhausted() {
703+
openTelemetryTracingTracer.attemptStarted(new Object(), 1);
704+
openTelemetryTracingTracer.attemptFailedRetriesExhausted(new RuntimeException());
705+
verify(scope).close();
706+
}
707+
708+
@Test
709+
void testAttemptEnded_closesScope_failedDuration() {
710+
openTelemetryTracingTracer.attemptStarted(new Object(), 1);
711+
openTelemetryTracingTracer.attemptFailedDuration(
712+
new RuntimeException(), java.time.Duration.ofMillis(100));
713+
verify(scope).close();
714+
}
715+
716+
@Test
717+
void testAttemptEnded_closesScope_permanentFailure() {
718+
openTelemetryTracingTracer.attemptStarted(new Object(), 1);
719+
openTelemetryTracingTracer.attemptPermanentFailure(new RuntimeException());
720+
verify(scope).close();
721+
}
722+
723+
@Test
724+
void testAttemptEnded_closesScope_cancelled() {
725+
openTelemetryTracingTracer.attemptStarted(new Object(), 1);
726+
openTelemetryTracingTracer.attemptCancelled();
727+
verify(scope).close();
728+
}
683729
}

0 commit comments

Comments
 (0)