diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cLastParentIdTest.groovy b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cLastParentIdTest.groovy deleted file mode 100644 index ec8316aa062..00000000000 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cLastParentIdTest.groovy +++ /dev/null @@ -1,24 +0,0 @@ -package opentelemetry14.context.propagation - -import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_KEEP - -class W3cLastParentIdTest extends AgentPropagatorTest { - @Override - String style() { - return 'tracecontext' - } - - @Override - def values() { - // spotless:off - return [ - [['traceparent': '00-11111111111111111111111111111111-2222222222222222-01','tracestate': 'dd=s:2;p:1948e0b51aee0bfa'], '11111111111111111111111111111111', '2222222222222222', SAMPLER_KEEP] - ] - // spotless:on - } - - @Override - void assertInjectedHeaders(Map headers, String traceId, String spanId, byte sampling) { - assert headers['tracestate'].contains("p:1948e0b51aee0bfa") - } -} diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cPropagatorTest.groovy b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cPropagatorTest.groovy index 0623881a726..ebf31c3c84d 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cPropagatorTest.groovy +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cPropagatorTest.groovy @@ -23,7 +23,5 @@ class W3cPropagatorTest extends AgentPropagatorTest { void assertInjectedHeaders(Map headers, String traceId, String spanId, byte sampling) { def traceFlags = sampling == SAMPLER_KEEP ? '01' : '00' assert headers['traceparent'] == "00-$traceId-$spanId-$traceFlags" - // 'p:' not expected in tracestate here because 'p' is not supplied in values() - assert !headers['tracestate'].contains("p:") } } diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cPropagatorTracestateTest.groovy b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cPropagatorTracestateTest.groovy index a2af81adceb..83e92808c6f 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cPropagatorTracestateTest.groovy +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cPropagatorTracestateTest.groovy @@ -2,9 +2,11 @@ package opentelemetry14.context.propagation import datadog.trace.agent.test.AgentTestRunner import io.opentelemetry.api.GlobalOpenTelemetry -import io.opentelemetry.context.Context import spock.lang.Subject +import static io.opentelemetry.context.Context.current +import static io.opentelemetry.context.Context.root + class W3cPropagatorTracestateTest extends AgentTestRunner { @Subject def tracer = GlobalOpenTelemetry.get().tracerProvider.get("tracecontext-propagator-tracestate") @@ -33,10 +35,10 @@ class W3cPropagatorTracestateTest extends AgentTestRunner { } when: - def context = propagator.extract(Context.root(), headers, TextMap.INSTANCE) + def context = propagator.extract(root(), headers, TextMap.INSTANCE) then: - context != Context.root() + context != root() when: def localSpan = tracer.spanBuilder("some-name") @@ -44,7 +46,7 @@ class W3cPropagatorTracestateTest extends AgentTestRunner { .startSpan() def scope = localSpan.makeCurrent() Map injectedHeaders = [:] - propagator.inject(Context.current(), injectedHeaders, TextMap.INSTANCE) + propagator.inject(current(), injectedHeaders, TextMap.INSTANCE) scope.close() localSpan.end() @@ -55,16 +57,20 @@ class W3cPropagatorTracestateTest extends AgentTestRunner { // Check tracestate contains extracted members plus the Datadog one in first position def injectedMembers = injectedTracestate.split(',') injectedMembers.length == Math.min(1 + members.length, 32) - injectedMembers[0] == expect + // Check datadog member (should be injected as first member) + injectedMembers[0] == "dd=s:0;p:${localSpan.spanContext.spanId};t.tid:1111111111111111" + // Check all other members for (int i = 0; i< Math.min(members.length, 31); i++) { assert injectedMembers[i+1] == members[i] } where: - tracestate |expect - "foo=1,bar=2" |"dd=s:0;t.tid:1111111111111111" - "dd=s:0,foo=1,bar=2" |"dd=s:0;t.tid:1111111111111111" - "foo=1,dd=s:0,bar=2" |"dd=s:0;t.tid:1111111111111111" - "dd=s:3" |"dd=s:0;t.tid:1111111111111111" + // spotless:off + tracestate << [ + "foo=1,bar=2", + "dd=s:0,foo=1,bar=2", + "foo=1,dd=s:0,bar=2", + ] + // spotless:on } } 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 aab90613169..1e44edd3f89 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 @@ -4,6 +4,7 @@ import static datadog.trace.api.DDTags.DJM_ENABLED; import static datadog.trace.api.DDTags.DSM_ENABLED; import static datadog.trace.api.DDTags.PROFILING_CONTEXT_ENGINE; +import static datadog.trace.api.TracePropagationBehaviorExtract.RESTART; import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.BAGGAGE_CONCERN; import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.DSM_CONCERN; import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.TRACING_CONCERN; @@ -31,7 +32,6 @@ import datadog.trace.api.IdGenerationStrategy; import datadog.trace.api.StatsDClient; import datadog.trace.api.TraceConfig; -import datadog.trace.api.TracePropagationBehaviorExtract; import datadog.trace.api.config.GeneralConfig; import datadog.trace.api.datastreams.AgentDataStreamsMonitoring; import datadog.trace.api.datastreams.PathwayContext; @@ -1326,7 +1326,6 @@ public CoreSpanBuilder ignoreActiveSpan() { } private DDSpan buildSpan() { - addTerminatedContextAsLinks(); DDSpan span = DDSpan.create(instrumentationName, timestampMicro, buildSpanContext(), links); if (span.isLocalRootSpan()) { EndpointTracker tracker = tracer.onRootSpanStarted(span); @@ -1337,6 +1336,22 @@ private DDSpan buildSpan() { return span; } + private void addParentContextAsLinks(AgentSpanContext parentContext) { + SpanLink link; + if (parentContext instanceof ExtractedContext) { + String headers = ((ExtractedContext) parentContext).getPropagationStyle().toString(); + SpanAttributes attributes = + SpanAttributes.builder() + .put("reason", "propagation_behavior_extract") + .put("context_headers", headers) + .build(); + link = DDSpanLink.from((ExtractedContext) parentContext, attributes); + } else { + link = SpanLink.from(parentContext); + } + withLink(link); + } + private void addTerminatedContextAsLinks() { if (this.parent instanceof TagContext) { List terminatedContextLinks = @@ -1504,6 +1519,7 @@ private DDSpanContext buildSpanContext() { spanId = this.spanId; } + // Find the parent context AgentSpanContext parentContext = parent; if (parentContext == null && !ignoreScope) { // use the Scope as parent unless overridden or ignored. @@ -1512,37 +1528,25 @@ private DDSpanContext buildSpanContext() { parentContext = activeSpan.context(); } } - - String parentServiceName = null; - boolean isRemote = false; - - TracePropagationBehaviorExtract behaviorExtract = - Config.get().getTracePropagationBehaviorExtract(); + // Handle remote terminated context as span links if (parentContext != null && parentContext.isRemote()) { - if (behaviorExtract == TracePropagationBehaviorExtract.IGNORE) { - // reset links that may have come terminated span links - links = new ArrayList<>(); - parentContext = null; - } else if (behaviorExtract == TracePropagationBehaviorExtract.RESTART) { - links = new ArrayList<>(); - SpanLink link = - (parentContext instanceof ExtractedContext) - ? DDSpanLink.from( - (ExtractedContext) parentContext, - SpanAttributes.builder() - .put("reason", "propagation_behavior_extract") - .put( - "context_headers", - ((ExtractedContext) parentContext).getPropagationStyle().toString()) - .build()) - : SpanLink.from(parentContext); - links.add(link); - parentContext = null; + switch (Config.get().getTracePropagationBehaviorExtract()) { + case RESTART: + addParentContextAsLinks(parentContext); + parentContext = null; + break; + case IGNORE: + parentContext = null; + break; + case CONTINUE: + default: + addTerminatedContextAsLinks(); } } + String parentServiceName = null; // Propagate internal trace. - // Note: if we are not in the context of distributed tracing and we are starting the first + // Note: if we are not in the context of distributed tracing, and we are starting the first // root span, parentContext will be null at this point. if (parentContext instanceof DDSpanContext) { final DDSpanContext ddsc = (DDSpanContext) parentContext; @@ -1574,7 +1578,6 @@ private DDSpanContext buildSpanContext() { if (parentContext instanceof ExtractedContext) { // Propagate external trace - isRemote = true; final ExtractedContext extractedContext = (ExtractedContext) parentContext; traceId = extractedContext.getTraceId(); parentSpanId = extractedContext.getSpanId(); @@ -1715,8 +1718,7 @@ private DDSpanContext buildSpanContext() { disableSamplingMechanismValidation, propagationTags, profilingContextIntegration, - injectBaggageAsTags, - isRemote); + injectBaggageAsTags); // By setting the tags on the context we apply decorators to any tags that have been set via // the builder. This is the order that the tags were added previously, but maybe the `tags` @@ -1725,9 +1727,7 @@ private DDSpanContext buildSpanContext() { context.setAllTags(tags); context.setAllTags(coreTags); context.setAllTags(rootSpanTags); - if (contextualTags != null) { - context.setAllTags(contextualTags); - } + context.setAllTags(contextualTags); return context; } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 1277b16aa7b..b28247c6914 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -1,5 +1,6 @@ package datadog.trace.core; +import static datadog.trace.api.DDTags.PARENT_ID; import static datadog.trace.api.DDTags.SPAN_LINKS; import static datadog.trace.api.cache.RadixTreeCache.HTTP_STATUSES; import static datadog.trace.bootstrap.instrumentation.api.ErrorPriorities.UNSET; @@ -145,7 +146,6 @@ public class DDSpanContext private volatile int encodedOperationName; private volatile int encodedResourceName; private volatile CharSequence lastParentId; - private final boolean isRemote; /** * Metastruct keys are associated to the current span, they will not propagate to the children @@ -196,8 +196,7 @@ public DDSpanContext( disableSamplingMechanismValidation, propagationTags, ProfilingContextIntegration.NoOp.INSTANCE, - true, - false); + true); } public DDSpanContext( @@ -243,8 +242,7 @@ public DDSpanContext( disableSamplingMechanismValidation, propagationTags, ProfilingContextIntegration.NoOp.INSTANCE, - injectBaggageAsTags, - false); + injectBaggageAsTags); } public DDSpanContext( @@ -290,8 +288,7 @@ public DDSpanContext( disableSamplingMechanismValidation, propagationTags, profilingContextIntegration, - true, - false); + true); } public DDSpanContext( @@ -316,8 +313,7 @@ public DDSpanContext( final boolean disableSamplingMechanismValidation, final PropagationTags propagationTags, final ProfilingContextIntegration profilingContextIntegration, - final boolean injectBaggageAsTags, - final boolean isRemote) { + final boolean injectBaggageAsTags) { assert traceCollector != null; this.traceCollector = traceCollector; @@ -376,8 +372,7 @@ public DDSpanContext( if (samplingPriority != PrioritySampling.UNSET) { setSamplingPriority(samplingPriority, SamplingMechanism.UNKNOWN); } - setLastParentId(this.propagationTags.getLastParentId()); - this.isRemote = isRemote; + setTag(PARENT_ID, this.propagationTags.getLastParentId()); } @Override @@ -1055,20 +1050,8 @@ public void setMetaStructCurrent(String field, Object value) { setMetaStruct(field, value); } - public CharSequence getLastParentId() { - return lastParentId; - } - - public void setLastParentId(CharSequence lastParentId) { - if (lastParentId != null) { - synchronized (unsafeTags) { - unsafeSetTag("_dd.parent_id", lastParentId); - } - this.lastParentId = lastParentId; - } - } - + @Override public boolean isRemote() { - return isRemote; + return false; } } 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 98157a2b135..c201047fd01 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 @@ -81,17 +81,7 @@ private void injectTraceParent(DDSpanContext context, C carrier, CarrierSett private void injectTraceState(DDSpanContext context, C carrier, CarrierSetter setter) { PropagationTags propagationTags = context.getPropagationTags(); - if (propagationTags.getLastParentId() == null) { - if (context.isRemote()) { - CharSequence lastParentId = context.getLastParentId(); - if (lastParentId != null) { - propagationTags.updateLastParentId(lastParentId); - } - } else { - propagationTags.updateLastParentId(DDSpanId.toHexStringPadded(context.getSpanId())); - } - } - + propagationTags.updateLastParentId(DDSpanId.toHexStringPadded(context.getSpanId())); String tracestate = propagationTags.headerValue(W3C); if (tracestate != null && !tracestate.isEmpty()) { setter.set(carrier, TRACE_STATE_KEY, tracestate); 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 9d48317022f..59521b95ab1 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 @@ -374,9 +374,7 @@ class CoreSpanBuilderTest extends DDCoreSpecification { span.traceId != extractedContext.traceId span.parentId != extractedContext.spanId span.samplingPriority() == PrioritySampling.UNSET - - - assert span.links.size() == 0 + span.links.empty } def "TagContext should populate default span details"() { 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 726399972ca..06c41767168 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 @@ -166,7 +166,57 @@ class W3CHttpInjectorTest extends DDCoreSpecification { tracer.close() } + def "update last parent id on child span"() { + setup: + def writer = new ListWriter() + def tracer = tracerBuilder().writer(writer).build() + final Map carrier = [:] + + when: 'injecting root span context' + def rootSpan = tracer.startSpan('test', 'root') + def rootSpanId = rootSpan.spanId + def rootScope = tracer.activateSpan(rootSpan) + + injector.inject(rootSpan.context() as DDSpanContext, carrier, MapSetter.INSTANCE) + def lastParentId = extractLastParentId(carrier) + + then: 'trace state has root span id as last parent' + lastParentId == rootSpanId + + when: 'injecting child span context' + def childSpan = tracer.startSpan('test', 'child') + def childSpanId = childSpan.spanId + carrier.clear() + injector.inject(childSpan.context() as DDSpanContext, carrier, MapSetter.INSTANCE) + lastParentId = extractLastParentId(carrier) + + then: 'trace state has child span id as last parent' + lastParentId == childSpanId + + when: 'injecting root span again' + childSpan.finish() + carrier.clear() + injector.inject(rootSpan.context() as DDSpanContext, carrier, MapSetter.INSTANCE) + lastParentId = extractLastParentId(carrier) + + then: 'trace state has root span is as last parent again' + lastParentId == rootSpanId + + cleanup: + rootScope.close() + rootSpan.finish() + } + static String buildTraceParent(String traceId, String spanId, int samplingPriority) { return "00-${DDTraceId.from(traceId).toHexString()}-${DDSpanId.toHexStringPadded(DDSpanId.from(spanId))}-${samplingPriority > 0 ? '01': '00'}" } + + static long extractLastParentId(Map carrier) { + def traceState = carrier[TRACE_STATE_KEY] + def traceStateMembers = traceState.split(',') + def ddTraceStateMember = traceStateMembers.find { it.startsWith("dd=")}.substring(3) + def parts = ddTraceStateMember.split(';') + def spanIdHex = parts.find { it.startsWith('p:')}.substring(2) + DDSpanId.fromHex(spanIdHex) + } } diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/test/DDCoreSpecification.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/test/DDCoreSpecification.groovy index 812da453572..7060b7a5084 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/test/DDCoreSpecification.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/test/DDCoreSpecification.groovy @@ -81,8 +81,7 @@ abstract class DDCoreSpecification extends DDSpecification { false, propagationTags, ProfilingContextIntegration.NoOp.INSTANCE, - true, - false) + true) def span = DDSpan.create("test", timestamp, context, null) for (Map.Entry e : tags.entrySet()) {