From 518aae0ddfc916f036800851c802eba8e4b8ddb2 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 7 Mar 2025 18:00:10 -0500 Subject: [PATCH 01/16] initial implementation of TRACE_PROPAGATION_BEHAVIOR_EXTRACT --- .../datadog/trace/api/ConfigDefaults.java | 1 + .../trace/api/config/TracerConfig.java | 1 + .../java/datadog/trace/core/CoreTracer.java | 23 +++++++++++++++---- .../trace/core/propagation/B3HttpCodec.java | 2 +- .../trace/core/propagation/HttpCodec.java | 15 ++++++++---- .../core/propagation/HttpExtractorTest.groovy | 2 ++ .../main/java/datadog/trace/api/Config.java | 21 +++++++++++++++-- .../instrumentation/api/TagContext.java | 4 ++++ 8 files changed, 57 insertions(+), 12 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index f3b58a88f71..2f1051e12b2 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -91,6 +91,7 @@ public final class ConfigDefaults { static final int DEFAULT_CLOCK_SYNC_PERIOD = 30; // seconds + static final String DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT = "continue"; static final boolean DEFAULT_TRACE_PROPAGATION_EXTRACT_FIRST = false; static final boolean DEFAULT_JMX_FETCH_MULTIPLE_RUNTIME_SERVICES_ENABLED = false; diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/TracerConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/TracerConfig.java index e4cebef3308..152b5e1b994 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/TracerConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/TracerConfig.java @@ -90,6 +90,7 @@ public final class TracerConfig { public static final String TRACE_PROPAGATION_STYLE = "trace.propagation.style"; public static final String TRACE_PROPAGATION_STYLE_EXTRACT = "trace.propagation.style.extract"; public static final String TRACE_PROPAGATION_STYLE_INJECT = "trace.propagation.style.inject"; + public static final String TRACE_PROPAGATION_BEHAVIOR_EXTRACT = "trace.propagation.behavior.extract"; public static final String TRACE_PROPAGATION_EXTRACT_FIRST = "trace.propagation.extract.first"; public static final String TRACE_BAGGAGE_MAX_ITEMS = "trace.baggage.max.items"; public static final String TRACE_BAGGAGE_MAX_BYTES = "trace.baggage.max.bytes"; 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 49eb2fe5728..92a86dae9ca 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 @@ -63,6 +63,7 @@ import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; import datadog.trace.bootstrap.instrumentation.api.ScopeSource; import datadog.trace.bootstrap.instrumentation.api.ScopeState; +import datadog.trace.bootstrap.instrumentation.api.SpanAttributes; import datadog.trace.bootstrap.instrumentation.api.TagContext; import datadog.trace.civisibility.interceptor.CiVisibilityApmProtocolInterceptor; import datadog.trace.civisibility.interceptor.CiVisibilityTelemetryInterceptor; @@ -1493,6 +1494,8 @@ private DDSpanContext buildSpanContext() { final PathwayContext pathwayContext; final PropagationTags propagationTags; + System.out.println("inside buildSpanContext"); + if (this.spanId == 0) { spanId = idGenerationStrategy.generateSpanId(); } else { @@ -1515,6 +1518,7 @@ private DDSpanContext buildSpanContext() { // Note: if we are not in the context of distributed tracing and we are starting the first // root span, parentContext will be null at this point. if (parentContext instanceof DDSpanContext) { + System.out.println("inside DDSPANCONTEXT"); final DDSpanContext ddsc = (DDSpanContext) parentContext; traceId = ddsc.getTraceId(); parentSpanId = ddsc.getSpanId(); @@ -1546,11 +1550,20 @@ private DDSpanContext buildSpanContext() { // Propagate external trace isRemote = true; final ExtractedContext extractedContext = (ExtractedContext) parentContext; - traceId = extractedContext.getTraceId(); - parentSpanId = extractedContext.getSpanId(); - samplingPriority = extractedContext.getSamplingPriority(); - endToEndStartTime = extractedContext.getEndToEndStartTime(); - propagationTags = extractedContext.getPropagationTags(); + + if(Config.get().getTracePropagationBehaviorExtract().equals("restart")){ + traceId = idGenerationStrategy.generateTraceId(); + parentSpanId = 0; + samplingPriority = parentContext.getSamplingPriority(); + endToEndStartTime = 0; + propagationTags = propagationTagsFactory.empty(); + }else{ + traceId = extractedContext.getTraceId(); + parentSpanId = extractedContext.getSpanId(); + samplingPriority = extractedContext.getSamplingPriority(); + endToEndStartTime = extractedContext.getEndToEndStartTime(); + propagationTags = extractedContext.getPropagationTags(); + } } else if (parentContext != null) { traceId = parentContext.getTraceId() == DDTraceId.ZERO diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java index 910449b7dd1..32885ec8089 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java @@ -164,7 +164,7 @@ static HttpCodec.Extractor newExtractor( final List extractors = new ArrayList<>(2); extractors.add(newSingleExtractor(config, traceConfigSupplier)); extractors.add(newMultiExtractor(config, traceConfigSupplier)); - return new HttpCodec.CompoundExtractor(extractors, config.isTracePropagationExtractFirst()); + return new HttpCodec.CompoundExtractor(extractors, config.isTracePropagationExtractFirst(), config.getTracePropagationBehaviorExtract()); } public static HttpCodec.Extractor newMultiExtractor( diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java index 9835d2e1c03..7a185af5c5d 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java @@ -171,10 +171,8 @@ public static Extractor createExtractor( switch (extractors.size()) { case 0: return StubExtractor.INSTANCE; - case 1: - return extractors.get(0); default: - return new CompoundExtractor(extractors, config.isTracePropagationExtractFirst()); + return new CompoundExtractor(extractors, config.isTracePropagationExtractFirst(), config.getTracePropagationBehaviorExtract()); } } @@ -208,10 +206,12 @@ public TagContext extract(C carrier, AgentPropagation.ContextVisitor gett public static class CompoundExtractor implements Extractor { private final List extractors; private final boolean extractFirst; + private final String extractBehavior; - public CompoundExtractor(final List extractors, boolean extractFirst) { + public CompoundExtractor(final List extractors, boolean extractFirst, String extractBehavior) { this.extractors = extractors; this.extractFirst = extractFirst; + this.extractBehavior = extractBehavior; } @Override @@ -262,6 +262,13 @@ else if (extracted != null && partialContext == null) { } if (context != null) { + if(extractBehavior.equals("restart")){ + context.resetTerminatedContextLink(); + context.addTerminatedContextLink(DDSpanLink.from(context, SpanAttributes.builder() + .put("reason", "propagation_behavior_extract") + .put("context_headers", context.getPropagationStyle().toString()) + .build())); + } log.debug("Extract complete context {}", context); return context; } else if (partialContext != null) { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy index e9642463fe9..a85f110bff3 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy @@ -20,6 +20,7 @@ import static datadog.trace.core.CoreTracer.TRACE_ID_MAX class HttpExtractorTest extends DDSpecification { static final W3C_TRACE_ID = "00000000000000000000000000000001" static final W3C_SPAN_ID = "123456789abcdef0" + static final W3C_HEX_SPAN_ID = "1311768467463790320" static final W3C_TRACE_PARENT = "00-$W3C_TRACE_ID-$W3C_SPAN_ID-01" static final W3C_TRACE_STATE_WITH_P = "dd=p:456789abcdef0123" static final W3C_TRACE_STATE_NO_P = "dd=s:2,foo=1" @@ -171,6 +172,7 @@ class HttpExtractorTest extends DDSpecification { setup: Config config = Mock(Config) { getTracePropagationStylesToExtract() >> styles + getTracePropagationBehaviorExtract() >> "continue" } DynamicConfig dynamicConfig = DynamicConfig.create().apply() HttpCodec.Extractor extractor = HttpCodec.createExtractor(config, { dynamicConfig.captureTraceConfig() }) 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 3dba6f5b8f4..ef0e0f05ef6 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -189,6 +189,7 @@ public static String getHostName() { private final boolean tracePropagationStyleB3PaddingEnabled; private final Set tracePropagationStylesToExtract; private final Set tracePropagationStylesToInject; + private final String tracePropagationBehaviorExtract; private final boolean tracePropagationExtractFirst; private final int traceBaggageMaxItems; private final int traceBaggageMaxBytes; @@ -924,6 +925,11 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins tracePropagationStyleB3PaddingEnabled = isEnabled(true, TRACE_PROPAGATION_STYLE, ".b3.padding.enabled"); + + tracePropagationBehaviorExtract = + configProvider.getString( + TRACE_PROPAGATION_BEHAVIOR_EXTRACT, DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT); + { // The dd.propagation.style.(extract|inject) settings have been deprecated in // favor of dd.trace.propagation.style(|.extract|.inject) settings. @@ -995,8 +1001,13 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins logOverriddenDeprecatedSettingWarning(PROPAGATION_STYLE_INJECT, injectOrigin, inject); } // Now we can check if we should pick the default injection/extraction - tracePropagationStylesToExtract = - extract.isEmpty() ? DEFAULT_TRACE_PROPAGATION_STYLE : extract; + + if (extract.isEmpty()){ + extract = DEFAULT_TRACE_PROPAGATION_STYLE; + } + + tracePropagationStylesToExtract = tracePropagationBehaviorExtract.equals("ignore") ? new HashSet<>() : extract; + tracePropagationStylesToInject = inject.isEmpty() ? DEFAULT_TRACE_PROPAGATION_STYLE : inject; traceBaggageMaxItems = @@ -2260,6 +2271,10 @@ public Set getTracePropagationStylesToInject() { return tracePropagationStylesToInject; } + public String getTracePropagationBehaviorExtract() { + return tracePropagationBehaviorExtract; + } + public boolean isTracePropagationExtractFirst() { return tracePropagationExtractFirst; } @@ -4441,6 +4456,8 @@ public String toString() { + tracePropagationStylesToExtract + ", tracePropagationStylesToInject=" + tracePropagationStylesToInject + + ", tracePropagationBehaviorExtract=" + + tracePropagationBehaviorExtract + ", tracePropagationExtractFirst=" + tracePropagationExtractFirst + ", clockSyncPeriod=" diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java index 226dca2bc30..4ec1b42ec02 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java @@ -89,6 +89,10 @@ public void addTerminatedContextLink(AgentSpanLink link) { this.terminatedContextLinks.add(link); } + public void resetTerminatedContextLink() { + this.terminatedContextLinks = null; + } + @Override public String getForwarded() { return httpHeaders.forwarded; From 8ff205bf16ac4e750c6974317266b725e1185e09 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Mon, 10 Mar 2025 16:21:17 -0400 Subject: [PATCH 02/16] initial implementation and unit tests for TRACE_PROPAGATION_BEHAVIOR_EXTRACT --- .../trace/api/config/TracerConfig.java | 3 +- .../java/datadog/trace/core/CoreTracer.java | 8 +-- .../main/java/datadog/trace/core/DDSpan.java | 4 ++ .../trace/core/propagation/B3HttpCodec.java | 5 +- .../trace/core/propagation/HttpCodec.java | 21 +++++--- .../trace/core/CoreSpanBuilderTest.groovy | 26 ++++++++++ .../core/propagation/HttpExtractorTest.groovy | 49 ++++++++++++++++++- .../main/java/datadog/trace/api/Config.java | 5 +- .../datadog/trace/api/ConfigTest.groovy | 19 +++++++ 9 files changed, 122 insertions(+), 18 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/TracerConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/TracerConfig.java index 152b5e1b994..7b9c2a6ee9c 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/TracerConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/TracerConfig.java @@ -90,7 +90,8 @@ public final class TracerConfig { public static final String TRACE_PROPAGATION_STYLE = "trace.propagation.style"; public static final String TRACE_PROPAGATION_STYLE_EXTRACT = "trace.propagation.style.extract"; public static final String TRACE_PROPAGATION_STYLE_INJECT = "trace.propagation.style.inject"; - public static final String TRACE_PROPAGATION_BEHAVIOR_EXTRACT = "trace.propagation.behavior.extract"; + public static final String TRACE_PROPAGATION_BEHAVIOR_EXTRACT = + "trace.propagation.behavior.extract"; public static final String TRACE_PROPAGATION_EXTRACT_FIRST = "trace.propagation.extract.first"; public static final String TRACE_BAGGAGE_MAX_ITEMS = "trace.baggage.max.items"; public static final String TRACE_BAGGAGE_MAX_BYTES = "trace.baggage.max.bytes"; 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 92a86dae9ca..33ddf55c7f1 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 @@ -63,7 +63,6 @@ import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; import datadog.trace.bootstrap.instrumentation.api.ScopeSource; import datadog.trace.bootstrap.instrumentation.api.ScopeState; -import datadog.trace.bootstrap.instrumentation.api.SpanAttributes; import datadog.trace.bootstrap.instrumentation.api.TagContext; import datadog.trace.civisibility.interceptor.CiVisibilityApmProtocolInterceptor; import datadog.trace.civisibility.interceptor.CiVisibilityTelemetryInterceptor; @@ -1494,8 +1493,6 @@ private DDSpanContext buildSpanContext() { final PathwayContext pathwayContext; final PropagationTags propagationTags; - System.out.println("inside buildSpanContext"); - if (this.spanId == 0) { spanId = idGenerationStrategy.generateSpanId(); } else { @@ -1518,7 +1515,6 @@ private DDSpanContext buildSpanContext() { // Note: if we are not in the context of distributed tracing and we are starting the first // root span, parentContext will be null at this point. if (parentContext instanceof DDSpanContext) { - System.out.println("inside DDSPANCONTEXT"); final DDSpanContext ddsc = (DDSpanContext) parentContext; traceId = ddsc.getTraceId(); parentSpanId = ddsc.getSpanId(); @@ -1551,13 +1547,13 @@ private DDSpanContext buildSpanContext() { isRemote = true; final ExtractedContext extractedContext = (ExtractedContext) parentContext; - if(Config.get().getTracePropagationBehaviorExtract().equals("restart")){ + if (Config.get().getTracePropagationBehaviorExtract().equals("restart")) { traceId = idGenerationStrategy.generateTraceId(); parentSpanId = 0; samplingPriority = parentContext.getSamplingPriority(); endToEndStartTime = 0; propagationTags = propagationTagsFactory.empty(); - }else{ + } else { traceId = extractedContext.getTraceId(); parentSpanId = extractedContext.getSpanId(); samplingPriority = extractedContext.getSamplingPriority(); 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 0b576654936..41964a93088 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 @@ -856,4 +856,8 @@ public boolean isOutbound() { Object spanKind = context.getTag(Tags.SPAN_KIND); return Tags.SPAN_KIND_CLIENT.equals(spanKind) || Tags.SPAN_KIND_PRODUCER.equals(spanKind); } + + public List getLinks() { + return links; + } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java index 32885ec8089..dce68583031 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java @@ -164,7 +164,10 @@ static HttpCodec.Extractor newExtractor( final List extractors = new ArrayList<>(2); extractors.add(newSingleExtractor(config, traceConfigSupplier)); extractors.add(newMultiExtractor(config, traceConfigSupplier)); - return new HttpCodec.CompoundExtractor(extractors, config.isTracePropagationExtractFirst(), config.getTracePropagationBehaviorExtract()); + return new HttpCodec.CompoundExtractor( + extractors, + config.isTracePropagationExtractFirst(), + config.getTracePropagationBehaviorExtract()); } public static HttpCodec.Extractor newMultiExtractor( diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java index 7a185af5c5d..06967576d41 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java @@ -172,7 +172,10 @@ public static Extractor createExtractor( case 0: return StubExtractor.INSTANCE; default: - return new CompoundExtractor(extractors, config.isTracePropagationExtractFirst(), config.getTracePropagationBehaviorExtract()); + return new CompoundExtractor( + extractors, + config.isTracePropagationExtractFirst(), + config.getTracePropagationBehaviorExtract()); } } @@ -208,7 +211,8 @@ public static class CompoundExtractor implements Extractor { private final boolean extractFirst; private final String extractBehavior; - public CompoundExtractor(final List extractors, boolean extractFirst, String extractBehavior) { + public CompoundExtractor( + final List extractors, boolean extractFirst, String extractBehavior) { this.extractors = extractors; this.extractFirst = extractFirst; this.extractBehavior = extractBehavior; @@ -262,12 +266,15 @@ else if (extracted != null && partialContext == null) { } if (context != null) { - if(extractBehavior.equals("restart")){ + if (extractBehavior.equals("restart")) { context.resetTerminatedContextLink(); - context.addTerminatedContextLink(DDSpanLink.from(context, SpanAttributes.builder() - .put("reason", "propagation_behavior_extract") - .put("context_headers", context.getPropagationStyle().toString()) - .build())); + context.addTerminatedContextLink( + DDSpanLink.from( + context, + SpanAttributes.builder() + .put("reason", "propagation_behavior_extract") + .put("context_headers", context.getPropagationStyle().toString()) + .build())); } log.debug("Extract complete context {}", context); return context; diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy index 28ceda7c1c4..f94fa7c3130 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy @@ -342,6 +342,32 @@ class CoreSpanBuilderTest extends DDCoreSpecification { new ExtractedContext(DDTraceId.from(3), 4, PrioritySampling.SAMPLER_KEEP, "some-origin", 0, ["asdf": "qwer"], [(ORIGIN_KEY): "some-origin", "zxcv": "1234"], null, PropagationTags.factory().empty(), null, DATADOG) | _ } + def "build context from ExtractedContext with TRACE_PROPAGATION_BEHAVIOR_EXTRACT=restart"() { + setup: + injectSysConfig("trace.propagation.behavior.extract", "restart") + def thread = Thread.currentThread() + TagContext context = extractedContext + context.addTerminatedContextLink(DDSpanLink.from(extractedContext)) + final DDSpan span = tracer.buildSpan("test", "op name") + .asChildOf(context).start() + + expect: + span.traceId != extractedContext.traceId + span.parentId != extractedContext.spanId + + def spanLinks = span.links + + assert spanLinks.size() == 1 + def link = spanLinks[0] + link.traceId() == extractedContext.traceId + link.spanId() == extractedContext.spanId + link.traceState() == extractedContext.propagationTags.headerValue(PropagationTags.HeaderType.W3C) + + where: + extractedContext | _ + new ExtractedContext(DDTraceId.ONE, 2, PrioritySampling.SAMPLER_DROP, null, 0, [:], [:], null, PropagationTags.factory().fromHeaderValue(PropagationTags.HeaderType.DATADOG, "_dd.p.dm=934086a686-4,_dd.p.anytag=value"), null, DATADOG) | _ + } + def "TagContext should populate default span details"() { setup: def thread = Thread.currentThread() diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy index a85f110bff3..3b28c9c639e 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy @@ -20,7 +20,6 @@ import static datadog.trace.core.CoreTracer.TRACE_ID_MAX class HttpExtractorTest extends DDSpecification { static final W3C_TRACE_ID = "00000000000000000000000000000001" static final W3C_SPAN_ID = "123456789abcdef0" - static final W3C_HEX_SPAN_ID = "1311768467463790320" static final W3C_TRACE_PARENT = "00-$W3C_TRACE_ID-$W3C_SPAN_ID-01" static final W3C_TRACE_STATE_WITH_P = "dd=p:456789abcdef0123" static final W3C_TRACE_STATE_NO_P = "dd=s:2,foo=1" @@ -34,6 +33,7 @@ class HttpExtractorTest extends DDSpecification { Config config = Mock(Config) { getTracePropagationStylesToExtract() >> styles isTracePropagationExtractFirst() >> extractFirst + getTracePropagationBehaviorExtract() >> "continue" } DynamicConfig dynamicConfig = DynamicConfig.create() .setHeaderTags(["SOME_HEADER": "some-tag"]) @@ -120,6 +120,7 @@ class HttpExtractorTest extends DDSpecification { setup: Config config = Mock(Config) { getTracePropagationStylesToExtract() >> styles + getTracePropagationBehaviorExtract() >> "continue" } DynamicConfig dynamicConfig = DynamicConfig.create().apply() HttpCodec.Extractor extractor = HttpCodec.createExtractor(config, { dynamicConfig.captureTraceConfig() }) @@ -208,4 +209,50 @@ class HttpExtractorTest extends DDSpecification { [TRACECONTEXT, B3MULTI, DATADOG] | "2" | "2" | "1" | "b" | W3C_TRACE_PARENT | W3C_TRACE_STATE_NO_P | [DATADOG] // spotless:on } + + def "verify extract behavior with TRACE_PROPAGATION_BEHAVIOR_EXTRACT"() { + setup: + Config config = Mock(Config) { + getTracePropagationStylesToExtract() >> [DATADOG, TRACECONTEXT] + getTracePropagationBehaviorExtract() >> behaviorExtract + isTracePropagationExtractFirst() >> extractFirst + } + DynamicConfig dynamicConfig = DynamicConfig.create().apply() + HttpCodec.Extractor extractor = HttpCodec.createExtractor(config, { dynamicConfig.captureTraceConfig() }) + + final Map actual = [ + (DatadogHttpCodec.TRACE_ID_KEY.toUpperCase()): datadogTraceId, + (DatadogHttpCodec.SPAN_ID_KEY.toUpperCase()) : datadogSpanId, + (W3CHttpCodec.TRACE_PARENT_KEY.toUpperCase()): w3cTraceParent, + (W3CHttpCodec.TRACE_STATE_KEY.toUpperCase()) : traceState + ] + + when: + final TagContext context = extractor.extract(actual, ContextVisitors.stringValuesMap()) + + then: + def links = context.getTerminatedContextLinks() + assert links.size() == expectedLinkStyle.size() + for (int i = 0; i < links.size(); i++) { + assert links[i].traceId().toString() == linkTraceId + assert links[i].spanId().toString() == linkSpanId + assert links[i].attributes().asMap()["reason"] == linkReason + assert links[i].attributes().asMap()["context_headers"] == expectedLinkStyle[i].toString() + if (expectedLinkStyle[i] == TRACECONTEXT || linkReason == "propagation_behavior_extract" && datadogTraceId == getW3C_TRACE_ID()) { + assert links[i].traceState() == traceState + } + } + + // no tests for behaviorExtract=ignore since no extraction will occur in this situation + where: + // spotless:off + behaviorExtract | extractFirst | datadogTraceId | datadogSpanId | w3cTraceParent | traceState | expectedLinkStyle | linkTraceId | linkSpanId | linkReason + "continue" | false | "1" | "2" | W3C_TRACE_PARENT | "" | [] | "" | "1311768467463790320" | "" + "continue" | false | "2" | "2" | W3C_TRACE_PARENT | W3C_TRACE_STATE_NO_P | [TRACECONTEXT] | "1" | "1311768467463790320" | "terminated_context" + "restart" | false | "1" | "2" | W3C_TRACE_PARENT | "dd=foo=1" | [DATADOG] | "1" | "1311768467463790320" | "propagation_behavior_extract" + "restart" | false | "2" | "2" | W3C_TRACE_PARENT | "" | [DATADOG] | "2" | "2" | "propagation_behavior_extract" + "restart" | true | "2" | "2" | W3C_TRACE_PARENT | "" | [DATADOG] | "2" | "2" | "propagation_behavior_extract" + "restart" | true | "1" | "2" | W3C_TRACE_PARENT | "" | [DATADOG] | "1" | "2" | "propagation_behavior_extract" + // spotless:on + } } 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 ef0e0f05ef6..7ad4819b447 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -1002,11 +1002,12 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins } // Now we can check if we should pick the default injection/extraction - if (extract.isEmpty()){ + if (extract.isEmpty()) { extract = DEFAULT_TRACE_PROPAGATION_STYLE; } - tracePropagationStylesToExtract = tracePropagationBehaviorExtract.equals("ignore") ? new HashSet<>() : extract; + tracePropagationStylesToExtract = + tracePropagationBehaviorExtract.equals("ignore") ? new HashSet<>() : extract; tracePropagationStylesToInject = inject.isEmpty() ? DEFAULT_TRACE_PROPAGATION_STYLE : inject; diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index df5509b13ef..e34c93d5a93 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -123,6 +123,7 @@ import static datadog.trace.api.config.TracerConfig.SPLIT_BY_TAGS import static datadog.trace.api.config.TracerConfig.TRACE_AGENT_PORT import static datadog.trace.api.config.TracerConfig.TRACE_AGENT_URL import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_EXTRACT_FIRST +import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_BEHAVIOR_EXTRACT import static datadog.trace.api.config.TracerConfig.TRACE_RATE_LIMIT import static datadog.trace.api.config.TracerConfig.TRACE_REPORT_HOSTNAME import static datadog.trace.api.config.TracerConfig.TRACE_RESOLVER_ENABLED @@ -205,6 +206,7 @@ class ConfigTest extends DDSpecification { prop.setProperty(PROPAGATION_STYLE_EXTRACT, "Datadog, B3") prop.setProperty(PROPAGATION_STYLE_INJECT, "B3, Datadog") prop.setProperty(TRACE_PROPAGATION_EXTRACT_FIRST, "false") + prop.setProperty(TRACE_PROPAGATION_BEHAVIOR_EXTRACT, "restart") prop.setProperty(JMX_FETCH_ENABLED, "false") prop.setProperty(JMX_FETCH_METRICS_CONFIGS, "/foo.yaml,/bar.yaml") prop.setProperty(JMX_FETCH_CHECK_PERIOD, "100") @@ -295,6 +297,7 @@ class ConfigTest extends DDSpecification { config.tracePropagationStylesToExtract.toList() == [DATADOG, B3SINGLE, B3MULTI] config.tracePropagationStylesToInject.toList() == [B3SINGLE, B3MULTI, DATADOG] config.tracePropagationExtractFirst == false + config.tracePropagationBehaviorExtract == "restart" config.jmxFetchEnabled == false config.jmxFetchMetricsConfigs == ["/foo.yaml", "/bar.yaml"] config.jmxFetchCheckPeriod == 100 @@ -386,6 +389,7 @@ class ConfigTest extends DDSpecification { System.setProperty(PREFIX + PROPAGATION_STYLE_EXTRACT, "Datadog, B3") System.setProperty(PREFIX + PROPAGATION_STYLE_INJECT, "B3, Datadog") System.setProperty(PREFIX + TRACE_PROPAGATION_EXTRACT_FIRST, "false") + System.setProperty(PREFIX + TRACE_PROPAGATION_BEHAVIOR_EXTRACT, "restart") System.setProperty(PREFIX + JMX_FETCH_ENABLED, "false") System.setProperty(PREFIX + JMX_FETCH_METRICS_CONFIGS, "/foo.yaml,/bar.yaml") System.setProperty(PREFIX + JMX_FETCH_CHECK_PERIOD, "100") @@ -475,6 +479,7 @@ class ConfigTest extends DDSpecification { config.tracePropagationStylesToExtract.toList() == [DATADOG, B3SINGLE, B3MULTI] config.tracePropagationStylesToInject.toList() == [B3SINGLE, B3MULTI, DATADOG] config.tracePropagationExtractFirst == false + config.tracePropagationBehaviorExtract == "restart" config.jmxFetchEnabled == false config.jmxFetchMetricsConfigs == ["/foo.yaml", "/bar.yaml"] config.jmxFetchCheckPeriod == 100 @@ -2604,4 +2609,18 @@ class ConfigTest extends DDSpecification { config.finalDebuggerSnapshotUrl == "http://localhost:8126/debugger/v1/input" config.finalDebuggerSymDBUrl == "http://localhost:8126/symdb/v1/input" } + + def "specify overrides for PROPAGATION_STYLE_EXTRACT when TRACE_PROPAGATION_BEHAVIOR_EXTRACT=ignore"() { + setup: + def prop = new Properties() + prop.setProperty(PROPAGATION_STYLE_EXTRACT, "Datadog, B3") + prop.setProperty(TRACE_PROPAGATION_BEHAVIOR_EXTRACT, "ignore") + + when: + Config config = Config.get(prop) + + then: + config.tracePropagationBehaviorExtract == "ignore" + config.tracePropagationStylesToExtract.toList() == [] + } } From 30e7c6cd896c2fcfcbd1cccf44b090e4ce264cd2 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Mon, 10 Mar 2025 16:32:18 -0400 Subject: [PATCH 03/16] change to sampling --- dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java | 2 +- .../test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) 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 33ddf55c7f1..2caf2e732bf 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 @@ -1550,7 +1550,7 @@ private DDSpanContext buildSpanContext() { if (Config.get().getTracePropagationBehaviorExtract().equals("restart")) { traceId = idGenerationStrategy.generateTraceId(); parentSpanId = 0; - samplingPriority = parentContext.getSamplingPriority(); + samplingPriority = PrioritySampling.UNSET; endToEndStartTime = 0; propagationTags = propagationTagsFactory.empty(); } else { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy index f94fa7c3130..920a3e84ff8 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy @@ -354,6 +354,7 @@ class CoreSpanBuilderTest extends DDCoreSpecification { expect: span.traceId != extractedContext.traceId span.parentId != extractedContext.spanId + span.samplingPriority() == PrioritySampling.UNSET def spanLinks = span.links From 3527a7f7484c38447304ae3e83743eb50b428a4f Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Wed, 12 Mar 2025 10:46:53 -0400 Subject: [PATCH 04/16] comments for future work --- dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java | 2 ++ .../java/datadog/trace/core/propagation/ExtractedContext.java | 1 + .../trace/bootstrap/instrumentation/api/AgentSpanContext.java | 2 ++ 3 files changed, 5 insertions(+) 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 2caf2e732bf..e3f67eddd96 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 @@ -1505,6 +1505,7 @@ private DDSpanContext buildSpanContext() { final AgentSpan activeSpan = scopeManager.activeSpan(); if (activeSpan != null) { parentContext = activeSpan.context(); + //check if restart, if so, parentContext=null, and add span links } } @@ -1553,6 +1554,7 @@ private DDSpanContext buildSpanContext() { samplingPriority = PrioritySampling.UNSET; endToEndStartTime = 0; propagationTags = propagationTagsFactory.empty(); + //adding span link from extracted context } else { traceId = extractedContext.getTraceId(); parentSpanId = extractedContext.getSpanId(); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java index e799688401d..f576f756fa8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java @@ -62,6 +62,7 @@ public ExtractedContext( this.spanId = spanId; this.endToEndStartTime = endToEndStartTime; this.propagationTags = propagationTags; + //set remote flag } @Override diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java index 139ea4e4281..cd6636163a4 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java @@ -14,6 +14,8 @@ * contextualize the associated Span instance. */ public interface AgentSpanContext { + + //add remote field. Deafult is false unless coming from tag or extracted /** * Gets the TraceId of the span's trace. * From 7ebfa65d81b78c1efd8f6cdbb4ba7de530d6de2c Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Wed, 12 Mar 2025 15:20:49 -0400 Subject: [PATCH 05/16] adding isremote() to all AgentSpanContexts --- .../shim/trace/OtelExtractedContext.java | 5 +++ .../spark/DatabricksParentContext.java | 5 +++ .../spark/OpenlineageParentContext.java | 5 +++ .../java/datadog/trace/core/CoreTracer.java | 41 +++++++++++-------- .../trace/core/propagation/B3HttpCodec.java | 5 +-- .../core/propagation/ExtractedContext.java | 2 +- .../trace/core/propagation/HttpCodec.java | 20 +-------- .../trace/core/CoreSpanBuilderTest.groovy | 4 +- .../instrumentation/api/AgentSpanContext.java | 3 +- .../instrumentation/api/NoopSpanContext.java | 5 +++ .../instrumentation/api/TagContext.java | 5 +++ 11 files changed, 56 insertions(+), 44 deletions(-) diff --git a/dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelExtractedContext.java b/dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelExtractedContext.java index 7b3551919d8..c8ad8fc4b2a 100644 --- a/dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelExtractedContext.java +++ b/dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelExtractedContext.java @@ -78,4 +78,9 @@ public Iterable> baggageItems() { public PathwayContext getPathwayContext() { return null; } + + @Override + public boolean isRemote() { + return true; + } } diff --git a/dd-java-agent/instrumentation/spark/src/main/java/datadog/trace/instrumentation/spark/DatabricksParentContext.java b/dd-java-agent/instrumentation/spark/src/main/java/datadog/trace/instrumentation/spark/DatabricksParentContext.java index 5b8d51a5dc8..19b83f07e4e 100644 --- a/dd-java-agent/instrumentation/spark/src/main/java/datadog/trace/instrumentation/spark/DatabricksParentContext.java +++ b/dd-java-agent/instrumentation/spark/src/main/java/datadog/trace/instrumentation/spark/DatabricksParentContext.java @@ -93,4 +93,9 @@ public Iterable> baggageItems() { public PathwayContext getPathwayContext() { return null; } + + @Override + public boolean isRemote() { + return false; + } } diff --git a/dd-java-agent/instrumentation/spark/src/main/java/datadog/trace/instrumentation/spark/OpenlineageParentContext.java b/dd-java-agent/instrumentation/spark/src/main/java/datadog/trace/instrumentation/spark/OpenlineageParentContext.java index 23b543211f1..6a0b28a70c0 100644 --- a/dd-java-agent/instrumentation/spark/src/main/java/datadog/trace/instrumentation/spark/OpenlineageParentContext.java +++ b/dd-java-agent/instrumentation/spark/src/main/java/datadog/trace/instrumentation/spark/OpenlineageParentContext.java @@ -143,6 +143,11 @@ public PathwayContext getPathwayContext() { return null; } + @Override + public boolean isRemote() { + return false; + } + public String getParentJobNamespace() { return parentJobNamespace; } 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 e3f67eddd96..c2070773961 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 @@ -63,6 +63,7 @@ import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; import datadog.trace.bootstrap.instrumentation.api.ScopeSource; import datadog.trace.bootstrap.instrumentation.api.ScopeState; +import datadog.trace.bootstrap.instrumentation.api.SpanAttributes; import datadog.trace.bootstrap.instrumentation.api.TagContext; import datadog.trace.civisibility.interceptor.CiVisibilityApmProtocolInterceptor; import datadog.trace.civisibility.interceptor.CiVisibilityTelemetryInterceptor; @@ -1499,18 +1500,33 @@ private DDSpanContext buildSpanContext() { spanId = this.spanId; } + boolean isRemote = false; + AgentSpanContext parentContext = parent; if (parentContext == null && !ignoreScope) { // use the Scope as parent unless overridden or ignored. final AgentSpan activeSpan = scopeManager.activeSpan(); if (activeSpan != null) { parentContext = activeSpan.context(); - //check if restart, if so, parentContext=null, and add span links + // check if parentContext is remote + if (parentContext.isRemote() && parentContext instanceof ExtractedContext) { + ExtractedContext pc = (ExtractedContext) parentContext; + if (Config.get().getTracePropagationBehaviorExtract().equals("restart")) { + links.add( + DDSpanLink.from( + pc, + SpanAttributes.builder() + .put("reason", "propagation_behavior_extract") + .put("context_headers", pc.getPropagationStyle().toString()) + .build())); + parentContext = null; + isRemote = true; + } + } } } String parentServiceName = null; - boolean isRemote = false; // Propagate internal trace. // Note: if we are not in the context of distributed tracing and we are starting the first @@ -1547,21 +1563,11 @@ private DDSpanContext buildSpanContext() { // Propagate external trace isRemote = true; final ExtractedContext extractedContext = (ExtractedContext) parentContext; - - if (Config.get().getTracePropagationBehaviorExtract().equals("restart")) { - traceId = idGenerationStrategy.generateTraceId(); - parentSpanId = 0; - samplingPriority = PrioritySampling.UNSET; - endToEndStartTime = 0; - propagationTags = propagationTagsFactory.empty(); - //adding span link from extracted context - } else { - traceId = extractedContext.getTraceId(); - parentSpanId = extractedContext.getSpanId(); - samplingPriority = extractedContext.getSamplingPriority(); - endToEndStartTime = extractedContext.getEndToEndStartTime(); - propagationTags = extractedContext.getPropagationTags(); - } + traceId = extractedContext.getTraceId(); + parentSpanId = extractedContext.getSpanId(); + samplingPriority = extractedContext.getSamplingPriority(); + endToEndStartTime = extractedContext.getEndToEndStartTime(); + propagationTags = extractedContext.getPropagationTags(); } else if (parentContext != null) { traceId = parentContext.getTraceId() == DDTraceId.ZERO @@ -1573,6 +1579,7 @@ private DDSpanContext buildSpanContext() { propagationTags = propagationTagsFactory.empty(); } else { // Start a new trace + System.out.println("Starting a new trace"); traceId = idGenerationStrategy.generateTraceId(); parentSpanId = DDSpanId.ZERO; samplingPriority = PrioritySampling.UNSET; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java index dce68583031..910449b7dd1 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/B3HttpCodec.java @@ -164,10 +164,7 @@ static HttpCodec.Extractor newExtractor( final List extractors = new ArrayList<>(2); extractors.add(newSingleExtractor(config, traceConfigSupplier)); extractors.add(newMultiExtractor(config, traceConfigSupplier)); - return new HttpCodec.CompoundExtractor( - extractors, - config.isTracePropagationExtractFirst(), - config.getTracePropagationBehaviorExtract()); + return new HttpCodec.CompoundExtractor(extractors, config.isTracePropagationExtractFirst()); } public static HttpCodec.Extractor newMultiExtractor( diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java index f576f756fa8..08aa37007a6 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java @@ -62,7 +62,7 @@ public ExtractedContext( this.spanId = spanId; this.endToEndStartTime = endToEndStartTime; this.propagationTags = propagationTags; - //set remote flag + // set remote flag } @Override diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java index 06967576d41..1c9537f9bef 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java @@ -172,10 +172,7 @@ public static Extractor createExtractor( case 0: return StubExtractor.INSTANCE; default: - return new CompoundExtractor( - extractors, - config.isTracePropagationExtractFirst(), - config.getTracePropagationBehaviorExtract()); + return new CompoundExtractor(extractors, config.isTracePropagationExtractFirst()); } } @@ -209,13 +206,10 @@ public TagContext extract(C carrier, AgentPropagation.ContextVisitor gett public static class CompoundExtractor implements Extractor { private final List extractors; private final boolean extractFirst; - private final String extractBehavior; - public CompoundExtractor( - final List extractors, boolean extractFirst, String extractBehavior) { + public CompoundExtractor(final List extractors, boolean extractFirst) { this.extractors = extractors; this.extractFirst = extractFirst; - this.extractBehavior = extractBehavior; } @Override @@ -266,16 +260,6 @@ else if (extracted != null && partialContext == null) { } if (context != null) { - if (extractBehavior.equals("restart")) { - context.resetTerminatedContextLink(); - context.addTerminatedContextLink( - DDSpanLink.from( - context, - SpanAttributes.builder() - .put("reason", "propagation_behavior_extract") - .put("context_headers", context.getPropagationStyle().toString()) - .build())); - } log.debug("Extract complete context {}", context); return context; } else if (partialContext != null) { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy index 920a3e84ff8..a6ced73b1d4 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy @@ -346,10 +346,8 @@ class CoreSpanBuilderTest extends DDCoreSpecification { setup: injectSysConfig("trace.propagation.behavior.extract", "restart") def thread = Thread.currentThread() - TagContext context = extractedContext - context.addTerminatedContextLink(DDSpanLink.from(extractedContext)) final DDSpan span = tracer.buildSpan("test", "op name") - .asChildOf(context).start() + .asChildOf(extractedContext).start() expect: span.traceId != extractedContext.traceId diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java index cd6636163a4..974255661e8 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java @@ -15,7 +15,6 @@ */ public interface AgentSpanContext { - //add remote field. Deafult is false unless coming from tag or extracted /** * Gets the TraceId of the span's trace. * @@ -54,6 +53,8 @@ public interface AgentSpanContext { default void mergePathwayContext(PathwayContext pathwayContext) {} + boolean isRemote(); + interface Extracted extends AgentSpanContext { /** * Gets the span links related to the other terminated context. diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NoopSpanContext.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NoopSpanContext.java index 3a15ff15a2d..876c0167f51 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NoopSpanContext.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NoopSpanContext.java @@ -45,6 +45,11 @@ public PathwayContext getPathwayContext() { return NoopPathwayContext.INSTANCE; } + @Override + public boolean isRemote() { + return false; + } + @Override public List getTerminatedContextLinks() { return emptyList(); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java index 4ec1b42ec02..9b6ec7780c1 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java @@ -240,6 +240,11 @@ public PathwayContext getPathwayContext() { return this.pathwayContext; } + @Override + public boolean isRemote() { + return true; + } + public TagContext withPathwayContext(PathwayContext pathwayContext) { this.pathwayContext = pathwayContext; return this; From 9ef2479cde2ee7c16785bd3ed535011e94f45e62 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Thu, 13 Mar 2025 14:53:07 -0400 Subject: [PATCH 06/16] updating span link logic for restart --- .../java/datadog/trace/core/CoreTracer.java | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) 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 c2070773961..aa3670386d6 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 @@ -64,6 +64,7 @@ import datadog.trace.bootstrap.instrumentation.api.ScopeSource; import datadog.trace.bootstrap.instrumentation.api.ScopeState; import datadog.trace.bootstrap.instrumentation.api.SpanAttributes; +import datadog.trace.bootstrap.instrumentation.api.SpanLink; import datadog.trace.bootstrap.instrumentation.api.TagContext; import datadog.trace.civisibility.interceptor.CiVisibilityApmProtocolInterceptor; import datadog.trace.civisibility.interceptor.CiVisibilityTelemetryInterceptor; @@ -1500,34 +1501,38 @@ private DDSpanContext buildSpanContext() { spanId = this.spanId; } - boolean isRemote = false; - AgentSpanContext parentContext = parent; if (parentContext == null && !ignoreScope) { // use the Scope as parent unless overridden or ignored. final AgentSpan activeSpan = scopeManager.activeSpan(); if (activeSpan != null) { parentContext = activeSpan.context(); - // check if parentContext is remote - if (parentContext.isRemote() && parentContext instanceof ExtractedContext) { - ExtractedContext pc = (ExtractedContext) parentContext; - if (Config.get().getTracePropagationBehaviorExtract().equals("restart")) { - links.add( - DDSpanLink.from( - pc, - SpanAttributes.builder() - .put("reason", "propagation_behavior_extract") - .put("context_headers", pc.getPropagationStyle().toString()) - .build())); - parentContext = null; - isRemote = true; - } - } } } String parentServiceName = null; + boolean isRemote = false; + if (parentContext != null && parentContext.isRemote()) { + if (Config.get().getTracePropagationBehaviorExtract().equals("restart")) { + SpanLink link; + if(parentContext instanceof ExtractedContext){ + ExtractedContext pc = (ExtractedContext) parentContext; + link = DDSpanLink.from( + pc, + SpanAttributes.builder() + .put("reason", "propagation_behavior_extract") + .put("context_headers", pc.getPropagationStyle().toString()) + .build()); + } else { + link = SpanLink.from(parentContext); + } + //reset links that may have come terminated span links + links = new ArrayList<>(); + links.add(link); + parentContext = null; + } + } // Propagate internal trace. // Note: if we are not in the context of distributed tracing and we are starting the first // root span, parentContext will be null at this point. From 14891f7180a0a2edb7870d646e7f02909f99ea37 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Thu, 13 Mar 2025 15:24:10 -0400 Subject: [PATCH 07/16] updating enum --- .../datadog/trace/api/ConfigDefaults.java | 3 +- .../api/TracePropagationBehaviorExtract.java | 21 +++++++++ .../java/datadog/trace/core/CoreTracer.java | 17 +++---- .../core/propagation/HttpExtractorTest.groovy | 46 ------------------- .../main/java/datadog/trace/api/Config.java | 14 ++++-- .../config/provider/ConfigProvider.java | 3 +- .../datadog/trace/api/ConfigTest.groovy | 6 +-- 7 files changed, 46 insertions(+), 64 deletions(-) create mode 100644 dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index 2f1051e12b2..6543781f108 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -91,7 +91,8 @@ public final class ConfigDefaults { static final int DEFAULT_CLOCK_SYNC_PERIOD = 30; // seconds - static final String DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT = "continue"; + static final TracePropagationBehaviorExtract DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT = + TracePropagationBehaviorExtract.CONTINUE; static final boolean DEFAULT_TRACE_PROPAGATION_EXTRACT_FIRST = false; static final boolean DEFAULT_JMX_FETCH_MULTIPLE_RUNTIME_SERVICES_ENABLED = false; diff --git a/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java b/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java new file mode 100644 index 00000000000..e86889033f9 --- /dev/null +++ b/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java @@ -0,0 +1,21 @@ +package datadog.trace.api; + +import java.util.Locale; + +/** Trace propagation styles for injecting and extracting trace propagation headers. */ +public enum TracePropagationBehaviorExtract { + CONTINUE, + RESTART, + IGNORE; + + private String displayName; + + @Override + public String toString() { + String string = displayName; + if (displayName == null) { + string = displayName = name().toLowerCase(Locale.ROOT); + } + return string; + } +} 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 aa3670386d6..6a82fb1c490 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 @@ -1516,18 +1516,19 @@ private DDSpanContext buildSpanContext() { if (parentContext != null && parentContext.isRemote()) { if (Config.get().getTracePropagationBehaviorExtract().equals("restart")) { SpanLink link; - if(parentContext instanceof ExtractedContext){ + if (parentContext instanceof ExtractedContext) { ExtractedContext pc = (ExtractedContext) parentContext; - link = DDSpanLink.from( - pc, - SpanAttributes.builder() - .put("reason", "propagation_behavior_extract") - .put("context_headers", pc.getPropagationStyle().toString()) - .build()); + link = + DDSpanLink.from( + pc, + SpanAttributes.builder() + .put("reason", "propagation_behavior_extract") + .put("context_headers", pc.getPropagationStyle().toString()) + .build()); } else { link = SpanLink.from(parentContext); } - //reset links that may have come terminated span links + // reset links that may have come terminated span links links = new ArrayList<>(); links.add(link); parentContext = null; diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy index 3b28c9c639e..2eef64d76a7 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy @@ -209,50 +209,4 @@ class HttpExtractorTest extends DDSpecification { [TRACECONTEXT, B3MULTI, DATADOG] | "2" | "2" | "1" | "b" | W3C_TRACE_PARENT | W3C_TRACE_STATE_NO_P | [DATADOG] // spotless:on } - - def "verify extract behavior with TRACE_PROPAGATION_BEHAVIOR_EXTRACT"() { - setup: - Config config = Mock(Config) { - getTracePropagationStylesToExtract() >> [DATADOG, TRACECONTEXT] - getTracePropagationBehaviorExtract() >> behaviorExtract - isTracePropagationExtractFirst() >> extractFirst - } - DynamicConfig dynamicConfig = DynamicConfig.create().apply() - HttpCodec.Extractor extractor = HttpCodec.createExtractor(config, { dynamicConfig.captureTraceConfig() }) - - final Map actual = [ - (DatadogHttpCodec.TRACE_ID_KEY.toUpperCase()): datadogTraceId, - (DatadogHttpCodec.SPAN_ID_KEY.toUpperCase()) : datadogSpanId, - (W3CHttpCodec.TRACE_PARENT_KEY.toUpperCase()): w3cTraceParent, - (W3CHttpCodec.TRACE_STATE_KEY.toUpperCase()) : traceState - ] - - when: - final TagContext context = extractor.extract(actual, ContextVisitors.stringValuesMap()) - - then: - def links = context.getTerminatedContextLinks() - assert links.size() == expectedLinkStyle.size() - for (int i = 0; i < links.size(); i++) { - assert links[i].traceId().toString() == linkTraceId - assert links[i].spanId().toString() == linkSpanId - assert links[i].attributes().asMap()["reason"] == linkReason - assert links[i].attributes().asMap()["context_headers"] == expectedLinkStyle[i].toString() - if (expectedLinkStyle[i] == TRACECONTEXT || linkReason == "propagation_behavior_extract" && datadogTraceId == getW3C_TRACE_ID()) { - assert links[i].traceState() == traceState - } - } - - // no tests for behaviorExtract=ignore since no extraction will occur in this situation - where: - // spotless:off - behaviorExtract | extractFirst | datadogTraceId | datadogSpanId | w3cTraceParent | traceState | expectedLinkStyle | linkTraceId | linkSpanId | linkReason - "continue" | false | "1" | "2" | W3C_TRACE_PARENT | "" | [] | "" | "1311768467463790320" | "" - "continue" | false | "2" | "2" | W3C_TRACE_PARENT | W3C_TRACE_STATE_NO_P | [TRACECONTEXT] | "1" | "1311768467463790320" | "terminated_context" - "restart" | false | "1" | "2" | W3C_TRACE_PARENT | "dd=foo=1" | [DATADOG] | "1" | "1311768467463790320" | "propagation_behavior_extract" - "restart" | false | "2" | "2" | W3C_TRACE_PARENT | "" | [DATADOG] | "2" | "2" | "propagation_behavior_extract" - "restart" | true | "2" | "2" | W3C_TRACE_PARENT | "" | [DATADOG] | "2" | "2" | "propagation_behavior_extract" - "restart" | true | "1" | "2" | W3C_TRACE_PARENT | "" | [DATADOG] | "1" | "2" | "propagation_behavior_extract" - // spotless:on - } } 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 7ad4819b447..4c6295feb0e 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -189,7 +189,7 @@ public static String getHostName() { private final boolean tracePropagationStyleB3PaddingEnabled; private final Set tracePropagationStylesToExtract; private final Set tracePropagationStylesToInject; - private final String tracePropagationBehaviorExtract; + private final TracePropagationBehaviorExtract tracePropagationBehaviorExtract; private final boolean tracePropagationExtractFirst; private final int traceBaggageMaxItems; private final int traceBaggageMaxBytes; @@ -927,8 +927,10 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins isEnabled(true, TRACE_PROPAGATION_STYLE, ".b3.padding.enabled"); tracePropagationBehaviorExtract = - configProvider.getString( - TRACE_PROPAGATION_BEHAVIOR_EXTRACT, DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT); + configProvider.getEnum( + TRACE_PROPAGATION_BEHAVIOR_EXTRACT, + TracePropagationBehaviorExtract.class, + DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT); { // The dd.propagation.style.(extract|inject) settings have been deprecated in @@ -1007,7 +1009,9 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins } tracePropagationStylesToExtract = - tracePropagationBehaviorExtract.equals("ignore") ? new HashSet<>() : extract; + tracePropagationBehaviorExtract == TracePropagationBehaviorExtract.IGNORE + ? new HashSet<>() + : extract; tracePropagationStylesToInject = inject.isEmpty() ? DEFAULT_TRACE_PROPAGATION_STYLE : inject; @@ -2272,7 +2276,7 @@ public Set getTracePropagationStylesToInject() { return tracePropagationStylesToInject; } - public String getTracePropagationBehaviorExtract() { + public TracePropagationBehaviorExtract getTracePropagationBehaviorExtract() { return tracePropagationBehaviorExtract; } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index 90e43cd30ab..2432c698f2c 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -14,6 +14,7 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Properties; import java.util.Set; @@ -60,7 +61,7 @@ public > T getEnum(String key, Class enumType, T defaultVal String value = getString(key); if (null != value) { try { - return Enum.valueOf(enumType, value); + return Enum.valueOf(enumType, value.toUpperCase(Locale.ROOT)); } catch (Exception ignoreAndUseDefault) { log.debug("failed to parse {} for {}, defaulting to {}", value, key, defaultValue); } diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index e34c93d5a93..adcdee061a3 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -297,7 +297,7 @@ class ConfigTest extends DDSpecification { config.tracePropagationStylesToExtract.toList() == [DATADOG, B3SINGLE, B3MULTI] config.tracePropagationStylesToInject.toList() == [B3SINGLE, B3MULTI, DATADOG] config.tracePropagationExtractFirst == false - config.tracePropagationBehaviorExtract == "restart" + config.tracePropagationBehaviorExtract == TracePropagationBehaviorExtract.RESTART config.jmxFetchEnabled == false config.jmxFetchMetricsConfigs == ["/foo.yaml", "/bar.yaml"] config.jmxFetchCheckPeriod == 100 @@ -479,7 +479,7 @@ class ConfigTest extends DDSpecification { config.tracePropagationStylesToExtract.toList() == [DATADOG, B3SINGLE, B3MULTI] config.tracePropagationStylesToInject.toList() == [B3SINGLE, B3MULTI, DATADOG] config.tracePropagationExtractFirst == false - config.tracePropagationBehaviorExtract == "restart" + config.tracePropagationBehaviorExtract == TracePropagationBehaviorExtract.RESTART config.jmxFetchEnabled == false config.jmxFetchMetricsConfigs == ["/foo.yaml", "/bar.yaml"] config.jmxFetchCheckPeriod == 100 @@ -2620,7 +2620,7 @@ class ConfigTest extends DDSpecification { Config config = Config.get(prop) then: - config.tracePropagationBehaviorExtract == "ignore" + config.tracePropagationBehaviorExtract == TracePropagationBehaviorExtract.IGNORE config.tracePropagationStylesToExtract.toList() == [] } } From 623e7ba08cd88bab568d1a5d3cda204b6f95a052 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Thu, 13 Mar 2025 15:26:11 -0400 Subject: [PATCH 08/16] cleanup --- dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java | 1 - .../java/datadog/trace/core/propagation/ExtractedContext.java | 1 - 2 files changed, 2 deletions(-) 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 6a82fb1c490..5f0f2c9c354 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 @@ -1585,7 +1585,6 @@ private DDSpanContext buildSpanContext() { propagationTags = propagationTagsFactory.empty(); } else { // Start a new trace - System.out.println("Starting a new trace"); traceId = idGenerationStrategy.generateTraceId(); parentSpanId = DDSpanId.ZERO; samplingPriority = PrioritySampling.UNSET; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java index 08aa37007a6..e799688401d 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ExtractedContext.java @@ -62,7 +62,6 @@ public ExtractedContext( this.spanId = spanId; this.endToEndStartTime = endToEndStartTime; this.propagationTags = propagationTags; - // set remote flag } @Override From f4538a767066be7832cfcf6838ead9dee68ce6df Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 14 Mar 2025 11:05:37 -0400 Subject: [PATCH 09/16] adding override to new class --- .../bootstrap/instrumentation/api/NotSampledSpanContext.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NotSampledSpanContext.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NotSampledSpanContext.java index db84c70cf16..0dea41e2cb3 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NotSampledSpanContext.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NotSampledSpanContext.java @@ -42,4 +42,9 @@ public Iterable> baggageItems() { public PathwayContext getPathwayContext() { return delegate.getPathwayContext(); } + + @Override + public boolean isRemote() { + return false; + } } From 2c51a7827676301666b484f28a3ffc18314ac16d Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 14 Mar 2025 13:17:56 -0400 Subject: [PATCH 10/16] updating enum and tests --- .../src/main/java/datadog/trace/core/CoreTracer.java | 4 +++- .../test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) 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 e997338b1f9..4e44a9d968a 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 @@ -33,6 +33,7 @@ import datadog.trace.api.InstrumenterConfig; import datadog.trace.api.StatsDClient; import datadog.trace.api.TraceConfig; +import datadog.trace.api.TracePropagationBehaviorExtract; import datadog.trace.api.config.GeneralConfig; import datadog.trace.api.datastreams.AgentDataStreamsMonitoring; import datadog.trace.api.datastreams.PathwayContext; @@ -1511,7 +1512,8 @@ private DDSpanContext buildSpanContext() { boolean isRemote = false; if (parentContext != null && parentContext.isRemote()) { - if (Config.get().getTracePropagationBehaviorExtract().equals("restart")) { + if (Config.get().getTracePropagationBehaviorExtract() + == TracePropagationBehaviorExtract.RESTART) { SpanLink link; if (parentContext instanceof ExtractedContext) { ExtractedContext pc = (ExtractedContext) parentContext; diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy index a6ced73b1d4..e4ec79819ee 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy @@ -345,7 +345,6 @@ class CoreSpanBuilderTest extends DDCoreSpecification { def "build context from ExtractedContext with TRACE_PROPAGATION_BEHAVIOR_EXTRACT=restart"() { setup: injectSysConfig("trace.propagation.behavior.extract", "restart") - def thread = Thread.currentThread() final DDSpan span = tracer.buildSpan("test", "op name") .asChildOf(extractedContext).start() From d0bd3f5047a736f6986e9eaca9df48a8b9b5ddf1 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 14 Mar 2025 13:44:20 -0400 Subject: [PATCH 11/16] refactoring capturing config --- .../src/main/java/datadog/trace/core/DDSpan.java | 4 ---- .../src/main/java/datadog/trace/api/Config.java | 10 ++++++---- .../bootstrap/config/provider/ConfigProvider.java | 3 +-- 3 files changed, 7 insertions(+), 10 deletions(-) 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 68e167ef483..5fafa0f29a8 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 @@ -856,8 +856,4 @@ public boolean isOutbound() { Object spanKind = context.getTag(Tags.SPAN_KIND); return Tags.SPAN_KIND_CLIENT.equals(spanKind) || Tags.SPAN_KIND_PRODUCER.equals(spanKind); } - - public List getLinks() { - return links; - } } 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 8521786c963..30f67d7e8cb 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -941,10 +941,12 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins isEnabled(true, TRACE_PROPAGATION_STYLE, ".b3.padding.enabled"); tracePropagationBehaviorExtract = - configProvider.getEnum( - TRACE_PROPAGATION_BEHAVIOR_EXTRACT, - TracePropagationBehaviorExtract.class, - DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT); + TracePropagationBehaviorExtract.valueOf( + configProvider + .getString( + TRACE_PROPAGATION_BEHAVIOR_EXTRACT, + DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT.toString()) + .toUpperCase(Locale.ROOT)); { // The dd.propagation.style.(extract|inject) settings have been deprecated in diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java index 0d83a8c281d..0a2715dd11a 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/ConfigProvider.java @@ -15,7 +15,6 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Properties; import java.util.Set; @@ -62,7 +61,7 @@ public > T getEnum(String key, Class enumType, T defaultVal String value = getString(key); if (null != value) { try { - return Enum.valueOf(enumType, value.toUpperCase(Locale.ROOT)); + return Enum.valueOf(enumType, value); } catch (Exception ignoreAndUseDefault) { log.debug("failed to parse {} for {}, defaulting to {}", value, key, defaultValue); } From 4975d230033d41d52ba6ae206282d7b2f81cb4f5 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 14 Mar 2025 14:43:56 -0400 Subject: [PATCH 12/16] forcing enum to be built by build time --- .../nativeimage/NativeImageGeneratorRunnerInstrumentation.java | 1 + 1 file changed, 1 insertion(+) diff --git a/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/NativeImageGeneratorRunnerInstrumentation.java b/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/NativeImageGeneratorRunnerInstrumentation.java index eea77052645..1dc8679b989 100644 --- a/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/NativeImageGeneratorRunnerInstrumentation.java +++ b/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/NativeImageGeneratorRunnerInstrumentation.java @@ -97,6 +97,7 @@ public static void onEnter(@Advice.Argument(value = 0, readOnly = false) String[ + "datadog.trace.api.ResolverCacheConfig$4:build_time," + "datadog.trace.api.ResolverCacheConfig$5:build_time," + "datadog.trace.api.TracePropagationStyle:build_time," + + "datadog.trace.api.TracePropagationBehaviorExtract:build_time," + "datadog.trace.api.telemetry.OtelEnvMetricCollector:build_time," + "datadog.trace.api.profiling.ProfilingEnablement:build_time," + "datadog.trace.bootstrap.config.provider.ConfigConverter:build_time," From dc499ea60d37ea181c420d2b7d2e37867d48204e Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Fri, 14 Mar 2025 15:19:23 -0400 Subject: [PATCH 13/16] adding enum to skip test coverae --- dd-trace-api/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/dd-trace-api/build.gradle b/dd-trace-api/build.gradle index 7c25ddeb164..1cf92606826 100644 --- a/dd-trace-api/build.gradle +++ b/dd-trace-api/build.gradle @@ -15,6 +15,7 @@ excludedClassesCoverage += [ 'datadog.trace.api.GlobalTracer*', 'datadog.trace.api.PropagationStyle', 'datadog.trace.api.TracePropagationStyle', + 'datadog.trace.api.TracePropagationBehaviorExtract', 'datadog.trace.api.SpanCorrelation*', 'datadog.trace.api.internal.TraceSegment', 'datadog.trace.api.internal.TraceSegment.NoOp', From 269d68a4458f3c856876a8fd1955a057340dc6f6 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Mon, 17 Mar 2025 15:13:51 -0400 Subject: [PATCH 14/16] responding to PR comments --- .../api/TracePropagationBehaviorExtract.java | 10 +++-- .../java/datadog/trace/core/CoreTracer.java | 40 +++++++++---------- .../trace/core/propagation/HttpCodec.java | 2 + .../trace/core/CoreSpanBuilderTest.groovy | 5 +-- .../core/propagation/HttpExtractorTest.groovy | 5 +-- .../main/java/datadog/trace/api/Config.java | 21 ++++++---- .../instrumentation/api/AgentSpanContext.java | 5 +++ .../api/NotSampledSpanContext.java | 2 +- .../instrumentation/api/TagContext.java | 4 -- .../datadog/trace/api/ConfigTest.groovy | 12 ++++++ 10 files changed, 64 insertions(+), 42 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java b/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java index e86889033f9..807310045ee 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java @@ -4,12 +4,16 @@ /** Trace propagation styles for injecting and extracting trace propagation headers. */ public enum TracePropagationBehaviorExtract { - CONTINUE, - RESTART, - IGNORE; + CONTINUE("continue"), + RESTART("restart"), + IGNORE("ignore"); private String displayName; + TracePropagationBehaviorExtract(String displayName) { + this.displayName = displayName; + } + @Override public String toString() { String string = displayName; 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 4e44a9d968a..9f3af42e501 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 @@ -1511,27 +1511,27 @@ private DDSpanContext buildSpanContext() { String parentServiceName = null; boolean isRemote = false; - if (parentContext != null && parentContext.isRemote()) { - if (Config.get().getTracePropagationBehaviorExtract() - == TracePropagationBehaviorExtract.RESTART) { - SpanLink link; - if (parentContext instanceof ExtractedContext) { - ExtractedContext pc = (ExtractedContext) parentContext; - link = - DDSpanLink.from( - pc, - SpanAttributes.builder() - .put("reason", "propagation_behavior_extract") - .put("context_headers", pc.getPropagationStyle().toString()) - .build()); - } else { - link = SpanLink.from(parentContext); - } - // reset links that may have come terminated span links - links = new ArrayList<>(); - links.add(link); - parentContext = null; + if (parentContext != null + && parentContext.isRemote() + && Config.get().getTracePropagationBehaviorExtract() + == TracePropagationBehaviorExtract.RESTART) { + SpanLink link; + if (parentContext instanceof ExtractedContext) { + ExtractedContext pc = (ExtractedContext) parentContext; + link = + DDSpanLink.from( + pc, + SpanAttributes.builder() + .put("reason", "propagation_behavior_extract") + .put("context_headers", pc.getPropagationStyle().toString()) + .build()); + } else { + link = SpanLink.from(parentContext); } + // reset links that may have come terminated span links + links = new ArrayList<>(); + links.add(link); + parentContext = null; } // Propagate internal trace. // Note: if we are not in the context of distributed tracing and we are starting the first diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java index 1c9537f9bef..9835d2e1c03 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java @@ -171,6 +171,8 @@ public static Extractor createExtractor( switch (extractors.size()) { case 0: return StubExtractor.INSTANCE; + case 1: + return extractors.get(0); default: return new CompoundExtractor(extractors, config.isTracePropagationExtractFirst()); } diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy index e4ec79819ee..a9a86f5cdad 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy @@ -345,6 +345,7 @@ class CoreSpanBuilderTest extends DDCoreSpecification { def "build context from ExtractedContext with TRACE_PROPAGATION_BEHAVIOR_EXTRACT=restart"() { setup: injectSysConfig("trace.propagation.behavior.extract", "restart") + def extractedContext = new ExtractedContext(DDTraceId.ONE, 2, PrioritySampling.SAMPLER_DROP, null, 0, [:], [:], null, PropagationTags.factory().fromHeaderValue(PropagationTags.HeaderType.DATADOG, "_dd.p.dm=934086a686-4,_dd.p.anytag=value"), null, DATADOG) final DDSpan span = tracer.buildSpan("test", "op name") .asChildOf(extractedContext).start() @@ -360,10 +361,6 @@ class CoreSpanBuilderTest extends DDCoreSpecification { link.traceId() == extractedContext.traceId link.spanId() == extractedContext.spanId link.traceState() == extractedContext.propagationTags.headerValue(PropagationTags.HeaderType.W3C) - - where: - extractedContext | _ - new ExtractedContext(DDTraceId.ONE, 2, PrioritySampling.SAMPLER_DROP, null, 0, [:], [:], null, PropagationTags.factory().fromHeaderValue(PropagationTags.HeaderType.DATADOG, "_dd.p.dm=934086a686-4,_dd.p.anytag=value"), null, DATADOG) | _ } def "TagContext should populate default span details"() { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy index 2eef64d76a7..a10b600d5c0 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy @@ -1,5 +1,7 @@ package datadog.trace.core.propagation +import datadog.trace.api.TracePropagationBehaviorExtract + import static datadog.trace.api.DDTags.PARENT_ID import static datadog.trace.api.TracePropagationStyle.NONE @@ -33,7 +35,6 @@ class HttpExtractorTest extends DDSpecification { Config config = Mock(Config) { getTracePropagationStylesToExtract() >> styles isTracePropagationExtractFirst() >> extractFirst - getTracePropagationBehaviorExtract() >> "continue" } DynamicConfig dynamicConfig = DynamicConfig.create() .setHeaderTags(["SOME_HEADER": "some-tag"]) @@ -120,7 +121,6 @@ class HttpExtractorTest extends DDSpecification { setup: Config config = Mock(Config) { getTracePropagationStylesToExtract() >> styles - getTracePropagationBehaviorExtract() >> "continue" } DynamicConfig dynamicConfig = DynamicConfig.create().apply() HttpCodec.Extractor extractor = HttpCodec.createExtractor(config, { dynamicConfig.captureTraceConfig() }) @@ -173,7 +173,6 @@ class HttpExtractorTest extends DDSpecification { setup: Config config = Mock(Config) { getTracePropagationStylesToExtract() >> styles - getTracePropagationBehaviorExtract() >> "continue" } DynamicConfig dynamicConfig = DynamicConfig.create().apply() HttpCodec.Extractor extractor = HttpCodec.createExtractor(config, { dynamicConfig.captureTraceConfig() }) 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 30f67d7e8cb..afaa7011929 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -940,13 +940,20 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins tracePropagationStyleB3PaddingEnabled = isEnabled(true, TRACE_PROPAGATION_STYLE, ".b3.padding.enabled"); - tracePropagationBehaviorExtract = - TracePropagationBehaviorExtract.valueOf( - configProvider - .getString( - TRACE_PROPAGATION_BEHAVIOR_EXTRACT, - DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT.toString()) - .toUpperCase(Locale.ROOT)); + TracePropagationBehaviorExtract tmpTracePropagationBehaviorExtract; + try { + tmpTracePropagationBehaviorExtract = + TracePropagationBehaviorExtract.valueOf( + configProvider + .getString( + TRACE_PROPAGATION_BEHAVIOR_EXTRACT, + DEFAULT_TRACE_PROPAGATION_BEHAVIOR_EXTRACT.toString()) + .toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + tmpTracePropagationBehaviorExtract = TracePropagationBehaviorExtract.CONTINUE; + log.warn("Error while parsing TRACE_PROPAGATION_BEHAVIOR_EXTRACT, defaulting to `continue`"); + } + tracePropagationBehaviorExtract = tmpTracePropagationBehaviorExtract; { // The dd.propagation.style.(extract|inject) settings have been deprecated in diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java index 974255661e8..fd05ab149db 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpanContext.java @@ -53,6 +53,11 @@ public interface AgentSpanContext { default void mergePathwayContext(PathwayContext pathwayContext) {} + /** + * Gets whether the span context used is part of the local trace or from another service + * + * @return boolean representing if the span context is part of the local trace + */ boolean isRemote(); interface Extracted extends AgentSpanContext { diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NotSampledSpanContext.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NotSampledSpanContext.java index 0dea41e2cb3..743f9c3bf66 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NotSampledSpanContext.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/NotSampledSpanContext.java @@ -45,6 +45,6 @@ public PathwayContext getPathwayContext() { @Override public boolean isRemote() { - return false; + return delegate.isRemote(); } } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java index 9b6ec7780c1..f5185d6292c 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContext.java @@ -89,10 +89,6 @@ public void addTerminatedContextLink(AgentSpanLink link) { this.terminatedContextLinks.add(link); } - public void resetTerminatedContextLink() { - this.terminatedContextLinks = null; - } - @Override public String getForwarded() { return httpHeaders.forwarded; diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 35afb07bbed..9370b2ea238 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -2674,4 +2674,16 @@ class ConfigTest extends DDSpecification { config.tracePropagationBehaviorExtract == TracePropagationBehaviorExtract.IGNORE config.tracePropagationStylesToExtract.toList() == [] } + + def "verify try/catch behavior for invalid strings for TRACE_PROPAGATION_BEHAVIOR_EXTRACT"() { + setup: + def prop = new Properties() + prop.setProperty(TRACE_PROPAGATION_BEHAVIOR_EXTRACT, "test") + + when: + Config config = Config.get(prop) + + then: + config.tracePropagationBehaviorExtract == TracePropagationBehaviorExtract.CONTINUE + } } From 7a0fcd59c5b8f57b58e908b2fc773dbc4a517a2f Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Mon, 17 Mar 2025 15:50:30 -0400 Subject: [PATCH 15/16] spotless --- .../datadog/trace/core/propagation/HttpExtractorTest.groovy | 2 -- 1 file changed, 2 deletions(-) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy index a10b600d5c0..e9642463fe9 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/HttpExtractorTest.groovy @@ -1,7 +1,5 @@ package datadog.trace.core.propagation -import datadog.trace.api.TracePropagationBehaviorExtract - import static datadog.trace.api.DDTags.PARENT_ID import static datadog.trace.api.TracePropagationStyle.NONE From 9d103931d49e32fdd3a6583bbf84e66ff401cd35 Mon Sep 17 00:00:00 2001 From: Matthew Li Date: Tue, 18 Mar 2025 14:46:13 -0400 Subject: [PATCH 16/16] modifying enum constructor --- .../api/TracePropagationBehaviorExtract.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java b/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java index 807310045ee..daa03b580af 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/TracePropagationBehaviorExtract.java @@ -4,22 +4,21 @@ /** Trace propagation styles for injecting and extracting trace propagation headers. */ public enum TracePropagationBehaviorExtract { - CONTINUE("continue"), - RESTART("restart"), - IGNORE("ignore"); + CONTINUE, + RESTART, + IGNORE; private String displayName; - TracePropagationBehaviorExtract(String displayName) { - this.displayName = displayName; + TracePropagationBehaviorExtract() { + this.displayName = name().toLowerCase(Locale.ROOT); } @Override public String toString() { - String string = displayName; if (displayName == null) { - string = displayName = name().toLowerCase(Locale.ROOT); + displayName = name().toLowerCase(Locale.ROOT); } - return string; + return displayName; } }