Skip to content

Commit f797537

Browse files
carlosalbertojack-berg
carlosalberto
and
jack-berg
authored
OpenTracing Shim: Update Tracer.close() (#5151)
* Update the Tracer.close() implementation. * Unobfuscate SdkTracerProvider * Use AtomicBoolean instead of volatile --------- Co-authored-by: Jack Berg <[email protected]>
1 parent b5e8bc6 commit f797537

File tree

3 files changed

+107
-20
lines changed

3 files changed

+107
-20
lines changed

‎opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/OpenTracingShim.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,6 @@ public static io.opentracing.Tracer createTracerShim(
4444
TracerProvider provider,
4545
TextMapPropagator textMapPropagator,
4646
TextMapPropagator httpPropagator) {
47-
return new TracerShim(provider.get("opentracing-shim"), textMapPropagator, httpPropagator);
47+
return new TracerShim(provider, textMapPropagator, httpPropagator);
4848
}
4949
}

‎opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java

+40-5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.opentracingshim;
77

8+
import io.opentelemetry.api.trace.TracerProvider;
89
import io.opentelemetry.context.propagation.TextMapPropagator;
910
import io.opentracing.Scope;
1011
import io.opentracing.ScopeManager;
@@ -14,23 +15,29 @@
1415
import io.opentracing.propagation.Format;
1516
import io.opentracing.propagation.TextMapExtract;
1617
import io.opentracing.propagation.TextMapInject;
18+
import java.io.Closeable;
19+
import java.io.IOException;
20+
import java.lang.reflect.Field;
21+
import java.util.concurrent.atomic.AtomicBoolean;
1722
import java.util.logging.Level;
1823
import java.util.logging.Logger;
1924
import javax.annotation.Nullable;
2025

