Skip to content

Commit

Permalink
OpenTracing Shim: Update Tracer.close() (#5151)
Browse files Browse the repository at this point in the history 
* Update the Tracer.close() implementation.

* Unobfuscate SdkTracerProvider

* Use AtomicBoolean instead of volatile

---------

Co-authored-by: Jack Berg <[email protected]>
  • Loading branch information
@carlosalberto @jack-berg
carlosalberto and jack-berg authored Mar 7, 2023
1 parent b5e8bc6 commit f797537
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 20 deletions.
2 changes: 1 addition & 1 deletion opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/OpenTracingShim.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,6 @@ public static io.opentracing.Tracer createTracerShim(
TracerProvider provider,
TextMapPropagator textMapPropagator,
TextMapPropagator httpPropagator) {
return new TracerShim(provider.get("opentracing-shim"), textMapPropagator, httpPropagator);
return new TracerShim(provider, textMapPropagator, httpPropagator);
}
}
45 changes: 40 additions & 5 deletions opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.opentracingshim;

import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentracing.Scope;
import io.opentracing.ScopeManager;
Expand All @@ -14,23 +15,29 @@
import io.opentracing.propagation.Format;
import io.opentracing.propagation.TextMapExtract;
import io.opentracing.propagation.TextMapInject;
import java.io.Closeable;
import java.io.IOException;
import java.lang.reflect.Field;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;

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

private final io.opentelemetry.api.trace.TracerProvider provider;
private final io.opentelemetry.api.trace.Tracer tracer;
private final ScopeManager scopeManagerShim;
private final Propagation propagation;
private volatile boolean isClosed;
private final AtomicBoolean isShutdown = new AtomicBoolean();

