From a62b2858821412a03910cc97e918562b32c0fc5f Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 9 Oct 2026 10:02:03 -0400 Subject: [PATCH 1/2] Stop repeating global tags as OTLP span attributes Global tags are exported once per payload as OTLP resource attributes, but were also written as attributes on every span because they live in each span's read-through parent TagMap. The OTLP span encoders now write a span's local tags plus only those parent entries the trace resource does not already carry (cached per parent), keeping DD_SPAN_TAGS, env and tracer flags on spans. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../core/otlp/common/OtlpResourceProto.java | 17 +++ .../trace/core/otlp/trace/OtlpSpanTags.java | 78 +++++++++++ .../trace/core/otlp/trace/OtlpTraceJson.java | 7 +- .../otlp/trace/OtlpTraceJsonCollector.java | 5 +- .../trace/core/otlp/trace/OtlpTraceProto.java | 4 +- .../otlp/trace/OtlpSpanTagsForkedTest.java | 89 +++++++++++++ .../core/otlp/trace/OtlpSpanTagsTest.java | 125 ++++++++++++++++++ .../main/java/datadog/trace/api/TagMap.java | 24 ++++ .../trace/api/TagMapReadThroughTest.java | 25 ++++ 9 files changed, 370 insertions(+), 4 deletions(-) create mode 100644 dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpSpanTags.java create mode 100644 dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpSpanTagsForkedTest.java create mode 100644 dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpSpanTagsTest.java diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/common/OtlpResourceProto.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/common/OtlpResourceProto.java index 3a2d4c0c460..d887d5b05d8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/otlp/common/OtlpResourceProto.java +++ b/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 TRACE_RESOURCE_ATTRIBUTES = + buildResourceAttributes(Config.get(), traceResourceAttributes(Config.get())); + + static Map buildResourceAttributes( + Config config, Map extraAttributes) { + Map attributes = new HashMap<>(); + visitResourceAttributes( + config, extraAttributes, (key, value) -> attributes.put(key, String.valueOf(value))); + return unmodifiableMap(attributes); + } + static byte[] buildResourceMessage(Config config, Map extraAttributes) { GrowableBuffer buf = new GrowableBuffer(512); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpSpanTags.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpSpanTags.java new file mode 100644 index 00000000000..cf159c02098 --- /dev/null +++ b/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. + * + *

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. + * + *

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. + * + *

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 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 resourceAttributes) { + this.resourceAttributes = resourceAttributes; + } + + void forEach(TagMap tags, T thisObj, BiConsumer 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)) { + // identity check: skips leftovers the span overrode locally (already written) or removed + if (tags.getEntry(leftover.tag()) == leftover) { + consumer.accept(thisObj, leftover); + } + } + } + + private TagMap.EntryReader[] leftovers(TagMap parent) { + if (parent != cachedParent) { + List 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())); + } +} diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJson.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJson.java index 845f870b965..6330c948e75 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJson.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJson.java @@ -170,12 +170,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. */ @@ -220,7 +223,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; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJsonCollector.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJsonCollector.java index cbb39ed615b..d4f593cf8a5 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJsonCollector.java +++ b/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(); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceProto.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceProto.java index 707225cf5a6..74a5f0ff1a8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceProto.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceProto.java @@ -27,6 +27,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; @@ -247,6 +248,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; @@ -297,7 +299,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; diff --git a/dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpSpanTagsForkedTest.java b/dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpSpanTagsForkedTest.java new file mode 100644 index 00000000000..7c4bb288070 --- /dev/null +++ b/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 resourceSpans = parse(collector.collectTraces()); + + Map resourceAttributes = attributes(resource(resourceSpans)); + assertEquals("core", resourceAttributes.get("team")); + assertEquals("us1", resourceAttributes.get("region")); + + Map 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 parse(OtlpPayload payload) throws IOException { + byte[] bytes = new byte[payload.getContentLength()]; + payload.getContent().get(bytes); + Map root = JsonMapper.fromJsonToMap(new String(bytes, StandardCharsets.UTF_8)); + return (Map) ((List) root.get("resourceSpans")).get(0); + } + + @SuppressWarnings("unchecked") + private static Map resource(Map resourceSpans) { + return (Map) resourceSpans.get("resource"); + } + + @SuppressWarnings("unchecked") + private static Map onlySpan(Map resourceSpans) { + List scopeSpans = (List) resourceSpans.get("scopeSpans"); + List spans = (List) ((Map) scopeSpans.get(0)).get("spans"); + assertEquals(1, spans.size()); + return (Map) spans.get(0); + } + + /** Flattens OTLP JSON {@code KeyValue} attributes, failing on duplicate keys. */ + @SuppressWarnings("unchecked") + private static Map attributes(Map owner) { + Map result = new HashMap<>(); + for (Object attribute : (List) owner.get("attributes")) { + Map keyValue = (Map) attribute; + Map value = (Map) keyValue.get("value"); + Object previous = result.put((String) keyValue.get("key"), value.values().iterator().next()); + assertNull(previous, "duplicate attribute " + keyValue.get("key")); + } + return result; + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpSpanTagsTest.java b/dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpSpanTagsTest.java new file mode 100644 index 00000000000..862e7ec89ea --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpSpanTagsTest.java @@ -0,0 +1,125 @@ +package datadog.trace.core.otlp.trace; + +import static java.util.Collections.emptyMap; +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.trace.api.TagMap; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class OtlpSpanTagsTest { + + private static final Map RESOURCE_TAGS = resourceTags(); + + private static Map resourceTags() { + Map tags = new HashMap<>(); + tags.put("team", "core"); + tags.put("region", "us1"); + return tags; + } + + /** Mirrors the tracer-wide parent: global tags plus DD_SPAN_TAGS, env and tracer flags. */ + private static TagMap tracerTags() { + TagMap parent = TagMap.create(); + parent.set("team", "core"); + parent.set("region", "us1"); + parent.set("owner", "alice"); + parent.set("env", "prod"); + parent.set("_dd.dsm.enabled", 1); + return parent.freeze(); + } + + /** Collects visited tags, failing if any key is visited twice. */ + private static Map collect(OtlpSpanTags spanTags, TagMap tags) { + Map out = new LinkedHashMap<>(); + spanTags.forEach( + tags, + out, + (m, e) -> { + Object previous = m.put(e.tag(), e.objectValue()); + assertNull(previous, "duplicate attribute " + e.tag()); + }); + return out; + } + + @Test + void skipsParentTagsCarriedByResource() { + TagMap span = TagMap.createFromParent(tracerTags()); + span.set("http.method", "GET"); + + Map expected = new HashMap<>(); + expected.put("http.method", "GET"); + expected.put("owner", "alice"); + expected.put("env", "prod"); + expected.put("_dd.dsm.enabled", 1); + assertEquals(expected, collect(new OtlpSpanTags(RESOURCE_TAGS), span)); + } + + @Test + void writesLocalOverridesOnce() { + TagMap span = TagMap.createFromParent(tracerTags()); + span.set("team", "payments"); // overrides a resource-covered key + span.set("owner", "bob"); // overrides a leftover key + + Map tags = collect(new OtlpSpanTags(RESOURCE_TAGS), span); + assertEquals("payments", tags.get("team")); + assertEquals("bob", tags.get("owner")); + assertFalse(tags.containsKey("region")); + } + + @Test + void skipsLeftoversRemovedFromSpan() { + TagMap span = TagMap.createFromParent(tracerTags()); + span.remove("owner"); + + Map tags = collect(new OtlpSpanTags(RESOURCE_TAGS), span); + assertFalse(tags.containsKey("owner")); + assertEquals("prod", tags.get("env")); + } + + @Test + void writesEverythingWithoutParent() { + TagMap span = TagMap.create(); + span.putAll(tracerTags()); + span.set("http.method", "GET"); + + assertEquals(6, collect(new OtlpSpanTags(RESOURCE_TAGS), span).size()); + } + + @Test + void writesEverythingWhenResourceHasNoGlobalTags() { + TagMap span = TagMap.createFromParent(tracerTags()); + + assertEquals(5, collect(new OtlpSpanTags(emptyMap()), span).size()); + } + + @Test + void keepsTypedEntriesWhoseStringFormMatchesResource() { + Map resource = new HashMap<>(RESOURCE_TAGS); + resource.put("_dd.dsm.enabled", "1"); + TagMap span = TagMap.createFromParent(tracerTags()); + + // resource only has the string "1", so the numeric entry is still written to keep its type + assertEquals(1, collect(new OtlpSpanTags(resource), span).get("_dd.dsm.enabled")); + } + + @Test + void recomputesLeftoversWhenParentChanges() { + OtlpSpanTags spanTags = new OtlpSpanTags(RESOURCE_TAGS); + assertFalse(collect(spanTags, TagMap.createFromParent(tracerTags())).containsKey("team")); + + // e.g. remote config replaced the tracer tags: the static resource still says team=core + TagMap updated = TagMap.create(); + updated.set("team", "payments"); + updated.set("region", "us1"); + TagMap span = TagMap.createFromParent(updated.freeze()); + + Map tags = collect(spanTags, span); + assertEquals("payments", tags.get("team")); + assertFalse(tags.containsKey("region")); + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index 2f2423d5239..00b38520df3 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -1900,6 +1900,24 @@ public void forEach(T thisObj, BiConsumer con } } + /** + * Visits only this map's own (local) entries, skipping everything read through from the parent + * chain. Pair with {@link #parent()} when the caller handles the shared parent entries itself. + */ + public void forEachLocal(T thisObj, BiConsumer consumer) { + Object[] thisBuckets = this.buckets; + for (int i = 0; i < thisBuckets.length; ++i) { + Object thisBucket = thisBuckets[i]; + if (thisBucket instanceof Entry) { + Entry thisEntry = (Entry) thisBucket; + consumer.accept(thisObj, thisEntry); + } else if (thisBucket instanceof BucketGroup) { + BucketGroup thisGroup = (BucketGroup) thisBucket; + thisGroup.forEachInChain(thisObj, consumer); + } + } + } + private void forEachParent(T thisObj, BiConsumer consumer) { for (TagMap ancestor = this.parent; ancestor != null; ancestor = ancestor.parent) { Object[] parentBuckets = ancestor.buckets; @@ -1996,6 +2014,12 @@ public boolean isFrozen() { return this.frozen; } + /** The frozen read-through parent, or {@code null} when this map has none. */ + @Nullable + public TagMap parent() { + return this.parent; + } + public void checkWriteAccess() { if (this.frozen) throw new IllegalStateException("TagMap frozen"); } diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java index b424c5793ce..077cfedc907 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -192,6 +193,30 @@ void biConsumerForEachAlsoEmitsUnion() { assertEquals("child-c", out.get("c")); } + @Test + void forEachLocalSkipsParentEntries() { + TagMap child = TagMap.createFromParent(frozenParent()); // parent {a, b} + child.set("b", "child-b"); // shadows parent "b" + child.set("c", "child-c"); + child.remove("a"); // tombstone only affects parent entries + + Map out = new HashMap<>(); + child.forEachLocal(out, (m, e) -> m.put(e.tag(), e.objectValue())); + assertEquals(2, out.size()); + assertEquals("child-b", out.get("b")); + assertEquals("child-c", out.get("c")); + } + + @Test + void parentExposesAttachedParent() { + TagMap parent = frozenParent(); + TagMap child = TagMap.createFromParent(parent); + + assertSame(parent, child.parent()); + assertNull(parent.parent()); + assertNull(TagMap.createFromParent(TagMap.create().freeze()).parent()); // empty is dropped + } + @Test void sizeIsExactUnion() { TagMap child = TagMap.createFromParent(frozenParent()); From 85cfb8bc5048719d7b36502410d0b61e64866ac9 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 9 Oct 2026 10:54:51 -0400 Subject: [PATCH 2/2] Skip OTLP leftovers the span holds locally, even as a shared Entry TagMap shares immutable Entry instances across maps, so copying another span's tags (e.g. setAllTags(otherSpan.getTags())) can install the shared parent's own Entry locally. The identity check then also emitted it as a leftover, writing duplicate OTLP attribute keys. Check whether the key reads through to the parent instead (no local entry, not removed), which also replaces getEntry's local+parent probe with a single local probe. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../trace/core/otlp/trace/OtlpSpanTags.java | 4 ++-- .../trace/core/otlp/trace/OtlpSpanTagsTest.java | 11 +++++++++++ .../src/main/java/datadog/trace/api/TagMap.java | 11 +++++++++++ .../trace/api/TagMapReadThroughTest.java | 17 +++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpSpanTags.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpSpanTags.java index cf159c02098..361b2fa82ea 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpSpanTags.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpSpanTags.java @@ -45,8 +45,8 @@ void forEach(TagMap tags, T thisObj, BiConsumer tags = collect(new OtlpSpanTags(RESOURCE_TAGS), span); + assertEquals("alice", tags.get("owner")); // collect() fails on duplicate keys + } + @Test void skipsLeftoversRemovedFromSpan() { TagMap span = TagMap.createFromParent(tracerTags()); diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index 00b38520df3..8e04e10c045 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -2020,6 +2020,17 @@ public TagMap parent() { return this.parent; } + /** + * Whether a lookup of {@code tag} falls through to the parent chain: there is a parent, no local + * entry shadows it (even one sharing the parent's {@link Entry} instance), and it was not removed + * locally. Does not check whether the parent actually holds {@code tag}. + */ + public boolean readsThroughToParent(String tag) { + return this.parent != null + && this.getLocalEntry(tag) == null + && (this.removedFromParent == null || !this.removedFromParent.contains(tag)); + } + public void checkWriteAccess() { if (this.frozen) throw new IllegalStateException("TagMap frozen"); } diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java index 077cfedc907..dfca288ee09 100644 --- a/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java +++ b/internal-api/src/test/java/datadog/trace/api/TagMapReadThroughTest.java @@ -207,6 +207,23 @@ void forEachLocalSkipsParentEntries() { assertEquals("child-c", out.get("c")); } + @Test + void readsThroughToParentUnlessShadowedOrRemoved() { + TagMap parent = frozenParent(); // {a, b} + TagMap child = TagMap.createFromParent(parent); + child.set("c", "child-c"); + child.set(parent.getEntry("b")); // local copy sharing the parent's Entry instance + + assertTrue(child.readsThroughToParent("a")); + assertFalse(child.readsThroughToParent("b")); // shadowed locally, even by the same Entry + assertFalse(child.readsThroughToParent("c")); // local only + assertTrue(child.readsThroughToParent("missing")); // would fall through, parent lacks it + + child.remove("a"); // tombstone + assertFalse(child.readsThroughToParent("a")); + assertFalse(parent.readsThroughToParent("a")); // no parent + } + @Test void parentExposesAttachedParent() { TagMap parent = frozenParent();