Keep global tags apart from span defaults in the config snapshot (OTLP trace/resource-level split)

dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java

@@ -221,11 +221,13 @@ 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_TRACE_SPAN_TAGS}, or the tags given to the
+   * builder. A remote config {@code tracing_tags} override replaces these along with the global
+   * tags while it's active.
+   */
   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 +545,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 +739,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 +752,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 +2591,27 @@ public void run() {
   protected class ConfigSnapshot extends DynamicConfig.Snapshot {
     final Sampler sampler;
 
+    /**
+     * The global tags ({@code DD_TAGS}, or their remote-config override, which keeps the configured
+     * {@code env} and {@code version}). 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. Only the tracer-defined tags while a remote config {@code
+     * tracing_tags} override is active.
+     */
+    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,20 +2628,53 @@ 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
+          && isTracingTagsOverridden() == oldSnapshot.isTracingTagsOverridden()
+          && 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;
+        Map<String, String> tracingTags = getTracingTags();
+        boolean overridden = isTracingTagsOverridden();
+        // A remote config tracing_tags override stands in for all the user's tags, span defaults
+        // included, until it's removed. It doesn't change the service's env or version.
+        TagMap defaultSpanTags = overridden ? TagMap.EMPTY : CoreTracer.this.defaultSpanTags;
+        globalTags =
+            overridden
+                ? withIdentityTags(tracingTags, config.getGlobalTags())
+                : TagMap.fromMapImmutable(tracingTags);
+        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);
       }
     }
   }
 
+  /** {@code overrideTags} with the {@code env} and {@code version} from {@code configTags}. */
+  private static TagMap withIdentityTags(
+      Map<String, String> overrideTags, Map<String, String> configTags) {
+    TagMap result = TagMap.create(overrideTags.size() + 2);
+    result.putAll(overrideTags);
+    String env = configTags.get(Tags.ENV);
+    if (env != null) {
+      result.set(Tags.ENV, env);
+    }
+    String version = configTags.get(Tags.VERSION);
+    if (version != null) {
+      result.set(Tags.VERSION, version);
+    }
+    return result.freeze();
+  }
+
   /**
    * Tags added by the tracer to all spans; combines user-supplied tags with tracer-defined tags.
    */

dd-trace-core/src/main/java/datadog/trace/core/TracingConfigPoller.java

@@ -255,7 +255,7 @@ void applyConfigOverrides(LibConfig libConfig) {
     }
     maybeOverride(builder::setTraceSampleRate, libConfig.traceSampleRate);
 
-    maybeOverride(builder::setTracingTags, parseTagListToMap(libConfig.tracingTags));
+    maybeOverride(builder::overrideTracingTags, parseTagListToMap(libConfig.tracingTags));
     DebuggerConfigBridge.updateConfig(
         new DebuggerConfigUpdate(
             libConfig.dynamicInstrumentationEnabled,

dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java

@@ -50,7 +50,6 @@
 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;
@@ -601,10 +600,21 @@ 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}.
+   * TestDynamicConfigV2.test_tracing_client_tracing_tags}. The override replaces the span tags
+   * ({@code DD_TRACE_SPAN_TAGS}) too, even when it's empty.
    */
