Merge pull request #7399 from DataDog/mtoff/otel-sdk-attributes

Move span Attributes implementation outside of SpanLink class
DataDog · Aug 14, 2024 · d37fbd5 · d37fbd5
2 parents b4db634 + 96dcbbe
commit d37fbd5
Show file tree

Hide file tree

Showing 12 changed files with 105 additions and 97 deletions.
diff --git a/.../agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelConventions.java b/.../agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelConventions.java
@@ -15,9 +15,12 @@
 import static java.util.Locale.ROOT;
 
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
+import datadog.trace.bootstrap.instrumentation.api.SpanAttributes;
 import datadog.trace.bootstrap.instrumentation.api.Tags;
 import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.SpanKind;
+import java.util.List;
 import javax.annotation.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -224,4 +227,46 @@ private static String getStringAttribute(AgentSpan span, String key) {
     }
     return (String) tag;
   }
+
+  public static AgentSpan.Attributes convertAttributes(Attributes attributes) {
+    if (attributes.isEmpty()) {
+      return SpanAttributes.EMPTY;
+    }
+    SpanAttributes.Builder builder = SpanAttributes.builder();
+    attributes.forEach(
+        (attributeKey, value) -> {
+          String key = attributeKey.getKey();
+          switch (attributeKey.getType()) {
+            case STRING:
+              builder.put(key, (String) value);
+              break;
+            case BOOLEAN:
+              builder.put(key, (boolean) value);
+              break;
+            case LONG:
+              builder.put(key, (long) value);
+              break;
+            case DOUBLE:
+              builder.put(key, (double) value);
+              break;
+            case STRING_ARRAY:
+              //noinspection unchecked
+              builder.putStringArray(key, (List<String>) value);
+              break;
+            case BOOLEAN_ARRAY:
+              //noinspection unchecked
+              builder.putBooleanArray(key, (List<Boolean>) value);
+              break;
+            case LONG_ARRAY:
+              //noinspection unchecked
+              builder.putLongArray(key, (List<Long>) value);
+              break;
+            case DOUBLE_ARRAY:
+              //noinspection unchecked
+              builder.putDoubleArray(key, (List<Double>) value);
+              break;
+          }
+        });
+    return builder.build();
+  }
 }
diff --git a/...ent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelSpanLink.java b/...ent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelSpanLink.java
@@ -1,12 +1,12 @@
 package datadog.opentelemetry.shim.trace;
 
+import static datadog.opentelemetry.shim.trace.OtelConventions.convertAttributes;
+
 import datadog.opentelemetry.shim.context.propagation.TraceStateHelper;
 import datadog.trace.api.DDSpanId;
 import datadog.trace.api.DDTraceId;
 import datadog.trace.bootstrap.instrumentation.api.SpanLink;
-import datadog.trace.bootstrap.instrumentation.api.SpanLinkAttributes;
 import io.opentelemetry.api.trace.SpanContext;
-import java.util.List;
 
 public class OtelSpanLink extends SpanLink {
   public OtelSpanLink(SpanContext spanContext) {
@@ -21,46 +21,4 @@ public OtelSpanLink(SpanContext spanContext, io.opentelemetry.api.common.Attribu
         TraceStateHelper.encodeHeader(spanContext.getTraceState()),
         convertAttributes(attributes));
   }
-
-  private static Attributes convertAttributes(io.opentelemetry.api.common.Attributes attributes) {
-    if (attributes.isEmpty()) {
-      return SpanLinkAttributes.EMPTY;
-    }
-    SpanLinkAttributes.Builder builder = SpanLinkAttributes.builder();
-    attributes.forEach(
-        (attributeKey, value) -> {
-          String key = attributeKey.getKey();
-          switch (attributeKey.getType()) {
-            case STRING:
-              builder.put(key, (String) value);
-              break;
-            case BOOLEAN:
-              builder.put(key, (boolean) value);
-              break;
-            case LONG:
-              builder.put(key, (long) value);
-              break;
-            case DOUBLE:
-              builder.put(key, (double) value);
-              break;
-            case STRING_ARRAY:
-              //noinspection unchecked
-              builder.putStringArray(key, (List<String>) value);
-              break;
-            case BOOLEAN_ARRAY:
-              //noinspection unchecked
-              builder.putBooleanArray(key, (List<Boolean>) value);
-              break;
-            case LONG_ARRAY:
-              //noinspection unchecked
-              builder.putLongArray(key, (List<Long>) value);
-              break;
-            case DOUBLE_ARRAY:
-              //noinspection unchecked
-              builder.putDoubleArray(key, (List<Double>) value);
-              break;
-          }
-        });
-    return builder.build();
-  }
 }
