diff --git a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/ScopeManagerShim.java b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/ScopeManagerShim.java index 89cbb2a0b9a..e7428017ba6 100644 --- a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/ScopeManagerShim.java +++ b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/ScopeManagerShim.java @@ -50,9 +50,10 @@ public Span activeSpan() { @Override @SuppressWarnings("MustBeClosedChecker") public Scope activate(@Nullable Span span) { - if (span == null) { + SpanShim spanShim = ShimUtil.getSpanShim(span); + if (spanShim == null) { return new ScopeShim(Context.current().with(NOOP_SPANSHIM).makeCurrent()); } - return new ScopeShim(Context.current().with(ShimUtil.getSpanShim(span)).makeCurrent()); + return new ScopeShim(Context.current().with(spanShim).makeCurrent()); } } diff --git a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/ShimUtil.java b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/ShimUtil.java index bc7effedaa1..475490eb717 100644 --- a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/ShimUtil.java +++ b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/ShimUtil.java @@ -7,23 +7,40 @@ import io.opentracing.Span; import io.opentracing.SpanContext; +import io.opentracing.noop.NoopSpan; import java.util.function.Supplier; +import java.util.logging.Level; +import java.util.logging.Logger; import javax.annotation.Nullable; class ShimUtil { + private static final Logger logger = Logger.getLogger(ShimUtil.class.getName()); private ShimUtil() {} - static SpanContextShim getContextShim(SpanContext context) { + @Nullable + static SpanContextShim getContextShim(@Nullable SpanContext context) { + if (context == null) { + return null; + } + if (!(context instanceof SpanContextShim)) { - throw new IllegalArgumentException( - "context is not a valid SpanContextShim object: " + className(context)); + logger.log( + Level.INFO, + "Expected to have an OpenTelemetry SpanContext but got {0}", + className(context)); + return null; } return (SpanContextShim) context; } - static SpanShim getSpanShim(Span span) { + @Nullable + static SpanShim getSpanShim(@Nullable Span span) { + if (span == null || span instanceof NoopSpan) { + return null; + } + if (!(span instanceof SpanShim)) { if (span instanceof Supplier) { // allow libraries to implement a delegate span, @@ -32,11 +49,13 @@ static SpanShim getSpanShim(Span span) { if (wrapped instanceof Span) { return getSpanShim((Span) wrapped); } else { - throw new IllegalArgumentException( - "span wrapper didn't return a span: " + className(wrapped)); + logger.log(Level.INFO, "Span wrapper didn't return a span: {0}", className(wrapped)); + return null; } } - throw new IllegalArgumentException("span is not a valid SpanShim object: " + className(span)); + + logger.log(Level.INFO, "Expected to have an OpenTelemetry Span but got {0}", className(span)); + return null; } return (SpanShim) span; diff --git a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java index 64cf4f0a19d..97f60b9e5ff 100644 --- a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java +++ b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java @@ -23,7 +23,6 @@ import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.Tracer.SpanBuilder; -import io.opentracing.noop.NoopSpan; import io.opentracing.tag.Tag; import io.opentracing.tag.Tags; import java.util.ArrayList; @@ -64,11 +63,12 @@ public SpanBuilderShim(TelemetryInfo telemetryInfo, String spanName) { @Override public SpanBuilder asChildOf(Span parent) { - if (parent == null || parent instanceof NoopSpan) { - return this; - } SpanShim spanShim = ShimUtil.getSpanShim(parent); - return addReference(References.CHILD_OF, spanShim.context()); + if (spanShim != null) { + addReference(References.CHILD_OF, spanShim.context()); + } + + return this; } @Override @@ -78,7 +78,8 @@ public SpanBuilder asChildOf(SpanContext parent) { @Override public SpanBuilder addReference(@Nullable String referenceType, SpanContext referencedContext) { - if (referencedContext == null) { + SpanContextShim contextShim = ShimUtil.getContextShim(referencedContext); + if (contextShim == null) { return this; } @@ -92,8 +93,6 @@ public SpanBuilder addReference(@Nullable String referenceType, SpanContext refe return this; } - SpanContextShim contextShim = ShimUtil.getContextShim(referencedContext); - // Optimization for 99% situations, when there is only one parent. if (allParents.size() == 0) { allParents = diff --git a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java index 27d61129976..6d907a8aeaa 100644 --- a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java +++ b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java @@ -61,7 +61,10 @@ public void inject(SpanContext context, Format format, C carrier) { return; } - SpanContextShim contextShim = getContextShim(context); + SpanContextShim contextShim = ShimUtil.getContextShim(context); + if (contextShim == null) { + return; + } if (format == Format.Builtin.TEXT_MAP || format == Format.Builtin.TEXT_MAP_INJECT @@ -94,12 +97,4 @@ public SpanContext extract(Format format, C carrier) { public void close() { isClosed = true; } - - static SpanContextShim getContextShim(SpanContext context) { - if (!(context instanceof SpanContextShim)) { - throw new IllegalArgumentException("context is not a valid SpanContextShim object"); - } - - return (SpanContextShim) context; - } } diff --git a/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/ShimUtilTest.java b/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/ShimUtilTest.java index 464363096c5..3da899b13a8 100644 --- a/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/ShimUtilTest.java +++ b/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/ShimUtilTest.java @@ -6,7 +6,6 @@ package io.opentelemetry.opentracingshim; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.Tracer; @@ -40,20 +39,14 @@ void spanWrapper() { SpanShim shim = new SpanShim(telemetryInfo, span); assertThat(ShimUtil.getSpanShim(shim)).isEqualTo(shim); assertThat(ShimUtil.getSpanShim(new SpanWrapper(shim))).isEqualTo(shim); - assertThatThrownBy(() -> ShimUtil.getSpanShim(new SpanWrapper("not a span"))) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("span wrapper didn't return a span: java.lang.String"); - assertThatThrownBy(() -> ShimUtil.getSpanShim(null)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("span is not a valid SpanShim object: null"); + assertThat(ShimUtil.getSpanShim(new SpanWrapper("not a span"))).isNull(); + assertThat(ShimUtil.getSpanShim(null)).isNull(); } @Test void getContextShim() { SpanContextShim contextShim = new SpanContextShim(new SpanShim(telemetryInfo, span)); assertThat(ShimUtil.getContextShim(contextShim)).isEqualTo(contextShim); - assertThatThrownBy(() -> ShimUtil.getContextShim(null)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("context is not a valid SpanContextShim object: null"); + assertThat(ShimUtil.getContextShim(null)).isNull(); } } diff --git a/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/TracerShimTest.java b/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/TracerShimTest.java index a9986875f04..eb9b0178f38 100644 --- a/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/TracerShimTest.java +++ b/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/TracerShimTest.java @@ -6,6 +6,7 @@ package io.opentelemetry.opentracingshim; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator; import io.opentelemetry.api.trace.Tracer; @@ -262,6 +263,13 @@ void inject_nullContext() { assertThat(map).isEmpty(); } + @Test + void inject_invalid() { + Map map = new HashMap<>(); + tracerShim.inject(mock(SpanContext.class), Format.Builtin.TEXT_MAP, new TextMapAdapter(map)); + assertThat(map).isEmpty(); + } + @Test void inject_textMap() { Map map = new HashMap<>();