diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 8f52b49052c..e2f327f97b3 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -221,11 +221,12 @@ private static final class LocalRootSpanTags { */ private final boolean apmTracingEnabled; - /** A set of tags that are added to every span */ + /** + * Span defaults added to every span: {@code DD_SPAN_TAGS}, or the tags given to the builder. + * Unlike the global tags ({@code DD_TAGS}), remote config doesn't replace these. + */ private final TagMap defaultSpanTags; - private final boolean defaultSpanTagsNeedsIntercept; - /** number of spans in a pending trace before they get flushed */ private final int partialFlushMinSpans; @@ -543,7 +544,7 @@ public CoreTracerBuilder config(final Config config) { invertMap(config.getBaggageMapping()))); // Explicitly skip setting scope manager because it depends on statsDClient localRootSpanTags(config.getLocalRootSpanTags()); - defaultSpanTags(withTracerTags(config.getMergedSpanTags(), config, null)); + defaultSpanTags(config.getSpanTags()); serviceNameMappings(config.getServiceMapping()); taggedHeaders(config.getRequestHeaderTags()); baggageMapping(config.getBaggageMapping()); @@ -737,7 +738,6 @@ private CoreTracer( null == tagInterceptor ? new TagInterceptor(new RuleFlags(config)) : tagInterceptor; this.defaultSpanTags = defaultSpanTags; - this.defaultSpanTagsNeedsIntercept = this.tagInterceptor.needsIntercept(this.defaultSpanTags); this.dynamicConfig = DynamicConfig.create(ConfigSnapshot::new) @@ -751,7 +751,7 @@ private CoreTracer( .setTraceSampleRate(config.getTraceSampleRate()) .setSpanSamplingRules(spanSamplingRules.getRules()) .setTraceSamplingRules(traceSamplingRules.getRules(), traceSamplingRulesJson) - .setTracingTags(config.getMergedSpanTags()) + .setTracingTags(config.getGlobalTags()) .setDataStreamsTransactionExtractors(dataStreamsTransactionExtractors.getExtractors()) .apply(); @@ -2590,7 +2590,25 @@ public void run() { protected class ConfigSnapshot extends DynamicConfig.Snapshot { final Sampler sampler; + /** + * The global tags ({@code DD_TAGS}, or their remote-config override), as given. These are what + * an OTLP export carries as resource attributes. + */ + final TagMap globalTags; + + /** + * The span defaults with the tracer-defined tags ({@code env}, {@code _dd.dsm.enabled}, ...), + * but without the global tags. + */ + final TagMap spanDefaultTags; + + /** + * The tags every span starts with: the global tags merged with the span defaults (span defaults + * win), then given the tracer-defined tags. {@code version} is dropped here even when it's a + * global tag, unless it's a split-by tag. + */ final TagMap mergedTracerTags; + final boolean mergedTracerTagsNeedsIntercept; protected ConfigSnapshot( @@ -2607,14 +2625,22 @@ protected ConfigSnapshot( sampler = Sampler.Builder.forConfig(CoreTracer.this.initialConfig, this, agentSampler); } - if (null == oldSnapshot) { - mergedTracerTags = CoreTracer.this.defaultSpanTags.immutableCopy(); - this.mergedTracerTagsNeedsIntercept = CoreTracer.this.defaultSpanTagsNeedsIntercept; - } else if (getTracingTags().equals(oldSnapshot.getTracingTags())) { + if (null != oldSnapshot && getTracingTags().equals(oldSnapshot.getTracingTags())) { + globalTags = oldSnapshot.globalTags; + spanDefaultTags = oldSnapshot.spanDefaultTags; mergedTracerTags = oldSnapshot.mergedTracerTags; mergedTracerTagsNeedsIntercept = oldSnapshot.mergedTracerTagsNeedsIntercept; } else { - mergedTracerTags = withTracerTags(getTracingTags(), CoreTracer.this.initialConfig, this); + Config config = CoreTracer.this.initialConfig; + TagMap defaultSpanTags = CoreTracer.this.defaultSpanTags; + globalTags = TagMap.fromMapImmutable(getTracingTags()); + spanDefaultTags = withTracerTags(defaultSpanTags, config, this); + + // span defaults take precedence over global tags + TagMap userSpanTags = TagMap.create(globalTags.size() + defaultSpanTags.size()); + userSpanTags.putAll(globalTags); + userSpanTags.putAll(defaultSpanTags); + mergedTracerTags = withTracerTags(userSpanTags, config, this); mergedTracerTagsNeedsIntercept = CoreTracer.this.tagInterceptor.needsIntercept(mergedTracerTags); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 9281c377afb..5f3ab56b602 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -1529,11 +1529,14 @@ void processTagsAndBaggage( : serializedPropagationTags.createTagMap(); } + CoreTracer.ConfigSnapshot traceConfig = traceCollector.getTraceConfig(); consumer.accept( new Metadata( threadId, threadName, unsafeTags, + traceConfig.globalTags, + traceConfig.spanDefaultTags, baggageItemsWithPropagationTags, getSamplingPriority(), measured, diff --git a/dd-trace-core/src/main/java/datadog/trace/core/Metadata.java b/dd-trace-core/src/main/java/datadog/trace/core/Metadata.java index d10187d1861..3bf2a3fe9b5 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/Metadata.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/Metadata.java @@ -14,6 +14,8 @@ public final class Metadata { private final UTF8BytesString threadName; private final int httpStatusCode; private final TagMap tags; + private final TagMap globalTags; + private final TagMap spanDefaultTags; private final Map baggage; private final int samplingPriority; @@ -30,6 +32,8 @@ public Metadata( long threadId, UTF8BytesString threadName, TagMap tags, + TagMap globalTags, + TagMap spanDefaultTags, Map baggage, int samplingPriority, boolean measured, @@ -45,6 +49,8 @@ public Metadata( this.threadName = threadName; this.httpStatusCode = httpStatusCode; this.tags = tags; + this.globalTags = globalTags; + this.spanDefaultTags = spanDefaultTags; this.baggage = baggage; this.samplingPriority = samplingPriority; this.measured = measured; @@ -98,6 +104,23 @@ public TagMap getTags() { return this.tags; } + /** + * The trace's global tags ({@code DD_TAGS}, or their remote-config override). {@link #getTags()} + * already includes them, below the span defaults and the span's own tags; this lets a serializer + * that writes them once per payload, such as an OTLP resource, tell them apart. + */ + public TagMap getGlobalTags() { + return globalTags; + } + + /** + * The trace's span defaults ({@code DD_SPAN_TAGS} and tracer-defined tags such as {@code env}), + * without the global tags. {@link #getTags()} already includes them, below the span's own tags. + */ + public TagMap getSpanDefaultTags() { + return spanDefaultTags; + } + public Map getBaggage() { return baggage; } diff --git a/dd-trace-core/src/test/java/datadog/trace/common/writer/FileBasedPayloadDispatcherTest.java b/dd-trace-core/src/test/java/datadog/trace/common/writer/FileBasedPayloadDispatcherTest.java index 2704dff3d3e..f116a1589f0 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/writer/FileBasedPayloadDispatcherTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/writer/FileBasedPayloadDispatcherTest.java @@ -307,6 +307,8 @@ private static CoreSpan mockSpan(CharSequence type, Map tags) Thread.currentThread().getId(), null, TagMap.fromMap(tags), + TagMap.EMPTY, + TagMap.EMPTY, Collections.emptyMap(), 0, false, diff --git a/dd-trace-core/src/test/java/datadog/trace/common/writer/PayloadDispatcherImplTest.java b/dd-trace-core/src/test/java/datadog/trace/common/writer/PayloadDispatcherImplTest.java index cf756003e7c..44467412346 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/writer/PayloadDispatcherImplTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/writer/PayloadDispatcherImplTest.java @@ -181,6 +181,12 @@ void testTraceAndSpanCountsAreResetAfterAccess() { } DDSpan realSpan() throws Exception { + // A real config snapshot, since serialization reads the trace's tag levels from it. Its type + // isn't visible from this package, so it's held as an Object and matched by name below. + CoreTracer tracer = CoreTracer.builder().writer(new ListWriter()).build(); + Object traceConfig = tracer.captureTraceConfig(); + tracer.close(); + // getTracer() and mapServiceName() are package-private in TraceCollector; use a custom // Answer to handle them at runtime without compile-time accessibility issues PendingTrace trace = @@ -193,6 +199,9 @@ DDSpan realSpan() throws Exception { // behavior) return mock(CoreTracer.class); } + if (returnType.getSimpleName().equals("ConfigSnapshot")) { + return traceConfig; + } if (returnType == String.class) { Object[] args = invocation.getArguments(); // mapServiceName(String) - return the argument unchanged diff --git a/dd-trace-core/src/test/java/datadog/trace/common/writer/TraceGenerator.java b/dd-trace-core/src/test/java/datadog/trace/common/writer/TraceGenerator.java index c570b2ae7e4..c4ea0720df3 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/writer/TraceGenerator.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/writer/TraceGenerator.java @@ -238,6 +238,8 @@ public PojoSpan( Thread.currentThread().getId(), UTF8BytesString.create(Thread.currentThread().getName()), TagMap.fromMap(tags), + TagMap.EMPTY, + TagMap.EMPTY, baggage, samplingPriority, measured, diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index 666431d8f1f..a891203838f 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -6,7 +6,9 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeFalse; import static org.mockito.ArgumentMatchers.any; @@ -50,6 +52,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Properties; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import okhttp3.HttpUrl; @@ -597,6 +600,163 @@ void verifyConfigurationPollingWithCustomTags( } } + /** + * Remote config {@code tracing_tags} overrides the global tags ({@code DD_TAGS}), and removing + * the override restores them, matching system-tests' {@code + * TestDynamicConfigV2.test_tracing_client_tracing_tags}. Span tags ({@code DD_SPAN_TAGS}) are + * separate local config, so they're kept alongside the override. + */ + @TableTest({ + "scenario | localTagsKey | expectedWithOverride ", + "DD_TAGS | tags | [rc_key: rc_val] ", + "DD_SPAN_TAGS | trace.span.tags | [local_key: local_val, rc_key: rc_val]" + }) + void remoteConfigTracingTagsOverrideGlobalTags( + String scenario, String localTagsKey, Map expectedWithOverride) + throws Exception { + ParsedConfigKey key = ParsedConfigKey.parse("datadog/2/APM_TRACING/config_overrides/config"); + ConfigurationPoller poller = mock(ConfigurationPoller.class); + SharedCommunicationObjects sco = createScoWithPoller(poller); + + ProductListener[] capturedUpdater = {null}; + doAnswer( + inv -> { + capturedUpdater[0] = inv.getArgument(1, ProductListener.class); + return null; + }) + .when(poller) + .addListener(eq(Product.APM_TRACING), any(ProductListener.class)); + + Properties properties = new Properties(); + properties.setProperty(localTagsKey, "local_key:local_val"); + CoreTracer tracer = + CoreTracer.builder() + .withProperties(properties) + .sharedCommunicationObjects(sco) + .pollForTracingConfiguration() + .build(); + unclosedTracers.add(tracer); + + try { + Map localTags = buildStringMap("local_key", "local_val"); + assertEquals(localTags, tracer.captureTraceConfig().mergedTracerTags); + + String json = "{\"lib_config\":{\"tracing_tags\": [\"rc_key:rc_val\"]}}"; + capturedUpdater[0].accept(key, json.getBytes(StandardCharsets.UTF_8), null); + capturedUpdater[0].commit(null); + + assertEquals(expectedWithOverride, tracer.captureTraceConfig().mergedTracerTags); + + capturedUpdater[0].remove(key, null); + capturedUpdater[0].commit(null); + + assertEquals(localTags, tracer.captureTraceConfig().mergedTracerTags); + } finally { + tracer.close(); + } + } + + /** + * The config snapshot keeps the global tags apart from the span defaults, so an OTLP export can + * carry the global tags as resource attributes; remote config replaces only the global tags. + */ + @Test + void configSnapshotSeparatesGlobalTagsFromSpanDefaults() throws Exception { + ParsedConfigKey key = ParsedConfigKey.parse("datadog/2/APM_TRACING/config_overrides/config"); + ConfigurationPoller poller = mock(ConfigurationPoller.class); + SharedCommunicationObjects sco = createScoWithPoller(poller); + + ProductListener[] capturedUpdater = {null}; + doAnswer( + inv -> { + capturedUpdater[0] = inv.getArgument(1, ProductListener.class); + return null; + }) + .when(poller) + .addListener(eq(Product.APM_TRACING), any(ProductListener.class)); + + Properties properties = new Properties(); + properties.setProperty(GeneralConfig.TAGS, "global_key:global_val,shared_key:global_val"); + properties.setProperty(TracerConfig.SPAN_TAGS, "span_key:span_val,shared_key:span_val"); + properties.setProperty(GeneralConfig.ENV, "prod"); + CoreTracer tracer = + CoreTracer.builder() + .withProperties(properties) + .sharedCommunicationObjects(sco) + .pollForTracingConfiguration() + .build(); + unclosedTracers.add(tracer); + + try { + Map spanDefaults = + buildStringMap("span_key", "span_val", "shared_key", "span_val", "env", "prod"); + + ConfigSnapshot config = tracer.captureTraceConfig(); + // Config adds DD_ENV to the global tags + assertEquals( + buildStringMap("global_key", "global_val", "shared_key", "global_val", "env", "prod"), + config.globalTags); + assertEquals(spanDefaults, config.spanDefaultTags); + assertEquals( + buildStringMap( + "global_key", "global_val", + "span_key", "span_val", + "shared_key", "span_val", + "env", "prod"), + config.mergedTracerTags); + + String json = "{\"lib_config\":{\"tracing_tags\": [\"rc_key:rc_val\"]}}"; + capturedUpdater[0].accept(key, json.getBytes(StandardCharsets.UTF_8), null); + capturedUpdater[0].commit(null); + + config = tracer.captureTraceConfig(); + assertEquals(buildStringMap("rc_key", "rc_val"), config.globalTags); + assertEquals(spanDefaults, config.spanDefaultTags); + assertEquals( + buildStringMap( + "rc_key", "rc_val", + "span_key", "span_val", + "shared_key", "span_val", + "env", "prod"), + config.mergedTracerTags); + } finally { + tracer.close(); + } + } + + /** + * Serializers get the global tags and span defaults of the snapshot the trace started with, so a + * config change mid-trace can't split one trace across two sets. + */ + @Test + void metadataCarriesTheTraceConfigTagLevels() { + Properties properties = new Properties(); + properties.setProperty(GeneralConfig.TAGS, "global_key:global_val"); + properties.setProperty(TracerConfig.SPAN_TAGS, "span_key:span_val"); + CoreTracer tracer = CoreTracer.builder().withProperties(properties).build(); + unclosedTracers.add(tracer); + + try { + ConfigSnapshot traceStartConfig = tracer.captureTraceConfig(); + DDSpan span = (DDSpan) tracer.buildSpan("datadog", "my_span").start(); + + Properties changed = new Properties(); + changed.setProperty(GeneralConfig.TAGS, "other_key:other_val"); + tracer.rebuildTraceConfig(Config.get(changed)); + assertNotSame(traceStartConfig.globalTags, tracer.captureTraceConfig().globalTags); + + Metadata[] captured = {null}; + span.processTagsAndBaggage(metadata -> captured[0] = metadata, 0); + + assertSame(traceStartConfig.globalTags, captured[0].getGlobalTags()); + assertSame(traceStartConfig.spanDefaultTags, captured[0].getSpanDefaultTags()); + assertEquals(buildStringMap("global_key", "global_val"), captured[0].getGlobalTags()); + assertEquals(buildStringMap("span_key", "span_val"), captured[0].getSpanDefaultTags()); + } finally { + tracer.close(); + } + } + static final String ACTION_JSON = "'{\"action\": \"enable\", \"lib_config\":" + "{\"tracing_sampling_rate\": null," diff --git a/dd-trace-core/src/test/java/datadog/trace/core/MetadataTest.java b/dd-trace-core/src/test/java/datadog/trace/core/MetadataTest.java index bbc5c607233..d844c42ea54 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/MetadataTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/MetadataTest.java @@ -54,6 +54,8 @@ private static Metadata metadataWithStatus(int status) { Thread.currentThread().getId(), UTF8BytesString.create("main"), TagMap.fromMap(emptyMap()), + TagMap.EMPTY, + TagMap.EMPTY, emptyMap(), 0, false, diff --git a/dd-trace-core/src/traceAgentTest/java/TraceGenerator.java b/dd-trace-core/src/traceAgentTest/java/TraceGenerator.java index 0fe52affee4..1050191cff4 100644 --- a/dd-trace-core/src/traceAgentTest/java/TraceGenerator.java +++ b/dd-trace-core/src/traceAgentTest/java/TraceGenerator.java @@ -175,6 +175,8 @@ static class PojoSpan implements CoreSpan { currentThread().getId(), UTF8BytesString.create(currentThread().getName()), fromMap(tags), + TagMap.EMPTY, + TagMap.EMPTY, baggage, UNSET, measured, diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 3d81035b6a8..6a67188cbf9 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -5555,6 +5555,11 @@ public String getVersion() { return version; } + /** Tags set on every span ({@code DD_SPAN_TAGS}), without the global tags. */ + public Map getSpanTags() { + return spanTags; + } + public Map getMergedSpanTags() { // Do not include runtimeId into span tags: we only want that added to the root span final Map result = newHashMap(getGlobalTags().size() + spanTags.size());