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..361b2fa82ea --- /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)) { + // 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 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 17963da3514..ccc6341d0d1 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 @@ -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; 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 e7b0310bc42..4a66756be00 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 @@ -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; 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..51894835b74 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/otlp/trace/OtlpSpanTagsTest.java @@ -0,0 +1,136 @@ +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 writesLeftoversCopiedFromAnotherSpanOnce() { + TagMap parent = tracerTags(); + TagMap span = TagMap.createFromParent(parent); + // e.g. setAllTags(otherSpan.getTags()): copies share the parent's immutable Entry instances + span.putAll(TagMap.createFromParent(parent)); + + Map 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()); + 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 52d2ef32a2b..f253fda2799 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -1953,6 +1953,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; @@ -2049,6 +2067,23 @@ 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; + } + + /** + * 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 b424c5793ce..dfca288ee09 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,47 @@ 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 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(); + 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());