From a70d27b6477d3ca34bbbbb5f10a16cb348cc180c Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Thu, 2 Nov 2023 18:39:12 +0100 Subject: [PATCH 01/12] feat(core): Add multiple context extraction Extract multiple propagated context according the new W3C trace context initiative. Add a config to limit to the first valid one. --- .../src/test/groovy/OpenTelemetryTest.groovy | 6 +- .../src/test/groovy/OpenTracing31Test.groovy | 3 +- .../src/test/groovy/OpenTracing32Test.groovy | 3 +- .../datadog/trace/api/ConfigDefaults.java | 2 + .../trace/api/config/TracerConfig.java | 1 + .../trace/core/propagation/B3HttpCodec.java | 15 ++++- .../core/propagation/ContextInterpreter.java | 22 ++++-- .../core/propagation/DatadogHttpCodec.java | 7 ++ .../core/propagation/ExtractedContext.java | 22 ++++-- .../core/propagation/HaystackHttpCodec.java | 7 ++ .../trace/core/propagation/HttpCodec.java | 67 +++++++++++++++---- .../trace/core/propagation/W3CHttpCodec.java | 7 ++ .../trace/core/propagation/XRayHttpCodec.java | 7 ++ .../datadog/trace/lambda/LambdaHandler.java | 8 ++- .../trace/core/CoreSpanBuilderTest.groovy | 7 +- .../DDSpanContextPropagationTagsTest.groovy | 9 +-- .../trace/core/DDSpanContextTest.groovy | 3 +- .../datadog/trace/core/DDSpanTest.groovy | 19 +++--- .../DefaultPathwayContextTest.groovy | 3 +- .../main/java/datadog/trace/api/Config.java | 13 ++++ .../instrumentation/api/TagContext.java | 14 +++- .../datadog/trace/api/ConfigTest.groovy | 8 +++ 22 files changed, 205 insertions(+), 48 deletions(-) diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/OpenTelemetryTest.groovy b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/OpenTelemetryTest.groovy index 6b856c119ff..fa10dd5bd6f 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/OpenTelemetryTest.groovy +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/OpenTelemetryTest.groovy @@ -4,6 +4,8 @@ import datadog.trace.api.DDTags import datadog.trace.api.DDTraceId import datadog.trace.api.interceptor.MutableSpan import datadog.trace.core.propagation.PropagationTags + +import static datadog.trace.api.TracePropagationStyle.NONE import static datadog.trace.api.sampling.PrioritySampling.* import static datadog.trace.api.sampling.SamplingMechanism.* import datadog.trace.context.TraceScope @@ -136,11 +138,11 @@ class OpenTelemetryTest extends AgentTestRunner { setup: def builder = tracer.spanBuilder("some name") if (parentId) { - def ctx = new ExtractedContext(DDTraceId.ONE, parentId, SAMPLER_DROP, null, PropagationTags.factory().empty()) + def ctx = new ExtractedContext(DDTraceId.ONE, parentId, SAMPLER_DROP, null, PropagationTags.factory().empty(), NONE) builder.setParent(tracer.converter.toSpanContext(ctx)) } if (linkId) { - def ctx = new ExtractedContext(DDTraceId.ONE, linkId, SAMPLER_DROP, null, PropagationTags.factory().empty()) + def ctx = new ExtractedContext(DDTraceId.ONE, linkId, SAMPLER_DROP, null, PropagationTags.factory().empty(), NONE) builder.addLink(tracer.converter.toSpanContext(ctx)) } def result = builder.startSpan() diff --git a/dd-java-agent/instrumentation/opentracing/api-0.31/src/test/groovy/OpenTracing31Test.groovy b/dd-java-agent/instrumentation/opentracing/api-0.31/src/test/groovy/OpenTracing31Test.groovy index 1fbc4fa7971..b1bc139d3fd 100644 --- a/dd-java-agent/instrumentation/opentracing/api-0.31/src/test/groovy/OpenTracing31Test.groovy +++ b/dd-java-agent/instrumentation/opentracing/api-0.31/src/test/groovy/OpenTracing31Test.groovy @@ -10,6 +10,7 @@ import datadog.trace.instrumentation.opentracing31.OTTracer import datadog.trace.instrumentation.opentracing31.TypeConverter import spock.lang.Shared +import static datadog.trace.api.TracePropagationStyle.NONE import static datadog.trace.api.sampling.PrioritySampling.* import static datadog.trace.api.sampling.SamplingMechanism.* import datadog.trace.context.TraceScope @@ -45,7 +46,7 @@ class OpenTracing31Test extends AgentTestRunner { .withTag("boolean", true) } if (addReference) { - def ctx = new ExtractedContext(DDTraceId.ONE, 2, SAMPLER_DROP, null, PropagationTags.factory().empty()) + def ctx = new ExtractedContext(DDTraceId.ONE, 2, SAMPLER_DROP, null, PropagationTags.factory().empty(), NONE) builder.addReference(addReference, tracer.tracer.converter.toSpanContext(ctx)) } def result = builder.start() diff --git a/dd-java-agent/instrumentation/opentracing/api-0.32/src/test/groovy/OpenTracing32Test.groovy b/dd-java-agent/instrumentation/opentracing/api-0.32/src/test/groovy/OpenTracing32Test.groovy index e4195d08803..5d0df28bf83 100644 --- a/dd-java-agent/instrumentation/opentracing/api-0.32/src/test/groovy/OpenTracing32Test.groovy +++ b/dd-java-agent/instrumentation/opentracing/api-0.32/src/test/groovy/OpenTracing32Test.groovy @@ -23,6 +23,7 @@ import spock.lang.Shared import spock.lang.Subject import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace +import static datadog.trace.api.TracePropagationStyle.NONE import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_DROP import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_KEEP import static datadog.trace.api.sampling.PrioritySampling.UNSET @@ -50,7 +51,7 @@ class OpenTracing32Test extends AgentTestRunner { .withTag("boolean", true) } if (addReference) { - def ctx = new ExtractedContext(DDTraceId.ONE, 2, SAMPLER_DROP, null, PropagationTags.factory().empty()) + def ctx = new ExtractedContext(DDTraceId.ONE, 2, SAMPLER_DROP, null, PropagationTags.factory().empty(), NONE) builder.addReference(addReference, tracer.tracer.converter.toSpanContext(ctx)) } def result = builder.start() 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 cf32591ca5a..649e405db98 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 @@ -79,6 +79,8 @@ public final class ConfigDefaults { static final int DEFAULT_CLOCK_SYNC_PERIOD = 30; // seconds + static final boolean DEFAULT_TRACE_PROPAGATION_EXTRACT_FIRST = false; + static final boolean DEFAULT_JMX_FETCH_MULTIPLE_RUNTIME_SERVICES_ENABLED = false; static final int DEFAULT_JMX_FETCH_MULTIPLE_RUNTIME_SERVICES_LIMIT = 10; 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 3971d64eaa4..a4600c314a1 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 @@ -84,6 +84,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_EXTRACT_FIRST = "trace.propagation.extract.first"; public static final String ENABLE_TRACE_AGENT_V05 = "trace.agent.v0.5.enabled"; 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 5748c6a72d7..e39f0745851 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 @@ -1,5 +1,7 @@ package datadog.trace.core.propagation; +import static datadog.trace.api.TracePropagationStyle.B3MULTI; +import static datadog.trace.api.TracePropagationStyle.B3SINGLE; import static datadog.trace.core.propagation.HttpCodec.firstHeaderValue; import datadog.trace.api.Config; @@ -7,6 +9,7 @@ import datadog.trace.api.DDSpanId; import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; +import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.core.DDSpanContext; @@ -161,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); + return new HttpCodec.CompoundExtractor(extractors, config.isTracePropagationExtractFirst()); } public static HttpCodec.Extractor newMultiExtractor( @@ -212,6 +215,11 @@ private B3MultiContextInterpreter(Config config) { super(config); } + @Override + public TracePropagationStyle style() { + return B3MULTI; + } + @Override public boolean accept(final String key, final String value) { if (null == key || key.isEmpty() || null == value || value.isEmpty()) { @@ -267,6 +275,11 @@ public B3SingleContextInterpreter(Config config) { super(config); } + @Override + public TracePropagationStyle style() { + return B3SINGLE; + } + @Override public boolean accept(String key, String value) { try { diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java index 8897339d9f6..f7e385ee502 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java @@ -21,6 +21,7 @@ import datadog.trace.api.DDTraceId; import datadog.trace.api.Functions; import datadog.trace.api.TraceConfig; +import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.cache.DDCache; import datadog.trace.api.cache.DDCaches; import datadog.trace.api.sampling.PrioritySampling; @@ -58,7 +59,7 @@ public abstract class ContextInterpreter implements AgentPropagation.KeyClassifi protected static final boolean LOG_EXTRACT_HEADER_NAMES = Config.get().isLogExtractHeaderNames(); private static final DDCache CACHE = DDCaches.newFixedSizeCache(64); - protected String toLowerCase(String key) { + protected static String toLowerCase(String key) { return CACHE.computeIfAbsent(key, Functions.LowerCase.INSTANCE); } @@ -68,9 +69,12 @@ protected ContextInterpreter(Config config) { this.clientIpWithoutAppSec = config.isClientIpEnabled(); } - public interface Factory { - ContextInterpreter create(); - } + /** + * Gets the propagation style handled by the context interpreter. + * + * @return The propagation style handled. + */ + public abstract TracePropagationStyle style(); protected final boolean handledForwarding(String key, String value) { if (value == null || !collectIpHeaders) { @@ -241,7 +245,8 @@ protected TagContext build() { tags, httpHeaders, propagationTags, - traceConfig); + traceConfig, + style()); return context; } else if (origin != null || !tags.isEmpty() @@ -254,7 +259,8 @@ protected TagContext build() { httpHeaders, baggage, samplingPriorityOrDefault(traceId, samplingPriority), - traceConfig); + traceConfig, + style()); } } return null; @@ -284,4 +290,8 @@ private int samplingPriorityOrDefault(DDTraceId traceId, int samplingPriority) { ? defaultSamplingPriority() : samplingPriority; } + + public interface Factory { + ContextInterpreter create(); + } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java index a2162875fd6..648773ef0d4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java @@ -1,5 +1,6 @@ package datadog.trace.core.propagation; +import static datadog.trace.api.TracePropagationStyle.DATADOG; import static datadog.trace.core.propagation.HttpCodec.firstHeaderValue; import static datadog.trace.core.propagation.XRayHttpCodec.XRayContextInterpreter.handleXRayTraceHeader; import static datadog.trace.core.propagation.XRayHttpCodec.X_AMZN_TRACE_ID; @@ -12,6 +13,7 @@ import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; +import datadog.trace.api.TracePropagationStyle; import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.bootstrap.instrumentation.api.TagContext; import datadog.trace.core.DDSpanContext; @@ -109,6 +111,11 @@ private DatadogContextInterpreter(Config config) { datadogTagsFactory = PropagationTags.factory(config); } + @Override + public TracePropagationStyle style() { + return DATADOG; + } + @Override public boolean accept(String key, String value) { if (null == key || key.isEmpty()) { 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 0cfe87e6911..d164eec30b9 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 @@ -2,6 +2,7 @@ import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; +import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.bootstrap.instrumentation.api.TagContext; import java.util.Map; @@ -20,8 +21,20 @@ public ExtractedContext( final long spanId, final int samplingPriority, final CharSequence origin, - final PropagationTags propagationTags) { - this(traceId, spanId, samplingPriority, origin, 0, null, null, null, propagationTags, null); + final PropagationTags propagationTags, + final TracePropagationStyle propagationStyle) { + this( + traceId, + spanId, + samplingPriority, + origin, + 0, + null, + null, + null, + propagationTags, + null, + propagationStyle); } public ExtractedContext( @@ -34,8 +47,9 @@ public ExtractedContext( final Map tags, final HttpHeaders httpHeaders, final PropagationTags propagationTags, - final TraceConfig traceConfig) { - super(origin, tags, httpHeaders, baggage, samplingPriority, traceConfig); + final TraceConfig traceConfig, + final TracePropagationStyle propagationStyle) { + super(origin, tags, httpHeaders, baggage, samplingPriority, traceConfig, propagationStyle); this.traceId = traceId; this.spanId = spanId; this.endToEndStartTime = endToEndStartTime; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HaystackHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HaystackHttpCodec.java index af293ec42c3..0d349a4a858 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HaystackHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HaystackHttpCodec.java @@ -1,5 +1,6 @@ package datadog.trace.core.propagation; +import static datadog.trace.api.TracePropagationStyle.HAYSTACK; import static datadog.trace.core.propagation.HttpCodec.firstHeaderValue; import datadog.trace.api.Config; @@ -7,6 +8,7 @@ import datadog.trace.api.DDSpanId; import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; +import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.core.DDSpanContext; @@ -139,6 +141,11 @@ private HaystackContextInterpreter(Config config) { super(config); } + @Override + public TracePropagationStyle style() { + return HAYSTACK; + } + @Override public boolean accept(String key, String value) { if (null == key || key.isEmpty()) { 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 2e2172c9665..d25017d6a4a 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 @@ -1,5 +1,7 @@ package datadog.trace.core.propagation; +import static datadog.trace.api.TracePropagationStyle.TRACECONTEXT; + import datadog.trace.api.Config; import datadog.trace.api.TraceConfig; import datadog.trace.api.TracePropagationStyle; @@ -46,7 +48,17 @@ void inject( final DDSpanContext context, final C carrier, final AgentPropagation.Setter setter); } + /** This interface defines propagated context extractor. */ public interface Extractor { + /** + * Extracts a propagated context from the given carrier using the provided getter. + * + * @param carrier The carrier containing the propagated context. + * @param getter The getter used to extract data from the carrier. + * @param The type of the carrier. + * @return {@code null} for failed context extraction, a {@link TagContext} instance for partial + * context extraction or an {@link ExtractedContext} for complete context extraction. + */ TagContext extract(final C carrier, final AgentPropagation.ContextVisitor getter); } @@ -136,7 +148,7 @@ public static Extractor createExtractor( break; } } - return new CompoundExtractor(extractors); + return new CompoundExtractor(extractors, config.isTracePropagationExtractFirst()); } public static class CompoundInjector implements Injector { @@ -159,27 +171,58 @@ public void inject( public static class CompoundExtractor implements Extractor { private final List extractors; + private final boolean extractFirst; - public CompoundExtractor(final List extractors) { + public CompoundExtractor(final List extractors, boolean extractFirst) { this.extractors = extractors; + this.extractFirst = extractFirst; } @Override public TagContext extract( final C carrier, final AgentPropagation.ContextVisitor getter) { - TagContext context = null; - - for (final Extractor extractor : extractors) { - context = extractor.extract(carrier, getter); - // Use incomplete TagContext only as last resort - if (context instanceof ExtractedContext) { - log.debug("Extract complete context {}", context); - return context; + ExtractedContext context = null; + TagContext partialContext = null; + + for (final Extractor extractor : this.extractors) { + TagContext extracted = extractor.extract(carrier, getter); + // Check if context is valid + if (extracted instanceof ExtractedContext) { + // If no prior valid context, store it as first valid context + if (context == null) { + context = (ExtractedContext) extracted; + // Stop extraction if only extracting first valid context and drop everything else + if (this.extractFirst) { + break; + } + } + // If another valid context is extracted + else { + boolean comingFromTraceContext = extracted.getPropagationStyle() == TRACECONTEXT; + boolean traceIdMatches = context.getTraceId().equals(extracted.getTraceId()); + if (comingFromTraceContext && traceIdMatches) { + // Propagate newly extracted W3C tracestate to first valid context + // TODO + // context. + } + } + } + // Check if context is at least partial to keep it as first valid partial context found + else if (extracted != null && partialContext == null) { + partialContext = extracted; } } - log.debug("Extract incomplete context {}", context); - return context; + if (context != null) { + log.debug("Extract complete context {}", context); + return context; + } else if (partialContext != null) { + log.debug("Extract incomplete context {}", partialContext); + return partialContext; + } else { + log.debug("Extract no context"); + return null; + } } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java index 13258b270f9..b72e7f84679 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java @@ -1,5 +1,6 @@ package datadog.trace.core.propagation; +import static datadog.trace.api.TracePropagationStyle.TRACECONTEXT; import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_DROP; import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_KEEP; import static datadog.trace.core.propagation.HttpCodec.firstHeaderValue; @@ -12,6 +13,7 @@ import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; +import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.internal.util.LongStringUtils; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.sampling.SamplingMechanism; @@ -110,6 +112,11 @@ private W3CContextInterpreter(Config config) { datadogTagsFactory = PropagationTags.factory(config); } + @Override + public TracePropagationStyle style() { + return TRACECONTEXT; + } + @Override public ContextInterpreter reset(TraceConfig traceConfig) { tracestateHeader = null; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/XRayHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/XRayHttpCodec.java index 489e12abe1e..15e7d54e476 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/XRayHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/XRayHttpCodec.java @@ -1,6 +1,7 @@ package datadog.trace.core.propagation; import static datadog.trace.api.DDTags.ORIGIN_KEY; +import static datadog.trace.api.TracePropagationStyle.XRAY; import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_DROP; import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_KEEP; import static java.util.concurrent.TimeUnit.MILLISECONDS; @@ -12,6 +13,7 @@ import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; +import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.core.DDSpanContext; @@ -136,6 +138,11 @@ private XRayContextInterpreter(Config config) { super(config); } + @Override + public TracePropagationStyle style() { + return XRAY; + } + @Override public boolean accept(String key, String value) { if (null == key || key.isEmpty()) { diff --git a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaHandler.java b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaHandler.java index bcf0ffc8834..43e791d3880 100644 --- a/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaHandler.java +++ b/dd-trace-core/src/main/java/datadog/trace/lambda/LambdaHandler.java @@ -1,5 +1,6 @@ package datadog.trace.lambda; +import static datadog.trace.api.TracePropagationStyle.DATADOG; import static java.util.concurrent.TimeUnit.SECONDS; import com.squareup.moshi.JsonAdapter; @@ -92,7 +93,12 @@ public static AgentSpan.Context notifyStartInvocation( propagationTagsFactory.fromHeaderValue( PropagationTags.HeaderType.DATADOG, response.headers().get(DATADOG_TAGS_KEY)); return new ExtractedContext( - DDTraceId.from(traceID), DDSpanId.ZERO, samplingPriority, null, propagationTags); + DDTraceId.from(traceID), + DDSpanId.ZERO, + samplingPriority, + null, + propagationTags, + DATADOG); } else { log.debug( "could not find traceID or sampling priority in notifyStartInvocation, not injecting the 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 735b6d313cc..f01640a5997 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 @@ -25,6 +25,7 @@ import static datadog.trace.api.DDTags.PID_TAG import static datadog.trace.api.DDTags.RUNTIME_ID_TAG import static datadog.trace.api.DDTags.THREAD_ID import static datadog.trace.api.DDTags.THREAD_NAME +import static datadog.trace.api.TracePropagationStyle.DATADOG import static java.util.concurrent.TimeUnit.MILLISECONDS class CoreSpanBuilderTest extends DDCoreSpecification { @@ -335,9 +336,9 @@ class CoreSpanBuilderTest extends DDCoreSpecification { span.context().propagationTags.headerValue(PropagationTags.HeaderType.DATADOG) == extractedContext.propagationTags.headerValue(PropagationTags.HeaderType.DATADOG) 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) | _ - 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) | _ + 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) | _ + 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 "TagContext should populate default span details"() { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextPropagationTagsTest.groovy index fc56a3377c2..50ef4ff544f 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextPropagationTagsTest.groovy @@ -7,6 +7,7 @@ import datadog.trace.core.propagation.ExtractedContext import datadog.trace.core.propagation.PropagationTags import datadog.trace.core.test.DDCoreSpecification +import static datadog.trace.api.TracePropagationStyle.DATADOG import static datadog.trace.api.sampling.PrioritySampling.* import static datadog.trace.api.sampling.SamplingMechanism.* @@ -23,7 +24,7 @@ class DDSpanContextPropagationTagsTest extends DDCoreSpecification { setup: tracer = tracerBuilder().writer(writer).build() def propagationTags = tracer.propagationTagsFactory.fromHeaderValue(PropagationTags.HeaderType.DATADOG, header) - def extracted = new ExtractedContext(DDTraceId.from(123), 456, priority, "789", propagationTags) + def extracted = new ExtractedContext(DDTraceId.from(123), 456, priority, "789", propagationTags, DATADOG) .withRequestContextDataAppSec("dummy") def span = (DDSpan) tracer.buildSpan("top") .asChildOf((AgentSpan.Context) extracted) @@ -52,7 +53,7 @@ class DDSpanContextPropagationTagsTest extends DDCoreSpecification { setup: tracer = tracerBuilder().writer(writer).build() def propagationTags = tracer.propagationTagsFactory.fromHeaderValue(PropagationTags.HeaderType.DATADOG, header) - def extracted = new ExtractedContext(DDTraceId.from(123), 456, priority, "789", propagationTags) + def extracted = new ExtractedContext(DDTraceId.from(123), 456, priority, "789", propagationTags, DATADOG) .withRequestContextDataAppSec("dummy") def rootSpan = (DDSpan) tracer.buildSpan("top") .asChildOf((AgentSpan.Context) extracted) @@ -82,7 +83,7 @@ class DDSpanContextPropagationTagsTest extends DDCoreSpecification { setup: tracer = tracerBuilder().writer(writer).build() def propagationTags = tracer.propagationTagsFactory.fromHeaderValue(PropagationTags.HeaderType.DATADOG, header) - def extracted = new ExtractedContext(DDTraceId.from(123), 456, priority, "789", propagationTags) + def extracted = new ExtractedContext(DDTraceId.from(123), 456, priority, "789", propagationTags, DATADOG) .withRequestContextDataAppSec("dummy") def span = (DDSpan) tracer.buildSpan("top") .asChildOf((AgentSpan.Context) extracted) @@ -109,7 +110,7 @@ class DDSpanContextPropagationTagsTest extends DDCoreSpecification { setup: tracer = tracerBuilder().writer(writer).build() def propagationTags = tracer.propagationTagsFactory.fromHeaderValue(PropagationTags.HeaderType.DATADOG, header) - def extracted = new ExtractedContext(DDTraceId.from(123), 456, priority, "789", propagationTags) + def extracted = new ExtractedContext(DDTraceId.from(123), 456, priority, "789", propagationTags, DATADOG) .withRequestContextDataAppSec("dummy") def rootSpan = (DDSpan) tracer.buildSpan("top") .asChildOf((AgentSpan.Context) extracted) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextTest.groovy index 0f6c6db868b..69a79e57822 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextTest.groovy @@ -9,6 +9,7 @@ import datadog.trace.common.writer.ListWriter import datadog.trace.core.propagation.ExtractedContext import datadog.trace.core.test.DDCoreSpecification +import static datadog.trace.api.TracePropagationStyle.DATADOG import static datadog.trace.api.sampling.PrioritySampling.* import static datadog.trace.api.sampling.SamplingMechanism.* import static datadog.trace.core.DDSpanContext.SPAN_SAMPLING_MECHANISM_TAG @@ -181,7 +182,7 @@ class DDSpanContextTest extends DDCoreSpecification { def "set TraceSegment tags and data on correct span"() { setup: - def extracted = new ExtractedContext(DDTraceId.from(123), 456, SAMPLER_KEEP, "789", tracer.getPropagationTagsFactory().empty()) + def extracted = new ExtractedContext(DDTraceId.from(123), 456, SAMPLER_KEEP, "789", tracer.getPropagationTagsFactory().empty(), DATADOG) .withRequestContextDataAppSec("dummy") def top = tracer.buildSpan("top").asChildOf((AgentSpan.Context) extracted).start() diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy index ecccd01274d..8fbcd58005d 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy @@ -18,6 +18,7 @@ import spock.lang.Shared import java.util.concurrent.TimeUnit +import static datadog.trace.api.TracePropagationStyle.DATADOG import static datadog.trace.api.sampling.PrioritySampling.UNSET import static datadog.trace.api.sampling.SamplingMechanism.SPAN_SAMPLING_RATE import static datadog.trace.core.DDSpanContext.SPAN_SAMPLING_MAX_PER_SECOND_TAG @@ -272,9 +273,9 @@ class DDSpanTest extends DDCoreSpecification { child.@origin == null // Access field directly instead of getter. where: - extractedContext | _ - new TagContext("some-origin", [:]) | _ - new ExtractedContext(DDTraceId.ONE, 2, PrioritySampling.SAMPLER_DROP, "some-origin", propagationTagsFactory.empty()) | _ + extractedContext | _ + new TagContext("some-origin", [:]) | _ + new ExtractedContext(DDTraceId.ONE, 2, PrioritySampling.SAMPLER_DROP, "some-origin", propagationTagsFactory.empty(), DATADOG) | _ } def "isRootSpan() in and not in the context of distributed tracing"() { @@ -291,9 +292,9 @@ class DDSpanTest extends DDCoreSpecification { root.finish() where: - extractedContext | isTraceRootSpan - null | true - new ExtractedContext(DDTraceId.from(123), 456, PrioritySampling.SAMPLER_KEEP, "789", propagationTagsFactory.empty()) | false + extractedContext | isTraceRootSpan + null | true + new ExtractedContext(DDTraceId.from(123), 456, PrioritySampling.SAMPLER_KEEP, "789", propagationTagsFactory.empty(), DATADOG) | false } def "getApplicationRootSpan() in and not in the context of distributed tracing"() { @@ -313,9 +314,9 @@ class DDSpanTest extends DDCoreSpecification { root.finish() where: - extractedContext | isTraceRootSpan - null | true - new ExtractedContext(DDTraceId.from(123), 456, PrioritySampling.SAMPLER_KEEP, "789", propagationTagsFactory.empty()) | false + extractedContext | isTraceRootSpan + null | true + new ExtractedContext(DDTraceId.from(123), 456, PrioritySampling.SAMPLER_KEEP, "789", propagationTagsFactory.empty(), DATADOG) | false } def 'publishing of root span closes the request context data'() { diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/datastreams/DefaultPathwayContextTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/datastreams/DefaultPathwayContextTest.groovy index 5888c87a291..8cb0d4338ae 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/datastreams/DefaultPathwayContextTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/datastreams/DefaultPathwayContextTest.groovy @@ -16,6 +16,7 @@ import datadog.trace.core.test.DDCoreSpecification import java.util.function.Consumer +import static datadog.trace.api.TracePropagationStyle.DATADOG import static datadog.trace.api.config.GeneralConfig.PRIMARY_TAG import static datadog.trace.core.datastreams.DefaultDataStreamsMonitoring.DEFAULT_BUCKET_DURATION_NANOS import static java.util.concurrent.TimeUnit.MILLISECONDS @@ -717,7 +718,7 @@ class DefaultPathwayContextTest extends DDCoreSpecification { @Override TagContext extract(C carrier, AgentPropagation.ContextVisitor getter) { - return new ExtractedContext(DDTraceId.ONE, 1, 0, null, 0, null, null, null, null, traceConfig ) + return new ExtractedContext(DDTraceId.ONE, 1, 0, null, 0, null, null, null, null, traceConfig, DATADOG) } } 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 15261fe4fb7..ed0633c73e4 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -104,6 +104,7 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_ANALYTICS_ENABLED; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_HTTP_RESOURCE_REMOVE_TRAILING_SLASH; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL; +import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_PROPAGATION_EXTRACT_FIRST; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_PROPAGATION_STYLE; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_RATE_LIMIT; import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_REPORT_HOSTNAME; @@ -391,6 +392,7 @@ import static datadog.trace.api.config.TracerConfig.TRACE_PEER_SERVICE_COMPONENT_OVERRIDES; import static datadog.trace.api.config.TracerConfig.TRACE_PEER_SERVICE_DEFAULTS_ENABLED; import static datadog.trace.api.config.TracerConfig.TRACE_PEER_SERVICE_MAPPING; +import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_EXTRACT_FIRST; import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_STYLE; import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_STYLE_EXTRACT; import static datadog.trace.api.config.TracerConfig.TRACE_PROPAGATION_STYLE_INJECT; @@ -576,6 +578,7 @@ static class HostNameHolder { private final boolean tracePropagationStyleB3PaddingEnabled; private final Set tracePropagationStylesToExtract; private final Set tracePropagationStylesToInject; + private final boolean tracePropagationExtractFirst; private final int clockSyncPeriod; private final boolean logsInjectionEnabled; @@ -1251,6 +1254,10 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins deprecatedInject.isEmpty() ? DEFAULT_PROPAGATION_STYLE : deprecatedInject; } + tracePropagationExtractFirst = + configProvider.getBoolean( + TRACE_PROPAGATION_EXTRACT_FIRST, DEFAULT_TRACE_PROPAGATION_EXTRACT_FIRST); + clockSyncPeriod = configProvider.getInteger(CLOCK_SYNC_PERIOD, DEFAULT_CLOCK_SYNC_PERIOD); logsInjectionEnabled = @@ -2148,6 +2155,10 @@ public Set getTracePropagationStylesToInject() { return tracePropagationStylesToInject; } + public boolean isTracePropagationExtractFirst() { + return tracePropagationExtractFirst; + } + public int getClockSyncPeriod() { return clockSyncPeriod; } @@ -3825,6 +3836,8 @@ public String toString() { + tracePropagationStylesToExtract + ", tracePropagationStylesToInject=" + tracePropagationStylesToInject + + ", tracePropagationExtractFirst=" + + tracePropagationExtractFirst + ", clockSyncPeriod=" + clockSyncPeriod + ", jmxFetchEnabled=" 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 4c730abfdfc..c5b9009e059 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 @@ -1,8 +1,11 @@ package datadog.trace.bootstrap.instrumentation.api; +import static datadog.trace.api.TracePropagationStyle.NONE; + import datadog.trace.api.DDSpanId; import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; +import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.sampling.PrioritySampling; import java.util.Collections; import java.util.Map; @@ -25,13 +28,14 @@ public class TagContext implements AgentSpan.Context.Extracted { private final Map baggage; private final int samplingPriority; private final TraceConfig traceConfig; + private final TracePropagationStyle propagationStyle; public TagContext() { this(null, null); } public TagContext(final String origin, final Map tags) { - this(origin, tags, null, null, PrioritySampling.UNSET, null); + this(origin, tags, null, null, PrioritySampling.UNSET, null, NONE); } public TagContext( @@ -40,19 +44,25 @@ public TagContext( final HttpHeaders httpHeaders, final Map baggage, final int samplingPriority, - final TraceConfig traceConfig) { + final TraceConfig traceConfig, + final TracePropagationStyle propagationStyle) { this.origin = origin; this.tags = tags; this.httpHeaders = httpHeaders == null ? EMPTY_HTTP_HEADERS : httpHeaders; this.baggage = baggage == null ? Collections.emptyMap() : baggage; this.samplingPriority = samplingPriority; this.traceConfig = traceConfig; + this.propagationStyle = propagationStyle; } public TraceConfig getTraceConfig() { return traceConfig; } + public TracePropagationStyle getPropagationStyle() { + return this.propagationStyle; + } + public final CharSequence getOrigin() { return origin; } 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 e67942b4115..5793b4d2b1b 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -114,6 +114,7 @@ import static datadog.trace.api.config.TracerConfig.SPAN_TAGS 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_RATE_LIMIT import static datadog.trace.api.config.TracerConfig.TRACE_REPORT_HOSTNAME import static datadog.trace.api.config.TracerConfig.TRACE_RESOLVER_ENABLED @@ -145,6 +146,7 @@ class ConfigTest extends DDSpecification { private static final DD_JMX_TAGS_ENV = "DD_TRACE_JMX_TAGS" private static final DD_PROPAGATION_STYLE_EXTRACT = "DD_PROPAGATION_STYLE_EXTRACT" private static final DD_PROPAGATION_STYLE_INJECT = "DD_PROPAGATION_STYLE_INJECT" + private static final DD_TRACE_PROPAGATION_EXTRACT_FIRST = "DD_TRACE_PROPAGATION_EXTRACT_FIRST" private static final DD_JMXFETCH_METRICS_CONFIGS_ENV = "DD_JMXFETCH_METRICS_CONFIGS" private static final DD_TRACE_AGENT_PORT_ENV = "DD_TRACE_AGENT_PORT" private static final DD_AGENT_PORT_LEGACY_ENV = "DD_AGENT_PORT" @@ -191,6 +193,7 @@ class ConfigTest extends DDSpecification { prop.setProperty(RUNTIME_CONTEXT_FIELD_INJECTION, "false") prop.setProperty(PROPAGATION_STYLE_EXTRACT, "Datadog, B3") prop.setProperty(PROPAGATION_STYLE_INJECT, "B3, Datadog") + prop.setProperty(TRACE_PROPAGATION_EXTRACT_FIRST, "false") prop.setProperty(JMX_FETCH_ENABLED, "false") prop.setProperty(JMX_FETCH_METRICS_CONFIGS, "/foo.yaml,/bar.yaml") prop.setProperty(JMX_FETCH_CHECK_PERIOD, "100") @@ -279,6 +282,7 @@ class ConfigTest extends DDSpecification { config.propagationStylesToInject.toList() == [PropagationStyle.B3, PropagationStyle.DATADOG] config.tracePropagationStylesToExtract.toList() == [DATADOG, B3SINGLE, B3MULTI] config.tracePropagationStylesToInject.toList() == [B3SINGLE, B3MULTI, DATADOG] + config.tracePropagationExtractFirst == false config.jmxFetchEnabled == false config.jmxFetchMetricsConfigs == ["/foo.yaml", "/bar.yaml"] config.jmxFetchCheckPeriod == 100 @@ -368,6 +372,7 @@ class ConfigTest extends DDSpecification { System.setProperty(PREFIX + RUNTIME_CONTEXT_FIELD_INJECTION, "false") 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 + JMX_FETCH_ENABLED, "false") System.setProperty(PREFIX + JMX_FETCH_METRICS_CONFIGS, "/foo.yaml,/bar.yaml") System.setProperty(PREFIX + JMX_FETCH_CHECK_PERIOD, "100") @@ -456,6 +461,7 @@ class ConfigTest extends DDSpecification { config.propagationStylesToInject.toList() == [PropagationStyle.B3, PropagationStyle.DATADOG] config.tracePropagationStylesToExtract.toList() == [DATADOG, B3SINGLE, B3MULTI] config.tracePropagationStylesToInject.toList() == [B3SINGLE, B3MULTI, DATADOG] + config.tracePropagationExtractFirst == false config.jmxFetchEnabled == false config.jmxFetchMetricsConfigs == ["/foo.yaml", "/bar.yaml"] config.jmxFetchCheckPeriod == 100 @@ -522,6 +528,7 @@ class ConfigTest extends DDSpecification { environmentVariables.set(DD_PRIORITIZATION_TYPE_ENV, "EnsureTrace") environmentVariables.set(DD_PROPAGATION_STYLE_EXTRACT, "B3 Datadog") environmentVariables.set(DD_PROPAGATION_STYLE_INJECT, "Datadog B3") + environmentVariables.set(DD_TRACE_PROPAGATION_EXTRACT_FIRST, "false") environmentVariables.set(DD_JMXFETCH_METRICS_CONFIGS_ENV, "some/file") environmentVariables.set(DD_TRACE_REPORT_HOSTNAME, "true") environmentVariables.set(DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH, "42") @@ -540,6 +547,7 @@ class ConfigTest extends DDSpecification { config.propagationStylesToInject.toList() == [PropagationStyle.DATADOG, PropagationStyle.B3] config.tracePropagationStylesToExtract.toList() == [B3SINGLE, B3MULTI, DATADOG] config.tracePropagationStylesToInject.toList() == [DATADOG, B3SINGLE, B3MULTI] + config.tracePropagationExtractFirst == false config.jmxFetchMetricsConfigs == ["some/file"] config.reportHostName == true config.xDatadogTagsMaxLength == 42 From 7ebe31d8ced023b93d0837ac60db24315f0818cd Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Fri, 3 Nov 2023 14:08:17 +0100 Subject: [PATCH 02/12] fix(core): Fix B3 extractor tests --- .../trace/core/propagation/B3HttpExtractorTest.groovy | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/B3HttpExtractorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/B3HttpExtractorTest.groovy index 67ab99ac372..073908513e1 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/B3HttpExtractorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/B3HttpExtractorTest.groovy @@ -293,7 +293,8 @@ class B3HttpExtractorTest extends DDSpecification { TagContext context = extractor.extract(headers, ContextVisitors.stringValuesMap()) then: - context == null + !(context instanceof ExtractedContext) + context.getTags() == ["some-tag": "my-interesting-info"] } def "extract http headers with out of range span ID"() { @@ -304,12 +305,12 @@ class B3HttpExtractorTest extends DDSpecification { SOME_HEADER : "my-interesting-info", ] - when: TagContext context = extractor.extract(headers, ContextVisitors.stringValuesMap()) then: - context == null + !(context instanceof ExtractedContext) + context.getTags() == ["some-tag": "my-interesting-info"] } def "extract ids while retaining the original string"() { From df0a27f691afe17cce2d544c7b04c49e0f3e1076 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Fri, 3 Nov 2023 14:08:48 +0100 Subject: [PATCH 03/12] feat(core): Add terminated extracted contexts as span links --- .../java/datadog/trace/core/CoreTracer.java | 7 +++++++ .../trace/core/propagation/HttpCodec.java | 16 +++++++++++----- .../instrumentation/api/AgentSpan.java | 8 ++++++++ .../instrumentation/api/AgentTracer.java | 9 ++++++++- .../instrumentation/api/TagContext.java | 17 +++++++++++++++++ 5 files changed, 51 insertions(+), 6 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 9caf0eebbd1..dae7c75cfce 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 @@ -1191,6 +1191,13 @@ public CoreSpanBuilder ignoreActiveSpan() { } private DDSpan buildSpan() { + if (parent instanceof TagContext) { + List terminatedContextLinks = + ((TagContext) parent).getTerminatedContextLinks(); + if (!terminatedContextLinks.isEmpty()) { + links.addAll(terminatedContextLinks); + } + } DDSpan span = DDSpan.create(instrumentationName, timestampMicro, buildSpanContext(), links); if (span.isLocalRootSpan()) { EndpointTracker tracker = tracer.onRootSpanStarted(span); 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 d25017d6a4a..fcca0fa1a4c 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 @@ -8,6 +8,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.bootstrap.instrumentation.api.TagContext; import datadog.trace.core.DDSpanContext; +import datadog.trace.core.DDSpanLink; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.net.URLEncoder; @@ -198,12 +199,17 @@ public TagContext extract( } // If another valid context is extracted else { - boolean comingFromTraceContext = extracted.getPropagationStyle() == TRACECONTEXT; boolean traceIdMatches = context.getTraceId().equals(extracted.getTraceId()); - if (comingFromTraceContext && traceIdMatches) { - // Propagate newly extracted W3C tracestate to first valid context - // TODO - // context. + if (traceIdMatches) { + boolean comingFromTraceContext = extracted.getPropagationStyle() == TRACECONTEXT; + if (comingFromTraceContext) { + // Propagate newly extracted W3C tracestate to first valid context + // TODO + } + } else { + // Terminate extracted context and add it as span link + context.addTerminatedContextLink(DDSpanLink.from((ExtractedContext) extracted)); + // TODO Note: Other vendor tracestate will be lost here } } } 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 20a22942540..463cfc4920e 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 @@ -6,6 +6,7 @@ import datadog.trace.api.gateway.RequestContext; import datadog.trace.api.interceptor.MutableSpan; import datadog.trace.api.sampling.PrioritySampling; +import java.util.List; import java.util.Map; public interface AgentSpan extends MutableSpan, IGSpanInfo { @@ -178,6 +179,13 @@ interface Context { default void mergePathwayContext(PathwayContext pathwayContext) {} interface Extracted extends Context { + /** + * Gets the span links related to the other terminated context. + * + * @return The span links to other extracted contexts found but terminated. + */ + List getTerminatedContextLinks(); + String getForwarded(); String getFastlyClientIp(); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index 3a1542987ea..000bc2c7235 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -1,6 +1,7 @@ package datadog.trace.bootstrap.instrumentation.api; import static datadog.trace.api.ConfigDefaults.DEFAULT_ASYNC_PROPAGATING; +import static java.util.Collections.emptyList; import datadog.trace.api.DDSpanId; import datadog.trace.api.DDTraceId; @@ -25,6 +26,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.function.Consumer; @@ -915,7 +917,7 @@ public int getSamplingPriority() { @Override public Iterable> baggageItems() { - return Collections.emptyList(); + return emptyList(); } @Override @@ -923,6 +925,11 @@ public PathwayContext getPathwayContext() { return NoopPathwayContext.INSTANCE; } + @Override + public List getTerminatedContextLinks() { + return emptyList(); + } + @Override public String getForwarded() { return null; 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 c5b9009e059..303b78a9ebd 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 @@ -1,13 +1,16 @@ package datadog.trace.bootstrap.instrumentation.api; import static datadog.trace.api.TracePropagationStyle.NONE; +import static java.util.Collections.emptyList; import datadog.trace.api.DDSpanId; import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; import datadog.trace.api.TracePropagationStyle; import datadog.trace.api.sampling.PrioritySampling; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Map; /** @@ -20,6 +23,7 @@ public class TagContext implements AgentSpan.Context.Extracted { private final CharSequence origin; private final Map tags; + private List terminatedContextLinks; private Object requestContextDataAppSec; private Object requestContextDataIast; private Object ciVisibilityContextData; @@ -48,6 +52,7 @@ public TagContext( final TracePropagationStyle propagationStyle) { this.origin = origin; this.tags = tags; + this.terminatedContextLinks = null; this.httpHeaders = httpHeaders == null ? EMPTY_HTTP_HEADERS : httpHeaders; this.baggage = baggage == null ? Collections.emptyMap() : baggage; this.samplingPriority = samplingPriority; @@ -67,6 +72,18 @@ public final CharSequence getOrigin() { return origin; } + @Override + public List getTerminatedContextLinks() { + return this.terminatedContextLinks == null ? emptyList() : this.terminatedContextLinks; + } + + public void addTerminatedContextLink(AgentSpanLink link) { + if (this.terminatedContextLinks == null) { + this.terminatedContextLinks = new ArrayList<>(); + } + this.terminatedContextLinks.add(link); + } + @Override public String getForwarded() { return httpHeaders.forwarded; From c63ce7002eca28ffeb6562158cdc136593f8629e Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Fri, 3 Nov 2023 15:56:18 +0100 Subject: [PATCH 04/12] feat(core): Add parent span to tracestate --- .../core/propagation/PropagationTags.java | 14 +++++++++++ .../trace/core/propagation/W3CHttpCodec.java | 18 +++++++++++++- .../core/propagation/ptags/PTagsFactory.java | 19 +++++++++++++++ .../core/propagation/ptags/W3CPTagsCodec.java | 11 +++++++++ .../propagation/W3CHttpInjectorTest.groovy | 24 +++++++++---------- 5 files changed, 72 insertions(+), 14 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java index 79bbd3eb342..644ad225c60 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java @@ -66,6 +66,20 @@ public interface Factory { public abstract void updateTraceIdHighOrderBits(long highOrderBits); + /** + * Gets the parent span identifier to propagate as tag. + * + * @return The parent span identifiers (hexadecimal encoded, {@code null} if undefined). + */ + public abstract CharSequence getParentSpanId(); + + /** + * Updates the parent span identifier to propagate as tag. + * + * @param parentSpanId The parent span identifier (as an unsigned long, {@code 0} to unset). + */ + public abstract void updateParentSpanId(long parentSpanId); + /** * Constructs a header value that includes valid propagated _dd.p.* tags and possibly a new * sampling decision tag _dd.p.dm based on the current state. Returns null if the value length diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java index b72e7f84679..0b84a7b5169 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java @@ -63,6 +63,13 @@ public Injector(Map invertedBaggageMapping) { @Override public void inject( final DDSpanContext context, final C carrier, final AgentPropagation.Setter setter) { + injectTraceParent(context, carrier, setter); + injectTraceState(context, carrier, setter); + injectBaggage(context, carrier, setter); + } + + private void injectTraceParent( + DDSpanContext context, C carrier, AgentPropagation.Setter setter) { StringBuilder sb = new StringBuilder(TRACE_PARENT_LENGTH); sb.append("00-"); sb.append(context.getTraceId().toHexString()); @@ -70,11 +77,20 @@ public void inject( sb.append(DDSpanId.toHexStringPadded(context.getSpanId())); sb.append(context.getSamplingPriority() > 0 ? "-01" : "-00"); setter.set(carrier, TRACE_PARENT_KEY, sb.toString()); - String tracestate = context.getPropagationTags().headerValue(PropagationTags.HeaderType.W3C); + } + + private void injectTraceState( + DDSpanContext context, C carrier, AgentPropagation.Setter setter) { + PropagationTags propagationTags = context.getPropagationTags(); + propagationTags.updateParentSpanId(context.getSpanId()); + String tracestate = propagationTags.headerValue(PropagationTags.HeaderType.W3C); if (tracestate != null && !tracestate.isEmpty()) { setter.set(carrier, TRACE_STATE_KEY, tracestate); } + } + private void injectBaggage( + DDSpanContext context, C carrier, AgentPropagation.Setter setter) { long e2eStart = context.getEndToEndStartTime(); if (e2eStart > 0) { setter.set(carrier, E2E_START_KEY, Long.toString(NANOSECONDS.toMillis(e2eStart))); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index 22c7f58cb7b..03ea687ac09 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -5,6 +5,7 @@ import static datadog.trace.core.propagation.ptags.PTagsCodec.DECISION_MAKER_TAG; import static datadog.trace.core.propagation.ptags.PTagsCodec.TRACE_ID_TAG; +import datadog.trace.api.DDSpanId; import datadog.trace.api.internal.util.LongStringUtils; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.sampling.SamplingMechanism; @@ -88,6 +89,10 @@ static class PTags extends PropagationTags { * of the trace id, wrapped into a {@link TagValue}, null if not set. */ private volatile TagValue traceIdHighOrderBitsHexTagValue; + /** The parent span identifier (as an unsigned long, {@code 0}) if not set) */ + private volatile long parentSpanId; + /** The parent span identifier (hexadecimal encoded, {@code null} if not set) */ + private CharSequence parentSpanIdValue; protected volatile String error; @@ -205,6 +210,20 @@ public void updateTraceIdHighOrderBits(long highOrderBits) { } } + @Override + public CharSequence getParentSpanId() { + return this.parentSpanIdValue; + } + + @Override + public void updateParentSpanId(long parentSpanId) { + if (this.parentSpanId != parentSpanId) { + this.parentSpanId = parentSpanId; + this.parentSpanIdValue = parentSpanId == 0 ? null : DDSpanId.toHexString(parentSpanId); + clearCachedHeader(W3C); + } + } + @Override @SuppressWarnings("StringEquality") @SuppressFBWarnings("ES_COMPARING_STRINGS_WITH_EQ") diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index 94c84d656d5..62eab888c3c 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -205,10 +205,12 @@ protected int estimateHeaderSize(PTags pTags) { @Override protected int appendPrefix(StringBuilder sb, PTags ptags) { sb.append("dd="); + // Append sampling priority (s) if (ptags.getSamplingPriority() != PrioritySampling.UNSET) { sb.append("s:"); sb.append(ptags.getSamplingPriority()); } + // Append origin (o) CharSequence origin = ptags.getOrigin(); if (origin != null) { if (sb.length() > EMPTY_SIZE) { @@ -221,6 +223,15 @@ protected int appendPrefix(StringBuilder sb, PTags ptags) { sb.append(origin); } } + // Append parent span (p) + CharSequence parentSpanId = ptags.getParentSpanId(); + if (parentSpanId != null) { + if (sb.length() > EMPTY_SIZE) { + sb.append(';'); + } + sb.append("p:"); + sb.append(parentSpanId); + } return sb.length(); } diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CHttpInjectorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CHttpInjectorTest.groovy index 3b2aef1b762..153f74f5daf 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CHttpInjectorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CHttpInjectorTest.groovy @@ -47,17 +47,15 @@ class W3CHttpInjectorTest extends DDCoreSpecification { null, NoopPathwayContext.INSTANCE, false, - PropagationTags.factory().fromHeaderValue(PropagationTags.HeaderType.DATADOG, tracestate ? "_dd.p.usr=123" : "")) + PropagationTags.factory().fromHeaderValue(PropagationTags.HeaderType.DATADOG, "_dd.p.usr=123")) final Map carrier = [:] Map expected = [ - (TRACE_PARENT_KEY): buildTraceParent(traceId, spanId, samplingPriority), + (TRACE_PARENT_KEY) : buildTraceParent(traceId, spanId, samplingPriority), (OT_BAGGAGE_PREFIX + "k1"): "v1", (OT_BAGGAGE_PREFIX + "k2"): "v2", - "SOME_CUSTOM_HEADER": "some-value" + "SOME_CUSTOM_HEADER" : "some-value", + (TRACE_STATE_KEY) : tracestate ] - if (tracestate) { - expected.put(TRACE_STATE_KEY, tracestate) - } when: injector.inject(mockedContext, carrier, MapSetter.INSTANCE) @@ -70,11 +68,11 @@ class W3CHttpInjectorTest extends DDCoreSpecification { where: traceId | spanId | samplingPriority | origin | tracestate - "1" | "2" | UNSET | null | null - "1" | "2" | SAMPLER_KEEP | "saipan" | "dd=s:1;o:saipan;t.usr:123" - "$TRACE_ID_MAX" | "${TRACE_ID_MAX - 1}" | UNSET | "saipan" | "dd=o:saipan;t.usr:123" - "${TRACE_ID_MAX - 1}" | "$TRACE_ID_MAX" | SAMPLER_KEEP | null | "dd=s:1;t.usr:123" - "${TRACE_ID_MAX - 1}" | "$TRACE_ID_MAX" | SAMPLER_DROP | null | "dd=s:0;t.usr:123" + "1" | "2" | UNSET | null | "dd=p:2;t.usr:123" + "1" | "2" | SAMPLER_KEEP | "saipan" | "dd=s:1;o:saipan;p:2;t.usr:123" + "$TRACE_ID_MAX" | "${TRACE_ID_MAX - 1}" | UNSET | "saipan" | "dd=o:saipan;p:fffffffffffffffe;t.usr:123" + "${TRACE_ID_MAX - 1}" | "$TRACE_ID_MAX" | SAMPLER_KEEP | null | "dd=s:1;p:ffffffffffffffff;t.usr:123" + "${TRACE_ID_MAX - 1}" | "$TRACE_ID_MAX" | SAMPLER_DROP | null | "dd=s:0;p:ffffffffffffffff;t.usr:123" } def "inject http headers with end-to-end"() { @@ -113,7 +111,7 @@ class W3CHttpInjectorTest extends DDCoreSpecification { then: carrier == [ (TRACE_PARENT_KEY): buildTraceParent('1', '2', UNSET), - (TRACE_STATE_KEY): "dd=o:fakeOrigin;t.dm:-4;t.anytag:value", + (TRACE_STATE_KEY): "dd=o:fakeOrigin;p:2;t.dm:-4;t.anytag:value", (OT_BAGGAGE_PREFIX + "t0"): "${(long) (mockedContext.endToEndStartTime / 1000000L)}", (OT_BAGGAGE_PREFIX + "k1"): "v1", (OT_BAGGAGE_PREFIX + "k2"): "v2", @@ -159,7 +157,7 @@ class W3CHttpInjectorTest extends DDCoreSpecification { then: carrier == [ (TRACE_PARENT_KEY): buildTraceParent('1', '2', USER_KEEP), - (TRACE_STATE_KEY): "dd=s:2;o:fakeOrigin;t.dm:-4", + (TRACE_STATE_KEY): "dd=s:2;o:fakeOrigin;p:2;t.dm:-4", (OT_BAGGAGE_PREFIX + "k1"): "v1", (OT_BAGGAGE_PREFIX + "k2"): "v2", ] From cbecc275da1418ddd02a760b99638c9a7510dc53 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 6 Nov 2023 09:34:38 +0100 Subject: [PATCH 05/12] feat(core): Revert parent span addition to tracestate --- .../core/propagation/PropagationTags.java | 14 -------------- .../trace/core/propagation/W3CHttpCodec.java | 1 - .../core/propagation/ptags/PTagsFactory.java | 19 ------------------- .../core/propagation/ptags/W3CPTagsCodec.java | 9 --------- 4 files changed, 43 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java index 644ad225c60..79bbd3eb342 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java @@ -66,20 +66,6 @@ public interface Factory { public abstract void updateTraceIdHighOrderBits(long highOrderBits); - /** - * Gets the parent span identifier to propagate as tag. - * - * @return The parent span identifiers (hexadecimal encoded, {@code null} if undefined). - */ - public abstract CharSequence getParentSpanId(); - - /** - * Updates the parent span identifier to propagate as tag. - * - * @param parentSpanId The parent span identifier (as an unsigned long, {@code 0} to unset). - */ - public abstract void updateParentSpanId(long parentSpanId); - /** * Constructs a header value that includes valid propagated _dd.p.* tags and possibly a new * sampling decision tag _dd.p.dm based on the current state. Returns null if the value length diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java index 0b84a7b5169..82ba1a9d7fc 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java @@ -82,7 +82,6 @@ private void injectTraceParent( private void injectTraceState( DDSpanContext context, C carrier, AgentPropagation.Setter setter) { PropagationTags propagationTags = context.getPropagationTags(); - propagationTags.updateParentSpanId(context.getSpanId()); String tracestate = propagationTags.headerValue(PropagationTags.HeaderType.W3C); if (tracestate != null && !tracestate.isEmpty()) { setter.set(carrier, TRACE_STATE_KEY, tracestate); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index 03ea687ac09..22c7f58cb7b 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -5,7 +5,6 @@ import static datadog.trace.core.propagation.ptags.PTagsCodec.DECISION_MAKER_TAG; import static datadog.trace.core.propagation.ptags.PTagsCodec.TRACE_ID_TAG; -import datadog.trace.api.DDSpanId; import datadog.trace.api.internal.util.LongStringUtils; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.sampling.SamplingMechanism; @@ -89,10 +88,6 @@ static class PTags extends PropagationTags { * of the trace id, wrapped into a {@link TagValue}, null if not set. */ private volatile TagValue traceIdHighOrderBitsHexTagValue; - /** The parent span identifier (as an unsigned long, {@code 0}) if not set) */ - private volatile long parentSpanId; - /** The parent span identifier (hexadecimal encoded, {@code null} if not set) */ - private CharSequence parentSpanIdValue; protected volatile String error; @@ -210,20 +205,6 @@ public void updateTraceIdHighOrderBits(long highOrderBits) { } } - @Override - public CharSequence getParentSpanId() { - return this.parentSpanIdValue; - } - - @Override - public void updateParentSpanId(long parentSpanId) { - if (this.parentSpanId != parentSpanId) { - this.parentSpanId = parentSpanId; - this.parentSpanIdValue = parentSpanId == 0 ? null : DDSpanId.toHexString(parentSpanId); - clearCachedHeader(W3C); - } - } - @Override @SuppressWarnings("StringEquality") @SuppressFBWarnings("ES_COMPARING_STRINGS_WITH_EQ") diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index 62eab888c3c..c46d2d54a28 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -223,15 +223,6 @@ protected int appendPrefix(StringBuilder sb, PTags ptags) { sb.append(origin); } } - // Append parent span (p) - CharSequence parentSpanId = ptags.getParentSpanId(); - if (parentSpanId != null) { - if (sb.length() > EMPTY_SIZE) { - sb.append(';'); - } - sb.append("p:"); - sb.append(parentSpanId); - } return sb.length(); } From e518431a4ef18bef3d18513c550a3d577539e517 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 6 Nov 2023 10:58:41 +0100 Subject: [PATCH 06/12] feat(core): Clean up W3C propagation tags error handling --- .../core/propagation/ptags/W3CPTagsCodec.java | 10 ---------- .../propagation/W3CHttpInjectorTest.groovy | 20 ++++++++++--------- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index c46d2d54a28..c3e61bea25b 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -674,7 +674,6 @@ private static class W3CPTags extends PTags { private final int ddMemberStart; private final int ddMemberValueEnd; private final int maxUnknownSize; - private String error; public W3CPTags( PTagsFactory factory, @@ -694,15 +693,6 @@ public W3CPTags( this.ddMemberStart = ddMemberStart; this.ddMemberValueEnd = ddMemberValueEnd; this.maxUnknownSize = maxUnknownSize; - this.error = null; - } - - @Override - String getError() { - if (this.error != null) { - return this.error; - } - return super.getError(); } @Override diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CHttpInjectorTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CHttpInjectorTest.groovy index 153f74f5daf..c45b32c7e1a 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CHttpInjectorTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CHttpInjectorTest.groovy @@ -47,15 +47,17 @@ class W3CHttpInjectorTest extends DDCoreSpecification { null, NoopPathwayContext.INSTANCE, false, - PropagationTags.factory().fromHeaderValue(PropagationTags.HeaderType.DATADOG, "_dd.p.usr=123")) + PropagationTags.factory().fromHeaderValue(PropagationTags.HeaderType.DATADOG, tracestate ? "_dd.p.usr=123" : "")) final Map carrier = [:] Map expected = [ (TRACE_PARENT_KEY) : buildTraceParent(traceId, spanId, samplingPriority), (OT_BAGGAGE_PREFIX + "k1"): "v1", (OT_BAGGAGE_PREFIX + "k2"): "v2", "SOME_CUSTOM_HEADER" : "some-value", - (TRACE_STATE_KEY) : tracestate ] + if (tracestate) { + expected.put(TRACE_STATE_KEY, tracestate) + } when: injector.inject(mockedContext, carrier, MapSetter.INSTANCE) @@ -68,11 +70,11 @@ class W3CHttpInjectorTest extends DDCoreSpecification { where: traceId | spanId | samplingPriority | origin | tracestate - "1" | "2" | UNSET | null | "dd=p:2;t.usr:123" - "1" | "2" | SAMPLER_KEEP | "saipan" | "dd=s:1;o:saipan;p:2;t.usr:123" - "$TRACE_ID_MAX" | "${TRACE_ID_MAX - 1}" | UNSET | "saipan" | "dd=o:saipan;p:fffffffffffffffe;t.usr:123" - "${TRACE_ID_MAX - 1}" | "$TRACE_ID_MAX" | SAMPLER_KEEP | null | "dd=s:1;p:ffffffffffffffff;t.usr:123" - "${TRACE_ID_MAX - 1}" | "$TRACE_ID_MAX" | SAMPLER_DROP | null | "dd=s:0;p:ffffffffffffffff;t.usr:123" + "1" | "2" | UNSET | null | null + "1" | "2" | SAMPLER_KEEP | "saipan" | "dd=s:1;o:saipan;t.usr:123" + "$TRACE_ID_MAX" | "${TRACE_ID_MAX - 1}" | UNSET | "saipan" | "dd=o:saipan;t.usr:123" + "${TRACE_ID_MAX - 1}" | "$TRACE_ID_MAX" | SAMPLER_KEEP | null | "dd=s:1;t.usr:123" + "${TRACE_ID_MAX - 1}" | "$TRACE_ID_MAX" | SAMPLER_DROP | null | "dd=s:0;t.usr:123" } def "inject http headers with end-to-end"() { @@ -111,7 +113,7 @@ class W3CHttpInjectorTest extends DDCoreSpecification { then: carrier == [ (TRACE_PARENT_KEY): buildTraceParent('1', '2', UNSET), - (TRACE_STATE_KEY): "dd=o:fakeOrigin;p:2;t.dm:-4;t.anytag:value", + (TRACE_STATE_KEY): "dd=o:fakeOrigin;t.dm:-4;t.anytag:value", (OT_BAGGAGE_PREFIX + "t0"): "${(long) (mockedContext.endToEndStartTime / 1000000L)}", (OT_BAGGAGE_PREFIX + "k1"): "v1", (OT_BAGGAGE_PREFIX + "k2"): "v2", @@ -157,7 +159,7 @@ class W3CHttpInjectorTest extends DDCoreSpecification { then: carrier == [ (TRACE_PARENT_KEY): buildTraceParent('1', '2', USER_KEEP), - (TRACE_STATE_KEY): "dd=s:2;o:fakeOrigin;p:2;t.dm:-4", + (TRACE_STATE_KEY): "dd=s:2;o:fakeOrigin;t.dm:-4", (OT_BAGGAGE_PREFIX + "k1"): "v1", (OT_BAGGAGE_PREFIX + "k2"): "v2", ] From c7bc8420e11f96aa74ad1e52c9258dfce71966b9 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 6 Nov 2023 13:56:08 +0100 Subject: [PATCH 07/12] feat(core): Add tracestate propagation from different propagation tags --- .../trace/core/propagation/HttpCodec.java | 7 +- .../core/propagation/PropagationTags.java | 16 +++++ .../core/propagation/ptags/PTagsFactory.java | 20 +++++- .../core/propagation/ptags/W3CPTagsCodec.java | 67 ++++++++++++++----- .../propagation/W3CPropagationTagsTest.groovy | 28 ++++++++ 5 files changed, 117 insertions(+), 21 deletions(-) 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 fcca0fa1a4c..925d5134622 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 @@ -189,9 +189,10 @@ public TagContext extract( TagContext extracted = extractor.extract(carrier, getter); // Check if context is valid if (extracted instanceof ExtractedContext) { + ExtractedContext extractedContext = (ExtractedContext) extracted; // If no prior valid context, store it as first valid context if (context == null) { - context = (ExtractedContext) extracted; + context = extractedContext; // Stop extraction if only extracting first valid context and drop everything else if (this.extractFirst) { break; @@ -204,7 +205,9 @@ public TagContext extract( boolean comingFromTraceContext = extracted.getPropagationStyle() == TRACECONTEXT; if (comingFromTraceContext) { // Propagate newly extracted W3C tracestate to first valid context - // TODO + String extractedTracestate = + extractedContext.getPropagationTags().getW3CTracestate(); + context.getPropagationTags().updateW3CTracestate(extractedTracestate); } } else { // Terminate extracted context and add it as span link diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java index 79bbd3eb342..00ed3d0f53d 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java @@ -66,6 +66,22 @@ public interface Factory { public abstract void updateTraceIdHighOrderBits(long highOrderBits); + /** + * Gets the original W3C + * tracestate header value. + * + * @return The original W3C tracestate header value. + */ + public abstract String getW3CTracestate(); + + /** + * Stores the original W3C + * tracestate header value. + * + * @param tracestate The original W3C tracestate header value. + */ + public abstract void updateW3CTracestate(String tracestate); + /** * Constructs a header value that includes valid propagated _dd.p.* tags and possibly a new * sampling decision tag _dd.p.dm based on the current state. Returns null if the value length diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java index 22c7f58cb7b..21ab986a16b 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsFactory.java @@ -88,7 +88,15 @@ static class PTags extends PropagationTags { * of the trace id, wrapped into a {@link TagValue}, null if not set. */ private volatile TagValue traceIdHighOrderBitsHexTagValue; - + /** + * The original W3C tracestate + * header value. + */ + protected volatile String tracestate; + /** + * The {@link PTagsFactory#PROPAGATION_ERROR_TAG_KEY propagation tag error} value, {@code null + * if no error while parsing header}. + */ protected volatile String error; public PTags( @@ -292,6 +300,16 @@ TagValue getDecisionMakerTagValue() { return decisionMakerTagValue; } + @Override + public String getW3CTracestate() { + return this.tracestate; + } + + @Override + public void updateW3CTracestate(String tracestate) { + this.tracestate = tracestate; + } + String getError() { return this.error; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index c3e61bea25b..a6da560dcba 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -16,12 +16,14 @@ public class W3CPTagsCodec extends PTagsCodec { private static final Logger log = LoggerFactory.getLogger(W3CPTagsCodec.class); private static final int MAX_HEADER_SIZE = 256; - private static final int EMPTY_SIZE = 3; // 'dd=' + private static final String DATADOG_MEMBER_KEY = "dd="; + private static final int EMPTY_SIZE = DATADOG_MEMBER_KEY.length(); // 3 private static final char MEMBER_SEPARATOR = ','; private static final char ELEMENT_SEPARATOR = ';'; private static final char KEY_VALUE_SEPARATOR = ':'; private static final int MIN_ALLOWED_CHAR = 32; private static final int MAX_ALLOWED_CHAR = 126; + private static final int MAX_LIST_MEMBER_COUNT = 32; @Override PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { @@ -44,12 +46,12 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { int memberIndex = 0; int ddMemberIndex = -1; while (memberStart < len) { - if (memberIndex == 32) { + if (memberIndex == MAX_LIST_MEMBER_COUNT) { // TODO should we return one with an error? // TODO should we try to pick up the `dd` member anyway? return tagsFactory.empty(); } - if (ddMemberIndex == -1 && value.startsWith("dd=", memberStart)) { + if (ddMemberIndex == -1 && value.startsWith(DATADOG_MEMBER_KEY, memberStart)) { ddMemberStart = memberStart; ddMemberIndex = memberIndex; } @@ -196,15 +198,19 @@ protected int estimateHeaderSize(PTags pTags) { W3CPTags w3CPTags = (W3CPTags) pTags; size += w3CPTags.maxUnknownSize; if (w3CPTags.ddMemberStart != -1) { - size += (w3CPTags.original.length() - (w3CPTags.ddMemberValueEnd - w3CPTags.ddMemberStart)); + size += + (w3CPTags.tracestate.length() - (w3CPTags.ddMemberValueEnd - w3CPTags.ddMemberStart)); } + } else if (pTags.tracestate != null) { + // We assume there is no Datadog list-member + size += pTags.tracestate.length(); } return size; } @Override protected int appendPrefix(StringBuilder sb, PTags ptags) { - sb.append("dd="); + sb.append(DATADOG_MEMBER_KEY); // Append sampling priority (s) if (ptags.getSamplingPriority() != PrioritySampling.UNSET) { sb.append("s:"); @@ -233,24 +239,25 @@ protected int appendTag(StringBuilder sb, TagElement key, TagElement value, int @Override protected int appendSuffix(StringBuilder sb, PTags ptags, int size) { - if (size >= MAX_HEADER_SIZE || !(ptags instanceof W3CPTags)) { - return size; + // If there is room for appending unknown from W3CPTags + if (size < MAX_HEADER_SIZE && ptags instanceof W3CPTags) { + W3CPTags w3cPTags = (W3CPTags) ptags; + size = cleanUpAndAppendUnknown(sb, w3cPTags, size); } - W3CPTags w3cPTags = (W3CPTags) ptags; - size = cleanUpAndAppendUnknown(sb, w3cPTags, size); + // Check empty Datadog list-member only tracestate if (size == EMPTY_SIZE) { // If we haven't written anything but the 'dd=', then reset the StringBuilder sb.setLength(0); size = 0; } + // Append all other non-Datadog list-members // TODO we need to ensure that there are only 32 segments including our own :( - int newSize = cleanUpAndAppendSuffix(sb, w3cPTags, size); + int newSize = cleanUpAndAppendSuffix(sb, ptags, size); if (newSize != size) { // We don't care about the total size in bytes here, but only the fact that we added something // that should be returned size = Math.max(size, EMPTY_SIZE + 1); } - return size; } @@ -586,7 +593,7 @@ private static int cleanUpAndAppendUnknown(StringBuilder sb, W3CPTags w3CPTags, || w3CPTags.ddMemberStart >= w3CPTags.ddMemberValueEnd) { return size; } - String original = w3CPTags.original; + String original = w3CPTags.tracestate; int elementStart = w3CPTags.ddMemberStart + EMPTY_SIZE; // skip over 'dd=' int okSize = size; while (elementStart < w3CPTags.ddMemberValueEnd && size < MAX_HEADER_SIZE) { @@ -621,8 +628,13 @@ private static int cleanUpAndAppendUnknown(StringBuilder sb, W3CPTags w3CPTags, return size; } - private static int cleanUpAndAppendSuffix(StringBuilder sb, W3CPTags w3CPTags, int size) { - String original = w3CPTags.original; + private static int cleanUpAndAppendSuffix(StringBuilder sb, PTags ptags, int size) { + String original = ptags.tracestate; + if (original == null) { + return size; + } + int ddMemberStart = (ptags instanceof W3CPTags) ? ((W3CPTags) ptags).ddMemberStart : -1; + int remainingListMemberAllowed = size == 0 ? MAX_LIST_MEMBER_COUNT : MAX_LIST_MEMBER_COUNT - 1; int len = original.length(); int memberStart = findNextMember(original, 0); while (memberStart < len) { @@ -630,7 +642,12 @@ private static int cleanUpAndAppendSuffix(StringBuilder sb, W3CPTags w3CPTags, i if (memberEnd < 0) { memberEnd = len; } - if (memberStart != w3CPTags.ddMemberStart) { + if (ddMemberStart == -1) { + if (original.startsWith(DATADOG_MEMBER_KEY, memberStart)) { + ddMemberStart = memberStart; + } + } + if (memberStart != ddMemberStart) { if (sb.length() > 0) { sb.append(MEMBER_SEPARATOR); size++; @@ -639,7 +656,12 @@ private static int cleanUpAndAppendSuffix(StringBuilder sb, W3CPTags w3CPTags, i sb.append(original, memberStart, end); size += (end - memberStart); } - memberStart = findNextMember(original, memberEnd + 1); + remainingListMemberAllowed--; + if (remainingListMemberAllowed == 0) { + memberStart = len; + } else { + memberStart = findNextMember(original, memberEnd + 1); + } } return size; } @@ -669,10 +691,19 @@ private static W3CPTags empty( } private static class W3CPTags extends PTags { - private final String original; + /** The index of the first tracestate list-member position in {@link #tracestate}. */ private final int firstMemberStart; + /** + * The index of the Datadog tracestate list-member (dd=) position in {@link #tracestate}, {@code + * -1 if Datadog list-member not found}. + */ private final int ddMemberStart; + /** + * The index of the end Datadog tracestate list-member (dd=) in {@link #tracestate}, {@code -1 + * if Datadog list-member not found}. + */ private final int ddMemberValueEnd; + private final int maxUnknownSize; public W3CPTags( @@ -688,7 +719,7 @@ public W3CPTags( int ddMemberValueEnd, int maxUnknownSize) { super(factory, tagPairs, decisionMakerTagValue, traceIdTagValue, samplingPriority, origin); - this.original = original; + this.tracestate = original; this.firstMemberStart = firstMemberStart; this.ddMemberStart = ddMemberStart; this.ddMemberValueEnd = ddMemberValueEnd; diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy index 1d03696c37c..a3f994cf41e 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy @@ -168,6 +168,34 @@ class W3CPropagationTagsTest extends DDCoreSpecification { memberCount << (1..37) // some arbitrary number larger than 32 } + def "validate tracestate header number of members #memberCount when propagating original tracestate"() { + setup: + def config = Mock(Config) + config.getxDatadogTagsMaxLength() >> 512 + def propagationTagsFactory = PropagationTags.factory(config) + def header = (1..memberCount).collect { "k$it=v$it" }.join(',') + def expectedHeader = 'dd=t.dm:-4,' + ( + memberCount > 32 ? + '' : + (1..Math.min(memberCount, 31)).collect { "k$it=v$it" }.join(',')) + + when: + def datadogHeaderPT = propagationTagsFactory.fromHeaderValue(HeaderType.DATADOG, '_dd.p.dm=-4') + def headerPT = propagationTagsFactory.fromHeaderValue(HeaderType.W3C, header) + datadogHeaderPT.updateW3CTracestate(headerPT.getW3CTracestate()) + + then: + if (memberCount <= 32) { + assert datadogHeaderPT.headerValue(HeaderType.W3C) == expectedHeader // 'dd=t.dm:-4,' + header + } else { + assert datadogHeaderPT.headerValue(HeaderType.W3C) == 'dd=t.dm:-4' + } + datadogHeaderPT.createTagMap() == ['_dd.p.dm':'-4'] + + where: + memberCount << (1..37) // some arbitrary number larger than 32 + } + def "create propagation tags from header value #headerValue"() { setup: def config = Mock(Config) From ca900328d74a2f3aaa41b4fb3a95e857d80529af Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 6 Nov 2023 16:08:15 +0100 Subject: [PATCH 08/12] fix(core): Fix tracestate encoding when Datadog and other members are present using W3CPtags --- .../core/propagation/ptags/W3CPTagsCodec.java | 14 +++++++---- .../propagation/W3CPropagationTagsTest.groovy | 25 ++++++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index a6da560dcba..39b9a9ee5e3 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -23,7 +23,7 @@ public class W3CPTagsCodec extends PTagsCodec { private static final char KEY_VALUE_SEPARATOR = ':'; private static final int MIN_ALLOWED_CHAR = 32; private static final int MAX_ALLOWED_CHAR = 126; - private static final int MAX_LIST_MEMBER_COUNT = 32; + private static final int MAX_MEMBER_COUNT = 32; @Override PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { @@ -46,7 +46,7 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) { int memberIndex = 0; int ddMemberIndex = -1; while (memberStart < len) { - if (memberIndex == MAX_LIST_MEMBER_COUNT) { + if (memberIndex == MAX_MEMBER_COUNT) { // TODO should we return one with an error? // TODO should we try to pick up the `dd` member anyway? return tagsFactory.empty(); @@ -634,19 +634,22 @@ private static int cleanUpAndAppendSuffix(StringBuilder sb, PTags ptags, int siz return size; } int ddMemberStart = (ptags instanceof W3CPTags) ? ((W3CPTags) ptags).ddMemberStart : -1; - int remainingListMemberAllowed = size == 0 ? MAX_LIST_MEMBER_COUNT : MAX_LIST_MEMBER_COUNT - 1; + int remainingMemberAllowed = size == 0 ? MAX_MEMBER_COUNT : MAX_MEMBER_COUNT - 1; int len = original.length(); int memberStart = findNextMember(original, 0); while (memberStart < len) { + // Look for member end position int memberEnd = original.indexOf(MEMBER_SEPARATOR, memberStart); if (memberEnd < 0) { memberEnd = len; } + // Try to define Datadog member start if not already found if (ddMemberStart == -1) { if (original.startsWith(DATADOG_MEMBER_KEY, memberStart)) { ddMemberStart = memberStart; } } + // Skip Datadog member (already added with prefix and tags) if (memberStart != ddMemberStart) { if (sb.length() > 0) { sb.append(MEMBER_SEPARATOR); @@ -655,9 +658,10 @@ private static int cleanUpAndAppendSuffix(StringBuilder sb, PTags ptags, int siz int end = stripTrailingOWC(original, memberStart, memberEnd); sb.append(original, memberStart, end); size += (end - memberStart); + remainingMemberAllowed--; } - remainingListMemberAllowed--; - if (remainingListMemberAllowed == 0) { + // Check if remaining members are allowed + if (remainingMemberAllowed == 0) { memberStart = len; } else { memberStart = findNextMember(original, memberEnd + 1); diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy index a3f994cf41e..4d725fb91c4 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/propagation/W3CPropagationTagsTest.groovy @@ -145,7 +145,7 @@ class W3CPropagationTagsTest extends DDCoreSpecification { valueChar << (' '..'ΓΏ') - ((' '..'~') - [',', '=']) } - def "validate tracestate header number of members #memberCount"() { + def "validate tracestate header number of members #memberCount without Datadog member"() { setup: def config = Mock(Config) config.getxDatadogTagsMaxLength() >> 512 @@ -168,6 +168,29 @@ class W3CPropagationTagsTest extends DDCoreSpecification { memberCount << (1..37) // some arbitrary number larger than 32 } + def "validate tracestate header number of members #memberCount with Datadog member"() { + setup: + def config = Mock(Config) + config.getxDatadogTagsMaxLength() >> 512 + def propagationTagsFactory = PropagationTags.factory(config) + def header = 'dd=s:1,'+(1..memberCount).collect { "k$it=v$it" }.join(',') + + when: + def headerPT = propagationTagsFactory.fromHeaderValue(HeaderType.W3C, header) + + then: + if (memberCount + 1 <= 32) { + assert headerPT.headerValue(HeaderType.W3C) == header + } else { + assert headerPT.headerValue(HeaderType.W3C) == null + } + // we're not using any dd members in the tests + headerPT.createTagMap() == [:] + + where: + memberCount << (1..37) // some arbitrary number larger than 32 + } + def "validate tracestate header number of members #memberCount when propagating original tracestate"() { setup: def config = Mock(Config) From f0fa17913cbeeba850f2400d2943ee6bcf74d7cd Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 6 Nov 2023 16:08:57 +0100 Subject: [PATCH 09/12] fix(core): Fix terminated context addition in span builder --- .../java/datadog/trace/core/CoreTracer.java | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 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 dae7c75cfce..684b3794ba0 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 @@ -1191,13 +1191,7 @@ public CoreSpanBuilder ignoreActiveSpan() { } private DDSpan buildSpan() { - if (parent instanceof TagContext) { - List terminatedContextLinks = - ((TagContext) parent).getTerminatedContextLinks(); - if (!terminatedContextLinks.isEmpty()) { - links.addAll(terminatedContextLinks); - } - } + addTerminatedContextAsLinks(); DDSpan span = DDSpan.create(instrumentationName, timestampMicro, buildSpanContext(), links); if (span.isLocalRootSpan()) { EndpointTracker tracker = tracer.onRootSpanStarted(span); @@ -1206,6 +1200,19 @@ private DDSpan buildSpan() { return span; } + private void addTerminatedContextAsLinks() { + if (this.parent instanceof TagContext) { + List terminatedContextLinks = + ((TagContext) this.parent).getTerminatedContextLinks(); + if (!terminatedContextLinks.isEmpty()) { + if (this.links == null) { + this.links = new ArrayList<>(); + } + this.links.addAll(terminatedContextLinks); + } + } + } + @Override public AgentSpan start() { return buildSpan(); From 5a3e9b7a744587e6d236c064842fe5c13848b27c Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Tue, 7 Nov 2023 17:43:11 +0100 Subject: [PATCH 10/12] fix(core): Fix trace id comparison --- .../trace/core/propagation/HttpCodec.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) 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 925d5134622..32e482de0cd 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 @@ -3,6 +3,9 @@ import static datadog.trace.api.TracePropagationStyle.TRACECONTEXT; import datadog.trace.api.Config; +import datadog.trace.api.DD128bTraceId; +import datadog.trace.api.DD64bTraceId; +import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; import datadog.trace.api.TracePropagationStyle; import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; @@ -200,8 +203,7 @@ public TagContext extract( } // If another valid context is extracted else { - boolean traceIdMatches = context.getTraceId().equals(extracted.getTraceId()); - if (traceIdMatches) { + if (traceIdMatch(context.getTraceId(), extractedContext.getTraceId())) { boolean comingFromTraceContext = extracted.getPropagationStyle() == TRACECONTEXT; if (comingFromTraceContext) { // Propagate newly extracted W3C tracestate to first valid context @@ -235,6 +237,23 @@ else if (extracted != null && partialContext == null) { } } + /** + * Checks if trace identifier matches, even if they are not encoded using the same size (64-bit vs + * 128-bit). + * + * @param a A trace identifier to check. + * @param b Another trace identifier to check. + * @return {@code true} if the trace identifiers matches, {@code false} otherwise. + */ + private static boolean traceIdMatch(DDTraceId a, DDTraceId b) { + if (a instanceof DD128bTraceId && b instanceof DD128bTraceId + || a instanceof DD64bTraceId && b instanceof DD64bTraceId) { + return a.equals(b); + } else { + return a.toLong() == b.toLong(); + } + } + /** URL encode value */ static String encode(final String value) { String encoded = value; From 2692631f700a8309101c54cff05afab01b375f57 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Wed, 8 Nov 2023 08:48:18 +0100 Subject: [PATCH 11/12] chore: Clean up comments --- .../main/java/datadog/trace/core/propagation/W3CHttpCodec.java | 2 +- .../datadog/trace/core/propagation/ptags/W3CPTagsCodec.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java index 82ba1a9d7fc..81d576ad977 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java @@ -263,7 +263,7 @@ private boolean storeTraceState(String value) { */ @Override protected TagContext build() { - // If there is neither a traceparent or a tracestate header, then ignore this + // If there is neither a traceparent nor a tracestate header, then ignore this if (traceparentHeader == null && tracestateHeader == null) { onlyTagContext(); } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java index 39b9a9ee5e3..c88f1e6a95f 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java @@ -251,7 +251,6 @@ protected int appendSuffix(StringBuilder sb, PTags ptags, int size) { size = 0; } // Append all other non-Datadog list-members - // TODO we need to ensure that there are only 32 segments including our own :( int newSize = cleanUpAndAppendSuffix(sb, ptags, size); if (newSize != size) { // We don't care about the total size in bytes here, but only the fact that we added something From c1f32c9e2d1b5061658271d60b22f12798b5d587 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Wed, 8 Nov 2023 09:16:31 +0100 Subject: [PATCH 12/12] feat(core): Ensure extracted context always have at least an empty PTags instance It will prevent the only case where a DDSpanContext can have a null PTags. This will allow to store tracestate if found from another valid extracted context. --- .../core/propagation/ContextInterpreter.java | 32 +++++++------- .../core/propagation/DatadogHttpCodec.java | 9 ++-- .../trace/core/propagation/W3CHttpCodec.java | 43 +++++++++---------- 3 files changed, 40 insertions(+), 44 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java index f7e385ee502..7e2368bbc76 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java @@ -48,6 +48,7 @@ public abstract class ContextInterpreter implements AgentPropagation.KeyClassifi protected long endToEndStartTime; protected boolean valid; protected boolean fullContext; + protected final PropagationTags.Factory propagationTagsFactory; protected PropagationTags propagationTags; private TagContext.HttpHeaders httpHeaders; @@ -67,6 +68,7 @@ protected ContextInterpreter(Config config) { this.customIpHeaderName = config.getTraceClientIpHeader(); this.clientIpResolutionEnabled = config.isTraceClientIpResolverEnabled(); this.clientIpWithoutAppSec = config.isClientIpEnabled(); + this.propagationTagsFactory = PropagationTags.factory(config); } /** @@ -233,21 +235,21 @@ public ContextInterpreter reset(TraceConfig traceConfig) { protected TagContext build() { if (valid) { if (fullContext && !DDTraceId.ZERO.equals(traceId)) { - final ExtractedContext context; - context = - new ExtractedContext( - traceId, - spanId, - samplingPriorityOrDefault(traceId, samplingPriority), - origin, - endToEndStartTime, - baggage, - tags, - httpHeaders, - propagationTags, - traceConfig, - style()); - return context; + if (propagationTags == null) { + propagationTags = propagationTagsFactory.empty(); + } + return new ExtractedContext( + traceId, + spanId, + samplingPriorityOrDefault(traceId, samplingPriority), + origin, + endToEndStartTime, + baggage, + tags, + httpHeaders, + propagationTags, + traceConfig, + style()); } else if (origin != null || !tags.isEmpty() || httpHeaders != null diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java index 648773ef0d4..9624a54a50a 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java @@ -17,6 +17,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.bootstrap.instrumentation.api.TagContext; import datadog.trace.core.DDSpanContext; +import datadog.trace.core.propagation.PropagationTags.HeaderType; import java.util.Map; import java.util.TreeMap; import java.util.function.Supplier; @@ -77,8 +78,7 @@ public void inject( } // inject x-datadog-tags - String datadogTags = - context.getPropagationTags().headerValue(PropagationTags.HeaderType.DATADOG); + String datadogTags = context.getPropagationTags().headerValue(HeaderType.DATADOG); if (datadogTags != null) { setter.set(carrier, DATADOG_TAGS_KEY, datadogTags); } @@ -103,12 +103,10 @@ private static class DatadogContextInterpreter extends ContextInterpreter { private static final int IGNORE = -1; private final boolean isAwsPropagationEnabled; - private final PropagationTags.Factory datadogTagsFactory; private DatadogContextInterpreter(Config config) { super(config); isAwsPropagationEnabled = config.isAwsPropagationEnabled(); - datadogTagsFactory = PropagationTags.factory(config); } @Override @@ -187,8 +185,7 @@ public boolean accept(String key, String value) { endToEndStartTime = extractEndToEndStartTime(firstHeaderValue(value)); break; case DD_TAGS: - propagationTags = - datadogTagsFactory.fromHeaderValue(PropagationTags.HeaderType.DATADOG, value); + propagationTags = propagationTagsFactory.fromHeaderValue(HeaderType.DATADOG, value); break; case OT_BAGGAGE: { diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java index 81d576ad977..76ca949e228 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java @@ -4,6 +4,7 @@ import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_DROP; import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_KEEP; import static datadog.trace.core.propagation.HttpCodec.firstHeaderValue; +import static datadog.trace.core.propagation.PropagationTags.HeaderType.W3C; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.NANOSECONDS; @@ -82,7 +83,7 @@ private void injectTraceParent( private void injectTraceState( DDSpanContext context, C carrier, AgentPropagation.Setter setter) { PropagationTags propagationTags = context.getPropagationTags(); - String tracestate = propagationTags.headerValue(PropagationTags.HeaderType.W3C); + String tracestate = propagationTags.headerValue(W3C); if (tracestate != null && !tracestate.isEmpty()) { setter.set(carrier, TRACE_STATE_KEY, tracestate); } @@ -116,15 +117,12 @@ private static class W3CContextInterpreter extends ContextInterpreter { private static final int E2E_START = 3; private static final int IGNORE = -1; - private final PropagationTags.Factory datadogTagsFactory; - // We need to delay handling of the tracestate header until after traceparent private String tracestateHeader = null; private String traceparentHeader = null; private W3CContextInterpreter(Config config) { super(config); - datadogTagsFactory = PropagationTags.factory(config); } @Override @@ -274,8 +272,8 @@ protected TagContext build() { "Found no traceparent header but tracestate header '" + tracestateHeader + "'"); } // Now we know that we have at least a traceparent header - parseTraceParentHeader(traceparentHeader, this); - parseTraceStateHeader(tracestateHeader, this, datadogTagsFactory); + parseTraceParentHeader(traceparentHeader); + parseTraceStateHeader(tracestateHeader); } catch (RuntimeException e) { onlyTagContext(); log.debug("Exception when building context", e); @@ -284,7 +282,7 @@ protected TagContext build() { return super.build(); } - static void parseTraceParentHeader(String tp, ContextInterpreter context) { + void parseTraceParentHeader(String tp) { int length = tp == null ? 0 : tp.length(); if (length < TRACE_PARENT_LENGTH) { throw new IllegalStateException("The length of traceparent '" + tp + "' is too short"); @@ -301,9 +299,9 @@ static void parseTraceParentHeader(String tp, ContextInterpreter context) { "Illegal all zero 64 bit trace id " + tp.substring(TRACE_PARENT_TID_START, TRACE_PARENT_TID_END)); } - context.traceId = traceId; - context.spanId = DDSpanId.fromHex(tp, TRACE_PARENT_SID_START, 16, true); - if (context.spanId == 0) { + this.traceId = traceId; + this.spanId = DDSpanId.fromHex(tp, TRACE_PARENT_SID_START, 16, true); + if (this.spanId == 0) { throw new IllegalStateException( "Illegal all zero span id " + tp.substring(TRACE_PARENT_SID_START, TRACE_PARENT_SID_END)); @@ -313,35 +311,34 @@ static void parseTraceParentHeader(String tp, ContextInterpreter context) { } long flags = LongStringUtils.parseUnsignedLongHex(tp, TRACE_PARENT_FLAGS_START, 2, true); if ((flags & TRACE_PARENT_FLAGS_SAMPLED) != 0) { - context.samplingPriority = SAMPLER_KEEP; + this.samplingPriority = SAMPLER_KEEP; } else { - context.samplingPriority = SAMPLER_DROP; + this.samplingPriority = SAMPLER_DROP; } } - static void parseTraceStateHeader( - String ts, ContextInterpreter context, PropagationTags.Factory factory) { - if (ts == null || ts.isEmpty()) { - context.propagationTags = factory.empty(); + void parseTraceStateHeader(String tracestate) { + if (tracestate == null || tracestate.isEmpty()) { + this.propagationTags = this.propagationTagsFactory.empty(); } else { - context.propagationTags = factory.fromHeaderValue(PropagationTags.HeaderType.W3C, ts); + this.propagationTags = this.propagationTagsFactory.fromHeaderValue(W3C, tracestate); } - int ptagsPriority = context.propagationTags.getSamplingPriority(); - int contextPriority = context.samplingPriority; + int ptagsPriority = this.propagationTags.getSamplingPriority(); + int contextPriority = this.samplingPriority; if ((contextPriority == SAMPLER_DROP && ptagsPriority > 0) || (contextPriority == SAMPLER_KEEP && ptagsPriority <= 0) || ptagsPriority == PrioritySampling.UNSET) { // Override Datadog sampling priority with W3C one - context.propagationTags.updateTraceSamplingPriority( + this.propagationTags.updateTraceSamplingPriority( contextPriority, SamplingMechanism.EXTERNAL_OVERRIDE); } else { // Use more detailed Datadog sampling priority in context - context.samplingPriority = ptagsPriority; + this.samplingPriority = ptagsPriority; } // Use the origin - context.origin = context.propagationTags.getOrigin(); + this.origin = this.propagationTags.getOrigin(); // Ensure TraceId high-order bits match - context.propagationTags.updateTraceIdHighOrderBits(context.traceId.toHighOrderLong()); + this.propagationTags.updateTraceIdHighOrderBits(this.traceId.toHighOrderLong()); } private static String trim(String input) {