Merge pull request #6232 from DataDog/bbujon/otel-span-kind

Store OpenTelemetry span kind as tag instead of span type

Author: PerfectSlayer

dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/trace/OtelConventions.java

@@ -1,6 +1,7 @@
 package datadog.trace.instrumentation.opentelemetry14.trace;
 
 import static datadog.trace.api.DDTags.ANALYTICS_SAMPLE_RATE;
+import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND;
 import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND_CLIENT;
 import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND_CONSUMER;
 import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND_PRODUCER;
@@ -13,6 +14,7 @@
 import static java.util.Locale.ROOT;
 
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
+import datadog.trace.bootstrap.instrumentation.api.Tags;
 import io.opentelemetry.api.trace.SpanKind;
 import javax.annotation.Nullable;
 import org.slf4j.Logger;
@@ -22,20 +24,17 @@ public final class OtelConventions {
   static final String SPAN_KIND_INTERNAL = "internal";
   private static final Logger LOGGER = LoggerFactory.getLogger(OtelConventions.class);
   private static final String OPERATION_NAME_SPECIFIC_ATTRIBUTE = "operation.name";
-  private static final String SERVICE_NAME_SPECIFIC_ATTRIBUTE = "service.name";
-  private static final String RESOURCE_NAME_SPECIFIC_ATTRIBUTE = "resource.name";
-  private static final String SPAN_TYPE_SPECIFIC_ATTRIBUTES = "span.type";
   private static final String ANALYTICS_EVENT_SPECIFIC_ATTRIBUTES = "analytics.event";
 
   private OtelConventions() {}
 
   /**
-   * Convert OpenTelemetry {@link SpanKind} to Datadog span type.
+   * Convert OpenTelemetry {@link SpanKind} to {@link Tags#SPAN_KIND} value.
    *
    * @param spanKind The OpenTelemetry span kind to convert.
-   * @return The related Datadog span type.
+   * @return The {@link Tags#SPAN_KIND} value.
    */
-  public static String toSpanType(SpanKind spanKind) {
+  public static String toSpanKindTagValue(SpanKind spanKind) {
     switch (spanKind) {
       case CLIENT:
         return SPAN_KIND_CLIENT;
@@ -53,16 +52,16 @@ public static String toSpanType(SpanKind spanKind) {
   }
 
   /**
-   * Convert Datadog span type to OpenTelemetry {@link SpanKind}.
+   * Convert {@link Tags#SPAN_KIND} value to OpenTelemetry {@link SpanKind}.
    *
-   * @param spanType The span type to convert.
+   * @param spanKind The {@link Tags#SPAN_KIND} value to convert.
    * @return The related OpenTelemetry {@link SpanKind}.
    */
-  public static SpanKind toSpanKind(String spanType) {
-    if (spanType == null) {
+  public static SpanKind toOtelSpanKind(String spanKind) {
+    if (spanKind == null) {
       return INTERNAL;
     }
-    switch (spanType) {
+    switch (spanKind) {
       case SPAN_KIND_CLIENT:
         return CLIENT;
       case SPAN_KIND_SERVER:
@@ -77,26 +76,17 @@ public static SpanKind toSpanKind(String spanType) {
   }
 
   public static void applyConventions(AgentSpan span) {
-    String serviceName = getStringAttribute(span, SERVICE_NAME_SPECIFIC_ATTRIBUTE);
-    if (serviceName != null) {
-      span.setServiceName(serviceName);
-    }
-    String resourceName = getStringAttribute(span, RESOURCE_NAME_SPECIFIC_ATTRIBUTE);
-    if (resourceName != null) {
-      span.setResourceName(resourceName);
-    }
-    String spanType = getStringAttribute(span, SPAN_TYPE_SPECIFIC_ATTRIBUTES);
-    if (spanType != null) {
-      span.setSpanType(spanType);
-    }
+    // The following attributes are already handled by tag interceptors:
+    // - service.name
+    // - resource.name
+    // - span.type
+    // TODO Remaining questions:
+    // - Do the attributes must be added has tag or intercepted?
+    // - Does operation name method blocked if operation.name is present?
     Boolean analyticsEvent = getBooleanAttribute(span, ANALYTICS_EVENT_SPECIFIC_ATTRIBUTES);
     if (analyticsEvent != null) {
       span.setMetric(ANALYTICS_SAMPLE_RATE, analyticsEvent ? 1 : 0);
     }
-    applyOperationName(span);
-  }
-
-  private static void applyOperationName(AgentSpan span) {
     String operationName = getStringAttribute(span, OPERATION_NAME_SPECIFIC_ATTRIBUTE);
     if (operationName == null) {
       operationName = computeOperationName(span);
@@ -105,7 +95,9 @@ private static void applyOperationName(AgentSpan span) {
   }
 
   private static String computeOperationName(AgentSpan span) {
-    SpanKind spanKind = toSpanKind(span.getSpanType());
+    Object spanKingTag = span.getTag(SPAN_KIND);
+    SpanKind spanKind =
+        spanKingTag instanceof String ? toOtelSpanKind((String) spanKingTag) : INTERNAL;
     /*
      * HTTP convention: https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/http/
      */

dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/main/java/datadog/trace/instrumentation/opentelemetry14/trace/OtelSpanBuilder.java

@@ -1,6 +1,7 @@
 package datadog.trace.instrumentation.opentelemetry14.trace;
 
-import static datadog.trace.instrumentation.opentelemetry14.trace.OtelConventions.toSpanType;
+import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND;
+import static datadog.trace.instrumentation.opentelemetry14.trace.OtelConventions.toSpanKindTagValue;
 import static datadog.trace.instrumentation.opentelemetry14.trace.OtelExtractedContext.extract;
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL;
 
@@ -113,7 +114,7 @@ public <T> SpanBuilder setAttribute(AttributeKey<T> key, T value) {
   @Override
   public SpanBuilder setSpanKind(SpanKind spanKind) {
     if (spanKind != null) {
-      this.delegate.withSpanType(toSpanType(spanKind));
+      this.delegate.withTag(SPAN_KIND, toSpanKindTagValue(spanKind));
       this.spanKindSet = true;
     }
     return this;

dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/OpenTelemetry14ConventionsTest.groovy

@@ -5,9 +5,9 @@ import io.opentelemetry.context.Context
 import io.opentelemetry.context.ThreadLocalContextStorage
 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.toSpanType
+import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND
+import static datadog.trace.instrumentation.opentelemetry14.trace.OtelConventions.SPAN_KIND_INTERNAL
+import static datadog.trace.instrumentation.opentelemetry14.trace.OtelConventions.toSpanKindTagValue
 import static io.opentelemetry.api.trace.SpanKind.CLIENT
 import static io.opentelemetry.api.trace.SpanKind.CONSUMER
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL
@@ -38,9 +38,15 @@ class OpenTelemetry14ConventionsTest extends AgentTestRunner {
       trace(1) {
         span {
           parent()
-          spanType toSpanType(kind == null ? INTERNAL : kind)
           operationName "$expectedOperationName"
           resourceName "some-name"
+          tags {
+            defaultTags()
+            "$SPAN_KIND" "${toSpanKindTagValue(kind == null ? INTERNAL : kind)}"
+            attributes.forEach { key, value->
+              tag(key, value)
+            }
+          }
         }
       }
     }
@@ -91,28 +97,26 @@ class OpenTelemetry14ConventionsTest extends AgentTestRunner {
     tracer.spanBuilder("some-name")
       .setAttribute("service.name", "my-service")
       .setAttribute("resource.name", "/my-resource")
-      .setAttribute("span.type", "$type")
+      .setAttribute("span.type", "http")
       .startSpan()
       .end()
 
-
     then:
     assertTraces(1) {
       trace(1) {
         span {
           parent()
-          spanType "$type"
-          operationName "$expectedOperationName"
+          operationName "internal"
           resourceName "/my-resource"
           serviceName "my-service"
+          spanType "http"
+          tags {
+            defaultTags()
+            "$SPAN_KIND" "$SPAN_KIND_INTERNAL"
+          }
         }
       }
     }
-
-    where:
-    type             | expectedOperationName
-    SPAN_KIND_SERVER | "server.request"
-    SPAN_KIND_CLIENT | "client.request"
   }
 
   def "test span analytics.event specific tag"() {
@@ -129,9 +133,9 @@ class OpenTelemetry14ConventionsTest extends AgentTestRunner {
         span {
           parent()
           operationName "internal"
-          spanType "internal"
           tags {
             defaultTags()
+            "$SPAN_KIND" "$SPAN_KIND_INTERNAL"
             if (value != null) {
               "analytics.event" value
               "$DDTags.ANALYTICS_SAMPLE_RATE" expectedMetric