Fix possible invalid parent span using OpenTelemetry API (#5666)

DataDog · Jul 31, 2023 · 4ed183c · 4ed183c
1 parent f94340f
commit 4ed183c
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 6 deletions.
diff --git a/...1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/OtelExtractedContext.java b/...1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/OtelExtractedContext.java
@@ -11,8 +11,11 @@
 import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.context.Context;
 import java.util.Map;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 class OtelExtractedContext implements AgentSpan.Context {
+  private static final Logger LOGGER = LoggerFactory.getLogger(OtelExtractedContext.class);
   private final DDTraceId traceId;
   private final long spanId;
   private final int prioritySampling;
@@ -29,13 +32,17 @@ static AgentSpan.Context extract(Context context) {
     SpanContext spanContext = span.getSpanContext();
     if (spanContext instanceof OtelSpanContext) {
       return ((OtelSpanContext) spanContext).delegate;
-    } else {
+    } else if (spanContext.isValid()) {
       try {
         return new OtelExtractedContext(spanContext);
       } catch (NumberFormatException e) {
-        return AgentTracer.NoopContext.INSTANCE;
+        LOGGER.debug(
+            "Failed to convert span context with trace id = {} and span id = {}",
+            spanContext.getTraceId(),
+            spanContext.getSpanId());
       }
     }
+    return null;
   }
 
   @Override

diff --git a/...etry-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/OtelSpanBuilder.java b/...etry-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/OtelSpanBuilder.java
@@ -26,7 +26,10 @@ public OtelSpanBuilder(AgentTracer.SpanBuilder delegate) {
 
   @Override
   public SpanBuilder setParent(Context context) {
-    this.delegate.asChildOf(extract(context));
+    AgentSpan.Context extractedContext = extract(context);
+    if (extractedContext != null) {
+      this.delegate.asChildOf(extractedContext);
+    }
     return this;
   }
 

diff --git a/...y-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/OtelTracerProvider.java b/...y-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/OtelTracerProvider.java
@@ -5,12 +5,13 @@
 import io.opentelemetry.api.trace.TracerProvider;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.logging.Logger;
 import javax.annotation.ParametersAreNonnullByDefault;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @ParametersAreNonnullByDefault
 public class OtelTracerProvider implements TracerProvider {
-  private static final Logger logger = Logger.getLogger(OtelTracerProvider.class.getName());
+  private static final Logger LOGGER = LoggerFactory.getLogger(OtelTracerProvider.class);
   private static final String DEFAULT_TRACER_NAME = "";
   public static final OtelTracerProvider INSTANCE = new OtelTracerProvider();
 
@@ -47,7 +48,7 @@ public Tracer get(String instrumentationScopeName, String instrumentationScopeVe
   @Override
   public TracerBuilder tracerBuilder(String instrumentationScopeName) {
     if (instrumentationScopeName.trim().isEmpty()) {
-      logger.fine("Tracer requested without instrumentation scope name.");
+      LOGGER.debug("Tracer requested without instrumentation scope name.");
       instrumentationScopeName = DEFAULT_TRACER_NAME;
     }
     return new OtelTracerBuilder(instrumentationScopeName);

diff --git a/...nstrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14Test.groovy b/...nstrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14Test.groovy
@@ -100,6 +100,23 @@ class OpenTelemetry14Test extends AgentTestRunner {
     }
   }
 
+  def "test parent span using invalid reference"() {
+    when:
+    def invalidCurrentSpanContext = Context.root() // Contains a SpanContext with TID/SID to 0 as current span
+    def childSpan = tracer.spanBuilder("some-name")
+      .setParent(invalidCurrentSpanContext)
+      .startSpan()
+    childSpan.end()
+
+    TEST_WRITER.waitForTraces(1)
+    def trace = TEST_WRITER.firstTrace()
+
+
+    then:
+    trace.size() == 1
+    trace[0].spanId != 0
+  }
+
   def "test no parent to create new root span"() {
     setup:
     def parentSpan = tracer.spanBuilder("some-name").startSpan()