2126
final class TracerShim implements Tracer {
2227
private static final Logger logger = Logger.getLogger(TracerShim.class.getName());
2328

29+
private final io.opentelemetry.api.trace.TracerProvider provider;
2430
private final io.opentelemetry.api.trace.Tracer tracer;
2531
private final ScopeManager scopeManagerShim;
2632
private final Propagation propagation;
27-
private volatile boolean isClosed;
33+
private final AtomicBoolean isShutdown = new AtomicBoolean();
2834

2935
TracerShim(
30-
io.opentelemetry.api.trace.Tracer tracer,
36+
io.opentelemetry.api.trace.TracerProvider provider,
3137
TextMapPropagator textMapPropagator,
3238
TextMapPropagator httpPropagator) {
33-
this.tracer = tracer;
39+
this.provider = provider;
40+
this.tracer = provider.get("opentracing-shim");
3441
this.propagation = new Propagation(textMapPropagator, httpPropagator);
3542
this.scopeManagerShim = new ScopeManagerShim();
3643
}
@@ -62,7 +69,7 @@ public Scope activateSpan(Span span) {
6269

6370
@Override
6471
public SpanBuilder buildSpan(String operationName) {
65-
if (isClosed) {
72+
if (isShutdown.get()) {
6673
return new NoopSpanBuilderShim(operationName);
6774
}
6875

@@ -110,6 +117,34 @@ public <C> SpanContext extract(Format<C> format, C carrier) {
110117

111118
@Override
112119
public void close() {
113-
isClosed = true;
120+
if (!isShutdown.compareAndSet(false, true)) {
121+
return;
122+
}
123+
124+
TracerProvider provider = maybeUnobfuscate(this.provider);
125+
if (provider instanceof Closeable) {
126+
try {
127+
((Closeable) provider).close();
128+
} catch (RuntimeException | IOException e) {
129+
logger.log(Level.INFO, "Exception caught while closing TracerProvider.", e);
130+
}
131+
}
132+
}
133+
134+
private static TracerProvider maybeUnobfuscate(TracerProvider tracerProvider) {
135+
if (!tracerProvider.getClass().getSimpleName().equals("ObfuscatedTracerProvider")) {
136+
return tracerProvider;
137+
}
138+
try {
139+
Field delegateField = tracerProvider.getClass().getDeclaredField("delegate");
140+
delegateField.setAccessible(true);
141+
Object delegate = delegateField.get(tracerProvider);
142+
if (delegate instanceof TracerProvider) {
143+
return (TracerProvider) delegate;
144+
}
145+
} catch (NoSuchFieldException | IllegalAccessException e) {
146+
logger.log(Level.INFO, "Error trying to unobfuscate SdkTracerProvider", e);
147+
}
148+
return tracerProvider;
114149
}
115150
}

‎opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/TracerShimTest.java

+66-14
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,23 @@
66
package io.opentelemetry.opentracingshim;
77

88
import static org.assertj.core.api.Assertions.assertThat;
9+
import static org.mockito.Mockito.doThrow;
910
import static org.mockito.Mockito.mock;
11+
import static org.mockito.Mockito.verify;
1012

13+
import io.opentelemetry.api.GlobalOpenTelemetry;
14+
import io.opentelemetry.api.OpenTelemetry;
1115
import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
12-
import io.opentelemetry.api.trace.Tracer;
16+
import io.opentelemetry.api.trace.TracerProvider;
1317
import io.opentelemetry.context.Context;
1418
import io.opentelemetry.context.propagation.TextMapPropagator;
19+
import io.opentelemetry.sdk.OpenTelemetrySdk;
1520
import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension;
21+
import io.opentelemetry.sdk.trace.SdkTracerProvider;
1622
import io.opentracing.Scope;
1723
import io.opentracing.Span;
1824
import io.opentracing.SpanContext;
25+
import io.opentracing.Tracer;
1926
import io.opentracing.propagation.Format;
2027
import io.opentracing.propagation.TextMapAdapter;
2128
import io.opentracing.tag.BooleanTag;
@@ -26,6 +33,7 @@
2633
import java.util.Collections;
2734
import java.util.HashMap;
2835
import java.util.Map;
36+
import org.junit.jupiter.api.AfterEach;
2937
import org.junit.jupiter.api.BeforeEach;
3038
import org.junit.jupiter.api.Test;
3139
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -40,15 +48,22 @@ class TracerShimTest {
4048
@RegisterExtension static OpenTelemetryExtension otelTesting = OpenTelemetryExtension.create();
4149

4250
TracerShim tracerShim;
43-
Tracer tracer;
51+
TracerProvider provider;
4452

4553
@BeforeEach
4654
void setUp() {
47-
tracer = otelTesting.getOpenTelemetry().getTracer("opentracingshim");
55+
GlobalOpenTelemetry.resetForTest();
56+
GlobalOpenTelemetry.set(otelTesting.getOpenTelemetry());
57+
58+
provider = otelTesting.getOpenTelemetry().getTracerProvider();
4859
TextMapPropagator propagator =
4960
otelTesting.getOpenTelemetry().getPropagators().getTextMapPropagator();
50-
;
51-
tracerShim = new TracerShim(tracer, propagator, propagator);
61+
tracerShim = new TracerShim(provider, propagator, propagator);
62+
}
63+
64+
@AfterEach
65+
void cleanup() {
66+
GlobalOpenTelemetry.resetForTest();
5267
}
5368

5469
@Test
@@ -278,8 +293,8 @@ void inject_textMap() {
278293
Map<String, String> map = new HashMap<>();
279294
CustomTextMapPropagator textMapPropagator = new CustomTextMapPropagator();
280295
CustomTextMapPropagator httpHeadersPropagator = new CustomTextMapPropagator();
281-
tracerShim = new TracerShim(tracer, textMapPropagator, httpHeadersPropagator);
282-
io.opentelemetry.api.trace.Span span = tracer.spanBuilder("span").startSpan();
296+
tracerShim = new TracerShim(provider, textMapPropagator, httpHeadersPropagator);
297+
io.opentelemetry.api.trace.Span span = tracerShim.tracer().spanBuilder("span").startSpan();
283298
SpanContext context = new SpanShim(span).context();
284299

285300
tracerShim.inject(context, Format.Builtin.TEXT_MAP, new TextMapAdapter(map));
@@ -292,8 +307,8 @@ void inject_httpHeaders() {
292307
Map<String, String> map = new HashMap<>();
293308
CustomTextMapPropagator textMapPropagator = new CustomTextMapPropagator();
294309
CustomTextMapPropagator httpHeadersPropagator = new CustomTextMapPropagator();
295-
tracerShim = new TracerShim(tracer, textMapPropagator, httpHeadersPropagator);
296-
io.opentelemetry.api.trace.Span span = tracer.spanBuilder("span").startSpan();
310+
tracerShim = new TracerShim(provider, textMapPropagator, httpHeadersPropagator);
311+
io.opentelemetry.api.trace.Span span = tracerShim.tracer().spanBuilder("span").startSpan();
297312
SpanContext context = new SpanShim(span).context();
298313

299314
tracerShim.inject(context, Format.Builtin.HTTP_HEADERS, new TextMapAdapter(map));
@@ -306,7 +321,7 @@ void extract_textMap() {
306321
Map<String, String> map = new HashMap<>();
307322
CustomTextMapPropagator textMapPropagator = new CustomTextMapPropagator();
308323
CustomTextMapPropagator httpHeadersPropagator = new CustomTextMapPropagator();
309-
tracerShim = new TracerShim(tracer, textMapPropagator, httpHeadersPropagator);
324+
tracerShim = new TracerShim(provider, textMapPropagator, httpHeadersPropagator);
310325

311326
tracerShim.extract(Format.Builtin.TEXT_MAP, new TextMapAdapter(map));
312327
assertThat(textMapPropagator.isExtracted()).isTrue();
@@ -318,7 +333,7 @@ void extract_httpHeaders() {
318333
Map<String, String> map = new HashMap<>();
319334
CustomTextMapPropagator textMapPropagator = new CustomTextMapPropagator();
320335
CustomTextMapPropagator httpHeadersPropagator = new CustomTextMapPropagator();
321-
tracerShim = new TracerShim(tracer, textMapPropagator, httpHeadersPropagator);
336+
tracerShim = new TracerShim(provider, textMapPropagator, httpHeadersPropagator);
322337

323338
tracerShim.extract(Format.Builtin.HTTP_HEADERS, new TextMapAdapter(map));
324339
assertThat(textMapPropagator.isExtracted()).isFalse();
@@ -328,7 +343,7 @@ void extract_httpHeaders() {
328343
@Test
329344
void extract_onlyBaggage() {
330345
W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance();
331-
tracerShim = new TracerShim(tracer, propagator, TextMapPropagator.noop());
346+
tracerShim = new TracerShim(provider, propagator, TextMapPropagator.noop());
332347

333348
io.opentelemetry.api.baggage.Baggage baggage =
334349
io.opentelemetry.api.baggage.Baggage.builder().put("foo", "bar").build();
@@ -342,8 +357,44 @@ void extract_onlyBaggage() {
342357
}
343358

344359
@Test
345-
void close() {
360+
void close_OpenTelemetrySdk() {
361+
SdkTracerProvider sdkProvider = mock(SdkTracerProvider.class);
362+
doThrow(new RuntimeException("testing error")).when(sdkProvider).close();
363+
364+
Tracer tracerShim =
365+
OpenTracingShim.createTracerShim(
366+
OpenTelemetrySdk.builder().setTracerProvider(sdkProvider).build());
367+
tracerShim.close();
368+
369+
verify(sdkProvider).close();
370+
371+
Span otSpan = tracerShim.buildSpan(null).start();
372+
io.opentelemetry.api.trace.Span span = ((SpanShim) otSpan).getSpan();
373+
assertThat(span.getSpanContext().isValid()).isFalse();
374+
}
375+
376+
@Test
377+
void close_GlobalOpenTelemetry() {
378+
GlobalOpenTelemetry.resetForTest();
379+
SdkTracerProvider sdkProvider = mock(SdkTracerProvider.class);
380+
doThrow(new RuntimeException("testing error")).when(sdkProvider).close();
381+
GlobalOpenTelemetry.set(OpenTelemetrySdk.builder().setTracerProvider(sdkProvider).build());
382+
383+
Tracer tracerShim = OpenTracingShim.createTracerShim(GlobalOpenTelemetry.get());
346384
tracerShim.close();
385+
386+
verify(sdkProvider).close();
387+
388+
Span otSpan = tracerShim.buildSpan(null).start();
389+
io.opentelemetry.api.trace.Span span = ((SpanShim) otSpan).getSpan();
390+
assertThat(span.getSpanContext().isValid()).isFalse();
391+
}
392+
393+
@Test
394+
void close_NoopOpenTelemetry() {
395+
Tracer tracerShim = OpenTracingShim.createTracerShim(OpenTelemetry.noop());
396+
tracerShim.close();
397+
347398
Span otSpan = tracerShim.buildSpan(null).start();
348399
io.opentelemetry.api.trace.Span span = ((SpanShim) otSpan).getSpan();
349400
assertThat(span.getSpanContext().isValid()).isFalse();
@@ -390,7 +441,8 @@ void doesNotCrash() {
390441

391442
@Test
392443
void noopDoesNotCrash() {
393-
tracerShim.close();
444+
Tracer tracerShim = OpenTracingShim.createTracerShim(OpenTelemetry.noop());
445+
394446
Span span =
395447
tracerShim
396448
.buildSpan("test")

0 commit comments

Comments
 (0)