Ensure OTel span kind is set (#6205)

DataDog · Nov 13, 2023 · 36f0e4b · 36f0e4b
1 parent 172cca3
commit 36f0e4b
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 36 deletions.
diff --git a/....4/src/main/java/datadog/trace/instrumentation/opentelemetry14/trace/OtelSpanBuilder.java b/....4/src/main/java/datadog/trace/instrumentation/opentelemetry14/trace/OtelSpanBuilder.java
@@ -2,6 +2,7 @@
 
 import static datadog.trace.instrumentation.opentelemetry14.trace.OtelConventions.toSpanType;
 import static datadog.trace.instrumentation.opentelemetry14.trace.OtelExtractedContext.extract;
+import static io.opentelemetry.api.trace.SpanKind.INTERNAL;
 
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
@@ -19,9 +20,11 @@
 @ParametersAreNonnullByDefault
 public class OtelSpanBuilder implements SpanBuilder {
   private final AgentTracer.SpanBuilder delegate;
+  private boolean spanKindSet;
 
   public OtelSpanBuilder(AgentTracer.SpanBuilder delegate) {
     this.delegate = delegate;
+    this.spanKindSet = false;
   }
 
   @Override
@@ -105,7 +108,10 @@ public <T> SpanBuilder setAttribute(AttributeKey<T> key, T value) {
 
   @Override
   public SpanBuilder setSpanKind(SpanKind spanKind) {
-    this.delegate.withSpanType(toSpanType(spanKind));
+    if (spanKind != null) {
+      this.delegate.withSpanType(toSpanType(spanKind));
+      this.spanKindSet = true;
+    }
     return this;
   }
 
@@ -117,6 +123,10 @@ public SpanBuilder setStartTimestamp(long startTimestamp, TimeUnit unit) {
 
   @Override
   public Span startSpan() {
+    // Ensure the span kind is set
+    if (!this.spanKindSet) {
+      setSpanKind(INTERNAL);
+    }
     AgentSpan delegate = this.delegate.start();
     return new OtelSpan(delegate);
   }

diff --git a/...ion/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14ConventionsTest.groovy b/...ion/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14ConventionsTest.groovy
@@ -7,7 +7,6 @@ import spock.lang.Subject
 
 import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND_CLIENT
 import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND_SERVER
-import static datadog.trace.instrumentation.opentelemetry14.trace.OtelConventions.DEFAULT_OPERATION_NAME
 import static datadog.trace.instrumentation.opentelemetry14.trace.OtelConventions.toSpanType
 import static io.opentelemetry.api.trace.SpanKind.CLIENT
 import static io.opentelemetry.api.trace.SpanKind.CONSUMER
@@ -29,9 +28,7 @@ class OpenTelemetry14ConventionsTest extends AgentTestRunner {
   def "test span name conventions"() {
     when:
     def builder = tracer.spanBuilder("some-name")
-    if (kind != null) {
-      builder.setSpanKind(kind)
-    }
+      .setSpanKind(kind)
     attributes.forEach { key, value -> builder.setAttribute(key, value) }
     builder.startSpan()
       .end()
@@ -41,9 +38,7 @@ class OpenTelemetry14ConventionsTest extends AgentTestRunner {
       trace(1) {
         span {
           parent()
-          if (kind != null) {
-            spanType toSpanType(kind)
-          }
+          spanType toSpanType(kind == null ? INTERNAL : kind)
           operationName "$expectedOperationName"
           resourceName "some-name"
         }
@@ -53,7 +48,7 @@ class OpenTelemetry14ConventionsTest extends AgentTestRunner {
     where:
     kind     | attributes                                                                     | expectedOperationName
     // Fallback behavior
-    null     | [:]                                                                            | DEFAULT_OPERATION_NAME
+    null     | [:]                                                                            | "internal"
     // Internal spans
     INTERNAL | [:]                                                                            | "internal"
     // Server spans
@@ -133,7 +128,8 @@ class OpenTelemetry14ConventionsTest extends AgentTestRunner {
       trace(1) {
         span {
           parent()
-          operationName "$DEFAULT_OPERATION_NAME"
+          operationName "internal"
+          spanType "internal"
           tags {
             defaultTags()
             if (value != null) {

diff --git a/...nstrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14Test.groovy b/...nstrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14Test.groovy
@@ -44,13 +44,15 @@ class OpenTelemetry14Test extends AgentTestRunner {
       trace(2) {
         span {
           parent()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName "some-name"
+          spanType "internal"
         }
         span {
           childOfPrevious()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName "other-name"
+          spanType "internal"
         }
       }
     }
@@ -72,13 +74,15 @@ class OpenTelemetry14Test extends AgentTestRunner {
       trace(2) {
         span {
           parent()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName "some-name"
+          spanType "internal"
         }
         span {
           childOfPrevious()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName "other-name"
+          spanType "internal"
         }
       }
     }
@@ -118,15 +122,17 @@ class OpenTelemetry14Test extends AgentTestRunner {
       trace(1) {
         span {
           parent()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName"some-name"
+          spanType "internal"
         }
       }
       trace(1) {
         span {
           parent()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName"other-name"
+          spanType "internal"
         }
       }
     }
@@ -139,8 +145,6 @@ class OpenTelemetry14Test extends AgentTestRunner {
     anotherSpan.end()
 
     when:
-    // Adding link is not supported
-    builder.addLink(anotherSpan.getSpanContext())
     // Adding event is not supported
     def result = builder.startSpan()
     result.addEvent("some-event")
@@ -149,10 +153,13 @@ class OpenTelemetry14Test extends AgentTestRunner {
     then:
     assertTraces(2) {
       trace(1) {
-        span {}
+        span {
+          spanType "internal"}
       }
       trace(1) {
-        span {}
+        span {
+          spanType "internal"
+        }
       }
     }
   }
@@ -201,7 +208,8 @@ class OpenTelemetry14Test extends AgentTestRunner {
       trace(1) {
         span {
           parent()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
+          spanType "internal"
           if (tagSpan) {
             resourceName "other-resource"
           } else if (tagBuilder) {
@@ -303,8 +311,9 @@ class OpenTelemetry14Test extends AgentTestRunner {
       trace(1) {
         span {
           parent()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName "some-name"
+          spanType "internal"
           errored true
 
           tags {
@@ -355,8 +364,9 @@ class OpenTelemetry14Test extends AgentTestRunner {
       trace(1) {
         span {
           parent()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName "some-name"
+          spanType "internal"
           errored false
           tags {
             defaultTags()
@@ -396,8 +406,9 @@ class OpenTelemetry14Test extends AgentTestRunner {
       trace(1) {
         span {
           parent()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName "some-name"
+          spanType "internal"
           errored false
           tags {
             defaultTags()
@@ -458,8 +469,9 @@ class OpenTelemetry14Test extends AgentTestRunner {
       trace(1) {
         span {
           parent()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName"some-name"
+          spanType "internal"
           errored true
           tags {
             defaultTags()

diff --git a/...pentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/ContextTest.groovy b/...pentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/ContextTest.groovy
@@ -224,17 +224,19 @@ class ContextTest extends AgentTestRunner {
       trace(3) {
         span {
           parent()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName "some-name"
+          spanType "internal"
         }
         span {
           childOfPrevious()
           operationName "other-name"
         }
         span {
           childOfPrevious()
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName "another-name"
+          spanType "internal"
         }
       }
     }

diff --git a/...try-1.4/src/test/groovy/opentelemetry14/context/propagation/AbstractPropagatorTest.groovy b/...try-1.4/src/test/groovy/opentelemetry14/context/propagation/AbstractPropagatorTest.groovy
@@ -13,8 +13,6 @@ import spock.lang.Subject
 
 import javax.annotation.Nullable
 
-import static datadog.trace.instrumentation.opentelemetry14.trace.OtelConventions.DEFAULT_OPERATION_NAME
-
 abstract class AbstractPropagatorTest extends AgentTestRunner {
   @Subject
   def tracer = GlobalOpenTelemetry.get().tracerProvider.get("propagator" + Math.random()) // TODO FIX LATER
@@ -88,8 +86,9 @@ abstract class AbstractPropagatorTest extends AgentTestRunner {
     assertTraces(1) {
       trace(1) {
         span {
-          operationName DEFAULT_OPERATION_NAME
+          operationName "internal"
           resourceName "some-name"
+          spanType "internal"
           traceDDId(DD128bTraceId.fromHex(traceId))
           parentSpanId(DDSpanId.fromHex(spanId).toLong() as BigInteger)
         }

diff --git a/dd-smoke-tests/play-2.8-otel/src/test/groovy/datadog/smoketest/Play28OTelSmokeTest.groovy b/dd-smoke-tests/play-2.8-otel/src/test/groovy/datadog/smoketest/Play28OTelSmokeTest.groovy
@@ -78,9 +78,9 @@ abstract class Play28OTelSmokeTest extends AbstractServerSmokeTest {
     // is undefined.
     boolean isOk = true
     def allowed = /|^\[${serverProviderName()}.request:GET \/welcome[sj]
-                   |(\[otel_unknown:filter1(\[otel_unknown:filter\d])(\[otel_unknown:filter\d])(\[otel_unknown:filter\d])])?
-                   |\[play.request:GET \/welcome[sj]\[otel_unknown:action1\[otel_unknown:action2\[otel_unknown:do-get\[play-ws.request:GET \/hello\/\?]]]]]
-                   |(\[otel_unknown:filter1(\[otel_unknown:filter\d])(\[otel_unknown:filter\d])(\[otel_unknown:filter\d])])?
+                   |(\[internal:filter1(\[internal:filter\d])(\[internal:filter\d])(\[internal:filter\d])])?
+                   |\[play.request:GET \/welcome[sj]\[internal:action1\[internal:action2\[internal:do-get\[play-ws.request:GET \/hello\/\?]]]]]
+                   |(\[internal:filter1(\[internal:filter\d])(\[internal:filter\d])(\[internal:filter\d])])?
                    |]$/.stripMargin().replaceAll("[\n\r]", "")
 
     // Ignore [command_execution], which can be generated by hostname calls from Config/Telemetry on some systems.
@@ -94,9 +94,9 @@ abstract class Play28OTelSmokeTest extends AbstractServerSmokeTest {
            |traceCounts=${traceCounts}""".stripMargin()
       def matches = matcher.head().findAll{ it != null }
       isOk &= matches.size() == 5
-      isOk &= matches.contains("[otel_unknown:filter2]")
-      isOk &= matches.contains("[otel_unknown:filter3]")
-      isOk &= matches.contains("[otel_unknown:filter4]")
+      isOk &= matches.contains("[internal:filter2]")
+      isOk &= matches.contains("[internal:filter3]")
+      isOk &= matches.contains("[internal:filter4]")
       assert isOk :  """\
            |Trace ${it.key} does not match allowed pattern:
            |pattern=${allowed}