TracerShim(
io.opentelemetry.api.trace.Tracer tracer,
io.opentelemetry.api.trace.TracerProvider provider,
TextMapPropagator textMapPropagator,
TextMapPropagator httpPropagator) {
this.tracer = tracer;
this.provider = provider;
this.tracer = provider.get("opentracing-shim");
this.propagation = new Propagation(textMapPropagator, httpPropagator);
this.scopeManagerShim = new ScopeManagerShim();
}
Expand Down Expand Up @@ -62,7 +69,7 @@ public Scope activateSpan(Span span) {

@Override
public SpanBuilder buildSpan(String operationName) {
if (isClosed) {
if (isShutdown.get()) {
return new NoopSpanBuilderShim(operationName);
}

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

@Override
public void close() {
isClosed = true;
if (!isShutdown.compareAndSet(false, true)) {
return;
}

TracerProvider provider = maybeUnobfuscate(this.provider);
if (provider instanceof Closeable) {
try {
((Closeable) provider).close();
} catch (RuntimeException | IOException e) {
logger.log(Level.INFO, "Exception caught while closing TracerProvider.", e);
}
}
}

private static TracerProvider maybeUnobfuscate(TracerProvider tracerProvider) {
if (!tracerProvider.getClass().getSimpleName().equals("ObfuscatedTracerProvider")) {
return tracerProvider;
}
try {
Field delegateField = tracerProvider.getClass().getDeclaredField("delegate");
delegateField.setAccessible(true);
Object delegate = delegateField.get(tracerProvider);
if (delegate instanceof TracerProvider) {
return (TracerProvider) delegate;
}
} catch (NoSuchFieldException | IllegalAccessException e) {
logger.log(Level.INFO, "Error trying to unobfuscate SdkTracerProvider", e);
}
return tracerProvider;
}
}
80 changes: 66 additions & 14 deletions opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/TracerShimTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,23 @@
package io.opentelemetry.opentracingshim;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension;
import io.opentelemetry.sdk.trace.SdkTracerProvider;
import io.opentracing.Scope;
import io.opentracing.Span;
import io.opentracing.SpanContext;
import io.opentracing.Tracer;
import io.opentracing.propagation.Format;
import io.opentracing.propagation.TextMapAdapter;
import io.opentracing.tag.BooleanTag;
Expand All @@ -26,6 +33,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -40,15 +48,22 @@ class TracerShimTest {
@RegisterExtension static OpenTelemetryExtension otelTesting = OpenTelemetryExtension.create();

TracerShim tracerShim;
Tracer tracer;
TracerProvider provider;

@BeforeEach
void setUp() {
tracer = otelTesting.getOpenTelemetry().getTracer("opentracingshim");
GlobalOpenTelemetry.resetForTest();
GlobalOpenTelemetry.set(otelTesting.getOpenTelemetry());

provider = otelTesting.getOpenTelemetry().getTracerProvider();
TextMapPropagator propagator =
otelTesting.getOpenTelemetry().getPropagators().getTextMapPropagator();
;
tracerShim = new TracerShim(tracer, propagator, propagator);
tracerShim = new TracerShim(provider, propagator, propagator);
}

@AfterEach
void cleanup() {
GlobalOpenTelemetry.resetForTest();
}

@Test
Expand Down Expand Up @@ -278,8 +293,8 @@ void inject_textMap() {
Map<String, String> map = new HashMap<>();
CustomTextMapPropagator textMapPropagator = new CustomTextMapPropagator();
CustomTextMapPropagator httpHeadersPropagator = new CustomTextMapPropagator();
tracerShim = new TracerShim(tracer, textMapPropagator, httpHeadersPropagator);
io.opentelemetry.api.trace.Span span = tracer.spanBuilder("span").startSpan();
tracerShim = new TracerShim(provider, textMapPropagator, httpHeadersPropagator);
io.opentelemetry.api.trace.Span span = tracerShim.tracer().spanBuilder("span").startSpan();
SpanContext context = new SpanShim(span).context();

tracerShim.inject(context, Format.Builtin.TEXT_MAP, new TextMapAdapter(map));
Expand All @@ -292,8 +307,8 @@ void inject_httpHeaders() {
Map<String, String> map = new HashMap<>();
CustomTextMapPropagator textMapPropagator = new CustomTextMapPropagator();
CustomTextMapPropagator httpHeadersPropagator = new CustomTextMapPropagator();
tracerShim = new TracerShim(tracer, textMapPropagator, httpHeadersPropagator);
io.opentelemetry.api.trace.Span span = tracer.spanBuilder("span").startSpan();
tracerShim = new TracerShim(provider, textMapPropagator, httpHeadersPropagator);
io.opentelemetry.api.trace.Span span = tracerShim.tracer().spanBuilder("span").startSpan();
SpanContext context = new SpanShim(span).context();

tracerShim.inject(context, Format.Builtin.HTTP_HEADERS, new TextMapAdapter(map));
Expand All @@ -306,7 +321,7 @@ void extract_textMap() {
Map<String, String> map = new HashMap<>();
CustomTextMapPropagator textMapPropagator = new CustomTextMapPropagator();
CustomTextMapPropagator httpHeadersPropagator = new CustomTextMapPropagator();
tracerShim = new TracerShim(tracer, textMapPropagator, httpHeadersPropagator);
tracerShim = new TracerShim(provider, textMapPropagator, httpHeadersPropagator);

tracerShim.extract(Format.Builtin.TEXT_MAP, new TextMapAdapter(map));
assertThat(textMapPropagator.isExtracted()).isTrue();
Expand All @@ -318,7 +333,7 @@ void extract_httpHeaders() {
Map<String, String> map = new HashMap<>();
CustomTextMapPropagator textMapPropagator = new CustomTextMapPropagator();
CustomTextMapPropagator httpHeadersPropagator = new CustomTextMapPropagator();
tracerShim = new TracerShim(tracer, textMapPropagator, httpHeadersPropagator);
tracerShim = new TracerShim(provider, textMapPropagator, httpHeadersPropagator);

tracerShim.extract(Format.Builtin.HTTP_HEADERS, new TextMapAdapter(map));
assertThat(textMapPropagator.isExtracted()).isFalse();
Expand All @@ -328,7 +343,7 @@ void extract_httpHeaders() {
@Test
void extract_onlyBaggage() {
W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance();
tracerShim = new TracerShim(tracer, propagator, TextMapPropagator.noop());
tracerShim = new TracerShim(provider, propagator, TextMapPropagator.noop());

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

@Test
void close() {
void close_OpenTelemetrySdk() {
SdkTracerProvider sdkProvider = mock(SdkTracerProvider.class);
doThrow(new RuntimeException("testing error")).when(sdkProvider).close();

Tracer tracerShim =
OpenTracingShim.createTracerShim(
OpenTelemetrySdk.builder().setTracerProvider(sdkProvider).build());
tracerShim.close();

verify(sdkProvider).close();

Span otSpan = tracerShim.buildSpan(null).start();
io.opentelemetry.api.trace.Span span = ((SpanShim) otSpan).getSpan();
assertThat(span.getSpanContext().isValid()).isFalse();
}

@Test
void close_GlobalOpenTelemetry() {
GlobalOpenTelemetry.resetForTest();
SdkTracerProvider sdkProvider = mock(SdkTracerProvider.class);
doThrow(new RuntimeException("testing error")).when(sdkProvider).close();
GlobalOpenTelemetry.set(OpenTelemetrySdk.builder().setTracerProvider(sdkProvider).build());

Tracer tracerShim = OpenTracingShim.createTracerShim(GlobalOpenTelemetry.get());
tracerShim.close();

verify(sdkProvider).close();

Span otSpan = tracerShim.buildSpan(null).start();
io.opentelemetry.api.trace.Span span = ((SpanShim) otSpan).getSpan();
assertThat(span.getSpanContext().isValid()).isFalse();
}

@Test
void close_NoopOpenTelemetry() {
Tracer tracerShim = OpenTracingShim.createTracerShim(OpenTelemetry.noop());
tracerShim.close();

Span otSpan = tracerShim.buildSpan(null).start();
io.opentelemetry.api.trace.Span span = ((SpanShim) otSpan).getSpan();
assertThat(span.getSpanContext().isValid()).isFalse();
Expand Down Expand Up @@ -390,7 +441,8 @@ void doesNotCrash() {

@Test
void noopDoesNotCrash() {
tracerShim.close();
Tracer tracerShim = OpenTracingShim.createTracerShim(OpenTelemetry.noop());

Span span =
tracerShim
.buildSpan("test")
Expand Down

0 comments on commit f797537

Please sign in to comment.