Skip to content

Adding baggage limits for OT Baggage #12303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mhlidd wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Adding baggage limits for OT Baggage #12303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1b4215c
adding baggage limits for OT Baggage
mhlidd Aug 6, 2026
e8c5cb5
adding accurate utf length checking
mhlidd Aug 26, 2026
740ce66
updating test fix and early exit when ot-baggage extraction is disabled
mhlidd Aug 26, 2026
0048d3b
fixing haystack traceid spanid concatenation bug
mhlidd Aug 26, 2026
3424e8d
Merge branch 'master' into mhlidd/ot_baggage_fix
mhlidd Aug 26, 2026
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
83 changes: 79 additions & 4 deletions dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,17 @@
import java.util.Collections;
import java.util.Map;
import java.util.TreeMap;
import okio.Utf8;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* When adding new context fields to the ContextInterpreter class remember to clear them in the
* reset() method.
*/
public abstract class ContextInterpreter implements AgentPropagation.KeyClassifier {
private static final Logger LOG = LoggerFactory.getLogger(ContextInterpreter.class);

private TraceConfig traceConfig;

protected Map<String, String> headerTags;
Expand All @@ -47,6 +52,9 @@ public abstract class ContextInterpreter implements AgentPropagation.KeyClassifi
protected int samplingPriority;
protected TagMap.Ledger tagLedger;
protected Map<String, String> baggage;
private Map<String, Integer> baggageItemBytes;
private int baggageItems;
private int baggageBytes;

protected CharSequence lastParentId;
protected CharSequence origin;
Expand All @@ -63,6 +71,11 @@ public abstract class ContextInterpreter implements AgentPropagation.KeyClassifi
private final boolean aiGuardEnabled;
private boolean collectIpHeaders;
private final boolean requestHeaderTagsCommaAllowed;
private final int baggageMaxItems;
private final int baggageMaxBytes;

/** Longest value {@link #addReservedBaggageItem(String, String)} will store, in characters. */
private static final int MAX_RESERVED_BAGGAGE_LENGTH = 64;

protected static final boolean LOG_EXTRACT_HEADER_NAMES = Config.get().isLogExtractHeaderNames();
private static final DDCache<String, String> CACHE = DDCaches.newFixedSizeCache(64);
Expand All @@ -78,6 +91,8 @@ protected ContextInterpreter(Config config) {
this.aiGuardEnabled = config.isAiGuardEnabled();
this.propagationTagsFactory = PropagationTags.factory(config);
this.requestHeaderTagsCommaAllowed = config.isRequestHeaderTagsCommaAllowed();
this.baggageMaxItems = config.getTraceBaggageMaxItems();
this.baggageMaxBytes = config.getTraceBaggageMaxBytes();
}

final TagMap.Ledger tagLedger() {
Expand Down Expand Up @@ -216,15 +231,72 @@ protected final boolean handleMappedBaggage(String key, String value) {
final String lowerCaseKey = toLowerCase(key);
final String mappedKey = baggageMapping.get(lowerCaseKey);
if (null != mappedKey) {
if (baggage.isEmpty()) {
baggage = new TreeMap<>();
}
baggage.put(mappedKey, HttpCodec.decode(value));
addBaggageItem(mappedKey, value);
return true;
}
return false;
}

protected final boolean addBaggageItem(String key, String value) {
if (key == null || value == null || baggageMaxItems <= 0 || baggageMaxBytes <= 0) {

@PerfectSlayer PerfectSlayer Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ question: ‏Do we want to allow negative max item/bytes values as possible config value?
Or should we set the min as 0 during parsing? Having negative value as errors.

@mhlidd mhlidd Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting min to 0 is cleaner. This will affect W3C Baggage as well but positively. :)

return false;
}
final Integer previousItemBytes = baggageItemBytes.get(key);
final boolean newItem = previousItemBytes == null;
if (newItem && baggage.containsKey(key)) {
// A reserved propagation value already owns this key. Caller-controlled baggage must not
// make preservation of that value depend on header visitation order.
LOG.debug("Dropping baggage item {}: key is reserved for propagation", key);
return false;
}
if (newItem && baggageItems >= baggageMaxItems) {
LOG.debug("Dropping baggage item {}: item limit {} reached", key, baggageMaxItems);
return false;
Comment thread
mhlidd marked this conversation as resolved.
}
// Charge the raw key and value before URL decoding so percent-encoded input keeps its size.
final long itemBytes = Utf8.size(key) + Utf8.size(value);

@PerfectSlayer PerfectSlayer Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ question: ‏Can you use an approximate size here rather than depending on OkHttp lib?
We will need to remove it at some point (hopefully next quarter)

final long projectedBytes = (long) baggageBytes - (newItem ? 0 : previousItemBytes) + itemBytes;
if (projectedBytes > baggageMaxBytes) {
LOG.debug("Dropping baggage item {}: byte limit {} reached", key, baggageMaxBytes);
return false;
}
final String decodedValue = HttpCodec.decode(value);
if (baggage.isEmpty()) {
baggage = new TreeMap<>();
baggageItemBytes = new TreeMap<>();

@PerfectSlayer PerfectSlayer Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ question: ‏Why do we need ordering here? (TreeMap)

@mhlidd mhlidd Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TreeMap was used in the old diff when OT Baggage was introduced, so I kept it here. Unsure of reasons it was used then (commit message says "use TreeMap everywhere for now"), but if unnecessary I can replace it w/ a HashMap.

}
baggage.put(key, decodedValue);
// Accepted baggage is bounded by the int-valued baggageMaxBytes configuration.
baggageItemBytes.put(key, (int) itemBytes);
if (newItem) {
baggageItems++;
}
baggageBytes = (int) projectedBytes;
return true;
}

// Stores a value the tracer itself round-trips, outside the caller-controlled baggage budget so
// that caller-supplied headers cannot evict it. Exempt from the configured limits, but not
// unbounded: the value is capped here rather than left to each caller to validate, so the total
// retained stays within trace.baggage.max.bytes plus a fixed amount per reserved key.
Comment on lines +278 to +281

@PerfectSlayer PerfectSlayer Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 thought: ‏Why is there method comment that is not part of the Javadoc?

@mhlidd mhlidd Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll update here. Also will open a follow-up PR to add to AGENTS.md to ensure that method comments are Javadoc style.

protected final void addReservedBaggageItem(String key, String value) {

@PerfectSlayer PerfectSlayer Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 thought: ‏I don't get the whole reserved baggage… If it's only for XRay and in case of conflict, I don't see the point having it.

if (key == null || value == null || value.length() > MAX_RESERVED_BAGGAGE_LENGTH) {
return;
}
if (baggage.isEmpty()) {
baggage = new TreeMap<>();
baggageItemBytes = new TreeMap<>();
}
// A mapped baggage key could collide with a reserved key. Remove its charge before replacing
// it so the caller-controlled budget continues to describe the items actually retained.
final Integer previousItemBytes = baggageItemBytes.remove(key);
if (previousItemBytes != null) {
baggageItems--;
baggageBytes -= previousItemBytes;
}
baggage.put(key, value);
}

public ContextInterpreter reset(TraceConfig traceConfig) {
this.traceConfig = traceConfig;
traceId = DDTraceId.ZERO;
Expand All @@ -234,6 +306,9 @@ public ContextInterpreter reset(TraceConfig traceConfig) {
endToEndStartTime = 0;
if (tagLedger != null) tagLedger.reset();
baggage = Collections.emptyMap();
baggageItemBytes = Collections.emptyMap();
baggageItems = 0;
baggageBytes = 0;
valid = true;
fullContext = true;
httpHeaders = null;
Expand Down
9 changes: 1 addition & 8 deletions dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
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;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -188,13 +187,7 @@ public boolean accept(String key, String value) {
propagationTags = propagationTagsFactory.fromHeaderValue(HeaderType.DATADOG, value);
break;
case OT_BAGGAGE:
{
if (baggage.isEmpty()) {
baggage = new TreeMap<>();
}
baggage.put(
lowerCaseKey.substring(OT_BAGGAGE_PREFIX.length()), HttpCodec.decode(value));
}
addBaggageItem(lowerCaseKey.substring(OT_BAGGAGE_PREFIX.length()), value);
break;
default:
}
Expand Down
23 changes: 10 additions & 13 deletions dd-trace-core/src/main/java/datadog/trace/core/propagation/HaystackHttpCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import datadog.trace.core.DDSpanContext;
import de.thetaphi.forbiddenapis.SuppressForbidden;
import java.util.Map;
import java.util.TreeMap;
import java.util.function.Supplier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -40,7 +39,7 @@ class HaystackHttpCodec {

static final String HAYSTACK_TRACE_ID_BAGGAGE_KEY = "Haystack-Trace-ID";
static final String HAYSTACK_SPAN_ID_BAGGAGE_KEY = "Haystack-Span-ID";
private static final String HAYSTACK_PARENT_ID_BAGGAGE_KEY = "Haystack-Parent-ID";
static final String HAYSTACK_PARENT_ID_BAGGAGE_KEY = "Haystack-Parent-ID";

// public static final long DATADOG = new BigInteger("Datadog!".getBytes()).longValue();
public static final String DATADOG = "44617461-646f-6721";
Expand Down Expand Up @@ -202,15 +201,20 @@ public boolean accept(String key, String value) {
String firstValue = firstHeaderValue(value);
if (null != firstValue) {
switch (classification) {
// the trace and span ids are recorded by the tracer itself, not supplied as caller
// baggage: the injector reads them back to reproduce the original 128-bit ids, so
// they must not be evicted by caller-supplied Baggage-* headers
case TRACE_ID:
traceId = DD64bTraceId.fromHex(convertUUIDToHexString(value));
addBaggageItem(HAYSTACK_TRACE_ID_BAGGAGE_KEY, value);
traceId = DD64bTraceId.fromHex(convertUUIDToHexString(firstValue));
addReservedBaggageItem(HAYSTACK_TRACE_ID_BAGGAGE_KEY, firstValue);
break;
case SPAN_ID:
spanId = DDSpanId.fromHex(convertUUIDToHexString(value));
addBaggageItem(HAYSTACK_SPAN_ID_BAGGAGE_KEY, value);
spanId = DDSpanId.fromHex(convertUUIDToHexString(firstValue));
addReservedBaggageItem(HAYSTACK_SPAN_ID_BAGGAGE_KEY, firstValue);
break;
Comment thread
mhlidd marked this conversation as resolved.
case PARENT_ID:
// Nothing reads this back when injecting, so it is ordinary caller-supplied
// baggage rather than propagation bookkeeping, and is subject to the limits.
addBaggageItem(HAYSTACK_PARENT_ID_BAGGAGE_KEY, value);
break;
case BAGGAGE:
Expand Down Expand Up @@ -238,13 +242,6 @@ public boolean accept(String key, String value) {
return true;
}

private void addBaggageItem(String key, String value) {
if (baggage.isEmpty()) {
baggage = new TreeMap<>();
}
baggage.put(key, HttpCodec.decode(value));
}

@Override
protected int defaultSamplingPriority() {
return PrioritySampling.SAMPLER_KEEP;
Expand Down
9 changes: 1 addition & 8 deletions dd-trace-core/src/main/java/datadog/trace/core/propagation/W3CHttpCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import datadog.trace.bootstrap.instrumentation.api.TagContext;
import datadog.trace.core.DDSpanContext;
import java.util.Map;
import java.util.TreeMap;
import java.util.function.Supplier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -194,13 +193,7 @@ public boolean accept(String key, String value) {
endToEndStartTime = extractEndToEndStartTime(firstHeaderValue(value));
break;
case OT_BAGGAGE:
{
if (baggage.isEmpty()) {
baggage = new TreeMap<>();
}
baggage.put(
lowerCaseKey.substring(OT_BAGGAGE_PREFIX.length()), HttpCodec.decode(value));
}
addBaggageItem(lowerCaseKey.substring(OT_BAGGAGE_PREFIX.length()), value);
break;
default:
}
Expand Down
12 changes: 2 additions & 10 deletions dd-trace-core/src/main/java/datadog/trace/core/propagation/XRayHttpCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import datadog.trace.api.sampling.PrioritySampling;
import datadog.trace.core.DDSpanContext;
import java.util.Map;
import java.util.TreeMap;
import java.util.function.Supplier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -184,7 +183,7 @@ public boolean accept(String key, String value) {
if (!baggageMapping.isEmpty()) {
String mappedKey = baggageMapping.get(toLowerCase(key));
if (null != mappedKey) {
addBaggageItem(this, mappedKey, HttpCodec.decode(value));
addBaggageItem(mappedKey, value);
}
}
return true;
Expand Down Expand Up @@ -235,7 +234,7 @@ static void handleXRayTraceHeader(ContextInterpreter interpreter, String value)
} else {
int eqIndex = part.indexOf('=');
if (eqIndex > 0) {
addBaggageItem(interpreter, part.substring(0, eqIndex), part.substring(eqIndex + 1));
interpreter.addBaggageItem(part.substring(0, eqIndex), part.substring(eqIndex + 1));
}
}
startPart = endPart + 1;
Expand All @@ -255,12 +254,5 @@ private static long extractEndToEndStartTime(String value) {
private static int convertSamplingPriority(char samplingPriority) {
return '1' == samplingPriority ? SAMPLER_KEEP : SAMPLER_DROP;
}

private static void addBaggageItem(ContextInterpreter interpreter, String key, String value) {
if (interpreter.baggage.isEmpty()) {
interpreter.baggage = new TreeMap<>();
}
interpreter.baggage.put(key, HttpCodec.decode(value));
}
}
}
Loading
Loading