-  @Test
-  void remoteConfigTracingTagsOverrideGlobalTags() throws Exception {
+  @TableTest({
+    "scenario                  | localTagsKey    | overrideTags          | expectedWithOverride",
+    "DD_TAGS                   | tags            | '[\"rc_key:rc_val\"]' | [rc_key: rc_val]    ",
+    "DD_TRACE_SPAN_TAGS        | trace.span.tags | '[\"rc_key:rc_val\"]' | [rc_key: rc_val]    ",
+    "empty, DD_TRACE_SPAN_TAGS | trace.span.tags | '[]'                  | [:]                 "
+  })
+  void remoteConfigTracingTagsOverrideGlobalTags(
+      String scenario,
+      String localTagsKey,
+      String overrideTags,
+      Map<String, String> expectedWithOverride)
+      throws Exception {
     ParsedConfigKey key = ParsedConfigKey.parse("datadog/2/APM_TRACING/config_overrides/config");
     ConfigurationPoller poller = mock(ConfigurationPoller.class);
     SharedCommunicationObjects sco = createScoWithPoller(poller);
@@ -618,31 +628,95 @@ void remoteConfigTracingTagsOverrideGlobalTags() throws Exception {
         .when(poller)
         .addListener(eq(Product.APM_TRACING), any(ProductListener.class));
 
-    Properties properties = new Properties();
-    properties.setProperty(GeneralConfig.TAGS, "local_key:local_val");
+    injectSysConfig(localTagsKey, "local_key:local_val");
     CoreTracer tracer =
-        CoreTracer.builder()
-            .withProperties(properties)
-            .sharedCommunicationObjects(sco)
-            .pollForTracingConfiguration()
-            .build();
+        CoreTracer.builder().sharedCommunicationObjects(sco).pollForTracingConfiguration().build();
     unclosedTracers.add(tracer);
 
     try {
       Map<String, String> localTags = buildStringMap("local_key", "local_val");
       assertEquals(localTags, tracer.captureTraceConfig().mergedTracerTags);
 
+      String json = "{\"lib_config\":{\"tracing_tags\": " + overrideTags + "}}";
+      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. A remote config override replaces both.
+   */
+  @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));
+
+    injectSysConfig(GeneralConfig.TAGS, "global_key:global_val,shared_key:global_val");
+    injectSysConfig(TracerConfig.SPAN_TAGS, "span_key:span_val,shared_key:span_val");
+    injectSysConfig(GeneralConfig.ENV, "prod");
+    injectSysConfig(GeneralConfig.VERSION, "1.2.3");
+    CoreTracer tracer =
+        CoreTracer.builder().sharedCommunicationObjects(sco).pollForTracingConfiguration().build();
+    unclosedTracers.add(tracer);
+
+    try {
+      Map<String, String> spanDefaults =
+          buildStringMap("span_key", "span_val", "shared_key", "span_val", "env", "prod");
+
+      ConfigSnapshot config = tracer.captureTraceConfig();
+      // Config adds DD_ENV and DD_VERSION to the global tags
+      assertEquals(
+          buildStringMap(
+              "global_key", "global_val",
+              "shared_key", "global_val",
+              "env", "prod",
+              "version", "1.2.3"),
+          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();
+      // the override keeps the configured env and version
       assertEquals(
-          buildStringMap("rc_key", "rc_val"), tracer.captureTraceConfig().mergedTracerTags);
+          buildStringMap("rc_key", "rc_val", "env", "prod", "version", "1.2.3"), config.globalTags);
+      assertEquals(buildStringMap("env", "prod"), config.spanDefaultTags);
+      assertEquals(buildStringMap("rc_key", "rc_val", "env", "prod"), config.mergedTracerTags);
 
       capturedUpdater[0].remove(key, null);
       capturedUpdater[0].commit(null);
 
-      assertEquals(localTags, tracer.captureTraceConfig().mergedTracerTags);
+      config = tracer.captureTraceConfig();
+      assertEquals(spanDefaults, config.spanDefaultTags);
     } finally {
       tracer.close();
     }

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_TRACE_SPAN_TAGS}), without the global tags. */
+  public Map<String, String> getSpanTags() {
+    return spanTags;
+  }
+
   public Map<String, String> getMergedSpanTags() {
     // Do not include runtimeId into span tags: we only want that added to the root span
     final Map<String, String> result = newHashMap(getGlobalTags().size() + spanTags.size());

internal-api/src/main/java/datadog/trace/api/DynamicConfig.java

@@ -104,6 +104,7 @@ public final class Builder {
     Map<String, String> requestHeaderTags;
     Map<String, String> responseHeaderTags;
     Map<String, String> tracingTags;
+    boolean tracingTagsOverridden;
     Map<String, String> baggageMapping;
 
     List<? extends SpanSamplingRule> spanSamplingRules;
@@ -134,6 +135,7 @@ public final class Builder {
       this.traceSampleRate = snapshot.traceSampleRate;
 
       this.tracingTags = snapshot.tracingTags;
+      this.tracingTagsOverridden = snapshot.tracingTagsOverridden;
 
       this.preferredServiceNameAndSource = snapshot.preferredServiceNameAndSource;
       this.dataStreamsTransactionExtractors = snapshot.dataStreamsTransactionExtractors;
@@ -218,6 +220,14 @@ public Builder setTraceSamplingRules(
 
     public Builder setTracingTags(Map<String, String> tracingTags) {
       this.tracingTags = tracingTags;
+      this.tracingTagsOverridden = false;
+      return this;
+    }
+
+    /** Sets the tracing tags from a remote config override, even an empty one. */
+    public Builder overrideTracingTags(Map<String, String> tracingTags) {
+      this.tracingTags = tracingTags;
+      this.tracingTagsOverridden = true;
       return this;
     }
 
@@ -332,6 +342,7 @@ public static class Snapshot implements TraceConfig {
 
     final Double traceSampleRate;
     final Map<String, String> tracingTags;
+    final boolean tracingTagsOverridden;
 
     final Pair<String, CharSequence> preferredServiceNameAndSource;
     final List<DataStreamsTransactionExtractor> dataStreamsTransactionExtractors;
@@ -354,6 +365,7 @@ protected Snapshot(DynamicConfig<?>.Builder builder, Snapshot oldSnapshot) {
       this.traceSampleRate = builder.traceSampleRate;
 
       this.tracingTags = nullToEmpty(builder.tracingTags);
+      this.tracingTagsOverridden = builder.tracingTagsOverridden;
 
       this.preferredServiceNameAndSource = builder.preferredServiceNameAndSource;
       this.dataStreamsTransactionExtractors = builder.dataStreamsTransactionExtractors;
@@ -437,6 +449,11 @@ public Map<String, String> getTracingTags() {
       return tracingTags;
     }
 
+    /** True while the tracing tags come from a remote config override. */
+    public boolean isTracingTagsOverridden() {
+      return tracingTagsOverridden;
+    }
+
     @Override
     public String toString() {
       return "DynamicConfig{"