Add trace.builder.tags.precedence.enabled to let explicit builder tags win (phase 1a)

dd-trace-api/src/main/java/datadog/trace/api/config/TracerConfig.java

@@ -105,6 +105,14 @@ public final class TracerConfig {
   public static final String TRACE_BAGGAGE_MAX_BYTES = "trace.baggage.max.bytes";
   public static final String TRACE_BAGGAGE_TAG_KEYS = "trace.baggage.tag.keys";
 
+  /**
+   * When enabled, explicit per-span tags set via the span builder take precedence over
+   * tracer-injected tags (inbound header tags, root-span tags, contextual tags) by being applied
+   * last. Default off, preserving the historical ordering where those could override builder tags.
+   */
+  public static final String TRACE_BUILDER_TAGS_PRECEDENCE_ENABLED =
+      "trace.builder.tags.precedence.enabled";
+
   public static final String TRACE_INFERRED_PROXY_SERVICES_ENABLED =
       "trace.inferred.proxy.services.enabled";
 

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

@@ -253,6 +253,11 @@ public static CoreTracerBuilder builder() {
   private static final boolean SPAN_BUILDER_REUSE_ENABLED =
       Config.get().isSpanBuilderReuseEnabled();
 
+  // Instance field (not static final) so it honors per-tracer config, e.g. an embedded tracer
+  // built via CoreTracerBuilder#withProperties/#config rather than the global Config.get()
+  // singleton. See the tag-ordering block in buildSpanContext.
+  private final boolean builderTagsPrecedence;
+
   // Cache used by buildSpan - instance so it can capture the CoreTracer
   private final ReusableSingleSpanBuilderThreadLocalCache spanBuilderThreadLocalCache =
       SPAN_BUILDER_REUSE_ENABLED ? new ReusableSingleSpanBuilderThreadLocalCache(this) : null;
@@ -838,6 +843,8 @@ private CoreTracer(
 
     propagationTagsFactory = PropagationTags.factory(config);
 
+    builderTagsPrecedence = config.isTraceBuilderTagsPrecedenceEnabled();
+
     // Register context propagators
     HttpCodec.Extractor baseExtractor =
         extractor == null ? HttpCodec.createExtractor(config, this::captureTraceConfig) : extractor;
@@ -2245,8 +2252,11 @@ protected static final DDSpanContext buildSpanContext(
               mergedTracerTagsNeedsIntercept ? null : mergedTracerTags);
 
       // By setting the tags on the context we apply decorators to any tags that have been set via
-      // the builder. This is the order that the tags were added previously, but maybe the `tags`
-      // set in the builder should come last, so that they override other tags.
+      // the builder. The `mergedTracerTags` are always applied first (the precedence floor:
+      // everything overrides them). The remaining contributors are applied last-wins; with
+      // `builderTagsPrecedence` enabled, `tagLedger` (the explicit builder tags) moves to last so
+      // it wins collisions instead of being overridden by `coreTags`/`rootSpanTags`/
+      // `contextualTags` -- see the PR description for the historical context on this ordering.
       //
       // mergedTracerTags is trace-level shared state and the precedence floor (everything below
       // overrides it). When it carries no interceptable tags it is attached as a read-through
@@ -2256,16 +2266,23 @@ protected static final DDSpanContext buildSpanContext(
       if (mergedTracerTagsNeedsIntercept) {
         context.setAllTags(mergedTracerTags, true);
       }
-      context.setAllTags(tagLedger);
-      context.setAllTags(coreTags, coreTagsNeedsIntercept);
-      context.setAllTags(rootSpanTags, rootSpanTagsNeedsIntercept);
-      context.setAllTags(contextualTags);
+      if (tracer.builderTagsPrecedence) {
+        context.setAllTags(coreTags, coreTagsNeedsIntercept);
+        context.setAllTags(rootSpanTags, rootSpanTagsNeedsIntercept);
+        context.setAllTags(contextualTags);
+        context.setAllTags(tagLedger);
+      } else {
+        context.setAllTags(tagLedger);
+        context.setAllTags(coreTags, coreTagsNeedsIntercept);
+        context.setAllTags(rootSpanTags, rootSpanTagsNeedsIntercept);
+        context.setAllTags(contextualTags);
+      }
       // Version is added later by the postProcessor (InternalTagsAdder), only if not already set
       // during the request. Config version is kept out of the trace-level bundle (see
       // withTracerTags), so this removal now only wipes a version set via the span builder —
-      // keeping
-      // the existing semantics where a builder-set version is replaced by the config version. Under
-      // read-through this is a cheap local removal (version isn't in the parent, so no tombstone).
+      // keeping the existing semantics where a builder-set version is replaced by the config
+      // version. Under read-through this is a cheap local removal (version isn't in the parent,
+      // so no tombstone).
       context.removeTag(Tags.VERSION);
       return context;
     }

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

@@ -0,0 +1,149 @@
+package datadog.trace.core;
+
+import static datadog.trace.api.TracePropagationStyle.DATADOG;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import datadog.trace.api.DDTraceId;
+import datadog.trace.api.TagMap;
+import datadog.trace.api.config.TracerConfig;
+import datadog.trace.api.sampling.PrioritySampling;
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
+import datadog.trace.core.propagation.ExtractedContext;
+import datadog.trace.core.propagation.PropagationTags;
+import java.util.Collections;
+import java.util.Properties;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Characterization of the span-build tag-ordering wart (see {@code CoreTracer} span builder).
+ *
+ * <p>An inbound header-derived tag ({@code coreTags}) and an explicit per-span builder tag ({@code
+ * tagLedger}) that share a key are both applied to the span. Historically the builder tag is
+ * applied <em>before</em> {@code coreTags}, so the header tag silently OVERRIDES the explicit
+ * builder tag — flagged in-code since 2020 ("maybe the builder tags should come last").
+ *
+ * <p>{@link #headerTagOverridesBuilderTagByDefault} pins the <b>default</b> (flag-off) behavior.
+ * {@link #builderTagWinsWhenPrecedenceEnabled} exercises the {@code
+ * trace.builder.tags.precedence.enabled} flag, which inverts it so the explicit builder tag wins
+ * (the logical precedence) -- read per-tracer off the {@code CoreTracerBuilder}'s own {@code
+ * Config}, so no process property or forking is needed to flip it in-test.
+ */
+class BuilderTagsPrecedenceTest extends DDCoreJavaSpecification {
+
+  private static final String KEY = "test.collision.tag";
+  private static final String HEADER_VALUE = "from-header";
+  private static final String BUILDER_VALUE = "from-builder";
+
+  private CoreTracer tracer;
+
+  @AfterEach
+  void cleanup() {
+    if (tracer != null) {
+      tracer.close();
+    }
+  }
+
+  /** An extracted context carrying a header-derived tag ({@code coreTags}) on the given key. */
+  private static ExtractedContext extractedWithHeaderTag(String key, String value) {
+    return new ExtractedContext(
+        DDTraceId.ONE,
+        2,
+        PrioritySampling.SAMPLER_KEEP,
+        null,
+        0,
+        Collections.<String, String>emptyMap(),
+        TagMap.fromMap(Collections.singletonMap(key, value)),
+        null,
+        PropagationTags.factory().empty(),
+        null,
+        DATADOG);
+  }
+
+  /**
+   * Default config: the historical order applies, so the inbound header tag overrides the explicit
+   * builder tag. This documents the wart; flipping the default would (intentionally) break this.
+   */
+  @Test
+  void headerTagOverridesBuilderTagByDefault() {
+    tracer = tracerBuilder().build();
+    AgentSpan span =
+        tracer
+            .buildSpan("test", "root")
+            .asChildOf(extractedWithHeaderTag(KEY, HEADER_VALUE))
+            .withTag(KEY, BUILDER_VALUE)
+            .start();
+    try {
+      Object resolved = ((DDSpan) span).getTag(KEY);
+      assertEquals(
+          HEADER_VALUE,
+          resolved,
+          "By default the historical order lets the inbound header tag override the explicit "
+              + "builder tag (the documented wart). If this fails, the default ordering changed.");
+    } finally {
+      span.finish();
+    }
+  }
+
+  /**
+   * With the flag enabled, the explicit builder tag is applied last, so it wins over the inbound
+   * header tag -- the inversion this PR adds.
+   */
+  @Test
+  void builderTagWinsWhenPrecedenceEnabled() {
+    Properties properties = new Properties();
+    properties.setProperty(TracerConfig.TRACE_BUILDER_TAGS_PRECEDENCE_ENABLED, "true");
+    tracer = tracerBuilder().withProperties(properties).build();
+
+    AgentSpan span =
+        tracer
+            .buildSpan("test", "root")
+            .asChildOf(extractedWithHeaderTag(KEY, HEADER_VALUE))
+            .withTag(KEY, BUILDER_VALUE)
+            .start();
+    try {
+      Object resolved = ((DDSpan) span).getTag(KEY);
+      assertEquals(
+          BUILDER_VALUE,
+          resolved,
+          "With trace.builder.tags.precedence.enabled=true, the explicit builder tag must win "
+              + "over the inbound header tag.");
+    } finally {
+      span.finish();
+    }
+  }
+
+  /**
+   * Confirms the flag is read from the tracer's own config (set via {@code
+   * CoreTracerBuilder#withProperties}), not a cached global default -- the bug fixed alongside this
+   * test, per the review discussion on this PR.
+   */
+  @Test
+  void precedenceFlagIsPerTracerNotAGlobalDefault() {
+    tracer = tracerBuilder().build();
+    Properties properties = new Properties();
+    properties.setProperty(TracerConfig.TRACE_BUILDER_TAGS_PRECEDENCE_ENABLED, "true");
+    CoreTracer enabledTracer = tracerBuilder().withProperties(properties).build();
+    try {
+      AgentSpan defaultSpan =
+          tracer
+              .buildSpan("test", "root")
+              .asChildOf(extractedWithHeaderTag(KEY, HEADER_VALUE))
+              .withTag(KEY, BUILDER_VALUE)
+              .start();
+      defaultSpan.finish();
+      assertEquals(HEADER_VALUE, ((DDSpan) defaultSpan).getTag(KEY));
+
+      AgentSpan enabledSpan =
+          enabledTracer
+              .buildSpan("test", "root")
+              .asChildOf(extractedWithHeaderTag(KEY, HEADER_VALUE))
+              .withTag(KEY, BUILDER_VALUE)
+              .start();
+      enabledSpan.finish();
+      assertEquals(BUILDER_VALUE, ((DDSpan) enabledSpan).getTag(KEY));
+    } finally {
+      enabledTracer.close();
+    }
+  }
+}

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

@@ -688,6 +688,7 @@
 import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_BYTES;
 import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_ITEMS;
 import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_TAG_KEYS;
+import static datadog.trace.api.config.TracerConfig.TRACE_BUILDER_TAGS_PRECEDENCE_ENABLED;
 import static datadog.trace.api.config.TracerConfig.TRACE_CLIENT_IP_HEADER;
 import static datadog.trace.api.config.TracerConfig.TRACE_CLIENT_IP_RESOLVER_ENABLED;
 import static datadog.trace.api.config.TracerConfig.TRACE_CLOUD_PAYLOAD_TAGGING_MAX_DEPTH;
@@ -912,6 +913,7 @@ public static String getHostName() {
   private final boolean integrationSynapseLegacyOperationName;
   private final String writerType;
   private final boolean injectBaggageAsTagsEnabled;
+  private final boolean traceBuilderTagsPrecedenceEnabled;
   private final boolean injectLinksAsTagsEnabled;
   private final boolean agentConfiguredUsingDefault;
   private final String agentUrl;
@@ -1556,6 +1558,8 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins
     injectBaggageAsTagsEnabled =
         configProvider.getBoolean(WRITER_BAGGAGE_INJECT, isDatadogTraceWriter);
     injectLinksAsTagsEnabled = configProvider.getBoolean(WRITER_LINKS_INJECT, isDatadogTraceWriter);
+    traceBuilderTagsPrecedenceEnabled =
+        configProvider.getBoolean(TRACE_BUILDER_TAGS_PRECEDENCE_ENABLED, false);
     String lambdaInitType = getEnv("AWS_LAMBDA_INITIALIZATION_TYPE");
     String lambdaMicrovmImageArn = ConfigHelper.env("AWS_LAMBDA_MICROVM_IMAGE_ARN");
     if ((lambdaInitType != null && lambdaInitType.equals("snap-start"))
@@ -3625,6 +3629,10 @@ public boolean isInjectBaggageAsTagsEnabled() {
     return injectBaggageAsTagsEnabled;
   }
 
+  public boolean isTraceBuilderTagsPrecedenceEnabled() {
+    return traceBuilderTagsPrecedenceEnabled;
+  }
+
   public boolean isInjectLinksAsTagsEnabled() {
     return injectLinksAsTagsEnabled;
   }
@@ -6954,6 +6962,8 @@ public String toString() {
         + traceFlushIntervalSeconds
         + ", injectBaggageAsTagsEnabled="
         + injectBaggageAsTagsEnabled
+        + ", traceBuilderTagsPrecedenceEnabled="
+        + traceBuilderTagsPrecedenceEnabled
         + ", injectLinksAsTagsEnabled="
         + injectLinksAsTagsEnabled
         + ", logsInjectionEnabled="

metadata/supported-configurations.json

@@ -5108,6 +5108,14 @@
         "aliases": ["DD_TRACE_INTEGRATION_BEANSHELL_ENABLED", "DD_INTEGRATION_BEANSHELL_ENABLED"]
       }
     ],
+    "DD_TRACE_BUILDER_TAGS_PRECEDENCE_ENABLED": [
+      {
+        "version": "A",
+        "type": "boolean",
+        "default": "false",
+        "aliases": []
+      }
+    ],
     "DD_TRACE_CAFFEINE_ENABLED": [
       {
         "version": "A",