Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2035,7 +2035,9 @@ protected static final DDSpanContext buildSpanContext(
requestContextDataIast = null;
ciVisibilityContextData = null;
}
propagationTags = tracer.propagationTagsFactory.empty();
// Local children share the parent's PropagationTags (trace-level state; reads route to the
// root) instead of allocating an unused empty() per span.
propagationTags = ddsc.getPropagationTags();
} else {
long endToEndStartTime;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1238,14 +1238,22 @@ void processTagsAndBaggage(

// Baggage
Map<String, String> baggageItemsWithPropagationTags;
// Trace-level propagation tags (_dd.p.*, including the _dd.p.dm decision-maker) belong only
// on the local root. Local children now share the root's PropagationTags instance, so gate
// the emit on span position; otherwise the trace-level tags would be duplicated onto every
// child span.
final boolean isLocalRoot = getRootSpanContextIfDifferent() == null;
if (injectBaggageAsTags) {
baggageItemsWithPropagationTags = new HashMap<>(baggageItems);
if (w3cBaggage != null) {
injectW3CBaggageTags(baggageItemsWithPropagationTags);
}
propagationTags.fillTagMap(baggageItemsWithPropagationTags);
if (isLocalRoot) {
propagationTags.fillTagMap(baggageItemsWithPropagationTags);
}
} else {
baggageItemsWithPropagationTags = propagationTags.createTagMap();
baggageItemsWithPropagationTags =
isLocalRoot ? propagationTags.createTagMap() : new HashMap<>();
}

consumer.accept(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,6 @@ public interface Factory {

public abstract CharSequence getLastParentId();

public abstract void updateLastParentId(CharSequence lastParentId);

/**
* Gets the original <a href="https://www.w3.org/TR/trace-context/#tracestate-header">W3C
* tracestate header</a> value.
Expand All @@ -104,6 +102,15 @@ public interface Factory {
*/
public abstract String headerValue(HeaderType headerType);

/**
* Like {@link #headerValue(HeaderType)} but uses {@code lastParentIdOverride} for the W3C {@code
* p:} (last-parent-id) instead of the stored {@link #getLastParentId() last-parent-id}. Used at
* inject so the injecting span's id is supplied as a parameter rather than mutated into these
* (possibly trace-level, shared) tags — keeping transient per-injection identity out of shared
* state. A {@code null} override falls back to {@link #headerValue(HeaderType)}.
*/
public abstract String headerValue(HeaderType headerType, CharSequence lastParentIdOverride);

/**
* Fills a provided tagMap with valid propagated _dd.p.* tags and possibly a new sampling decision
* tags _dd.p.dm (root span only) based on the current state, or sets only an error tag if the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,11 @@ private <C> void injectTraceParent(DDSpanContext context, C carrier, CarrierSett

private <C> void injectTraceState(DDSpanContext context, C carrier, CarrierSetter<C> setter) {
PropagationTags propagationTags = context.getPropagationTags();
propagationTags.updateLastParentId(DDSpanId.toHexStringPadded(context.getSpanId()));
String tracestate = propagationTags.headerValue(W3C);
// Supply the injecting span's id for the W3C `p:` as a parameter rather than mutating it into
// the (possibly trace-level, shared) tags — keeps transient per-injection identity out of
// shared state, so concurrent sibling injects can't race on it.
String tracestate =
propagationTags.headerValue(W3C, DDSpanId.toHexStringPadded(context.getSpanId()));
if (tracestate != null && !tracestate.isEmpty()) {
setter.set(carrier, TRACE_STATE_KEY, tracestate);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,18 @@ abstract class PTagsCodec {
protected static final TagKey UPSTREAM_SERVICES_DEPRECATED_TAG = TagKey.from("upstream_services");

static String headerValue(PTagsCodec codec, PTags ptags) {
return headerValue(codec, ptags, null);
}

static String headerValue(PTagsCodec codec, PTags ptags, CharSequence lastParentIdOverride) {
int estimate = codec.estimateHeaderSize(ptags);
if (estimate == 0) {
return "";
}

// No encoding validation here because we don't allow arbitrary tag change
StringBuilder sb = new StringBuilder(estimate);
int size = codec.appendPrefix(sb, ptags);
int size = codec.appendPrefix(sb, ptags, lastParentIdOverride);
if (!ptags.isPropagationTagsDisabled()) {
if (ptags.getDecisionMakerTagValue() != null) {
size = codec.appendTag(sb, DECISION_MAKER_TAG, ptags.getDecisionMakerTagValue(), size);
Expand Down Expand Up @@ -174,6 +178,14 @@ static int calcXDatadogTagsSize(int size, TagKey tagKey, TagValue tagValue) {

protected abstract int appendPrefix(StringBuilder sb, PTags ptags);

/**
* Encode the prefix, using {@code lastParentIdOverride} for the W3C {@code p:} when non-null
* (inject-time). Codecs without a last-parent-id (e.g. Datadog) ignore the override.
*/
protected int appendPrefix(StringBuilder sb, PTags ptags, CharSequence lastParentIdOverride) {
return appendPrefix(sb, ptags);
}

protected abstract int appendTag(StringBuilder sb, TagElement key, TagElement value, int size);

protected abstract int appendSuffix(StringBuilder sb, PTags ptags, int size);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,14 +420,6 @@ public CharSequence getLastParentId() {
return lastParentId;
}

@Override
public void updateLastParentId(CharSequence lastParentId) {
if (!Objects.equals(this.lastParentId, lastParentId)) {
clearCachedHeader(W3C);
this.lastParentId = TagValue.from(lastParentId);
}
}

@Override
@SuppressWarnings("StringEquality")
@SuppressFBWarnings("ES_COMPARING_STRINGS_WITH_EQ")
Expand All @@ -448,6 +440,18 @@ public String headerValue(HeaderType headerType) {
return header;
}

@Override
public String headerValue(HeaderType headerType, CharSequence lastParentIdOverride) {
if (lastParentIdOverride == null) {
return headerValue(headerType);
}
// Inject-time path: encode fresh with the override; do NOT cache — the W3C `p:` is
// per-injecting-span and these tags may be shared across sibling spans.
String header =
PTagsCodec.headerValue(factory.getDecoderEncoder(headerType), this, lastParentIdOverride);
return (header == null || header.isEmpty()) ? null : header;
}

@Override
public void fillTagMap(Map<String, String> tagMap) {
PTagsCodec.fillTagMap(this, tagMap);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,11 @@ protected int estimateHeaderSize(PTags pTags) {

@Override
protected int appendPrefix(StringBuilder sb, PTags ptags) {
return appendPrefix(sb, ptags, null);
}

@Override
protected int appendPrefix(StringBuilder sb, PTags ptags, CharSequence lastParentIdOverride) {
sb.append(DATADOG_MEMBER_KEY);
// Append sampling priority (s)
if (ptags.getSamplingPriority() != PrioritySampling.UNSET) {
Expand All @@ -246,7 +251,8 @@ protected int appendPrefix(StringBuilder sb, PTags ptags) {
}
}
// append last ParentId (p)
CharSequence lastParent = ptags.getLastParentId();
CharSequence lastParent =
lastParentIdOverride != null ? lastParentIdOverride : ptags.getLastParentId();
if (lastParent != null) {
if (sb.length() > EMPTY_SIZE) {
sb.append(';');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package datadog.trace.core;

import static datadog.trace.api.TracePropagationStyle.DATADOG;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import datadog.trace.api.DDTraceId;
import datadog.trace.api.sampling.PrioritySampling;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.core.propagation.ExtractedContext;
import datadog.trace.core.propagation.PropagationTags;
import java.util.Collections;
import java.util.Map;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

/**
* Regression guard for the "local children share the parent's {@link PropagationTags}" optimization
* in {@code CoreTracer.buildSpanContext}: a non-root (local child) span must still carry the
* inbound distributed {@code _dd.p.*} tags when it injects.
*
* <p>Inbound {@code _dd.p.*} live on the root's {@code PropagationTags} (inherited from the {@link
* ExtractedContext}), and reads route to the root via {@code DDSpanContext.getPropagationTags()} —
* so a local child injects the same tags whether it holds its own instance or shares the root's.
* That invariant is what lets the child skip allocating its own {@code empty()} and share the
* root's instead. If {@link #localChildCarriesInboundDdpTags()} regresses, non-root injection is
* dropping inbound {@code _dd.p.*} on every hop.
*
* <p>Sharing has a second, opposite hazard on the <em>serialization</em> side: trace-level {@code
* _dd.p.*} must be emitted into a span's metadata only on the local root. Since children share the
* root's non-empty instance, {@code DDSpanContext.processTagsAndBaggage} gates the emit on span
* position — otherwise {@code _dd.p.dm} (and friends) would be duplicated onto every child span in
* the chunk, which the {@code test_sampling_span_tags} parametric test forbids. {@link
* #localChildDoesNotEmitTraceLevelDdpTags()} guards that direction.
*/
class PropagationTagsChildSpanTest extends DDCoreJavaSpecification {

private static final String INBOUND_HEADER = "_dd.p.dm=934086a686-4,_dd.p.anytag=value";
private static final String INBOUND_TAG = "_dd.p.anytag=value";

private CoreTracer tracer;

@BeforeEach
void setup() {
tracer = tracerBuilder().build();
}

private static ExtractedContext extractedWithDdpTags() {
return new ExtractedContext(
DDTraceId.ONE,
2,
PrioritySampling.SAMPLER_KEEP,
null,
0,
Collections.<String, String>emptyMap(),
Collections.<String, Object>emptyMap(),
null,
PropagationTags.factory()
.fromHeaderValue(PropagationTags.HeaderType.DATADOG, INBOUND_HEADER),
null,
DATADOG);
}

/** What the Datadog codec would inject for {@code x-datadog-tags} from this span. */
private static String injectedDdpHeader(AgentSpan span) {
return ((DDSpanContext) span.spanContext())
.getPropagationTags()
.headerValue(PropagationTags.HeaderType.DATADOG);
}

/** Baseline: the root span (built directly from the extracted context) carries inbound tags. */
@Test
void rootSpanCarriesInboundDdpTags() {
AgentSpan root = tracer.buildSpan("test", "root").asChildOf(extractedWithDdpTags()).start();
try {
String header = injectedDdpHeader(root);
assertTrue(
header != null && header.contains(INBOUND_TAG),
"root injected _dd.p.* header should contain inbound tag; was: " + header);
} finally {
root.finish();
}
}

/**
* THE open question: a local child of the root. Inbound {@code _dd.p.*} must survive injection
* from the child, or downstream services lose the distributed tags on every non-root hop.
*/
@Test
void localChildCarriesInboundDdpTags() {
AgentSpan root = tracer.buildSpan("test", "root").asChildOf(extractedWithDdpTags()).start();
AgentSpan child = tracer.buildSpan("test", "child").asChildOf(root.spanContext()).start();
try {
String header = injectedDdpHeader(child);
assertTrue(
header != null && header.contains(INBOUND_TAG),
"local child injected _dd.p.* header should contain inbound tag; was: " + header);
} finally {
child.finish();
root.finish();
}
}

/** The trace-level {@code _dd.p.*} a span would emit into its serialized metadata (baggage). */
private static Map<String, String> emittedPropagationTags(AgentSpan span) {
CapturingConsumer consumer = new CapturingConsumer();
((DDSpan) span).processTagsAndBaggage(consumer);
return consumer.metadata.getBaggage();
}

/** Serialization baseline: the local root emits the trace-level decision-maker tag. */
@Test
void rootSpanEmitsDecisionMakerTag() {
AgentSpan root = tracer.buildSpan("test", "root").asChildOf(extractedWithDdpTags()).start();
try {
Map<String, String> emitted = emittedPropagationTags(root);
assertTrue(
emitted.containsKey("_dd.p.dm"),
"root serialized metadata should carry _dd.p.dm; was: " + emitted);
} finally {
root.finish();
}
}

/**
* Regression guard for the {@code test_sampling_span_tags} parametric test: a non-root (local
* child) span must not emit trace-level {@code _dd.p.*} into its serialized metadata. Because the
* child shares the root's {@link PropagationTags}, an ungated emit duplicates {@code _dd.p.dm}
* onto every child span in the chunk.
*/
@Test
void localChildDoesNotEmitTraceLevelDdpTags() {
AgentSpan root = tracer.buildSpan("test", "root").asChildOf(extractedWithDdpTags()).start();
AgentSpan child = tracer.buildSpan("test", "child").asChildOf(root.spanContext()).start();
try {
Map<String, String> emitted = emittedPropagationTags(child);
assertFalse(
emitted.containsKey("_dd.p.dm"),
"local child serialized metadata must not carry trace-level _dd.p.dm; was: " + emitted);
assertFalse(
emitted.containsKey("_dd.p.anytag"),
"local child serialized metadata must not carry trace-level _dd.p.*; was: " + emitted);
} finally {
child.finish();
root.finish();
}
}

private static final class CapturingConsumer implements MetadataConsumer {
private Metadata metadata;

@Override
public void accept(Metadata metadata) {
this.metadata = metadata;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package datadog.trace.core.propagation;

import static datadog.trace.core.propagation.PropagationTags.HeaderType.W3C;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import org.junit.jupiter.api.Test;

/**
* The inject-time W3C last-parent-id ({@code p:}) is supplied as a parameter to {@link
* PropagationTags#headerValue(PropagationTags.HeaderType, CharSequence)} rather than mutated into
* the (possibly trace-level, shared) tags. This keeps transient per-injection identity out of
* shared state: a sibling span's inject can't pollute another span's header, and the stored inbound
* last-parent-id is never overwritten.
*/
class PropagationTagsLastParentIdTest {

private static final String SPAN_A = "00000000000000aa";
private static final String SPAN_B = "00000000000000bb";

private static PropagationTags w3c(String header) {
return PropagationTags.factory().fromHeaderValue(W3C, header);
}

@Test
void overrideSuppliesW3cLastParentId() {
PropagationTags tags = w3c("dd=s:1;o:rum");
assertTrue(tags.headerValue(W3C, SPAN_A).contains("p:" + SPAN_A));
}

@Test
void overrideDoesNotMutateSharedTags_noCrossTalk() {
// One tags instance, two sibling spans injecting through it (the shared-root scenario).
PropagationTags shared = w3c("dd=s:1;o:rum"); // no inbound p:

String headerA = shared.headerValue(W3C, SPAN_A);
String headerB = shared.headerValue(W3C, SPAN_B);
String headerAagain = shared.headerValue(W3C, SPAN_A);

assertTrue(headerA.contains("p:" + SPAN_A));
assertTrue(headerB.contains("p:" + SPAN_B));
// Injecting B did not change what A injects — no shared mutation.
assertEquals(headerA, headerAagain, "a sibling inject must not change another span's header");
// The override is never written into the shared tags (no-override header has no p:).
assertFalse(shared.headerValue(W3C).contains("p:"), "override must not mutate the stored tags");
}

@Test
void inboundLastParentIdPreservedAndUnmutatedByOverride() {
PropagationTags tags = w3c("dd=s:1;p:" + SPAN_A); // arrived carrying a last-parent-id

// No-override path (e.g. span-link traceState) keeps the inbound p:.
assertTrue(tags.headerValue(W3C).contains("p:" + SPAN_A));
// An inject override replaces it for that produced header...
assertTrue(tags.headerValue(W3C, SPAN_B).contains("p:" + SPAN_B));
// ...without mutating the stored inbound value.
assertTrue(
tags.headerValue(W3C).contains("p:" + SPAN_A), "inbound p: must survive override use");
}
}
Loading