diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java index 5afc104c675..108e5a121ee 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/ContextInterpreter.java @@ -31,12 +31,16 @@ import java.util.Collections; import java.util.Map; import java.util.TreeMap; +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 headerTags; @@ -48,6 +52,9 @@ public abstract class ContextInterpreter implements AgentPropagation.KeyClassifi protected TagMap.Ledger tagLedger; protected Map baggage; + private int baggageItemCount; + private int baggageSize; + protected CharSequence lastParentId; protected CharSequence origin; protected long endToEndStartTime; @@ -63,6 +70,8 @@ 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; protected static final boolean LOG_EXTRACT_HEADER_NAMES = Config.get().isLogExtractHeaderNames(); private static final DDCache CACHE = DDCaches.newFixedSizeCache(64); @@ -78,6 +87,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() { @@ -216,15 +227,38 @@ 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) { + return false; + } + final boolean newItem = !baggage.containsKey(key); + if (newItem && baggageItemCount >= baggageMaxItems) { + LOG.debug("Dropping baggage item {}: item limit {} reached", key, baggageMaxItems); + return false; + } + + final long projectedSize = (long) baggageSize + key.length() + value.length(); + if (projectedSize > baggageMaxBytes) { + LOG.debug("Dropping baggage item {}: byte limit {} reached", key, baggageMaxBytes); + return false; + } + if (baggage.isEmpty()) { + baggage = new TreeMap<>(); + } + baggage.put(key, HttpCodec.decode(value)); + if (newItem) { + baggageItemCount++; + } + baggageSize = (int) projectedSize; + return true; + } + public ContextInterpreter reset(TraceConfig traceConfig) { this.traceConfig = traceConfig; traceId = DDTraceId.ZERO; @@ -234,6 +268,8 @@ public ContextInterpreter reset(TraceConfig traceConfig) { endToEndStartTime = 0; if (tagLedger != null) tagLedger.reset(); baggage = Collections.emptyMap(); + baggageItemCount = 0; + baggageSize = 0; valid = true; fullContext = true; httpHeaders = null; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java index 6723dfdbc8c..f2db43cdb6c 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/DatadogHttpCodec.java @@ -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; @@ -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: } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HaystackHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HaystackHttpCodec.java index 90573ea763b..61e9a40e897 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HaystackHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HaystackHttpCodec.java @@ -40,7 +40,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"; @@ -131,6 +131,9 @@ private static class HaystackContextInterpreter extends ContextInterpreter { private static final String BAGGAGE_PREFIX_LC = "baggage-"; + // Largest reserved value we accept. Only relevant for traceID/spanID + private static final int MAX_RESERVED_ID_LENGTH = 64; + private static final int TRACE_ID = 0; private static final int SPAN_ID = 1; private static final int PARENT_ID = 2; @@ -203,15 +206,15 @@ public boolean accept(String key, String value) { if (null != firstValue) { switch (classification) { 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; case PARENT_ID: - addBaggageItem(HAYSTACK_PARENT_ID_BAGGAGE_KEY, value); + addBaggageItem(HAYSTACK_PARENT_ID_BAGGAGE_KEY, firstValue); break; case BAGGAGE: { @@ -238,7 +241,18 @@ public boolean accept(String key, String value) { return true; } - private void addBaggageItem(String key, String value) { + /** + * Records the value of a reserved key, e.g. traceID/spanID. Ignores baggage item and byte + * limits to ensure propagation of key headers. However, if the header exceeds + * MAX_RESERVED_ID_LENGTH, value is rejected. + * + * @param key the reserved baggage key. + * @param value the id as it arrived, ignored when longer than {@link #MAX_RESERVED_ID_LENGTH}. + */ + private void addReservedBaggageItem(String key, String value) { + if (value == null || value.length() > MAX_RESERVED_ID_LENGTH) { + return; + } if (baggage.isEmpty()) { baggage = new TreeMap<>(); } 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 1d1d55cecc4..86ea382b482 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 @@ -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; @@ -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: } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/XRayHttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/XRayHttpCodec.java index 8b1c20f19af..bce004eb054 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/XRayHttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/XRayHttpCodec.java @@ -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; @@ -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; @@ -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; @@ -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)); - } } } diff --git a/dd-trace-core/src/test/java/datadog/trace/core/propagation/DatadogHttpExtractorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/propagation/DatadogHttpExtractorTest.java index 4c3644f24ed..05314f7c337 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/propagation/DatadogHttpExtractorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/propagation/DatadogHttpExtractorTest.java @@ -1,6 +1,8 @@ package datadog.trace.core.propagation; import static datadog.trace.api.config.TracerConfig.REQUEST_HEADER_TAGS_COMMA_ALLOWED; +import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_BYTES; +import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_ITEMS; import static datadog.trace.api.sampling.PrioritySampling.UNSET; import static datadog.trace.bootstrap.instrumentation.api.ContextVisitors.stringValuesMap; import static datadog.trace.core.propagation.DatadogHttpCodec.DATADOG_TAGS_KEY; @@ -31,6 +33,7 @@ import datadog.trace.test.junit.utils.converter.PrioritySamplingConverter; import datadog.trace.test.junit.utils.converter.TraceIdConverter; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.function.Supplier; import org.junit.jupiter.api.Test; @@ -325,6 +328,112 @@ void baggageIsMappedOnContextCreation( } } + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_ITEMS, value = "3") + void extractOtBaggageStopsAtItemLimit() { + Map headers = otBaggageHeaders(50); + headers.put(SOME_CUSTOM_BAGGAGE_HEADER, "mappedBaggageValue"); + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals(3, context.getBaggage().size()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_ITEMS, value = "1") + void extractMappedBaggageStopsAtItemLimit() { + Map headers = otBaggageHeaders(50); + headers.put(SOME_CUSTOM_BAGGAGE_HEADER, "mappedBaggageValue"); + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals(1, context.getBaggage().size()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_BYTES, value = "24") + void extractOtBaggageStopsAtByteLimit() { + // with single digit indices each stored item is "keyN" + "valueN" = 10 bytes, so 2 fit in 24 + // bytes and a third would take the total to 30 + TagContext context = this.extractor.extract(otBaggageHeaders(10), stringValuesMap()); + + assertEquals(2, context.getBaggage().size()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_BYTES, value = "24") + void extractOtBaggageChargesRepeatedKeyEachTime() { + // headers are visited in insertion order, so the duplicate key is seen before the last item + Map headers = new LinkedHashMap<>(); + // "key0" + "val0" is 8 bytes, and the duplicate is charged again rather than replacing the + // first charge, taking the total to 16 + headers.put(OT_BAGGAGE_PREFIX + "key0", "val0"); + headers.put(OT_BAGGAGE_PREFIX + "KEY0", "val0"); + // leaving no room for these 11 bytes, even though only 8 bytes are actually retained + headers.put(OT_BAGGAGE_PREFIX + "a", "0123456789"); + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals(singletonMap("key0", "val0"), context.getBaggage()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_ITEMS, value = "1") + void extractOtBaggageAllowsReplacementAtItemLimit() { + Map headers = new LinkedHashMap<>(); + headers.put(OT_BAGGAGE_PREFIX + "key0", "old"); + headers.put(OT_BAGGAGE_PREFIX + "KEY0", "replacement"); + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals(singletonMap("key0", "replacement"), context.getBaggage()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_BYTES, value = "24") + void extractOtBaggageReplacesValueWithoutFreeingItsCharge() { + Map headers = new LinkedHashMap<>(); + headers.put(OT_BAGGAGE_PREFIX + "key0", "val0"); // 8 bytes + headers.put(OT_BAGGAGE_PREFIX + "KEY0", "012345678901"); // replaces the value, charges 16 more + headers.put(OT_BAGGAGE_PREFIX + "a", "0123456789"); // 11 bytes, no longer fits + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals(singletonMap("key0", "012345678901"), context.getBaggage()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_BYTES, value = "8") + void extractOtBaggageChargesEncodedValueSize() { + Map headers = new LinkedHashMap<>(); + headers.put(OT_BAGGAGE_PREFIX + "a", "b"); // 2 characters + headers.put(OT_BAGGAGE_PREFIX + "c", "%E2%99%A5"); // 1 character key + 9 character raw value + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals(singletonMap("a", "b"), context.getBaggage()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_BYTES, value = "3") + void extractOtBaggageChargesLiteralUtf8ByCharacterCount() { + Map headers = new LinkedHashMap<>(); + headers.put(OT_BAGGAGE_PREFIX + "a", "♥"); // 2 characters + headers.put(OT_BAGGAGE_PREFIX + "b", "c"); // 2 more characters, no longer fits + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals(singletonMap("a", "♥"), context.getBaggage()); + } + + private static Map otBaggageHeaders(int count) { + Map headers = new HashMap<>(); + for (int i = 0; i < count; i++) { + headers.put(OT_BAGGAGE_PREFIX + "key" + i, "value" + i); + } + return headers; + } + private static String asString(CharSequence cs) { return cs == null ? null : cs.toString(); } diff --git a/dd-trace-core/src/test/java/datadog/trace/core/propagation/HaystackHttpExtractorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/propagation/HaystackHttpExtractorTest.java index e416ccf1f69..794b071f283 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/propagation/HaystackHttpExtractorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/propagation/HaystackHttpExtractorTest.java @@ -1,10 +1,13 @@ package datadog.trace.core.propagation; +import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_ITEMS; import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_KEEP; import static datadog.trace.bootstrap.instrumentation.api.ContextVisitors.stringValuesMap; +import static datadog.trace.core.propagation.HaystackHttpCodec.HAYSTACK_PARENT_ID_BAGGAGE_KEY; import static datadog.trace.core.propagation.HaystackHttpCodec.HAYSTACK_SPAN_ID_BAGGAGE_KEY; import static datadog.trace.core.propagation.HaystackHttpCodec.HAYSTACK_TRACE_ID_BAGGAGE_KEY; import static datadog.trace.core.propagation.HaystackHttpCodec.OT_BAGGAGE_PREFIX; +import static datadog.trace.core.propagation.HaystackHttpCodec.PARENT_ID_KEY; import static datadog.trace.core.propagation.HaystackHttpCodec.SPAN_ID_KEY; import static datadog.trace.core.propagation.HaystackHttpCodec.TRACE_ID_KEY; import static datadog.trace.core.propagation.HttpCodecTestHelper.headers; @@ -20,6 +23,7 @@ import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; import datadog.trace.bootstrap.instrumentation.api.TagContext; +import datadog.trace.test.junit.utils.config.WithConfig; import datadog.trace.test.junit.utils.converter.TraceIdConverter; import java.util.HashMap; import java.util.Map; @@ -35,6 +39,127 @@ protected HttpCodec.Extractor newExtractor( return HaystackHttpCodec.newExtractor(config, traceConfigSupplier); } + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_ITEMS, value = "3") + void extractBaggageStopsAtItemLimit() { + Map headers = new HashMap<>(); + for (int i = 0; i < 50; i++) { + headers.put(OT_BAGGAGE_PREFIX + "key" + i, "value" + i); + } + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals(3, context.getBaggage().size()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_ITEMS, value = "1") + void extractKeepsHaystackIdsWhenBaggageLimitReached() { + // the Haystack ids are recorded by the tracer for lossless injection, so caller supplied + // Baggage-* headers must not be able to evict them by exhausting the item limit + Map headers = new HashMap<>(); + for (int i = 0; i < 50; i++) { + headers.put(OT_BAGGAGE_PREFIX + "key" + i, "value" + i); + } + headers.put(TRACE_ID_KEY, "44617461-646f-6721-0000-000000000001"); + headers.put(SPAN_ID_KEY, "44617461-646f-6721-0000-000000000002"); + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals( + "44617461-646f-6721-0000-000000000001", + context.getBaggage().get(HAYSTACK_TRACE_ID_BAGGAGE_KEY)); + assertEquals( + "44617461-646f-6721-0000-000000000002", + context.getBaggage().get(HAYSTACK_SPAN_ID_BAGGAGE_KEY)); + } + + @Test + void extractUsesFirstConcatenatedHaystackIdHeaderValue() { + String traceUuid = "44617461-646f-6721-0000-000000000001"; + String spanUuid = "44617461-646f-6721-0000-000000000002"; + String parentUuid = "44617461-646f-6721-0000-000000000005"; + Map headers = + headers( + TRACE_ID_KEY, + traceUuid + ",44617461-646f-6721-0000-000000000003", + SPAN_ID_KEY, + spanUuid + ",44617461-646f-6721-0000-000000000004", + PARENT_ID_KEY, + parentUuid + ",44617461-646f-6721-0000-000000000006"); + + ExtractedContext context = + (ExtractedContext) this.extractor.extract(headers, stringValuesMap()); + + assertEquals(DDTraceId.from(1), context.getTraceId()); + assertEquals(DDSpanId.from("2"), context.getSpanId()); + assertEquals(traceUuid, context.getBaggage().get(HAYSTACK_TRACE_ID_BAGGAGE_KEY)); + assertEquals(spanUuid, context.getBaggage().get(HAYSTACK_SPAN_ID_BAGGAGE_KEY)); + assertEquals(parentUuid, context.getBaggage().get(HAYSTACK_PARENT_ID_BAGGAGE_KEY)); + } + + @Test + void extractRetainsParentIdAsBaggage() { + // Parent-ID is not read back when injecting, so it is kept as ordinary caller baggage and + // still propagated downstream as Baggage-Haystack-Parent-ID + Map headers = + headers( + TRACE_ID_KEY, + "44617461-646f-6721-0000-000000000001", + SPAN_ID_KEY, + "44617461-646f-6721-0000-000000000002", + PARENT_ID_KEY, + "44617461-646f-6721-0000-000000000003"); + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals( + "44617461-646f-6721-0000-000000000003", + context.getBaggage().get(HAYSTACK_PARENT_ID_BAGGAGE_KEY)); + } + + @Test + void extractDropsOversizedParentId() { + // unlike the reserved trace and span ids, Parent-ID is subject to the baggage limits + Map headers = + headers( + TRACE_ID_KEY, + "44617461-646f-6721-0000-000000000001", + SPAN_ID_KEY, + "44617461-646f-6721-0000-000000000002", + PARENT_ID_KEY, + repeat('x', 10_000)); + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertFalse(context.getBaggage().containsKey(HAYSTACK_PARENT_ID_BAGGAGE_KEY)); + } + + @Test + void extractDoesNotReserveOversizedTraceId() { + // reserved values skip the baggage budget, so they are capped to keep the overshoot fixed + Map headers = + headers( + TRACE_ID_KEY, + repeat('a', 10_000) + "-646f-6721-0000-000000000001", + SPAN_ID_KEY, + "44617461-646f-6721-0000-000000000002"); + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + // only the last two UUID segments carry the DataDog id, so extraction still succeeds + assertEquals(DDTraceId.fromHex("0000000000000001"), context.getTraceId()); + assertFalse(context.getBaggage().containsKey(HAYSTACK_TRACE_ID_BAGGAGE_KEY)); + } + + private static String repeat(char value, int count) { + StringBuilder result = new StringBuilder(count); + for (int i = 0; i < count; i++) { + result.append(value); + } + return result.toString(); + } + @TableTest({ "scenario | traceId | spanId | traceUuid | spanUuid ", "small ids | '1' | '2' | '44617461-646f-6721-0000-000000000001' | '44617461-646f-6721-0000-000000000002'", diff --git a/dd-trace-core/src/test/java/datadog/trace/core/propagation/W3CHttpExtractorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/propagation/W3CHttpExtractorTest.java index 511ffe97ac7..e12aad95455 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/propagation/W3CHttpExtractorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/propagation/W3CHttpExtractorTest.java @@ -1,5 +1,7 @@ package datadog.trace.core.propagation; +import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_BYTES; +import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_ITEMS; import static datadog.trace.bootstrap.instrumentation.api.ContextVisitors.stringValuesMap; import static datadog.trace.core.propagation.HttpCodecTestHelper.headers; import static datadog.trace.core.propagation.W3CHttpCodec.OT_BAGGAGE_PREFIX; @@ -19,6 +21,7 @@ import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; import datadog.trace.bootstrap.instrumentation.api.TagContext; +import datadog.trace.test.junit.utils.config.WithConfig; import datadog.trace.test.junit.utils.converter.PrioritySamplingConverter; import datadog.trace.test.junit.utils.converter.SamplingMechanismConverter; import java.util.HashMap; @@ -49,6 +52,34 @@ protected HttpCodec.Extractor newExtractor( return W3CHttpCodec.newExtractor(config, traceConfigSupplier); } + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_ITEMS, value = "3") + void extractOtBaggageStopsAtItemLimit() { + Map headers = new HashMap<>(); + for (int i = 0; i < 50; i++) { + headers.put(OT_BAGGAGE_PREFIX + "key" + i, "value" + i); + } + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals(3, context.getBaggage().size()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_BYTES, value = "24") + void extractOtBaggageStopsAtByteLimit() { + // with single digit indices each stored item is "keyN" + "valueN" = 10 bytes, so 2 fit in 24 + // bytes and a third would take the total to 30 + Map headers = new HashMap<>(); + for (int i = 0; i < 10; i++) { + headers.put(OT_BAGGAGE_PREFIX + "key" + i, "value" + i); + } + + TagContext context = this.extractor.extract(headers, stringValuesMap()); + + assertEquals(2, context.getBaggage().size()); + } + @TableTest({ "scenario | traceparent | tpValid | traceId | spanId | priority ", "null traceparent | | false | | 0 | UNSET ", diff --git a/dd-trace-core/src/test/java/datadog/trace/core/propagation/XRayHttpExtractorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/propagation/XRayHttpExtractorTest.java index ac4566ce015..f904ce67593 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/propagation/XRayHttpExtractorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/propagation/XRayHttpExtractorTest.java @@ -1,8 +1,12 @@ package datadog.trace.core.propagation; +import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_BYTES; +import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_ITEMS; +import static datadog.trace.api.sampling.PrioritySampling.SAMPLER_KEEP; import static datadog.trace.bootstrap.instrumentation.api.ContextVisitors.stringValuesMap; import static datadog.trace.core.propagation.HttpCodecTestHelper.headers; import static datadog.trace.core.propagation.XRayHttpCodec.X_AMZN_TRACE_ID; +import static datadog.trace.core.propagation.XRayTestHelper.zeroPadId; import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -13,6 +17,7 @@ import datadog.trace.api.DDTraceId; import datadog.trace.api.TraceConfig; import datadog.trace.bootstrap.instrumentation.api.TagContext; +import datadog.trace.test.junit.utils.config.WithConfig; import datadog.trace.test.junit.utils.converter.PrioritySamplingConverter; import java.util.HashMap; import java.util.Map; @@ -28,6 +33,52 @@ protected HttpCodec.Extractor newExtractor( return XRayHttpCodec.newExtractor(config, traceConfigSupplier); } + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_ITEMS, value = "3") + void extractTraceHeaderBaggageStopsAtItemLimit() { + TagContext context = + this.extractor.extract(headers(X_AMZN_TRACE_ID, baggageHeader(50)), stringValuesMap()); + + assertEquals(3, context.getBaggage().size()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_BYTES, value = "24") + void extractTraceHeaderBaggageStopsAtByteLimit() { + // with single digit indices each stored item is "keyN" + "valueN" = 10 bytes, so 2 fit in 24 + // bytes and a third would take the total to 30 + TagContext context = + this.extractor.extract(headers(X_AMZN_TRACE_ID, baggageHeader(9)), stringValuesMap()); + + assertEquals(2, context.getBaggage().size()); + } + + @Test + @WithConfig(key = TRACE_BAGGAGE_MAX_ITEMS, value = "3") + void extractTraceHeaderKeepsParsingContextAfterBaggageLimit() { + // reaching the baggage limit must not stop the header being parsed: the trace context segments + // can appear after the `key=value` segments that exhausted the limit + TagContext context = + this.extractor.extract( + headers( + X_AMZN_TRACE_ID, baggageHeader(50) + ";Parent=" + zeroPadId("2") + ";Sampled=1"), + stringValuesMap()); + + assertEquals(3, context.getBaggage().size()); + assertEquals(zeroPadId("1"), context.getTraceId().toHexStringPadded(16)); + assertEquals(zeroPadId("2"), DDSpanId.toHexStringPadded(context.getSpanId())); + assertEquals(SAMPLER_KEEP, context.getSamplingPriority()); + } + + private static String baggageHeader(int itemCount) { + // a single X-Amzn-Trace-Id header carries an arbitrary number of `key=value` segments + StringBuilder header = new StringBuilder("Root=1-00000000-00000000").append(zeroPadId("1")); + for (int i = 0; i < itemCount; i++) { + header.append(";key").append(i).append("=value").append(i); + } + return header.toString(); + } + @TableTest({ "scenario | traceId | spanId | samplingPriority | expectedSamplingPriority", "no sampling | 1 | 2 | '' | UNSET ", @@ -44,9 +95,9 @@ void extractHttpHeaders( // spotless:off Map headers = headers( X_AMZN_TRACE_ID, "Root=1-00000000-00000000" - + XRayTestHelper.zeroPadId(traceId) + + zeroPadId(traceId) + ";Parent=" - + XRayTestHelper.zeroPadId(spanId) + + zeroPadId(spanId) + samplingPriority + ";=empty key;empty value=;=;;", SOME_HEADER, "my-interesting-info", @@ -131,17 +182,14 @@ void extractIdsWhileRetainingTheOriginalString( Map headers = headers( X_AMZN_TRACE_ID, - "Root=1-00000000-00000000" - + XRayTestHelper.zeroPadId(traceId) - + ";Parent=" - + XRayTestHelper.zeroPadId(spanId)); + "Root=1-00000000-00000000" + zeroPadId(traceId) + ";Parent=" + zeroPadId(spanId)); ExtractedContext context = (ExtractedContext) extractor.extract(headers, stringValuesMap()); assertEquals(DDTraceId.fromHex(expectedTraceIdHex), context.getTraceId()); - assertEquals(XRayTestHelper.zeroPadId(traceId), context.getTraceId().toHexStringPadded(16)); + assertEquals(zeroPadId(traceId), context.getTraceId().toHexStringPadded(16)); assertEquals(expectedSpanId, context.getSpanId()); - assertEquals(XRayTestHelper.zeroPadId(spanId), DDSpanId.toHexStringPadded(context.getSpanId())); + assertEquals(zeroPadId(spanId), DDSpanId.toHexStringPadded(context.getSpanId())); } @TableTest({ @@ -154,9 +202,9 @@ void extractHeadersWithEndToEnd(String traceId, String spanId, long endToEndStar headers( X_AMZN_TRACE_ID, "Root=1-00000000-00000000" - + XRayTestHelper.zeroPadId(traceId) + + zeroPadId(traceId) + ";Parent=" - + XRayTestHelper.zeroPadId(spanId) + + zeroPadId(spanId) + ";k1=v1;t0=" + endToEndStartTime + ";k2=v2"); diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index fade2b4c417..bf1fa10fdbf 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -1995,9 +1995,13 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins tracePropagationStylesToInject = inject.isEmpty() ? DEFAULT_TRACE_PROPAGATION_STYLE : inject; traceBaggageMaxItems = - configProvider.getInteger(TRACE_BAGGAGE_MAX_ITEMS, DEFAULT_TRACE_BAGGAGE_MAX_ITEMS); + nonNegativeBaggageLimit( + TRACE_BAGGAGE_MAX_ITEMS, + configProvider.getInteger(TRACE_BAGGAGE_MAX_ITEMS, DEFAULT_TRACE_BAGGAGE_MAX_ITEMS)); traceBaggageMaxBytes = - configProvider.getInteger(TRACE_BAGGAGE_MAX_BYTES, DEFAULT_TRACE_BAGGAGE_MAX_BYTES); + nonNegativeBaggageLimit( + TRACE_BAGGAGE_MAX_BYTES, + configProvider.getInteger(TRACE_BAGGAGE_MAX_BYTES, DEFAULT_TRACE_BAGGAGE_MAX_BYTES)); // These setting are here for backwards compatibility until they can be removed in a major // release of the tracer @@ -3432,6 +3436,14 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) log.debug("New instance: {}", this); } + private static int nonNegativeBaggageLimit(String setting, int value) { + if (value < 0) { + log.warn("Invalid {}: {}. The value must not be negative. Disabling baggage", setting, value); + return 0; + } + return value; + } + private static boolean isValidUrl(String url) { if (url == null || url.isEmpty()) { return false; diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index ef95d5e902c..0ef1c1faf5b 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -6,6 +6,8 @@ import static datadog.trace.api.ConfigDefaults.DEFAULT_FEATURE_FLAGGING_CONFIGUR import static datadog.trace.api.ConfigDefaults.DEFAULT_FEATURE_FLAGGING_CONFIGURATION_SOURCE_REQUEST_TIMEOUT_SECONDS import static datadog.trace.api.ConfigDefaults.DEFAULT_PARTIAL_FLUSH_MIN_SPANS import static datadog.trace.api.ConfigDefaults.DEFAULT_SERVICE_NAME +import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_BAGGAGE_MAX_BYTES +import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_BAGGAGE_MAX_ITEMS import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_LONG_RUNNING_FLUSH_INTERVAL import static datadog.trace.api.ConfigDefaults.DEFAULT_TRACE_LONG_RUNNING_INITIAL_FLUSH_INTERVAL import static datadog.trace.api.DDTags.HOST_TAG @@ -124,6 +126,8 @@ import static datadog.trace.api.config.TracerConfig.SPAN_TAGS import static datadog.trace.api.config.TracerConfig.SPLIT_BY_TAGS import static datadog.trace.api.config.TracerConfig.TRACE_AGENT_PORT import static datadog.trace.api.config.TracerConfig.TRACE_AGENT_URL +import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_BYTES +import static datadog.trace.api.config.TracerConfig.TRACE_BAGGAGE_MAX_ITEMS import static datadog.trace.api.config.TracerConfig.TRACE_EXPERIMENTAL_FEATURES_ENABLED import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_ENABLED import static datadog.trace.api.config.TracerConfig.TRACE_LONG_RUNNING_FLUSH_INTERVAL @@ -3126,6 +3130,33 @@ class ConfigTest extends DDSpecification { "\$.a,invalid" | "\$.b,invalid" | '[$[\'a\']]' | '[$[\'b\']]' } + def "baggage limits normalize negative values to zero"() { + setup: + def prop = new Properties() + if (maxItems != null) { + prop.setProperty(TRACE_BAGGAGE_MAX_ITEMS, maxItems) + } + if (maxBytes != null) { + prop.setProperty(TRACE_BAGGAGE_MAX_BYTES, maxBytes) + } + + when: + Config config = Config.get(prop) + + then: + config.traceBaggageMaxItems == expectedMaxItems + config.traceBaggageMaxBytes == expectedMaxBytes + + where: + maxItems | maxBytes | expectedMaxItems | expectedMaxBytes + null | null | DEFAULT_TRACE_BAGGAGE_MAX_ITEMS | DEFAULT_TRACE_BAGGAGE_MAX_BYTES + "0" | "0" | 0 | 0 + "8" | "512" | 8 | 512 + "-1" | null | 0 | DEFAULT_TRACE_BAGGAGE_MAX_BYTES + null | "-8192" | DEFAULT_TRACE_BAGGAGE_MAX_ITEMS | 0 + "-1" | "-1" | 0 | 0 + } + // Subclass for setting Strictness of ConfigHelper when using fake configs static class ConfigTestWithFakes extends ConfigTest {