Keep DD_SPAN_TAGS when remote config overrides tracing tags (OTLP trace/resource-level split)

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);
       }

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

@@ -601,10 +601,17 @@ 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}. Span tags ({@code DD_SPAN_TAGS}) are
+   * separate local config, so they're kept alongside the override.
    */
-  @Test
-  void remoteConfigTracingTagsOverrideGlobalTags() throws Exception {
+  @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<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);
@@ -619,7 +626,7 @@ void remoteConfigTracingTagsOverrideGlobalTags() throws Exception {
         .addListener(eq(Product.APM_TRACING), any(ProductListener.class));
 
     Properties properties = new Properties();
-    properties.setProperty(GeneralConfig.TAGS, "local_key:local_val");
+    properties.setProperty(localTagsKey, "local_key:local_val");
     CoreTracer tracer =
         CoreTracer.builder()
             .withProperties(properties)
@@ -636,8 +643,7 @@ void remoteConfigTracingTagsOverrideGlobalTags() throws Exception {
       capturedUpdater[0].accept(key, json.getBytes(StandardCharsets.UTF_8), null);
       capturedUpdater[0].commit(null);
 
-      assertEquals(
-          buildStringMap("rc_key", "rc_val"), tracer.captureTraceConfig().mergedTracerTags);
+      assertEquals(expectedWithOverride, tracer.captureTraceConfig().mergedTracerTags);
 
       capturedUpdater[0].remove(key, null);
       capturedUpdater[0].commit(null);
@@ -648,6 +654,74 @@ void remoteConfigTracingTagsOverrideGlobalTags() throws Exception {
     }
   }
 
+  /**
+   * 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<String, String> 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();
+    }
+  }
+
   static final String ACTION_JSON =
       "'{\"action\": \"enable\", \"lib_config\":"
           + "{\"tracing_sampling_rate\": null,"

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<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());