diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java index c72533768ef..709a838f893 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java @@ -13,6 +13,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; +import datadog.trace.bootstrap.instrumentation.api.SpanPrototype; import datadog.trace.bootstrap.instrumentation.api.Tags; import java.lang.reflect.Method; import java.net.Inet4Address; @@ -53,8 +54,9 @@ public String apply(Class clazz) { private final TagMap.Entry traceAnalyticsEntry; - // Deliberately not volatile, reading null and repeating the calculation is safe - private TagMap.Entry cachedComponentEntry = null; + // Deliberately not volatile: reading a stale null and rebuilding is safe. SpanPrototype is + // frozen, so a benign race produces two equivalent prototypes and either is fine. + private SpanPrototype cachedSpanPrototype = null; protected BaseDecorator() { final Config config = Config.get(); @@ -80,18 +82,38 @@ protected BaseDecorator() { protected abstract CharSequence component(); - /** Caches the component TagMap.Entry, so it isn't recreated for every trace */ - protected final TagMap.Entry componentEntry() { - // DQH = Tried calling component() in the constructor, but that had issues with static - // field ordering. That was caught be an integration test, but I didn't want to risk - // breaking other integrations where the test is not as thorough. - - // This approach while more complicated doesn't have any field initialization ordering issues. - TagMap.Entry componentEntry = cachedComponentEntry; - if (componentEntry == null) { - cachedComponentEntry = componentEntry = TagMap.Entry.create(Tags.COMPONENT, component()); + /** + * The baked-once {@link SpanPrototype} carrying this decorator's constant identity and tags: span + * type, component, integration name, and — via the {@link ServerDecorator} / {@link + * ClientDecorator} extensions — span kind and language. + * + *

Built lazily on first access, not in the constructor: {@link #component()}, {@link + * #spanType()}, and (in {@link ClientDecorator}) {@code spanKind()} are overridable and may + * reference statics that are not yet initialized while the decorator singleton is under + * construction. Deferring the build sidesteps that field-initialization-ordering hazard (the same + * one the old per-{@link TagMap.Entry} caches guarded against) while collapsing those several + * caches into a single object. Not volatile: {@link SpanPrototype} is frozen, so a benign race + * rebuilds an equivalent prototype. + */ + protected final SpanPrototype spanPrototype() { + SpanPrototype prototype = cachedSpanPrototype; + if (prototype == null) { + cachedSpanPrototype = prototype = buildSpanPrototype(); } - return componentEntry; + return prototype; + } + + /** + * Builds this decorator's {@link SpanPrototype}. Subclasses extend the chain with {@link + * SpanPrototype.Builder#extends_} to add their level's constants (see {@link ServerDecorator} / + * {@link ClientDecorator}), mirroring the decorator class hierarchy. Called once per decorator, + * lazily — see {@link #spanPrototype()}. + */ + protected SpanPrototype buildSpanPrototype() { + return SpanPrototype.builder() + .initSpanType(spanType()) + .initComponentAndIntegration(component()) + .build(); } protected boolean traceAnalyticsDefault() { @@ -109,16 +131,12 @@ public final void afterStart(final AgentSpan span) { } protected void doAfterStart(final AgentSpan span) { - if (spanType() != null) { - span.setSpanType(spanType()); - } - - span.setTag(componentEntry()); - - // DQH - Could retrieve the value from componentEntry and cast to avoid the virtual call, - // unclear which option is better here - final CharSequence component = component(); - span.spanContext().setIntegrationName(component); + // Stamps the prototype's constant span type, tags, and integration name, overwriting whatever + // is already present: this decorator's identity is authoritative and must win over anything + // seeded earlier, e.g. a global tag from DD_TAGS / DD_TRACE_SPAN_TAGS applied at construction + // -- + // matching the unconditional setTag/setSpanType calls this replaced. + span.applyOverwriting(spanPrototype()); // null handled by setMetric span.setMetric(traceAnalyticsEntry); diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ClientDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ClientDecorator.java index 9c21bca3cdc..f06beb4930a 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ClientDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ClientDecorator.java @@ -1,45 +1,36 @@ package datadog.trace.bootstrap.instrumentation.decorator; -import datadog.trace.api.TagMap; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.SpanPrototype; import datadog.trace.bootstrap.instrumentation.api.Tags; import javax.annotation.ParametersAreNonnullByDefault; @ParametersAreNonnullByDefault public abstract class ClientDecorator extends BaseDecorator { - // Deliberately not volatile, reading a stale null and creating an extra Entry is safe - private TagMap.Entry cachedSpanKindEntry = null; protected abstract String service(); - /** Caches span kind entry to reduce allocation */ - private final TagMap.Entry spanKindEntry() { - // DQH - I considered moving the creation of the TagMap.Entry into a ClientDecorator - // constructor, but that introduces a subtle ordering requirement. - - // If the spanKind method refers to a static that isn't yet initialized, - // then spanKind will return null when the Decorator singleton is being constructed. - - // Such an ordering problem did occur with similar changes in BaseDecorator, so I've - // decided to be cautious here, too. - TagMap.Entry kindEntry = cachedSpanKindEntry; - if (kindEntry == null) { - cachedSpanKindEntry = kindEntry = TagMap.Entry.create(Tags.SPAN_KIND, spanKind()); - } - return kindEntry; - } - protected String spanKind() { return Tags.SPAN_KIND_CLIENT; } + @Override + protected SpanPrototype buildSpanPrototype() { + // Extend the base prototype with the client-level span.kind. spanKind() is overridable and may + // read a not-yet-initialized static during singleton construction -- building lazily (via + // spanPrototype()) preserves the ordering safety the old cached spanKindEntry provided. + return SpanPrototype.builder() + .extends_(super.buildSpanPrototype()) + .initKind(spanKind()) + .build(); + } + @Override protected void doAfterStart(final AgentSpan span) { final String service = service(); if (service != null) { span.setServiceName(service, component()); } - span.setTag(spanKindEntry()); // Generate metrics for all client spans. span.setMeasured(true); diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ServerDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ServerDecorator.java index 7551e66474d..49c5655ebfd 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ServerDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ServerDecorator.java @@ -1,23 +1,21 @@ package datadog.trace.bootstrap.instrumentation.decorator; import datadog.trace.api.DDTags; -import datadog.trace.api.TagMap; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.SpanPrototype; import datadog.trace.bootstrap.instrumentation.api.Tags; import javax.annotation.ParametersAreNonnullByDefault; @ParametersAreNonnullByDefault public abstract class ServerDecorator extends BaseDecorator { - private static final TagMap.Entry SPAN_KIND_ENTRY = - TagMap.Entry.create(Tags.SPAN_KIND, Tags.SPAN_KIND_SERVER); - private static final TagMap.Entry LANG_ENTRY = - TagMap.Entry.create(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE); @Override - protected void doAfterStart(final AgentSpan span) { - span.setTag(SPAN_KIND_ENTRY); - span.setTag(LANG_ENTRY); - - super.doAfterStart(span); + protected SpanPrototype buildSpanPrototype() { + // Extend the base prototype with the server-level constants (span.kind=server, language). The + // prototype chain mirrors the decorator class hierarchy; base afterStart applies the whole set. + return SpanPrototype.builder() + .extends_(super.buildSpanPrototype()) + .initKind(Tags.SPAN_KIND_SERVER) + .initTag(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE) + .build(); } } diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy index 3151d655739..44b4f64ff26 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy @@ -2,7 +2,6 @@ package datadog.trace.bootstrap.instrumentation.decorator import datadog.appsec.api.blocking.BlockingException import datadog.context.Context -import datadog.trace.api.TagMap import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities @@ -12,6 +11,8 @@ import datadog.trace.test.util.DDSpecification import javax.annotation.Nonnull import spock.lang.Shared +import static datadog.trace.bootstrap.instrumentation.decorator.ExpectedSpanState.expectedSpan + class BaseDecoratorTest extends DDSpecification { def setupSpec() { @@ -28,25 +29,20 @@ class BaseDecoratorTest extends DDSpecification { def spanContext = Mock(AgentSpanContext) def "test afterStart"() { + setup: + def recordingSpan = new RecordingSpan() + when: - decorator.afterStart(span) + decorator.afterStart(recordingSpan) then: - 1 * span.setSpanType(decorator.spanType()) - 1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component")) - 1 * span.spanContext() >> spanContext - 1 * spanContext.setIntegrationName("test-component") - _ * span.setTag(_) - _ * span.setTag(_, _) // Want to allow other calls from child implementations. - _ * span.setTag(_) - _ * span.setMeasured(true) - _ * span.setMetric(_) - _ * span.setMetric(_, _) - _ * span.setMetric(_) - _ * span.setServiceName(_, _) - _ * span.setOperationName(_) - _ * span.setSamplingPriority(_) - 0 * _ + // The base spec runs polymorphically against every subclass decorator, so it only asserts the + // baseline identity every decorator applies, tolerating the tags subclasses layer on. Each + // level's exact tag set is asserted by its own afterStart spec. + expectedSpan() + .spanType(decorator.spanType()) + .component("test-component") + .assertIdentityAppliedTo(recordingSpan) } def "test onPeerConnection"() { diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy index fec5748f089..7e3fbd98f81 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy @@ -1,10 +1,8 @@ package datadog.trace.bootstrap.instrumentation.decorator -import datadog.trace.api.DDTags -import datadog.trace.api.TagMap import datadog.trace.bootstrap.instrumentation.api.AgentSpan -import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext -import datadog.trace.bootstrap.instrumentation.api.Tags + +import static datadog.trace.bootstrap.instrumentation.decorator.ExpectedSpanState.expectedSpan class ClientDecoratorTest extends BaseDecoratorTest { @@ -13,28 +11,24 @@ class ClientDecoratorTest extends BaseDecoratorTest { def "test afterStart"() { setup: def decorator = newDecorator((String) serviceName) - def spanContext = Mock(AgentSpanContext) + def recordingSpan = new RecordingSpan() when: - decorator.afterStart(span) + decorator.afterStart(recordingSpan) then: + def expected = expectedSpan() + .spanType(decorator.spanType()) + .component("test-component") + .spanKind("client") + .measured(true) + .analyticsSampleRate(1.0d) if (serviceName != null) { - 1 * span.setServiceName(serviceName, "test-component") + expected.serviceName(serviceName, "test-component") } - 1 * span.setMeasured(true) - 1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component")) - 1 * span.spanContext() >> spanContext - 1 * spanContext.setIntegrationName("test-component") - 1 * span.setTag(TagMap.Entry.create(Tags.SPAN_KIND, "client")) - 1 * span.setSpanType(decorator.spanType()) - 1 * span.setMetric(TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, 1.0)) - _ * span.setTag(_) - _ * span.setTag(_, _) // Want to allow other calls from child implementations. - _ * span.setTag(_) - _ * span.setServiceName(_) - _ * span.setOperationName(_) - 0 * _ + // Polymorphic parent spec: subclass decorators (e.g. DB-type processing) layer on extra tags in + // afterStart, so tolerate additional tags while asserting the client-level scalars exactly. + expected.assertAppliedAllowingExtraTags(recordingSpan) where: serviceName << ["test-service", "other-service", null] diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy index 93852ccc88c..f0e8d783239 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy @@ -1,14 +1,12 @@ package datadog.trace.bootstrap.instrumentation.decorator -import datadog.trace.api.DDTags -import datadog.trace.api.TagMap import datadog.trace.bootstrap.instrumentation.api.AgentSpan -import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext import datadog.trace.bootstrap.instrumentation.api.Tags import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_HOST import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX +import static datadog.trace.bootstrap.instrumentation.decorator.ExpectedSpanState.expectedSpan class DatabaseClientDecoratorTest extends ClientDecoratorTest { @@ -17,23 +15,22 @@ class DatabaseClientDecoratorTest extends ClientDecoratorTest { def "test afterStart"() { setup: def decorator = newDecorator((String) serviceName) - def spanContext = Mock(AgentSpanContext) + def recordingSpan = new RecordingSpan() when: - decorator.afterStart(span) + decorator.afterStart(recordingSpan) then: + def expected = expectedSpan() + .spanType("test-type") + .component("test-component") + .spanKind("client") + .measured(true) + .analyticsSampleRate(1.0d) if (serviceName != null) { - 1 * span.setServiceName(serviceName, "test-component") + expected.serviceName(serviceName, "test-component") } - 1 * span.setMeasured(true) - 1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component")) - 1 * span.spanContext() >> spanContext - 1 * spanContext.setIntegrationName("test-component") - 1 * span.setTag(TagMap.Entry.create(Tags.SPAN_KIND, "client")) - 1 * span.setSpanType("test-type") - 1 * span.setMetric(TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, 1.0)) - 0 * _ + expected.assertAppliedTo(recordingSpan) where: serviceName << ["test-service", "other-service", null] diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ServerDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ServerDecoratorTest.groovy index d60c1534627..a75542aa1e7 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ServerDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ServerDecoratorTest.groovy @@ -1,39 +1,29 @@ package datadog.trace.bootstrap.instrumentation.decorator -import datadog.trace.api.TagMap import datadog.trace.bootstrap.instrumentation.api.AgentSpan -import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext -import static datadog.trace.api.DDTags.ANALYTICS_SAMPLE_RATE -import static datadog.trace.api.DDTags.LANGUAGE_TAG_KEY -import static datadog.trace.api.DDTags.LANGUAGE_TAG_VALUE -import static datadog.trace.bootstrap.instrumentation.api.Tags.COMPONENT -import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND +import static datadog.trace.bootstrap.instrumentation.decorator.ExpectedSpanState.expectedSpan class ServerDecoratorTest extends BaseDecoratorTest { def span = Mock(AgentSpan) def "test afterStart"() { + setup: def decorator = newDecorator() - def spanContext = Mock(AgentSpanContext) + def recordingSpan = new RecordingSpan() when: - decorator.afterStart(span) + decorator.afterStart(recordingSpan) then: - 1 * span.setTag(TagMap.Entry.create(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE)) - 1 * span.setTag(TagMap.Entry.create(COMPONENT, "test-component")) - 1 * span.spanContext() >> spanContext - 1 * spanContext.setIntegrationName("test-component") - 1 * span.setTag(TagMap.Entry.create(SPAN_KIND, "server")) - 1 * span.setSpanType(decorator.spanType()) - if (decorator.traceAnalyticsEnabled) { - 1 * span.setMetric(TagMap.Entry.create(ANALYTICS_SAMPLE_RATE, 1.0)) - } else { - 1 * span.setMetric(null) - } - 0 * _ + expectedSpan() + .spanType(decorator.spanType()) + .component("test-component") + .spanKind("server") + .language() + .analyticsSampleRate(decorator.traceAnalyticsEnabled ? 1.0d : null) + .assertAppliedTo(recordingSpan) } def "test beforeFinish"() { diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/ExpectedSpanState.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/ExpectedSpanState.java new file mode 100644 index 00000000000..4f4140f3b36 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/ExpectedSpanState.java @@ -0,0 +1,157 @@ +package datadog.trace.bootstrap.instrumentation.decorator; + +import static datadog.trace.api.DDTags.ANALYTICS_SAMPLE_RATE; +import static datadog.trace.api.DDTags.LANGUAGE_TAG_KEY; +import static datadog.trace.api.DDTags.LANGUAGE_TAG_VALUE; +import static datadog.trace.bootstrap.instrumentation.api.Tags.COMPONENT; +import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND; +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 static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * The span state a decorator's {@code afterStart} is expected to apply, built up level by level to + * mirror the {@code buildSpanPrototype()} extension chain (base identity, then server/client kind, + * then any specialization). + * + *

To get an {@code ExpectedSpanState}, use the static factory method {@link #expectedSpan()} and + * use it as a fluent builder to define the expected state, mirroring {@code SpanMatcher} in the + * smoke tests. {@link #assertAppliedTo(RecordingSpan)} verifies the whole accumulated state at once + * instead of asserting individual mock interactions. + */ +final class ExpectedSpanState { + private CharSequence spanType; + private final Map tags = new LinkedHashMap<>(); + private CharSequence integrationName; + + private boolean expectService; + private String serviceName; + private CharSequence serviceNameSource; + + private boolean measured; + + // null => expect setMetric(null); non-null => expect the analytics-rate metric entry. + private Double analyticsSampleRate; + + /** + * Checks a span's expected {@code afterStart} state. + * + * @return A new {@link ExpectedSpanState} instance to configure the expected state. + */ + static ExpectedSpanState expectedSpan() { + return new ExpectedSpanState(); + } + + ExpectedSpanState spanType(CharSequence type) { + this.spanType = type; + return this; + } + + /** + * Baked component tag plus the integration name derived from it, as {@code BaseDecorator} does. + */ + ExpectedSpanState component(CharSequence component) { + tags.put(COMPONENT, String.valueOf(component)); + this.integrationName = component; + return this; + } + + ExpectedSpanState spanKind(CharSequence kind) { + tags.put(SPAN_KIND, String.valueOf(kind)); + return this; + } + + ExpectedSpanState language() { + tags.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE); + return this; + } + + ExpectedSpanState serviceName(String serviceName, CharSequence source) { + this.expectService = true; + this.serviceName = serviceName; + this.serviceNameSource = source; + return this; + } + + ExpectedSpanState measured(boolean measured) { + this.measured = measured; + return this; + } + + ExpectedSpanState analyticsSampleRate(Double rate) { + this.analyticsSampleRate = rate; + return this; + } + + /** + * Lenient baseline check for the polymorphic base {@code afterStart} spec, which runs against + * every subclass decorator: asserts the identity a decorator must apply (span type, the declared + * tags as a subset, integration name) while tolerating the extra tags/state a subclass layers on. + */ + void assertIdentityAppliedTo(RecordingSpan span) { + assertEquals(str(spanType), str(span.recordedSpanType()), "span type"); + assertEquals(str(integrationName), str(span.recordedIntegrationName()), "integration name"); + for (Map.Entry expectedTag : tags.entrySet()) { + assertEquals( + expectedTag.getValue(), + span.recordedTags().get(expectedTag.getKey()), + "tag " + expectedTag.getKey()); + } + } + + /** Exact check: the recorded state must match exactly, with no additional tags. */ + void assertAppliedTo(RecordingSpan span) { + assertAppliedTo(span, false); + } + + /** + * Scalar-exact check that tolerates additional tags, for a polymorphic parent spec (e.g. {@code + * ClientDecoratorTest}) whose subclass decorators layer on extra tags in {@code afterStart}. Span + * type, integration name, service, measured flag and metric are still asserted exactly. + */ + void assertAppliedAllowingExtraTags(RecordingSpan span) { + assertAppliedTo(span, true); + } + + private void assertAppliedTo(RecordingSpan span, boolean allowExtraTags) { + assertEquals(str(spanType), str(span.recordedSpanType()), "span type"); + if (allowExtraTags) { + for (Map.Entry expectedTag : tags.entrySet()) { + assertEquals( + expectedTag.getValue(), + span.recordedTags().get(expectedTag.getKey()), + "tag " + expectedTag.getKey()); + } + } else { + assertEquals(tags, span.recordedTags(), "applied tags"); + } + assertEquals(str(integrationName), str(span.recordedIntegrationName()), "integration name"); + + if (expectService) { + assertTrue(span.serviceNameSet(), "expected setServiceName to be called"); + assertEquals(serviceName, span.recordedServiceName(), "service name"); + assertEquals(str(serviceNameSource), str(span.recordedServiceNameSource()), "service source"); + } else { + assertFalse(span.serviceNameSet(), "did not expect setServiceName to be called"); + } + + assertEquals(measured, span.recordedMeasured(), "measured"); + + if (analyticsSampleRate == null) { + assertNull(span.recordedMetric(), "expected no analytics metric"); + } else { + assertTrue(span.metricSet(), "expected setMetric to be called"); + assertEquals(ANALYTICS_SAMPLE_RATE, span.recordedMetric().tag(), "analytics metric key"); + assertEquals( + analyticsSampleRate, span.recordedMetric().doubleValue(), "analytics metric value"); + } + } + + private static String str(CharSequence value) { + return value == null ? null : value.toString(); + } +} diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/RecordingSpan.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/RecordingSpan.java new file mode 100644 index 00000000000..0ba67d2642b --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/RecordingSpan.java @@ -0,0 +1,271 @@ +package datadog.trace.bootstrap.instrumentation.decorator; + +import datadog.trace.api.DDTraceId; +import datadog.trace.api.TagMap; +import datadog.trace.api.TraceConfig; +import datadog.trace.api.gateway.Flow.Action.RequestBlockingAction; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.sampling.PrioritySampling; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.bootstrap.instrumentation.api.ImmutableSpan; +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * A recording {@link AgentSpan} test double for decorator {@code afterStart} tests. Rather than + * verifying individual mock interactions, it accumulates the state a decorator applies (span type, + * tags, integration name, service name, measured flag, analytics metric) so a test can assert the + * resulting span state as a whole -- see {@link ExpectedSpanState}. + * + *

Only the mutators {@code afterStart} exercises are recorded; every other {@link AgentSpan} + * method inherits the inert {@link ImmutableSpan} / noop behavior. + */ +final class RecordingSpan extends ImmutableSpan { + private final RecordingSpanContext context = new RecordingSpanContext(); + private final Map tags = new LinkedHashMap<>(); + + private CharSequence spanType; + private String serviceName; + private CharSequence serviceNameSource; + private boolean serviceNameSet; + private boolean measured; + private boolean metricSet; + private TagMap.EntryReader metric; + + // ----- recorded mutators ----- + + @Override + public AgentSpan setSpanType(CharSequence type) { + this.spanType = type; + return this; + } + + @Override + public AgentSpan setAllTags(Map map) { + if (map == null || map.isEmpty()) { + return this; + } + if (map instanceof TagMap) { + ((TagMap) map) + .forEach(reader -> tags.put(reader.tag(), String.valueOf(reader.objectValue()))); + } else { + for (Map.Entry entry : map.entrySet()) { + tags.put(entry.getKey(), String.valueOf(entry.getValue())); + } + } + return this; + } + + @Override + public AgentSpan setTag(TagMap.EntryReader entry) { + if (entry != null) { + tags.put(entry.tag(), String.valueOf(entry.objectValue())); + } + return this; + } + + @Override + public AgentSpan setTag(String key, CharSequence value) { + tags.put(key, String.valueOf(value)); + return this; + } + + @Override + public AgentSpan setTag(String key, String value) { + tags.put(key, value); + return this; + } + + @Override + public AgentSpan setTag(String key, Object value) { + tags.put(key, String.valueOf(value)); + return this; + } + + @Override + public void setServiceName(String serviceName, CharSequence source) { + this.serviceNameSet = true; + this.serviceName = serviceName; + this.serviceNameSource = source; + } + + @Override + public AgentSpan setMeasured(boolean measured) { + this.measured = measured; + return this; + } + + @Override + public AgentSpan setMetric(TagMap.EntryReader metricEntry) { + this.metricSet = true; + this.metric = metricEntry; + return this; + } + + @Override + public AgentSpanContext spanContext() { + return context; + } + + // ----- recorded state accessors ----- + + CharSequence recordedSpanType() { + return spanType; + } + + Map recordedTags() { + return tags; + } + + CharSequence recordedIntegrationName() { + return context.recordedIntegrationName(); + } + + boolean serviceNameSet() { + return serviceNameSet; + } + + String recordedServiceName() { + return serviceName; + } + + CharSequence recordedServiceNameSource() { + return serviceNameSource; + } + + boolean recordedMeasured() { + return measured; + } + + boolean metricSet() { + return metricSet; + } + + TagMap.EntryReader recordedMetric() { + return metric; + } + + // ----- inert reads (mirror NoopSpan) ----- + + @Override + public DDTraceId getTraceId() { + return DDTraceId.ZERO; + } + + @Override + public long getSpanId() { + return 0; + } + + @Override + public RequestBlockingAction getRequestBlockingAction() { + return null; + } + + @Override + public boolean isError() { + return false; + } + + @Override + public Object getTag(String key) { + return tags.get(key); + } + + @Override + public long getStartTime() { + return 0; + } + + @Override + public long getDurationNano() { + return 0; + } + + @Override + public String getOperationName() { + return null; + } + + @Override + public String getServiceName() { + return serviceName; + } + + @Override + public CharSequence getResourceName() { + return null; + } + + @Override + public RequestContext getRequestContext() { + return RequestContext.Noop.INSTANCE; + } + + @Override + public Integer getSamplingPriority() { + return (int) PrioritySampling.UNSET; + } + + @Override + public String getSpanType() { + return spanType == null ? null : spanType.toString(); + } + + @Override + public TagMap getTags() { + return TagMap.EMPTY; + } + + @Override + public AgentSpan getRootSpan() { + return this; + } + + @Override + public short getHttpStatusCode() { + return 0; + } + + @Override + public AgentSpan getLocalRootSpan() { + return this; + } + + @Override + public boolean isSameTrace(AgentSpan otherSpan) { + return otherSpan == this; + } + + @Override + public String getBaggageItem(String key) { + return null; + } + + @Override + public String getSpanName() { + return ""; + } + + @Override + public boolean hasResourceName() { + return false; + } + + @Override + public byte getResourceNamePriority() { + return Byte.MAX_VALUE; + } + + @Override + public TraceConfig traceConfig() { + return AgentTracer.NoopTraceConfig.INSTANCE; + } + + @Override + public boolean isOutbound() { + return false; + } +} diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/RecordingSpanContext.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/RecordingSpanContext.java new file mode 100644 index 00000000000..dc3a0565fc9 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/RecordingSpanContext.java @@ -0,0 +1,62 @@ +package datadog.trace.bootstrap.instrumentation.decorator; + +import datadog.trace.api.DDTraceId; +import datadog.trace.api.datastreams.PathwayContext; +import datadog.trace.api.sampling.PrioritySampling; +import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; +import datadog.trace.bootstrap.instrumentation.api.AgentTraceCollector; +import java.util.Collections; +import java.util.Map; + +/** + * A recording {@link AgentSpanContext} test double for decorator {@code afterStart} tests. Captures + * the integration name applied by {@link datadog.trace.bootstrap.instrumentation.decorator} + * decorators; every other accessor returns an inert default. + */ +final class RecordingSpanContext implements AgentSpanContext { + private CharSequence integrationName; + + @Override + public void setIntegrationName(CharSequence componentName) { + this.integrationName = componentName; + } + + CharSequence recordedIntegrationName() { + return integrationName; + } + + @Override + public DDTraceId getTraceId() { + return DDTraceId.ZERO; + } + + @Override + public long getSpanId() { + return 0; + } + + @Override + public AgentTraceCollector getTraceCollector() { + return null; + } + + @Override + public int getSamplingPriority() { + return PrioritySampling.UNSET; + } + + @Override + public Iterable> baggageItems() { + return Collections.emptyList(); + } + + @Override + public PathwayContext getPathwayContext() { + return null; + } + + @Override + public boolean isRemote() { + return false; + } +} diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java index 57809e76069..ce64e816d7e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java @@ -679,6 +679,13 @@ public void apply(@Nonnull final SpanPrototype prototype) { context.apply(prototype); } + @Override + public void applyOverwriting(@Nonnull final SpanPrototype prototype) { + // Route straight to the context (owner of the tag map + future fast path) rather than through + // the interface default's per-setter delegation. + context.applyOverwriting(prototype); + } + // Getters @Override 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 1b211b5fae1..e3b2f81f047 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 @@ -4,6 +4,7 @@ import static datadog.trace.api.DDTags.SPAN_LINKS; import static datadog.trace.bootstrap.instrumentation.api.ErrorPriorities.UNSET; import static datadog.trace.bootstrap.instrumentation.api.ServiceNameSources.MANUAL; +import static datadog.trace.bootstrap.instrumentation.api.ServiceNameSources.SPLIT_BY_TAGS; import datadog.trace.api.Config; import datadog.trace.api.DDSpanId; @@ -30,6 +31,7 @@ import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import datadog.trace.bootstrap.instrumentation.api.SpanPrototype; +import datadog.trace.bootstrap.instrumentation.api.SplitByTagsPriorities; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.core.propagation.PropagationTags; @@ -146,6 +148,13 @@ public class DDSpanContext private volatile byte resourceNamePriority = ResourceNamePriorities.DEFAULT; + /** + * Tracks the highest-priority {@code trace.split-by-tags} candidate applied so far, so that e.g. + * {@code component} always wins over {@code language} regardless of which order they're processed + * in when several land on the span together. See {@link SplitByTagsPriorities}. + */ + private volatile byte splitByTagsPriority = SplitByTagsPriorities.UNSET; + /** Each span have an operation name describing the current span */ private volatile CharSequence operationName; @@ -507,6 +516,20 @@ public void setServiceName(String serviceName, @Nonnull CharSequence source) { setServiceNameSource(Objects.requireNonNull(source)); } + /** + * Sets the service name from a {@code trace.split-by-tags} candidate tag, but only if {@code + * priority} is at least as high as the last such candidate that won -- see {@link + * SplitByTagsPriorities}. Guards against split-by-tags candidates that land on the span together + * (e.g. {@code component} and {@code language} via a single {@code SpanPrototype} application) + * from overwriting each other in whatever order they happen to be processed. + */ + public void setSplitByTagsServiceName(String serviceName, byte priority) { + if (priority >= this.splitByTagsPriority) { + this.splitByTagsPriority = priority; + setServiceName(serviceName, SPLIT_BY_TAGS); + } + } + public CharSequence getServiceNameSource() { return serviceNameSource; } @@ -608,9 +631,10 @@ public void setSpanType(final CharSequence spanType) { * earlier decorator) wins. Because it never clobbers, {@code apply} is order-independent and * self-neutralizes once construction has already seeded the same prototype. * - *

This is the shared seam for both the construction path ({@code CoreSpanBuilder}) and - * decorator {@code afterStart} (via {@link DDSpan#apply}). The context owns the tag map, so the - * eventual cheaper bulk-share path (skipping interception for non-intercepted tags) and the + *

This is the construction-time seam ({@code CoreSpanBuilder}) -- see {@link + * #applyOverwriting} for the decorator {@code afterStart} seam (via {@link + * DDSpan#applyOverwriting}), which needs different precedence. The context owns the tag map, so + * the eventual cheaper bulk-share path (skipping interception for non-intercepted tags) and the * identity short-circuit will land here -- deferred to the dense-store / tag-registry work, which * exposes intercept status at the internal-api level. Until then the constant tags route through * the interceptor, identical to the per-tag calls this replaces. @@ -631,6 +655,24 @@ public void apply(@Nonnull final SpanPrototype prototype) { } } + /** + * Applies a {@link SpanPrototype} unconditionally: stamps its span type, constant tags, and + * integration name over whatever is already present. This is the decorator {@code afterStart} + * seam -- see {@link datadog.trace.bootstrap.instrumentation.api.AgentSpan#applyOverwriting} for + * why it needs different precedence than {@link #apply}. + */ + public void applyOverwriting(@Nonnull final SpanPrototype prototype) { + final CharSequence spanType = prototype.spanType(); + if (spanType != null) { + setSpanType(spanType); + } + setAllTags(prototype.tags(), true); + final CharSequence integrationName = prototype.integrationName(); + if (integrationName != null) { + setIntegrationName(integrationName); + } + } + /** * Seeds tags that are not already present, routed through the interceptor. Mirrors {@link * #setAllTags(TagMap, boolean)}'s intercepting path but skips any key already set, so explicit diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index d81a9cc8441..44f7a3dcc6e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -7,7 +7,6 @@ import static datadog.trace.api.sampling.PrioritySampling.USER_DROP; import static datadog.trace.bootstrap.instrumentation.api.InstrumentationTags.SERVLET_CONTEXT; import static datadog.trace.bootstrap.instrumentation.api.ServiceNameSources.SPLIT_BY_SERVLET_CONTEXT; -import static datadog.trace.bootstrap.instrumentation.api.ServiceNameSources.SPLIT_BY_TAGS; import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_METHOD; import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_STATUS; import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_URL; @@ -32,6 +31,7 @@ import datadog.trace.api.sampling.SamplingMechanism; import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; +import datadog.trace.bootstrap.instrumentation.api.SplitByTagsPriorities; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.URIUtils; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; @@ -244,7 +244,7 @@ private static void setResourceFromUrl( private boolean intercept(DDSpanContext span, String tag, Object value) { if (splitServiceTags.contains(tag)) { - span.setServiceName(String.valueOf(value), SPLIT_BY_TAGS); + span.setSplitByTagsServiceName(String.valueOf(value), SplitByTagsPriorities.of(tag)); return true; } return false; diff --git a/dd-trace-core/src/test/java/datadog/trace/core/DecoratorPrototypeGlobalTagPrecedenceTest.java b/dd-trace-core/src/test/java/datadog/trace/core/DecoratorPrototypeGlobalTagPrecedenceTest.java new file mode 100644 index 00000000000..6acc104b027 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/DecoratorPrototypeGlobalTagPrecedenceTest.java @@ -0,0 +1,85 @@ +package datadog.trace.core; + +import static datadog.trace.bootstrap.instrumentation.api.Tags.COMPONENT; +import static datadog.trace.test.junit.utils.config.WithConfigExtension.injectSysConfig; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import datadog.trace.bootstrap.instrumentation.api.SpanPrototype; +import datadog.trace.common.writer.ListWriter; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Regression test for a precedence bug between global tags ({@code DD_TAGS} / {@code + * DD_TRACE_SPAN_TAGS}, seeded onto the context during construction) and a decorator's own identity + * (applied in {@code BaseDecorator.afterStart}, mirrored here via {@code + * AgentSpan#applyOverwriting}). A global tag on a decorator-owned key (e.g. {@code component}) must + * never suppress the decorator's own value -- see {@code AgentSpan#apply} (fill-absent, + * construction seam) vs {@code AgentSpan#applyOverwriting} (unconditional, decorator seam). + */ +public class DecoratorPrototypeGlobalTagPrecedenceTest extends DDCoreJavaSpecification { + + private ListWriter writer; + + @BeforeEach + void setup() { + writer = new ListWriter(); + } + + @AfterEach + void cleanup() { + // no-op: each test builds and closes its own tracer + } + + @Test + void decoratorIdentityOverwritesGlobalTagOnSameKey() { + // Simulates `dd.trace.span.tags=component:global-value` clashing with a decorator's own + // component. + injectSysConfig("dd.trace.span.tags", "component:global-value"); + CoreTracer tracer = tracerBuilder().writer(writer).build(); + try { + DDSpan span = (DDSpan) tracer.buildSpan("test", "op name").withServiceName("foo").start(); + try { + // Global tag is seeded at construction time, before any decorator runs. + assertEquals("global-value", span.getTags().get(COMPONENT)); + + // Decorator's afterStart mirrors BaseDecorator.doAfterStart: applyOverwriting must win. + SpanPrototype decoratorPrototype = + SpanPrototype.builder().initComponentAndIntegration("decorator-component").build(); + span.applyOverwriting(decoratorPrototype); + + assertEquals("decorator-component", span.getTags().get(COMPONENT)); + } finally { + span.finish(); + } + } finally { + tracer.close(); + } + } + + @Test + void fillAbsentApplyWouldLetGlobalTagWinDemonstratingWhyOverwritingIsNeeded() { + // Documents the bug applyOverwriting fixes: the fill-absent `apply` seam is correct for + // construction-time seeding, but would leave the global tag in place if used for decorators. + injectSysConfig("dd.trace.span.tags", "component:global-value"); + CoreTracer tracer = tracerBuilder().writer(writer).build(); + try { + DDSpan span = (DDSpan) tracer.buildSpan("test", "op name").withServiceName("foo").start(); + try { + assertEquals("global-value", span.getTags().get(COMPONENT)); + + SpanPrototype decoratorPrototype = + SpanPrototype.builder().initComponentAndIntegration("decorator-component").build(); + span.apply(decoratorPrototype); + + // Fill-absent semantics: the key is already present (global tag), so it is left alone. + assertEquals("global-value", span.getTags().get(COMPONENT)); + } finally { + span.finish(); + } + } finally { + tracer.close(); + } + } +} diff --git a/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorTest.java index ae85244a729..eaffdb21674 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/taginterceptor/TagInterceptorTest.java @@ -48,8 +48,11 @@ import datadog.trace.test.junit.utils.config.WithConfig; import datadog.trace.test.junit.utils.converter.ConfigDefaultsConverter; import datadog.trace.test.junit.utils.converter.TagsConverter; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.function.Function; import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -191,14 +194,16 @@ void mappingCausesServletContextToNotChangeServiceName(String serviceName) { } private CoreTracer createSplittingTracer(String tag) { + return createSplittingTracer(Collections.singleton(tag)); + } + + private CoreTracer createSplittingTracer(Set tags) { return tracerBuilder() .serviceName("my-service") .writer(new LoggingWriter()) .sampler(new AllSampler()) - // equivalent to split-by-tags: tag - .tagInterceptor( - new TagInterceptor( - true, "my-service", Collections.singleton(tag), new RuleFlags(), false)) + // equivalent to split-by-tags: tags + .tagInterceptor(new TagInterceptor(true, "my-service", tags, new RuleFlags(), false)) .build(); } @@ -282,6 +287,33 @@ void splitByTagsThenPeerServiceViaSetTag() { assertEquals("peer-service", span.getServiceName()); } + /** + * A server decorator's own identity tags -- {@code component} and {@code language} -- can both be + * split-by-tags candidates. The decorator applies them together (e.g. via a single {@code + * SpanPrototype}), so {@code component} must win over {@code language} for the service name + * regardless of which order they happen to be processed in -- matching the old, sequential {@code + * setTag} calls that used to set language before component unconditionally. + */ + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void splitByTagsComponentWinsOverLanguageRegardlessOfOrder(boolean componentFirst) { + CoreTracer tracer = + createSplittingTracer( + new HashSet<>(Arrays.asList(Tags.COMPONENT, DDTags.LANGUAGE_TAG_KEY))); + + AgentSpan span = tracer.buildSpan("datadog", "some span").start(); + if (componentFirst) { + span.setTag(Tags.COMPONENT, "my-component"); + span.setTag(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE); + } else { + span.setTag(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE); + span.setTag(Tags.COMPONENT, "my-component"); + } + span.finish(); + + assertEquals("my-component", span.getServiceName()); + } + @Test void setResourceName() throws Exception { ListWriter writer = new ListWriter(); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java index dd17b8ccc7c..d0a86382793 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java @@ -227,11 +227,11 @@ default boolean isValid() { * order-independent, and self-neutralizes once construction has already seeded the same * prototype. * - *

This is the single seam through which a prototype's constant initial state is applied, - * shared by the construction path (buildSpan/startSpan) and decorator {@code afterStart}. Core - * spans override to route straight to the context, which owns the tag map and will host the - * eventual fast path (bulk share / identity short-circuit); this default is the best-effort - * fallback for other span implementations. + *

This is the fill-absent seam through which a prototype's constant initial state is applied + * at construction (buildSpan/startSpan) -- see {@link #applyOverwriting} for the decorator {@code + * afterStart} seam, which needs different precedence. Core spans override to route straight to + * the context, which owns the tag map and will host the eventual fast path (bulk share / identity + * short-circuit); this default is the best-effort fallback for other span implementations. */ default void apply(@Nonnull final SpanPrototype prototype) { if (getSpanType() == null) { @@ -260,6 +260,32 @@ default void apply(@Nonnull final SpanPrototype prototype) { } } + /** + * Applies a {@link SpanPrototype} unconditionally: stamps its span type, constant tags, and + * integration name over whatever is already present. This is the decorator {@code afterStart} + * seam -- a decorator's identity (its component, span kind, span type, integration name) is + * authoritative and must win over anything set earlier, such as a global tag from {@code DD_TAGS} + * / {@code DD_TRACE_SPAN_TAGS} seeded onto the span at construction, matching the unconditional + * {@code setTag}/{@code setSpanType} calls this replaced. + * + *

Kept separate from {@link #apply}, which is the fill-absent seam for construction-time + * seeding: the two callers have genuinely different precedence needs, and collapsing them + * previously let a global tag silently suppress a decorator's own value. + */ + default void applyOverwriting(@Nonnull final SpanPrototype prototype) { + final CharSequence spanType = prototype.spanType(); + if (spanType != null) { + setSpanType(spanType); + } + + prototype.tags().forEach((tag, value) -> setTag(tag, value)); + + final CharSequence integrationName = prototype.integrationName(); + if (integrationName != null) { + spanContext().setIntegrationName(integrationName); + } + } + default AgentSpan asAgentSpan() { return this; } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SplitByTagsPriorities.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SplitByTagsPriorities.java new file mode 100644 index 00000000000..2d29215cc3f --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SplitByTagsPriorities.java @@ -0,0 +1,39 @@ +package datadog.trace.bootstrap.instrumentation.api; + +import datadog.trace.api.DDTags; + +/** + * Precedence among the decorator-owned identity tags ({@code component}, {@code language}, {@code + * span.kind}) when more than one is configured as a {@code trace.split-by-tags} candidate and they + * land on a span together, e.g. via a single {@code SpanPrototype} application. Mirrors the + * precedence the old, sequential {@code setTag} calls in {@code ServerDecorator}/{@code + * BaseDecorator} used to give for free (span kind, then language, then component last). + * + *

Any other split-by-tags candidate (a user-configured custom tag, or one set directly by + * instrumentation code rather than via a decorator's prototype) is not one of these known identity + * tags, so it always outranks them -- matching the old behavior where such a tag's {@code setTag} + * call, being outside the decorator's fixed sequence, was never contended with span + * kind/language/component. + */ +public final class SplitByTagsPriorities { + public static final byte UNSET = 0; + public static final byte SPAN_KIND = 1; + public static final byte LANGUAGE = 2; + public static final byte COMPONENT = 3; + public static final byte OTHER = Byte.MAX_VALUE; + + public static byte of(String tag) { + if (Tags.SPAN_KIND.equals(tag)) { + return SPAN_KIND; + } + if (DDTags.LANGUAGE_TAG_KEY.equals(tag)) { + return LANGUAGE; + } + if (Tags.COMPONENT.equals(tag)) { + return COMPONENT; + } + return OTHER; + } + + private SplitByTagsPriorities() {} +}