Stop repeating global tags as OTLP span attributes

dd-trace-core/src/main/java/datadog/trace/core/otlp/common/OtlpResourceProto.java

@@ -8,11 +8,13 @@
 import static datadog.trace.core.otlp.common.OtlpResourceAttributes.datadogResourceAttributes;
 import static datadog.trace.core.otlp.common.OtlpResourceAttributes.traceResourceAttributes;
 import static datadog.trace.core.otlp.common.OtlpResourceAttributes.visitResourceAttributes;
+import static java.util.Collections.unmodifiableMap;
 
 import datadog.communication.serialization.GrowableBuffer;
 import datadog.communication.serialization.StreamingBuffer;
 import datadog.trace.api.Config;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -39,6 +41,21 @@ private OtlpResourceProto() {}
   public static final byte[] TRACE_RESOURCE_MESSAGE =
       buildResourceMessage(Config.get(), traceResourceAttributes(Config.get()));
 
+  /**
+   * Attributes carried by the OTLP trace resource ({@link #TRACE_RESOURCE_MESSAGE}, and the
+   * equivalent JSON fragment), so span encoders can avoid repeating them on every span.
+   */
+  public static final Map<String, String> TRACE_RESOURCE_ATTRIBUTES =
+      buildResourceAttributes(Config.get(), traceResourceAttributes(Config.get()));
+
+  static Map<String, String> buildResourceAttributes(
+      Config config, Map<String, Object> extraAttributes) {
+    Map<String, String> attributes = new HashMap<>();
+    visitResourceAttributes(
+        config, extraAttributes, (key, value) -> attributes.put(key, String.valueOf(value)));
+    return unmodifiableMap(attributes);
+  }
+
   static byte[] buildResourceMessage(Config config, Map<String, Object> extraAttributes) {
     GrowableBuffer buf = new GrowableBuffer(512);
 

dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpSpanTags.java

@@ -0,0 +1,78 @@
+package datadog.trace.core.otlp.trace;
+
+import datadog.trace.api.TagMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.function.BiConsumer;
+
+/**
+ * Visits the span tags to export over OTLP, leaving out tags the trace resource already carries.
+ *
+ * <p>Tracer-wide tags (global tags, {@code DD_SPAN_TAGS}, {@code env}, ...) are shared by spans as
+ * the frozen read-through parent of each span's {@link TagMap}. Global tags are also exported once
+ * per payload as resource attributes, so repeating them on every span is redundant. The parent's
+ * other "leftover" entries are worked out once per parent and cached; each span then writes its own
+ * local tags followed by whichever leftovers it can still see.
+ *
+ * <p>Spans without a read-through parent (e.g. when tracer-wide tags need interception and are
+ * copied into each span) have all their tags written, as before.
+ *
+ * <p>Not thread-safe: each encoder owns its own instance.
+ */
+final class OtlpSpanTags {
+  private static final TagMap.EntryReader[] NO_ENTRIES = new TagMap.EntryReader[0];
+
+  /** Attributes carried by the trace resource. */
+  private final Map<String, String> resourceAttributes;
+
+  /** Read-through parent the cached leftovers were computed from. */
+  private TagMap cachedParent;
+
+  /** Entries of {@link #cachedParent} not covered by the resource, so still written per span. */
+  private TagMap.EntryReader[] cachedLeftovers = NO_ENTRIES;
+
+  OtlpSpanTags(Map<String, String> resourceAttributes) {
+    this.resourceAttributes = resourceAttributes;
+  }
+
+  <T> void forEach(TagMap tags, T thisObj, BiConsumer<T, ? super TagMap.EntryReader> consumer) {
+    TagMap parent = tags.parent();
+    if (null == parent || resourceAttributes.isEmpty()) {
+      // nothing shared with the resource
+      tags.forEach(thisObj, consumer);
+      return;
+    }
+    tags.forEachLocal(thisObj, consumer);
+    for (TagMap.EntryReader leftover : leftovers(parent)) {
+      // skips leftovers the span holds locally (already written) or removed
+      if (tags.readsThroughToParent(leftover.tag())) {
+        consumer.accept(thisObj, leftover);
+      }
+    }
+  }
+
+  private TagMap.EntryReader[] leftovers(TagMap parent) {
+    if (parent != cachedParent) {
+      List<TagMap.EntryReader> leftovers = new ArrayList<>();
+      parent.forEach(
+          entry -> {
+            if (!coveredByResource(entry)) {
+              leftovers.add(entry);
+            }
+          });
+      cachedLeftovers = leftovers.toArray(NO_ENTRIES);
+      cachedParent = parent;
+    }
+    return cachedLeftovers;
+  }
+
+  /**
+   * Covered when the resource has the same key and value. Only object entries qualify, since those
+   * are written as string attributes just like the resource; numeric and boolean entries keep their
+   * type on the span.
+   */
+  private boolean coveredByResource(TagMap.EntryReader entry) {
+    return entry.isObject() && entry.stringValue().equals(resourceAttributes.get(entry.tag()));
+  }
+}

dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJson.java

@@ -192,12 +192,15 @@ private static void writeSpanTag(JsonWriter writer, UTF8BytesString key, long va
 
   public static class MetaWriter implements MetadataConsumer {
     private final JsonWriter writer;
+    private final OtlpSpanTags spanTags;
 
     private boolean includeProcessTags;
     private boolean includeSamplingTags;
 
-    public MetaWriter(JsonWriter writer) {
+    /** {@code spanTags} is owned by the collector so its cache survives across payloads. */
+    MetaWriter(JsonWriter writer, OtlpSpanTags spanTags) {
       this.writer = writer;
+      this.spanTags = spanTags;
     }
 
     /** Call this to ensure process tags are written out for the next span. */
@@ -247,7 +250,7 @@ public void accept(Metadata metadata) {
         writeSpanTag(writer, PROCESS_TAGS_KEY, metadata.processTags());
       }
 
-      metadata.getTags().forEach(writer, OtlpTraceJson::writeSpanTag);
+      spanTags.forEach(metadata.getTags(), writer, OtlpTraceJson::writeSpanTag);
 
       // reset for next span
       includeProcessTags = false;

dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJsonCollector.java

@@ -4,6 +4,7 @@
 import static datadog.trace.core.otlp.common.OtlpPayload.JSON_CONTENT_TYPE;
 import static datadog.trace.core.otlp.common.OtlpProtoBuffer.MAX_CAPACITY_BYTES;
 import static datadog.trace.core.otlp.common.OtlpResourceJson.TRACE_RESOURCE_FRAGMENT;
+import static datadog.trace.core.otlp.common.OtlpResourceProto.TRACE_RESOURCE_ATTRIBUTES;
 import static datadog.trace.core.otlp.trace.OtlpTraceJson.writeSpan;
 
 import datadog.json.JsonWriter;
@@ -36,6 +37,8 @@ public final class OtlpTraceJsonCollector extends OtlpTraceCollector {
   private static final OtelInstrumentationScope DEFAULT_TRACE_SCOPE =
       new OtelInstrumentationScope("", null, null);
 
+  private final OtlpSpanTags spanTags = new OtlpSpanTags(TRACE_RESOURCE_ATTRIBUTES);
+
   private JsonWriter writer;
   private OtlpTraceJson.MetaWriter metaWriter;
 
@@ -91,7 +94,7 @@ public OtlpPayload collectTraces() {
   /** Prepare temporary elements to collect trace data. */
   private void start() {
     writer = new JsonWriter();
-    metaWriter = new OtlpTraceJson.MetaWriter(writer);
+    metaWriter = new OtlpTraceJson.MetaWriter(writer, spanTags);
 
     writer.beginObject();
     writer.name("resourceSpans").beginArray();

dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceProto.java

@@ -26,6 +26,7 @@
 import static datadog.trace.core.otlp.common.OtlpCommonProto.writeString;
 import static datadog.trace.core.otlp.common.OtlpCommonProto.writeTag;
 import static datadog.trace.core.otlp.common.OtlpCommonProto.writeVarInt;
+import static datadog.trace.core.otlp.common.OtlpResourceProto.TRACE_RESOURCE_ATTRIBUTES;
 import static datadog.trace.core.otlp.common.OtlpTraceFlags.NO_TRACE_FLAGS;
 import static datadog.trace.core.otlp.common.OtlpTraceFlags.REMOTE_TRACE_FLAG;
 import static datadog.trace.core.otlp.common.OtlpTraceFlags.SAMPLED_TRACE_FLAG;
@@ -269,6 +270,7 @@ private static void writeSpanTag(StreamingBuffer buf, UTF8BytesString key, long
 
   public static class MetaWriter implements MetadataConsumer {
     private final StreamingBuffer buf;
+    private final OtlpSpanTags spanTags = new OtlpSpanTags(TRACE_RESOURCE_ATTRIBUTES);
 
     private boolean includeProcessTags;
     private boolean includeSamplingTags;
@@ -324,7 +326,7 @@ public void accept(Metadata metadata) {
         writeSpanTag(buf, PROCESS_TAGS_KEY, metadata.processTags());
       }
 
-      metadata.getTags().forEach(buf, OtlpTraceProto::writeSpanTag);
+      spanTags.forEach(metadata.getTags(), buf, OtlpTraceProto::writeSpanTag);
 
       // reset for next span
       includeProcessTags = false;

dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpSpanTagsForkedTest.java

@@ -0,0 +1,89 @@
+package datadog.trace.core.otlp.trace;
+
+import static java.util.Arrays.asList;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import datadog.json.JsonMapper;
+import datadog.trace.api.config.GeneralConfig;
+import datadog.trace.api.config.TracerConfig;
+import datadog.trace.common.writer.ListWriter;
+import datadog.trace.core.CoreSpan;
+import datadog.trace.core.CoreTracer;
+import datadog.trace.core.DDSpan;
+import datadog.trace.core.otlp.common.OtlpPayload;
+import datadog.trace.test.junit.utils.config.WithConfig;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.junit.jupiter.api.Test;
+
+/**
+ * End-to-end check that global tags exported as OTLP resource attributes are not repeated on each
+ * span, while span-only tracer tags still are. Forked because the trace resource is built once from
+ * {@code Config} on first use.
+ */
+@WithConfig(key = GeneralConfig.TAGS, value = "team:core,region:us1")
+@WithConfig(key = TracerConfig.SPAN_TAGS, value = "owner:alice")
+class OtlpSpanTagsForkedTest {
+
+  @Test
+  void globalTagsOnlyOnResource() throws IOException {
+    CoreTracer tracer = CoreTracer.builder().writer(new ListWriter()).build();
+    DDSpan span = (DDSpan) tracer.startSpan("test", "op");
+    span.setTag("http.method", "GET");
+    span.setTag("region", "eu1"); // span-level override of a global tag is kept
+    span.finish();
+
+    OtlpTraceJsonCollector collector = new OtlpTraceJsonCollector();
+    collector.addTrace(asList((CoreSpan<?>) span));
+    Map<String, Object> resourceSpans = parse(collector.collectTraces());
+
+    Map<String, Object> resourceAttributes = attributes(resource(resourceSpans));
+    assertEquals("core", resourceAttributes.get("team"));
+    assertEquals("us1", resourceAttributes.get("region"));
+
+    Map<String, Object> spanAttributes = attributes(onlySpan(resourceSpans));
+    assertFalse(spanAttributes.containsKey("team"));
+    assertEquals("eu1", spanAttributes.get("region"));
+    assertEquals("alice", spanAttributes.get("owner"));
+    assertEquals("GET", spanAttributes.get("http.method"));
+  }
+
+  @SuppressWarnings("unchecked")
+  private static Map<String, Object> parse(OtlpPayload payload) throws IOException {
+    byte[] bytes = new byte[payload.getContentLength()];
+    payload.getContent().get(bytes);
+    Map<String, Object> root = JsonMapper.fromJsonToMap(new String(bytes, StandardCharsets.UTF_8));
+    return (Map<String, Object>) ((List<Object>) root.get("resourceSpans")).get(0);
+  }
+
+  @SuppressWarnings("unchecked")
+  private static Map<String, Object> resource(Map<String, Object> resourceSpans) {
+    return (Map<String, Object>) resourceSpans.get("resource");
+  }
+
+  @SuppressWarnings("unchecked")
+  private static Map<String, Object> onlySpan(Map<String, Object> resourceSpans) {
+    List<Object> scopeSpans = (List<Object>) resourceSpans.get("scopeSpans");
+    List<Object> spans = (List<Object>) ((Map<String, Object>) scopeSpans.get(0)).get("spans");
+    assertEquals(1, spans.size());
+    return (Map<String, Object>) spans.get(0);
+  }
+
+  /** Flattens OTLP JSON {@code KeyValue} attributes, failing on duplicate keys. */
+  @SuppressWarnings("unchecked")
+  private static Map<String, Object> attributes(Map<String, Object> owner) {
+    Map<String, Object> result = new HashMap<>();
+    for (Object attribute : (List<Object>) owner.get("attributes")) {
+      Map<String, Object> keyValue = (Map<String, Object>) attribute;
+      Map<String, Object> value = (Map<String, Object>) keyValue.get("value");
+      Object previous = result.put((String) keyValue.get("key"), value.values().iterator().next());
+      assertNull(previous, "duplicate attribute " + keyValue.get("key"));
+    }
+    return result;
+  }
+}