diff --git a/...main/java/datadog/trace/instrumentation/opentelemetry14/OpenTelemetryInstrumentation.java b/...main/java/datadog/trace/instrumentation/opentelemetry14/OpenTelemetryInstrumentation.java
@@ -74,7 +74,6 @@ public String[] helperClassNames() {
       "datadog.opentelemetry.shim.trace.OtelSpanBuilder$1",
       "datadog.opentelemetry.shim.trace.OtelSpanContext",
       "datadog.opentelemetry.shim.trace.OtelSpanLink",
-      "datadog.opentelemetry.shim.trace.OtelSpanLink$1",
       "datadog.opentelemetry.shim.trace.OtelTracer",
       "datadog.opentelemetry.shim.trace.OtelTracerBuilder",
       "datadog.opentelemetry.shim.trace.OtelTracerProvider",

diff --git a/...og/trace/instrumentation/opentelemetry14/context/OpenTelemetryContextInstrumentation.java b/...og/trace/instrumentation/opentelemetry14/context/OpenTelemetryContextInstrumentation.java
@@ -68,7 +68,6 @@ public String[] helperClassNames() {
       "datadog.opentelemetry.shim.trace.OtelSpanBuilder$1",
       "datadog.opentelemetry.shim.trace.OtelSpanContext",
       "datadog.opentelemetry.shim.trace.OtelSpanLink",
-      "datadog.opentelemetry.shim.trace.OtelSpanLink$1",
       "datadog.opentelemetry.shim.trace.OtelTracer",
       "datadog.opentelemetry.shim.trace.OtelTracerBuilder",
       "datadog.opentelemetry.shim.trace.OtelTracerProvider",

diff --git a/...e/instrumentation/opentelemetry14/context/OpenTelemetryContextStorageInstrumentation.java b/...e/instrumentation/opentelemetry14/context/OpenTelemetryContextStorageInstrumentation.java
@@ -69,7 +69,6 @@ public String[] helperClassNames() {
       "datadog.opentelemetry.shim.trace.OtelSpanBuilder$1",
       "datadog.opentelemetry.shim.trace.OtelSpanContext",
       "datadog.opentelemetry.shim.trace.OtelSpanLink",
-      "datadog.opentelemetry.shim.trace.OtelSpanLink$1",
       "datadog.opentelemetry.shim.trace.OtelTracer",
       "datadog.opentelemetry.shim.trace.OtelTracerBuilder",
       "datadog.opentelemetry.shim.trace.OtelTracerProvider",

diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanLink.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanLink.java
@@ -1,16 +1,17 @@
 package datadog.trace.core;
 
-import static datadog.trace.bootstrap.instrumentation.api.SpanLinkAttributes.EMPTY;
+import static datadog.trace.bootstrap.instrumentation.api.SpanAttributes.EMPTY;
 
 import com.squareup.moshi.FromJson;
 import com.squareup.moshi.JsonAdapter;
 import com.squareup.moshi.Moshi;
 import com.squareup.moshi.ToJson;
 import datadog.trace.api.DDSpanId;
 import datadog.trace.api.DDTraceId;
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan.Attributes;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
+import datadog.trace.bootstrap.instrumentation.api.SpanAttributes;
 import datadog.trace.bootstrap.instrumentation.api.SpanLink;
-import datadog.trace.bootstrap.instrumentation.api.SpanLinkAttributes;
 import datadog.trace.core.propagation.ExtractedContext;
 import datadog.trace.core.propagation.PropagationTags;
 import java.util.List;
@@ -130,7 +131,7 @@ AgentSpanLink fromSpanLinkJson(SpanLinkJson json) {
           DDSpanId.fromHex(json.span_id),
           json.flags,
           json.tracestate,
-          SpanLinkAttributes.fromMap(json.attributes));
+          SpanAttributes.fromMap(json.attributes));
     }
   }
 

diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanLinkTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanLinkTest.groovy
@@ -5,8 +5,8 @@ import datadog.trace.api.DDSpanId
 import datadog.trace.api.DDTraceId
 import datadog.trace.api.DynamicConfig
 import datadog.trace.bootstrap.instrumentation.api.ContextVisitors
+import datadog.trace.bootstrap.instrumentation.api.SpanAttributes
 import datadog.trace.bootstrap.instrumentation.api.SpanLink
-import datadog.trace.bootstrap.instrumentation.api.SpanLinkAttributes
 import datadog.trace.common.writer.ListWriter
 import datadog.trace.core.propagation.ExtractedContext
 import datadog.trace.core.propagation.W3CHttpCodec
@@ -19,7 +19,7 @@ import java.util.stream.IntStream
 import static datadog.trace.api.DDTags.SPAN_LINKS
 import static datadog.trace.bootstrap.instrumentation.api.AgentSpanLink.DEFAULT_FLAGS
 import static datadog.trace.bootstrap.instrumentation.api.AgentSpanLink.SAMPLED_FLAG
-import static datadog.trace.bootstrap.instrumentation.api.SpanLinkAttributes.EMPTY
+import static datadog.trace.bootstrap.instrumentation.api.SpanAttributes.EMPTY
 import static datadog.trace.core.propagation.W3CHttpCodec.TRACE_PARENT_KEY
 import static datadog.trace.core.propagation.W3CHttpCodec.TRACE_STATE_KEY
 import static java.util.stream.Collectors.toList
@@ -172,7 +172,7 @@ class DDSpanLinkTest extends DDCoreSpecification {
       DDSpanId.fromHex(String.format("123456789abc%04d", index)),
       index % 2 == 0 ? SAMPLED_FLAG : DEFAULT_FLAGS,
       "",
-      SpanLinkAttributes.fromMap(attributes))
+      SpanAttributes.fromMap(attributes))
   }
 
   def assertLink(SpanLink expected, DDSpanLink.SpanLinkJson actual) {

diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java
@@ -228,4 +228,20 @@ interface Extracted extends Context {
       String getCustomIpHeader();
     }
   }
+
+  interface Attributes {
+    /**
+     * Gets the attributes as an immutable map.
+     *
+     * @return The attributes as an immutable map.
+     */
+    Map<String, String> asMap();
+
+    /**
+     * Checks whether the attributes are empty.
+     *
+     * @return {@code true} if the attributes are empty, {@code false} otherwise.
+     */
+    boolean isEmpty();
+  }
 }
diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanLink.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanLink.java
@@ -1,7 +1,7 @@
 package datadog.trace.bootstrap.instrumentation.api;
 
 import datadog.trace.api.DDTraceId;
-import java.util.Map;
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan.Attributes;
 
 /**
  * This interface describes a link to another span. The linked span could be part of the same trace
@@ -51,20 +51,4 @@ public interface AgentSpanLink {
    * @return The link attributes.
    */
   Attributes attributes();
-
-  interface Attributes {
-    /**
-     * Gets the attributes as an immutable map.
-     *
-     * @return The attributes as an immutable map.
-     */
-    Map<String, String> asMap();
-
-    /**
-     * Checks whether the attributes are empty.
-     *
-     * @return {@code true} if the attributes are empty, {@code false} otherwise.
-     */
-    boolean isEmpty();
-  }
 }
