Skip to content

Commit be63fb9

Browse files
PerfectSlayerPerfectSlayer
authored
Simplify context propagation (#8719)
* feat(core): Simplify context propagation by avoiding adding terminated contexts as span links and clearing them if not needed. * chore(core): Clean up * fix(core): Fix last parent id injection into trace state * feat(core): Add last parent id in trace state test
1 parent 20ce33b commit be63fb9

File tree

9 files changed

+111
-111
lines changed

9 files changed

+111
-111
lines changed

‎dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cLastParentIdTest.groovy‎

Lines changed: 0 additions & 24 deletions
This file was deleted.

‎dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cPropagatorTest.groovy‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,5 @@ class W3cPropagatorTest extends AgentPropagatorTest {
2323
void assertInjectedHeaders(Map<String, String> headers, String traceId, String spanId, byte sampling) {
2424
def traceFlags = sampling == SAMPLER_KEEP ? '01' : '00'
2525
assert headers['traceparent'] == "00-$traceId-$spanId-$traceFlags"
26-
// 'p:' not expected in tracestate here because 'p' is not supplied in values()
27-
assert !headers['tracestate'].contains("p:")
2826
}
2927
}

‎dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/W3cPropagatorTracestateTest.groovy‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ package opentelemetry14.context.propagation
22

33
import datadog.trace.agent.test.AgentTestRunner
44
import io.opentelemetry.api.GlobalOpenTelemetry
5-
import io.opentelemetry.context.Context
65
import spock.lang.Subject
76

7+
import static io.opentelemetry.context.Context.current
8+
import static io.opentelemetry.context.Context.root
9+
810
class W3cPropagatorTracestateTest extends AgentTestRunner {
911
@Subject
1012
def tracer = GlobalOpenTelemetry.get().tracerProvider.get("tracecontext-propagator-tracestate")
@@ -33,18 +35,18 @@ class W3cPropagatorTracestateTest extends AgentTestRunner {
3335
}
3436

3537
when:
36-
def context = propagator.extract(Context.root(), headers, TextMap.INSTANCE)
38+
def context = propagator.extract(root(), headers, TextMap.INSTANCE)
3739

3840
then:
39-
context != Context.root()
41+
context != root()
4042

4143
when:
4244
def localSpan = tracer.spanBuilder("some-name")
4345
.setParent(context)
4446
.startSpan()
4547
def scope = localSpan.makeCurrent()
4648
Map<String, String> injectedHeaders = [:]
47-
propagator.inject(Context.current(), injectedHeaders, TextMap.INSTANCE)
49+
propagator.inject(current(), injectedHeaders, TextMap.INSTANCE)
4850
scope.close()
4951
localSpan.end()
5052

@@ -55,16 +57,20 @@ class W3cPropagatorTracestateTest extends AgentTestRunner {
5557
// Check tracestate contains extracted members plus the Datadog one in first position
5658
def injectedMembers = injectedTracestate.split(',')
5759
injectedMembers.length == Math.min(1 + members.length, 32)
58-
injectedMembers[0] == expect
60+
// Check datadog member (should be injected as first member)
61+
injectedMembers[0] == "dd=s:0;p:${localSpan.spanContext.spanId};t.tid:1111111111111111"
62+
// Check all other members
5963
for (int i = 0; i< Math.min(members.length, 31); i++) {
6064
assert injectedMembers[i+1] == members[i]
6165
}
6266

6367
where:
64-
tracestate |expect
65-
"foo=1,bar=2" |"dd=s:0;t.tid:1111111111111111"
66-
"dd=s:0,foo=1,bar=2" |"dd=s:0;t.tid:1111111111111111"
67-
"foo=1,dd=s:0,bar=2" |"dd=s:0;t.tid:1111111111111111"
68-
"dd=s:3" |"dd=s:0;t.tid:1111111111111111"
68+
// spotless:off
69+
tracestate << [
70+
"foo=1,bar=2",
71+
"dd=s:0,foo=1,bar=2",
72+
"foo=1,dd=s:0,bar=2",
73+
]
74+
// spotless:on
6975
}
7076
}

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

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static datadog.trace.api.DDTags.DJM_ENABLED;
55
import static datadog.trace.api.DDTags.DSM_ENABLED;
66
import static datadog.trace.api.DDTags.PROFILING_CONTEXT_ENGINE;
7+
import static datadog.trace.api.TracePropagationBehaviorExtract.RESTART;
78
import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.BAGGAGE_CONCERN;
89
import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.DSM_CONCERN;
910
import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.TRACING_CONCERN;
@@ -31,7 +32,6 @@
3132
import datadog.trace.api.IdGenerationStrategy;
3233
import datadog.trace.api.StatsDClient;
3334
import datadog.trace.api.TraceConfig;
34-
import datadog.trace.api.TracePropagationBehaviorExtract;
3535
import datadog.trace.api.config.GeneralConfig;
3636
import datadog.trace.api.datastreams.AgentDataStreamsMonitoring;
3737
import datadog.trace.api.datastreams.PathwayContext;
@@ -1326,7 +1326,6 @@ public CoreSpanBuilder ignoreActiveSpan() {
13261326
}
13271327

13281328
private DDSpan buildSpan() {
1329-
addTerminatedContextAsLinks();
13301329
DDSpan span = DDSpan.create(instrumentationName, timestampMicro, buildSpanContext(), links);
13311330
if (span.isLocalRootSpan()) {
13321331
EndpointTracker tracker = tracer.onRootSpanStarted(span);
@@ -1337,6 +1336,22 @@ private DDSpan buildSpan() {
13371336
return span;
13381337
}
13391338

1339+
private void addParentContextAsLinks(AgentSpanContext parentContext) {
1340+
SpanLink link;
1341+
if (parentContext instanceof ExtractedContext) {
1342+
String headers = ((ExtractedContext) parentContext).getPropagationStyle().toString();
1343+
SpanAttributes attributes =
1344+
SpanAttributes.builder()
1345+
.put("reason", "propagation_behavior_extract")
1346+
.put("context_headers", headers)
1347+
.build();
1348+
link = DDSpanLink.from((ExtractedContext) parentContext, attributes);
1349+
} else {
1350+
link = SpanLink.from(parentContext);
1351+
}
1352+
withLink(link);
1353+
}
1354+
13401355
private void addTerminatedContextAsLinks() {
13411356
if (this.parent instanceof TagContext) {
13421357
List<AgentSpanLink> terminatedContextLinks =
@@ -1504,6 +1519,7 @@ private DDSpanContext buildSpanContext() {
15041519
spanId = this.spanId;
15051520
}
15061521

1522+
// Find the parent context
15071523
AgentSpanContext parentContext = parent;
15081524
if (parentContext == null && !ignoreScope) {
15091525
// use the Scope as parent unless overridden or ignored.
@@ -1512,37 +1528,25 @@ private DDSpanContext buildSpanContext() {
15121528
parentContext = activeSpan.context();
15131529
}
15141530
}
1515-
1516-
String parentServiceName = null;
1517-
boolean isRemote = false;
1518-
1519-
TracePropagationBehaviorExtract behaviorExtract =
1520-
Config.get().getTracePropagationBehaviorExtract();
1531+
// Handle remote terminated context as span links
15211532
if (parentContext != null && parentContext.isRemote()) {
1522-
if (behaviorExtract == TracePropagationBehaviorExtract.IGNORE) {
1523-
// reset links that may have come terminated span links
1524-
links = new ArrayList<>();
1525-
parentContext = null;
1526-
} else if (behaviorExtract == TracePropagationBehaviorExtract.RESTART) {
1527-
links = new ArrayList<>();
1528-
SpanLink link =
1529-
(parentContext instanceof ExtractedContext)
1530-
? DDSpanLink.from(
1531-
(ExtractedContext) parentContext,
1532-
SpanAttributes.builder()
1533-
.put("reason", "propagation_behavior_extract")
1534-
.put(
1535-
"context_headers",
1536-
((ExtractedContext) parentContext).getPropagationStyle().toString())
1537-
.build())
1538-
: SpanLink.from(parentContext);
1539-
links.add(link);
1540-
parentContext = null;
1533+
switch (Config.get().getTracePropagationBehaviorExtract()) {
1534+
case RESTART:
1535+
addParentContextAsLinks(parentContext);
1536+
parentContext = null;
1537+
break;
1538+
case IGNORE:
1539+
parentContext = null;
1540+
break;
1541+
case CONTINUE:
1542+
default:
1543+
addTerminatedContextAsLinks();
15411544
}
15421545
}
15431546

1547+
String parentServiceName = null;
15441548
// Propagate internal trace.
1545-
// Note: if we are not in the context of distributed tracing and we are starting the first
1549+
// Note: if we are not in the context of distributed tracing, and we are starting the first
15461550
// root span, parentContext will be null at this point.
15471551
if (parentContext instanceof DDSpanContext) {
15481552
final DDSpanContext ddsc = (DDSpanContext) parentContext;
@@ -1574,7 +1578,6 @@ private DDSpanContext buildSpanContext() {
15741578

15751579
if (parentContext instanceof ExtractedContext) {
15761580
// Propagate external trace
1577-
isRemote = true;
15781581
final ExtractedContext extractedContext = (ExtractedContext) parentContext;
15791582
traceId = extractedContext.getTraceId();
15801583
parentSpanId = extractedContext.getSpanId();
@@ -1715,8 +1718,7 @@ private DDSpanContext buildSpanContext() {
17151718
disableSamplingMechanismValidation,
17161719
propagationTags,
17171720
profilingContextIntegration,
1718-
injectBaggageAsTags,
1719-
isRemote);
1721+
injectBaggageAsTags);
17201722

17211723
// By setting the tags on the context we apply decorators to any tags that have been set via
17221724
// the builder. This is the order that the tags were added previously, but maybe the `tags`
@@ -1725,9 +1727,7 @@ private DDSpanContext buildSpanContext() {
17251727
context.setAllTags(tags);
17261728
context.setAllTags(coreTags);
17271729
context.setAllTags(rootSpanTags);
1728-
if (contextualTags != null) {
1729-
context.setAllTags(contextualTags);
1730-
}
1730+
context.setAllTags(contextualTags);
17311731
return context;
17321732
}
17331733
}

‎dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java‎

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package datadog.trace.core;
22

3+
import static datadog.trace.api.DDTags.PARENT_ID;
34
import static datadog.trace.api.DDTags.SPAN_LINKS;
45
import static datadog.trace.api.cache.RadixTreeCache.HTTP_STATUSES;
56
import static datadog.trace.bootstrap.instrumentation.api.ErrorPriorities.UNSET;
@@ -145,7 +146,6 @@ public class DDSpanContext
145146
private volatile int encodedOperationName;
146147
private volatile int encodedResourceName;
147148
private volatile CharSequence lastParentId;
148-
private final boolean isRemote;
149149

150150
/**
151151
* Metastruct keys are associated to the current span, they will not propagate to the children
@@ -196,8 +196,7 @@ public DDSpanContext(
196196
disableSamplingMechanismValidation,
197197
propagationTags,
198198
ProfilingContextIntegration.NoOp.INSTANCE,
199-
true,
200-
false);
199+
true);
201200
}
202201

203202
public DDSpanContext(
@@ -243,8 +242,7 @@ public DDSpanContext(
243242
disableSamplingMechanismValidation,
244243
propagationTags,
245244
ProfilingContextIntegration.NoOp.INSTANCE,
246-
injectBaggageAsTags,
247-
false);
245+
injectBaggageAsTags);
248246
}
249247

250248
public DDSpanContext(
@@ -290,8 +288,7 @@ public DDSpanContext(
290288
disableSamplingMechanismValidation,
291289
propagationTags,
292290
profilingContextIntegration,
293-
true,
294-
false);
291+
true);
295292
}
296293

297294
public DDSpanContext(
@@ -316,8 +313,7 @@ public DDSpanContext(
316313
final boolean disableSamplingMechanismValidation,
317314
final PropagationTags propagationTags,
318315
final ProfilingContextIntegration profilingContextIntegration,
319-
final boolean injectBaggageAsTags,
320-
final boolean isRemote) {
316+
final boolean injectBaggageAsTags) {
321317

322318
assert traceCollector != null;
323319
this.traceCollector = traceCollector;
@@ -376,8 +372,7 @@ public DDSpanContext(
376372
if (samplingPriority != PrioritySampling.UNSET) {
377373
setSamplingPriority(samplingPriority, SamplingMechanism.UNKNOWN);
378374
}
379-
setLastParentId(this.propagationTags.getLastParentId());
380-
this.isRemote = isRemote;
375+
setTag(PARENT_ID, this.propagationTags.getLastParentId());
381376
}
382377

383378
@Override
@@ -1055,20 +1050,8 @@ public void setMetaStructCurrent(String field, Object value) {
10551050
setMetaStruct(field, value);
10561051
}
10571052

1058-
public CharSequence getLastParentId() {
1059-
return lastParentId;
1060-
}
1061-
1062-
public void setLastParentId(CharSequence lastParentId) {
1063-
if (lastParentId != null) {
1064-
synchronized (unsafeTags) {
1065-
unsafeSetTag("_dd.parent_id", lastParentId);
1066-
}
1067-
this.lastParentId = lastParentId;
1068-
}
1069-
}
1070-
1053+
@Override
10711054
public boolean isRemote() {
1072-
return isRemote;
1055+
return false;
10731056
}
10741057
}

‎dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,17 +81,7 @@ private <C> void injectTraceParent(DDSpanContext context, C carrier, CarrierSett
8181

8282
private <C> void injectTraceState(DDSpanContext context, C carrier, CarrierSetter<C> setter) {
8383
PropagationTags propagationTags = context.getPropagationTags();
84-
if (propagationTags.getLastParentId() == null) {
85-
if (context.isRemote()) {
86-
CharSequence lastParentId = context.getLastParentId();
87-
if (lastParentId != null) {
88-
propagationTags.updateLastParentId(lastParentId);
89-
}
90-
} else {
91-
propagationTags.updateLastParentId(DDSpanId.toHexStringPadded(context.getSpanId()));
92-
}
93-
}
94-
84+
propagationTags.updateLastParentId(DDSpanId.toHexStringPadded(context.getSpanId()));
9585
String tracestate = propagationTags.headerValue(W3C);
9686
if (tracestate != null && !tracestate.isEmpty()) {
9787
setter.set(carrier, TRACE_STATE_KEY, tracestate);

‎dd-trace-core/src/test/groovy/datadog/trace/core/CoreSpanBuilderTest.groovy‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,7 @@ class CoreSpanBuilderTest extends DDCoreSpecification {
374374
span.traceId != extractedContext.traceId
375375
span.parentId != extractedContext.spanId
376376
span.samplingPriority() == PrioritySampling.UNSET
377-
378-
379-
assert span.links.size() == 0
377+
span.links.empty
380378
}
381379

382380
def "TagContext should populate default span details"() {

0 commit comments

Comments
 (0)