Refactored span events to avoid not needed JSON deserialization for v…

dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelConventions.java

@@ -7,7 +7,6 @@
 import static datadog.trace.api.DDTags.ERROR_MSG;
 import static datadog.trace.api.DDTags.ERROR_STACK;
 import static datadog.trace.api.DDTags.ERROR_TYPE;
-import static datadog.trace.api.DDTags.SPAN_EVENTS;
 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;
@@ -136,13 +135,6 @@ public static void applyNamingConvention(AgentSpan span) {
     }
   }
 
-  public static void setEventsAsTag(AgentSpan span, List<OtelSpanEvent> events) {
-    if (events == null || events.isEmpty()) {
-      return;
-    }
-    span.setTag(SPAN_EVENTS, OtelSpanEvent.toTag(events));
-  }
-
   public static void applySpanEventExceptionAttributesAsTags(
       AgentSpan span, Attributes exceptionAttributes) {
     span.setTag(ERROR_MSG, exceptionAttributes.get(EXCEPTION_MESSAGE_ATTRIBUTE_KEY));

dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelSpan.java

@@ -3,7 +3,6 @@
 import static datadog.opentelemetry.shim.trace.OtelConventions.applyNamingConvention;
 import static datadog.opentelemetry.shim.trace.OtelConventions.applyReservedAttribute;
 import static datadog.opentelemetry.shim.trace.OtelConventions.applySpanEventExceptionAttributesAsTags;
-import static datadog.opentelemetry.shim.trace.OtelConventions.setEventsAsTag;
 import static datadog.opentelemetry.shim.trace.OtelSpanEvent.EXCEPTION_SPAN_EVENT_NAME;
 import static datadog.opentelemetry.shim.trace.OtelSpanEvent.initializeExceptionAttributes;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
@@ -26,7 +25,6 @@
 import io.opentelemetry.api.trace.StatusCode;
 import io.opentelemetry.api.trace.TraceFlags;
 import io.opentelemetry.api.trace.TraceState;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import javax.annotation.ParametersAreNonnullByDefault;
@@ -37,9 +35,6 @@ public class OtelSpan implements Span, WithAgentSpan, SpanWrapper {
   private StatusCode statusCode;
   private boolean recording;
 
-  /** Span events ({@code null} until an event is added). */
-  private List<OtelSpanEvent> events;
-
   public OtelSpan(AgentSpan delegate) {
     this.delegate = delegate;
     if (delegate instanceof AttachableWrapper) {
@@ -85,21 +80,15 @@ public <T> Span setAttribute(AttributeKey<T> key, T value) {
   @Override
   public Span addEvent(String name, Attributes attributes) {
     if (this.recording) {
-      if (this.events == null) {
-        this.events = new ArrayList<>();
-      }
-      this.events.add(new OtelSpanEvent(name, attributes));
+      this.delegate.addEvent(new OtelSpanEvent(name, attributes));
     }
     return this;
   }
 
   @Override
   public Span addEvent(String name, Attributes attributes, long timestamp, TimeUnit unit) {
     if (this.recording) {
-      if (this.events == null) {
-        this.events = new ArrayList<>();
-      }
-      this.events.add(new OtelSpanEvent(name, attributes, timestamp, unit));
+      this.delegate.addEvent(new OtelSpanEvent(name, attributes, timestamp, unit));
     }
     return this;
   }
@@ -123,12 +112,9 @@ public Span setStatus(StatusCode statusCode, String description) {
   @Override
   public Span recordException(Throwable exception, Attributes additionalAttributes) {
     if (this.recording) {
-      if (this.events == null) {
-        this.events = new ArrayList<>();
-      }
       additionalAttributes = initializeExceptionAttributes(exception, additionalAttributes);
       applySpanEventExceptionAttributesAsTags(this.delegate, additionalAttributes);
-      this.events.add(new OtelSpanEvent(EXCEPTION_SPAN_EVENT_NAME, additionalAttributes));
+      this.delegate.addEvent(new OtelSpanEvent(EXCEPTION_SPAN_EVENT_NAME, additionalAttributes));
     }
     return this;
   }
@@ -179,7 +165,6 @@ public AgentSpan asAgentSpan() {
   @Override
   public void onSpanFinished() {
     applyNamingConvention(this.delegate);
-    setEventsAsTag(this.delegate, this.events);
   }
 
   private static class NoopSpan implements Span {

dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelSpanEvent.java

@@ -2,18 +2,20 @@
 
 import datadog.trace.api.time.SystemTimeSource;
 import datadog.trace.api.time.TimeSource;
+import datadog.trace.bootstrap.instrumentation.api.AgentSpanEvent;
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.common.AttributesBuilder;
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
-import javax.annotation.Nonnull;
 
-public class OtelSpanEvent {
+public class OtelSpanEvent implements AgentSpanEvent {
   public static final String EXCEPTION_SPAN_EVENT_NAME = "exception";
   public static final AttributeKey<String> EXCEPTION_MESSAGE_ATTRIBUTE_KEY =
       AttributeKey.stringKey("exception.message");
@@ -26,33 +28,69 @@ public class OtelSpanEvent {
   private static TimeSource timeSource = SystemTimeSource.INSTANCE;
 
   private final String name;
-  private final String attributes;
+  private final Attributes attributes;
 
   /** Event timestamp in nanoseconds. */
   private final long timestamp;
 
   public OtelSpanEvent(String name, Attributes attributes) {
-    this.name = name;
-    this.attributes = AttributesJsonParser.toJson(attributes);
-    this.timestamp = OtelSpanEvent.timeSource.getCurrentTimeNanos();
+    this(name, attributes, OtelSpanEvent.timeSource.getCurrentTimeNanos());
   }
 
   public OtelSpanEvent(String name, Attributes attributes, long timestamp, TimeUnit unit) {
+    this(name, attributes, unit.toNanos(timestamp));
+  }
+
+  private OtelSpanEvent(String name, Attributes attributes, long timestampNanos) {
     this.name = name;
-    this.attributes = AttributesJsonParser.toJson(attributes);
-    this.timestamp = unit.toNanos(timestamp);
+    this.attributes = attributes;
+    this.timestamp = timestampNanos;
   }
 
-  @Nonnull
-  public static String toTag(List<OtelSpanEvent> events) {
-    StringBuilder builder = new StringBuilder("[");
-    for (OtelSpanEvent event : events) {
-      if (builder.length() > 1) {
-        builder.append(',');
-      }
-      builder.append(event.toJson());
+  @Override
+  public String name() {
+    return this.name;
+  }
+
+  @Override
+  public long timeNanos() {
+    return this.timestamp;
+  }
+
+  /**
+   * Exposes the event attributes as typed values for native (V1) encoding. OpenTelemetry attribute
+   * values are already {@link String}, {@link Boolean}, {@link Long}, {@link Double} or a {@link
+   * List} of those, so they are passed through unchanged.
+   */
+  @Override
+  public Map<String, Object> attributes() {
+    if (this.attributes == null || this.attributes.isEmpty()) {
+      return Collections.emptyMap();
     }
-    return builder.append(']').toString();
+    Map<String, Object> map = new LinkedHashMap<>(this.attributes.size());
+    this.attributes.forEach((key, value) -> map.put(key.getKey(), value));
+    return map;
+  }
+
+  @Override
+  public CharSequence toJson() {
+    StringBuilder builder = new StringBuilder(128);
+    builder
+        .append('{')
+        .append("\"time_unix_nano\":")
+        .append(this.timestamp)
+        .append(',')
+        .append("\"name\":")
+        .append('"')
+        .append(this.name)
+        .append('"');
+
+    String attributesJson = AttributesJsonParser.toJson(this.attributes);
+    if (!attributesJson.isEmpty()) {
+      builder.append(",\"attributes\":").append(attributesJson);
+    }
+
+    return builder.append('}').toString();
   }
 
   /**
@@ -170,24 +208,14 @@ public static void setTimeSource(TimeSource newTimeSource) {
     timeSource = newTimeSource;
   }
 
-  public String toJson() {
-    StringBuilder builder =
-        new StringBuilder(
-            "{\"time_unix_nano\":" + this.timestamp + ",\"name\":\"" + this.name + "\"");
-    if (!this.attributes.isEmpty()) {
-      builder.append(",\"attributes\":").append(this.attributes);
-    }
-    return builder.append('}').toString();
-  }
-
   @Override
   public String toString() {
     return "OtelSpanEvent{timestamp="
         + this.timestamp
         + ", name='"
         + this.name
-        + "', attributes='"
+        + "', attributes="
         + this.attributes
-        + "'}";
+        + '}';
   }
 }

dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/TrackingSpanDecorator.groovy

@@ -9,6 +9,7 @@ import datadog.trace.api.interceptor.MutableSpan
 import datadog.trace.bootstrap.CallDepthThreadLocalMap
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext
+import datadog.trace.bootstrap.instrumentation.api.AgentSpanEvent
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink
 import datadog.trace.core.DDSpan
 
@@ -405,6 +406,11 @@ class TrackingSpanDecorator implements AgentSpan {
     delegate.addLink(link)
   }
 
+  @Override
+  void addEvent(AgentSpanEvent event) {
+    delegate.addEvent(event)
+  }
+
   @Override
   AgentSpan setMetaStruct(String field, Object value) {
     return delegate.setMetaStruct(field, value)

dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV1.java

@@ -17,6 +17,7 @@
 import datadog.trace.api.ProcessTags;
 import datadog.trace.api.TagMap;
 import datadog.trace.api.sampling.SamplingMechanism;
+import datadog.trace.bootstrap.instrumentation.api.AgentSpanEvent;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
 import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags;
 import datadog.trace.bootstrap.instrumentation.api.Tags;
@@ -89,7 +90,7 @@ public void map(List<? extends CoreSpan<?>> trace, Writable writable) {
     }
 
     CoreSpan<?> firstSpan = trace.get(0);
-    firstSpan.processTagsAndBaggageWithStructuredLinks(spanMetadata);
+    firstSpan.processTagsAndBaggageWithStructuredLinksAndEvents(spanMetadata);
     Metadata firstSpanMeta = spanMetadata.metadata;
 
     // encoded fields: 1..7, but skipping #5, as not required by tracers and set by the agent.
@@ -128,7 +129,7 @@ private void encodeSpans(Writable writable, int fieldId, List<? extends CoreSpan
     Metadata meta = spanMetadata.metadata;
     for (CoreSpan<?> span : spans) {
       if (meta == null) {
-        span.processTagsAndBaggageWithStructuredLinks(spanMetadata);
+        span.processTagsAndBaggageWithStructuredLinksAndEvents(spanMetadata);
         meta = spanMetadata.metadata;
       }
       TagMap tags = meta.getTags();
@@ -160,9 +161,9 @@ private void encodeSpans(Writable writable, int fieldId, List<? extends CoreSpan
       // (example values: web, db, lambda)
       encodeString(writable, 10, span.getType());
       // links = 11, a collection of links to other spans
-      encodeSpanLinks(writable, 11, meta.getSpanLinks());
+      encodeSpanLinks(writable, 11, meta.getLinks());
       // events = 12, a collection of events that occurred during this span
-      encodeSpanEvents(writable, 12, tags.getObject(DDTags.SPAN_EVENTS));
+      encodeSpanEvents(writable, 12, meta.getEvents());
       // env = 13, the optional string environment of this span
       encodeString(writable, 13, tags.getString(Tags.ENV));
       // version = 14, the optional string version of this span
@@ -200,48 +201,21 @@ private void encodeSpanLinks(
     }
   }
 
-  private void encodeSpanEvents(Writable writable, int fieldId, Object eventsObject) {
+  private void encodeSpanEvents(
+      Writable writable, int fieldId, List<? extends AgentSpanEvent> events) {
     writable.writeInt(fieldId);
-    if (!(eventsObject instanceof List) || ((List<?>) eventsObject).isEmpty()) {
+    if (events == null || events.isEmpty()) {
       writable.startArray(0);
       return;
     }
 
-    List<?> events = (List<?>) eventsObject;
-    int encodableCount = 0;
-    for (Object event : events) {
-      if (isEncodableSpanEvent(event)) {
-        encodableCount++;
-      }
-    }
-    writable.startArray(encodableCount);
-    for (Object event : events) {
-      if (!(event instanceof Map)) {
-        continue;
-      }
-      Map<?, ?> eventMap = (Map<?, ?>) event;
-      Long timeUnixNano = asLong(eventMap.get("time_unix_nano"));
-      Object nameObject = eventMap.get("name");
-      if (timeUnixNano == null || nameObject == null) {
-        continue;
-      }
-
-      Map<?, ?> attributes =
-          eventMap.get("attributes") instanceof Map ? (Map<?, ?>) eventMap.get("attributes") : null;
-
+    writable.startArray(events.size());
+    for (AgentSpanEvent event : events) {
       writable.startMap(3);
-      encodeLong(writable, 1, timeUnixNano);
-      encodeString(writable, 2, String.valueOf(nameObject));
-      encodeEventAttributes(writable, 3, attributes);
-    }
-  }
-
-  private boolean isEncodableSpanEvent(Object event) {
-    if (!(event instanceof Map)) {
-      return false;
+      encodeLong(writable, 1, event.timeNanos());
+      encodeString(writable, 2, event.name());
+      encodeEventAttributes(writable, 3, event.attributes());
     }
-    Map<?, ?> eventMap = (Map<?, ?>) event;
-    return eventMap.get("name") != null && asLong(eventMap.get("time_unix_nano")) != null;
   }
 
   private void encodeEventAttributes(Writable writable, int fieldId, Map<?, ?> attrs) {
@@ -340,20 +314,6 @@ private boolean isIntegralNumber(Number number) {
     return !(number instanceof Float || number instanceof Double);
   }
 
-  private Long asLong(Object value) {
-    if (value instanceof Number) {
-      return ((Number) value).longValue();
-    }
-    if (value instanceof CharSequence) {
-      try {
-        return Long.parseLong(value.toString());
-      } catch (NumberFormatException ignored) {
-        return null;
-      }
-    }
-    return null;
-  }
-
   private void encodeSpanAttributes(
       Writable writable, int fieldId, Metadata meta, Map<String, Object> metaStruct) {
     TagMap tags = meta.getTags();
@@ -364,9 +324,7 @@ private void encodeSpanAttributes(
     boolean writeTopLevel = meta.topLevel();
     int tagCount = 0;
     for (TagMap.EntryReader entry : tags) {
-      if (!DDTags.SPAN_EVENTS.equals(entry.tag())) {
-        tagCount += getFlatAttributeCount(entry);
-      }
+      tagCount += getFlatAttributeCount(entry);
     }
 
     writable.writeInt(fieldId);
@@ -387,9 +345,6 @@ private void encodeSpanAttributes(
     }
 
     for (TagMap.EntryReader entry : tags) {
-      if (DDTags.SPAN_EVENTS.equals(entry.tag())) {
-        continue;
-      }
       writeFlattenedTagAttribute(writable, entry);
     }
     if (writeHttpStatus) {
@@ -487,7 +442,7 @@ private void writeAttribute(Writable writable, String key, Object value) {
       return;
     }
 
-    if (!(value instanceof String) && value != null) {
+    if (!(value instanceof CharSequence) && value != null) {
       log.debug("Not a string value for key: {}, value: {}", key, value);
     }
     writable.writeInt(VALUE_TYPE_STRING);