diff --git a/...strumentation/api/SpanLinkAttributes.java → ...p/instrumentation/api/SpanAttributes.java b/...strumentation/api/SpanLinkAttributes.java → ...p/instrumentation/api/SpanAttributes.java
@@ -2,40 +2,40 @@
 
 import static java.util.Objects.requireNonNull;
 
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan.Attributes;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-/** This class is a base implementation of {@link AgentSpanLink.Attributes}. */
-public class SpanLinkAttributes implements AgentSpanLink.Attributes {
-  /** An empty span links attributes. */
-  public static final AgentSpanLink.Attributes EMPTY =
-      new SpanLinkAttributes(Collections.emptyMap());
+/** This class is a base implementation of {@link Attributes}. */
+public class SpanAttributes implements Attributes {
+  /** Represent an empty attributes. */
+  public static final Attributes EMPTY = new SpanAttributes(Collections.emptyMap());
 
   private final Map<String, String> attributes;
 
-  protected SpanLinkAttributes(Map<String, String> attributes) {
+  protected SpanAttributes(Map<String, String> attributes) {
     this.attributes = attributes;
   }
 
   /**
-   * Gets a builder to create span link attributes.
+   * Gets a builder to create attributes.
    *
-   * @return A builder to create span link attributes.
+   * @return A builder to create attributes.
    */
   public static Builder builder() {
     return new Builder();
   }
 
   /**
-   * Create span link attributes from its map representation.
+   * Create attributes from its map representation.
    *
-   * @param map A map representing the span link attributes.
-   * @return The related span link attributes.
+   * @param map A map representing the attributes.
+   * @return The related attributes.
    */
-  public static SpanLinkAttributes fromMap(Map<String, String> map) {
-    return new SpanLinkAttributes(new HashMap<>(map));
+  public static SpanAttributes fromMap(Map<String, String> map) {
+    return new SpanAttributes(new HashMap<>(map));
   }
 
   @Override
@@ -50,7 +50,7 @@ public boolean isEmpty() {
 
   @Override
   public String toString() {
-    return "SpanLinkAttributes{" + this.attributes + '}';
+    return "SpanAttributes{" + this.attributes + '}';
   }
 
   public static class Builder {
@@ -115,8 +115,8 @@ protected <T> Builder putArray(String key, List<T> array) {
       return this;
     }
 
-    public AgentSpanLink.Attributes build() {
-      return new SpanLinkAttributes(this.attributes);
+    public Attributes build() {
+      return new SpanAttributes(this.attributes);
     }
   }
 }
diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanLink.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SpanLink.java
@@ -1,6 +1,6 @@
 package datadog.trace.bootstrap.instrumentation.api;
 
-import static datadog.trace.bootstrap.instrumentation.api.SpanLinkAttributes.EMPTY;
+import static datadog.trace.bootstrap.instrumentation.api.SpanAttributes.EMPTY;
 
 import datadog.trace.api.DDTraceId;
 
@@ -10,10 +10,14 @@ public class SpanLink implements AgentSpanLink {
   private final long spanId;
   private final byte traceFlags;
   private final String traceState;
-  private final Attributes attributes;
+  private final AgentSpan.Attributes attributes;
 
   protected SpanLink(
-      DDTraceId traceId, long spanId, byte traceFlags, String traceState, Attributes attributes) {
+      DDTraceId traceId,
+      long spanId,
+      byte traceFlags,
+      String traceState,
+      AgentSpan.Attributes attributes) {
     this.traceId = traceId == null ? DDTraceId.ZERO : traceId;
     this.spanId = spanId;
     this.traceFlags = traceFlags;
@@ -43,7 +47,10 @@ public static SpanLink from(AgentSpan.Context context) {
    * @return A span link to the given context.
    */
   public static SpanLink from(
-      AgentSpan.Context context, byte traceFlags, String traceState, Attributes attributes) {
+      AgentSpan.Context context,
+      byte traceFlags,
+      String traceState,
+      AgentSpan.Attributes attributes) {
     if (context.getSamplingPriority() > 0) {
       traceFlags = (byte) (traceFlags | SAMPLED_FLAG);
     }
@@ -72,7 +79,7 @@ public String traceState() {
   }
 
   @Override
-  public Attributes attributes() {
+  public AgentSpan.Attributes attributes() {
     return this.attributes;
   }