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 @@ -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,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