From 86e545dd2bf60b4cb3cbd0cd5fd3dcd9a08136d5 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 19 Oct 2026 12:12:20 +0200 Subject: [PATCH 1/2] fix(core): Simplify TID header parsing error handling Revert the non-throwing id parsing from #12739: DD64bTraceId.fromOrNull and the LongStringUtils sentinel parser added public API, duplicated the decimal parser, and diverged from DD128bTraceId and DDSpanId. Keep the goal of keeping malformed headers out of telemetry by catching NumberFormatException during Datadog context extraction and logging it with EXCLUDE_TELEMETRY, without the stack trace. --- .../java/datadog/trace/api/DD64bTraceId.java | 20 --- .../api/internal/util/LongStringUtils.java | 86 ---------- .../java/datadog/trace/api/DDTraceIdTest.java | 41 ----- .../internal/util/LongStringUtilsTest.java | 154 ------------------ .../propagation/DatadogIdParseBenchmark.java | 135 --------------- .../core/propagation/DatadogHttpCodec.java | 35 +--- .../trace/core/propagation/HttpCodec.java | 15 +- .../propagation/DatadogHttpExtractorTest.java | 2 - .../core/propagation/HttpExtractorTest.java | 3 +- 9 files changed, 12 insertions(+), 479 deletions(-) delete mode 100644 dd-trace-api/src/test/java/datadog/trace/api/internal/util/LongStringUtilsTest.java delete mode 100644 dd-trace-core/src/jmh/java/datadog/trace/core/propagation/DatadogIdParseBenchmark.java diff --git a/dd-trace-api/src/main/java/datadog/trace/api/DD64bTraceId.java b/dd-trace-api/src/main/java/datadog/trace/api/DD64bTraceId.java index 6cc0f940f66..f40b47a89d4 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/DD64bTraceId.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/DD64bTraceId.java @@ -46,26 +46,6 @@ public static DD64bTraceId from(String s) throws NumberFormatException { return DD64bTraceId.create(LongStringUtils.parseUnsignedLong(s), s); } - /** - * Like {@link #from(String)}, but returns {@code null} instead of throwing when the given {@code - * String} is not a plain decimal unsigned 64 bit id. Stricter than {@link #from(String)}: a - * leading {@code +} and non-ASCII digits are rejected. - * - *

Internal API for parsing untrusted input, such as propagation headers, on the request path. - * Not intended for use outside the tracer. - * - * @param s String of unsigned 64 bit id - * @return DDTraceId, or {@code null} if the {@code String} is not a valid id - */ - public static DD64bTraceId fromOrNull(String s) { - int len = s == null ? 0 : s.length(); - long id = LongStringUtils.parseUnsignedLongOrSentinel(s, 0, len, 0L); - if (id == 0L && !LongStringUtils.isUnsignedLongZero(s, 0, len)) { - return null; - } - return DD64bTraceId.create(id, s); - } - /** * Create a new {@code DDTraceId} from the given {@code String} hex representation of the unsigned * 64 bit id. diff --git a/dd-trace-api/src/main/java/datadog/trace/api/internal/util/LongStringUtils.java b/dd-trace-api/src/main/java/datadog/trace/api/internal/util/LongStringUtils.java index 650f39c63b4..aa55b8943fb 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/internal/util/LongStringUtils.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/internal/util/LongStringUtils.java @@ -73,93 +73,7 @@ private static int firstNonZeroCharacter(CharSequence s, int start) { return firstNonZero; } - /** - * Parse the decimal representation of the unsigned 64 bit long from the {@code String}. - * - * @param s String in decimal of unsigned 64-bits long. - * @return long - * @throws NumberFormatException - */ public static long parseUnsignedLong(String s) throws NumberFormatException { - long result = parseUnsignedLongOrSentinel(s, 0, s == null ? 0 : s.length(), 0L); - if (result != 0L) { - return result; - } - // Zero is ambiguous with the invalid sentinel. Let the strict parser decide: it accepts input - // the non-throwing parser rejects (a leading '+', non-ASCII digits) and builds the exception. - return parseUnsignedLongStrict(s); - } - - /** - * Parse the decimal representation of the unsigned 64 bit long from the {@code CharSequence} - * without throwing, for untrusted input such as propagation headers. - * - *

Accepts 1 to 20 ASCII digits only: no sign, no whitespace, no non-ASCII digits. An unsigned - * 64 bit value uses every bit, so no return value can mean "invalid" on its own. Callers pick an - * {@code ifInvalid} that is impossible or harmless for their use, e.g. {@code 0} for trace and - * span ids. With {@code 0} as {@code ifInvalid}, {@link #isUnsignedLongZero} tells a real zero - * apart from invalid input. - * - * @param s CharSequence containing the decimal digits - * @param start the start index of the decimal value - * @param len the length of the decimal value - * @param ifInvalid the value returned for null, empty, malformed or out of range input - * @return the parsed long, or {@code ifInvalid} - */ - public static long parseUnsignedLongOrSentinel( - CharSequence s, int start, int len, long ifInvalid) { - if (s == null || len <= 0 || len > 20 || start < 0 || start + len > s.length()) { - return ifInvalid; - } - int end = start + len; - // Signed 64 bits holds any 18 digit number, so the first 18 digits cannot overflow - int fastEnd = Math.min(end, start + 18); - long result = 0; - int i = start; - for (; i < fastEnd; i++) { - int d = s.charAt(i) - '0'; - if (d < 0 || d > 9) { - return ifInvalid; - } - result = result * 10 + d; - } - for (; i < end; i++) { - int d = s.charAt(i) - '0'; - if (d < 0 || d > 9 || Long.compareUnsigned(result, MAX_FIRST_PART) > 0) { - return ifInvalid; - } - long scaled = result * 10; - result = scaled + d; - if (Long.compareUnsigned(result, scaled) < 0) { - return ifInvalid; - } - } - return result; - } - - /** - * Whether {@code s[start, start + len)} is a zero that {@link #parseUnsignedLongOrSentinel} - * accepts: 1 to 20 {@code '0'} characters. Callers that use {@code 0} as the invalid sentinel - * check this only when the parse returned {@code 0}. - * - * @param s CharSequence containing the decimal digits - * @param start the start index of the decimal value - * @param len the length of the decimal value - * @return true if the range parses to zero, false if it is zero-valued only as the sentinel - */ - public static boolean isUnsignedLongZero(CharSequence s, int start, int len) { - if (s == null || len <= 0 || len > 20 || start < 0 || start + len > s.length()) { - return false; - } - for (int i = start, end = start + len; i < end; i++) { - if (s.charAt(i) != '0') { - return false; - } - } - return true; - } - - private static long parseUnsignedLongStrict(String s) throws NumberFormatException { if (s == null) { throw new NumberFormatException("s can't be null"); } diff --git a/dd-trace-api/src/test/java/datadog/trace/api/DDTraceIdTest.java b/dd-trace-api/src/test/java/datadog/trace/api/DDTraceIdTest.java index 27ca226355b..98225bb9698 100644 --- a/dd-trace-api/src/test/java/datadog/trace/api/DDTraceIdTest.java +++ b/dd-trace-api/src/test/java/datadog/trace/api/DDTraceIdTest.java @@ -2,7 +2,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -71,46 +70,6 @@ void failParsingIllegal64BitIdStringRepresentation(String stringId) { assertThrows(NumberFormatException.class, () -> DD64bTraceId.from(stringId)); } - @TableTest({ - "scenario | stringId | expectedId ", - "zero | '0' | DD64bTraceId.ZERO ", - "one | '1' | DD64bTraceId.ONE ", - "max | '18446744073709551615' | DD64bTraceId.MAX ", - "long max | '9223372036854775807' | DD64bTraceId.LONG_MAX", - "long max plus one | '9223372036854775808' | DD64bTraceId.LONG_MIN" - }) - @ParameterizedTest(name = "fromOrNull parses valid 64-bit id String representation [{index}]") - void fromOrNullParsesValid64BitIdStringRepresentation(String stringId, DD64bTraceId expectedId) { - DD64bTraceId ddid = DD64bTraceId.fromOrNull(stringId); - - assertEquals(expectedId, ddid); - assertEquals(stringId, ddid.toString()); - } - - @Test - void fromOrNullKeepsTheParsedStringForToString() { - String stringId = "1234567890"; - - assertSame(stringId, DD64bTraceId.fromOrNull(stringId).toString()); - } - - @ParameterizedTest(name = "fromOrNull returns null for illegal 64-bit id String [{index}]") - @NullSource - @ValueSource( - strings = { - "", - "-1", - "+1", - "18446744073709551616", - "18446744073709551625", - "184467440737095516150", - "18446744073709551a1", - "184467440737095511a" - }) - void fromOrNullReturnsNullForIllegal64BitIdString(String stringId) { - assertNull(DD64bTraceId.fromOrNull(stringId)); - } - @TableTest({ "scenario | hexId | expectedId ", "zero | '0' | DD64bTraceId.ZERO ", diff --git a/dd-trace-api/src/test/java/datadog/trace/api/internal/util/LongStringUtilsTest.java b/dd-trace-api/src/test/java/datadog/trace/api/internal/util/LongStringUtilsTest.java deleted file mode 100644 index a18f74a3034..00000000000 --- a/dd-trace-api/src/test/java/datadog/trace/api/internal/util/LongStringUtilsTest.java +++ /dev/null @@ -1,154 +0,0 @@ -package datadog.trace.api.internal.util; - -import static datadog.trace.api.internal.util.LongStringUtils.parseUnsignedLong; -import static datadog.trace.api.internal.util.LongStringUtils.parseUnsignedLongOrSentinel; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; - -import datadog.trace.api.DDTraceApiTableTestConverters; -import java.math.BigInteger; -import java.util.concurrent.ThreadLocalRandom; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.NullSource; -import org.junit.jupiter.params.provider.ValueSource; -import org.tabletest.junit.TableTest; -import org.tabletest.junit.TypeConverterSources; - -@TypeConverterSources(DDTraceApiTableTestConverters.class) -class LongStringUtilsTest { - private static final long SENTINEL = 42L; - - @TableTest({ - "scenario | input | expected ", - "zero | '0' | 0 ", - "one | '1' | 1 ", - "leading zeros | '00001' | 1 ", - "18 digits | '999999999999999999' | 999999999999999999 ", - "19 digits | '1000000000000000000' | 1000000000000000000", - "long max | '9223372036854775807' | Long.MAX_VALUE ", - "long max plus one | '9223372036854775808' | Long.MIN_VALUE ", - "unsigned max | '18446744073709551615' | -1 ", - "20 digits with leading zero | '00000000000000000001' | 1 " - }) - @ParameterizedTest(name = "parse valid decimal [{index}]") - void parseValidDecimal(String input, long expected) { - assertEquals(expected, parseUnsignedLongOrSentinel(input, 0, input.length(), SENTINEL)); - assertEquals(expected, parseUnsignedLong(input)); - } - - @ParameterizedTest(name = "non-throwing parse returns sentinel for invalid input [{index}]") - @ValueSource( - strings = { - "", - "-1", - "+1", - " 1", - "1 ", - "12a", - "0x1a", - "١", // ARABIC-INDIC DIGIT ONE - "18446744073709551616", // unsigned max + 1 - "18446744073709551620", // first 19 digits already too large - "99999999999999999999", - "73659894405625460838", // JDK 8's parseUnsignedLong wraps this instead of throwing - "184467440737095516150", // 21 digits - "000000000000000000001" // 21 digits, even with leading zeros - }) - void nonThrowingParseReturnsSentinelForInvalidInput(String input) { - assertEquals(SENTINEL, parseUnsignedLongOrSentinel(input, 0, input.length(), SENTINEL)); - } - - @Test - void nonThrowingParseReturnsSentinelForNull() { - assertEquals(SENTINEL, parseUnsignedLongOrSentinel(null, 0, 0, SENTINEL)); - } - - @TableTest({ - "scenario | input | start | len | expected", - "middle | 'x123y' | 1 | 3 | 123 ", - "prefix | '123y' | 0 | 3 | 123 ", - "negative start | '123' | -1 | 2 | 42 ", - "past the end | '123' | 1 | 3 | 42 ", - "zero length | '123' | 0 | 0 | 42 ", - "includes bad char | 'x123y' | 1 | 4 | 42 " - }) - @ParameterizedTest(name = "non-throwing parse of a range [{index}]") - void nonThrowingParseOfARange(String input, int start, int len, long expected) { - assertEquals(expected, parseUnsignedLongOrSentinel(input, start, len, SENTINEL)); - } - - @TableTest({ - "scenario | input | start | len | expected", - "zero | '0' | 0 | 1 | true ", - "20 zeros | '00000000000000000000' | 0 | 20 | true ", - "zeros in a range | 'x00y' | 1 | 2 | true ", - "21 zeros | '000000000000000000000' | 0 | 21 | false ", - "non-zero digit | '01' | 0 | 2 | false ", - "not a digit | 'a' | 0 | 1 | false ", - "empty | '' | 0 | 0 | false ", - "past the end | '0' | 0 | 2 | false " - }) - @ParameterizedTest(name = "isUnsignedLongZero [{index}]") - void isUnsignedLongZero(String input, int start, int len, boolean expected) { - assertEquals(expected, LongStringUtils.isUnsignedLongZero(input, start, len)); - } - - @Test - void isUnsignedLongZeroOfNull() { - assertFalse(LongStringUtils.isUnsignedLongZero(null, 0, 0)); - } - - @TableTest({ - "scenario | input | expected", - "leading plus | '+1' | 1 ", - "non-ASCII digit | '١' | 1 " - }) - @ParameterizedTest(name = "throwing parse keeps accepting what Long.parseLong accepts [{index}]") - void throwingParseKeepsAcceptingWhatLongParseLongAccepts(String input, long expected) { - assertEquals(expected, parseUnsignedLong(input)); - } - - @ParameterizedTest(name = "throwing parse rejects invalid input [{index}]") - @NullSource - @ValueSource(strings = {"", "-1", "12a", "18446744073709551616", "184467440737095516150"}) - void throwingParseRejectsInvalidInput(String input) { - assertThrows(NumberFormatException.class, () -> parseUnsignedLong(input)); - } - - @Test - void matchesJdkForRandomUnsignedLongs() { - ThreadLocalRandom random = ThreadLocalRandom.current(); - for (int i = 0; i < 10_000; i++) { - long value = random.nextLong(); - String s = Long.toUnsignedString(value); - assertEquals(value, parseUnsignedLongOrSentinel(s, 0, s.length(), SENTINEL), s); - } - } - - @Test - void matchesExactValueForRandomDigitStrings() { - // Lengths around the 18 / 19 / 20 digit boundaries, where overflow handling changes. Longer - // input is always rejected, even with leading zeros, matching parseUnsignedLong(String). - // BigInteger is the oracle: JDK 8's parseUnsignedLong misses some 20-digit overflows and - // wraps them instead of throwing. - ThreadLocalRandom random = ThreadLocalRandom.current(); - char[] digits = new char[20]; - for (int i = 0; i < 10_000; i++) { - int len = 17 + random.nextInt(4); - for (int j = 0; j < len; j++) { - digits[j] = (char) ('0' + random.nextInt(10)); - } - String s = new String(digits, 0, len); - BigInteger exact = new BigInteger(s); - boolean valid = exact.bitLength() <= 64; - long expected = exact.longValue(); - if (valid) { - assertEquals(expected, parseUnsignedLongOrSentinel(s, 0, len, SENTINEL), s); - } else { - assertEquals(SENTINEL, parseUnsignedLongOrSentinel(s, 0, len, SENTINEL), s); - } - } - } -} diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/propagation/DatadogIdParseBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/propagation/DatadogIdParseBenchmark.java deleted file mode 100644 index 0708ada94d2..00000000000 --- a/dd-trace-core/src/jmh/java/datadog/trace/core/propagation/DatadogIdParseBenchmark.java +++ /dev/null @@ -1,135 +0,0 @@ -package datadog.trace.core.propagation; - -import static java.util.concurrent.TimeUnit.NANOSECONDS; -import static java.util.concurrent.TimeUnit.SECONDS; - -import ch.qos.logback.classic.Level; -import ch.qos.logback.classic.Logger; -import datadog.trace.api.Config; -import datadog.trace.api.DD64bTraceId; -import datadog.trace.api.DDTraceId; -import datadog.trace.api.DynamicConfig; -import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; -import datadog.trace.bootstrap.instrumentation.api.TagContext; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.Map; -import org.openjdk.jmh.annotations.Benchmark; -import org.openjdk.jmh.annotations.BenchmarkMode; -import org.openjdk.jmh.annotations.Fork; -import org.openjdk.jmh.annotations.Measurement; -import org.openjdk.jmh.annotations.Mode; -import org.openjdk.jmh.annotations.OutputTimeUnit; -import org.openjdk.jmh.annotations.Param; -import org.openjdk.jmh.annotations.Scope; -import org.openjdk.jmh.annotations.Setup; -import org.openjdk.jmh.annotations.State; -import org.openjdk.jmh.annotations.Warmup; -import org.slf4j.LoggerFactory; - -/** - * Cost of parsing an {@code x-datadog-trace-id} header value, valid and invalid, with the throwing - * {@link DD64bTraceId#from(String)} and the non-throwing {@link DD64bTraceId#fromOrNull(String)}, - * and of a full Datadog-style extraction carrying that value. - * - *

The invalid values are the two shapes behind the highest-volume propagation entries in Error - * Tracking: a value past the unsigned 64 bit range ({@code numberFormatOutOfLongRange}) and a value - * that is not decimal at all. Valid values cover both the 18 digit path and the 19 digit path, - * where most randomly generated 63 bit ids fall. - * - *

Results. Invalid ids cost 20-43 ns per extraction instead of 840-880 ns, because no - * exception is built. The benchmark's stack is shallow; a real server's is much deeper, so the - * saving in production is larger. Overflow (24 ns to parse) is slower than not-decimal (3 ns) - * because all 20 digits are read before the overflow shows. Valid ids parse about 9 ns faster than - * with {@code Long.parseLong}. Before this change, valid-id extraction varied between forks (the 19 - * digit case ran 119-598 ns); after, every fork is within a few ns. Numbers are the median of 5 - * forks, ns/op, with the per-fork range; "before" is the same benchmark on the parent commit, - * without the {@code fromOrNull} arm. - * Apple M1 Max, 10 CPUs - macOS/aarch64 - JDK 25 - * arm trace id header before after - * extract 18 digits 153 (152-153) 107 (100-109) - * extract 19 digits 149 (119-598) 109 (108-110) - * extract unsigned max + 1 840 (821-947) 43 (42-43) - * extract not decimal 882 (760-884) 21 (21-21) - * fromThrowing 18 digits 31 (31-31) 22 (22-23) - * fromThrowing 19 digits 32 (32-33) 24 (23-24) - * fromThrowing unsigned max + 1 875 (771-953) 919 (915-926) - * fromThrowing not decimal 815 (804-836) 804 (792-809) - * fromOrNull 18 digits - 22 (22-23) - * fromOrNull 19 digits - 24 (24-24) - * fromOrNull unsigned max + 1 - 24 (23-24) - * fromOrNull not decimal - 3 (3-3) - * - */ -@State(Scope.Benchmark) -@Warmup(iterations = 3, time = 2, timeUnit = SECONDS) -@Measurement(iterations = 5, time = 2, timeUnit = SECONDS) -@BenchmarkMode(Mode.AverageTime) -@OutputTimeUnit(NANOSECONDS) -@Fork(5) -public class DatadogIdParseBenchmark { - @Param({ - "123456789012345678", // valid, 18 digits - "8217536438929873290", // valid, 19 digits - "18446744073709551616", // invalid, unsigned max + 1 - "1a2b3c4d5e6f" // invalid, not decimal - }) - String traceIdHeader; - - HttpCodec.Extractor extractor; - Map headers; - - @Setup - public void setUp() { - // The test logback.xml on the JMH classpath sets the root logger to DEBUG. Production runs at - // INFO by default, and per-request debug output would swamp the invalid-header arms. - ((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).setLevel(Level.INFO); - - DynamicConfig dynamicConfig = - DynamicConfig.create() - .setHeaderTags(Collections.emptyMap()) - .setBaggageMapping(Collections.emptyMap()) - .apply(); - extractor = DatadogHttpCodec.newExtractor(Config.get(), dynamicConfig::captureTraceConfig); - - headers = new LinkedHashMap<>(); - headers.put(DatadogHttpCodec.TRACE_ID_KEY, traceIdHeader); - headers.put(DatadogHttpCodec.SPAN_ID_KEY, "2345678901234567890"); - headers.put(DatadogHttpCodec.SAMPLING_PRIORITY_KEY, "1"); - headers.put("user-agent", "benchmark"); - } - - @Benchmark - public DDTraceId fromThrowing() { - try { - return DD64bTraceId.from(traceIdHeader); - } catch (NumberFormatException e) { - return null; - } - } - - @Benchmark - public DDTraceId fromOrNull() { - return DD64bTraceId.fromOrNull(traceIdHeader); - } - - @Benchmark - public TagContext extract() { - return extractor.extract(headers, MAP_VISITOR); - } - - private static final AgentPropagation.ContextVisitor> MAP_VISITOR = - new MapContextVisitor(); - - private static final class MapContextVisitor - implements AgentPropagation.ContextVisitor> { - @Override - public void forEachKey(Map carrier, AgentPropagation.KeyClassifier classifier) { - for (Map.Entry entry : carrier.entrySet()) { - if (!classifier.accept(entry.getKey(), entry.getValue())) { - return; - } - } - } - } -} 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 11242afa81e..d9875db6779 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 @@ -1,8 +1,6 @@ package datadog.trace.core.propagation; import static datadog.trace.api.TracePropagationStyle.DATADOG; -import static datadog.trace.api.internal.util.LongStringUtils.isUnsignedLongZero; -import static datadog.trace.api.internal.util.LongStringUtils.parseUnsignedLongOrSentinel; import static datadog.trace.api.telemetry.LogCollector.EXCLUDE_TELEMETRY; import static datadog.trace.core.propagation.HttpCodec.firstHeaderValue; import static datadog.trace.core.propagation.XRayHttpCodec.XRayContextInterpreter.handleXRayTraceHeader; @@ -13,7 +11,6 @@ import datadog.context.propagation.CarrierSetter; import datadog.trace.api.Config; import datadog.trace.api.DD128bTraceId; -import datadog.trace.api.DD64bTraceId; import datadog.trace.api.DDSpanId; import datadog.trace.api.DDTags; import datadog.trace.api.DDTraceId; @@ -175,26 +172,10 @@ public boolean accept(String key, String value) { if (null != value) { switch (classification) { case TRACE_ID: - { - String traceIdValue = firstHeaderValue(value); - DDTraceId parsedTraceId = DD64bTraceId.fromOrNull(traceIdValue); - if (parsedTraceId == null) { - return rejectInvalidId(TRACE_ID_KEY, traceIdValue); - } - traceId = parsedTraceId; - } + traceId = DDTraceId.from(firstHeaderValue(value)); break; case SPAN_ID: - { - String spanIdValue = firstHeaderValue(value); - int len = spanIdValue.length(); - long parsedSpanId = - parseUnsignedLongOrSentinel(spanIdValue, 0, len, DDSpanId.ZERO); - if (parsedSpanId == DDSpanId.ZERO && !isUnsignedLongZero(spanIdValue, 0, len)) { - return rejectInvalidId(SPAN_ID_KEY, spanIdValue); - } - spanId = parsedSpanId; - } + spanId = DDSpanId.from(firstHeaderValue(value)); break; case ORIGIN: origin = firstHeaderValue(value); @@ -214,6 +195,10 @@ public boolean accept(String key, String value) { default: } } + } catch (NumberFormatException e) { + invalidateContext(); + log.debug(EXCLUDE_TELEMETRY, "Ignoring context with invalid {}: {}", key, value); + return false; } catch (RuntimeException e) { invalidateContext(); log.debug("Exception when extracting context", e); @@ -229,14 +214,6 @@ public boolean accept(String key, String value) { return true; } - // Malformed ids come from the caller, not from a tracer bug, and repeat on every request from - // that caller, so they are rejected without an exception and kept out of telemetry - private boolean rejectInvalidId(String headerName, String headerValue) { - invalidateContext(); - log.debug(EXCLUDE_TELEMETRY, "Ignoring context with invalid {}: {}", headerName, headerValue); - return false; - } - @Override protected TagContext build() { restore128bTraceId(); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java index b6df5b8455d..61e9a8f105e 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/propagation/HttpCodec.java @@ -2,8 +2,6 @@ import static datadog.trace.api.DDTags.PARENT_ID; import static datadog.trace.api.TracePropagationStyle.TRACECONTEXT; -import static datadog.trace.api.internal.util.LongStringUtils.isUnsignedLongZero; -import static datadog.trace.api.internal.util.LongStringUtils.parseUnsignedLongOrSentinel; import static datadog.trace.core.propagation.DatadogHttpCodec.SPAN_ID_KEY; import datadog.context.propagation.CarrierSetter; @@ -331,14 +329,11 @@ public boolean accept(String key, String value) { } private void cacheDatadogSpanId(String key, String value) { - if (SPAN_ID_KEY.equalsIgnoreCase(key) && value != null) { - // Parse numeric header value to format it as 16 hexadecimal character format. Same - // non-throwing parse as DatadogHttpCodec, so malformed ids cost no exception and both agree - // on which ids are valid. - int len = value.length(); - long spanId = parseUnsignedLongOrSentinel(value, 0, len, DDSpanId.ZERO); - if (spanId != DDSpanId.ZERO || isUnsignedLongZero(value, 0, len)) { - this.datadogSpanIdHex = DDSpanId.toHexStringPadded(spanId); + if (SPAN_ID_KEY.equalsIgnoreCase(key)) { + try { + // Parse numeric header value to format it as 16 hexadecimal character format + this.datadogSpanIdHex = DDSpanId.toHexStringPadded(DDSpanId.from(value)); + } catch (NumberFormatException ignored) { } } } 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 59372fa0459..3eaa1bf5aa1 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 @@ -269,8 +269,6 @@ void extractHttpHeadersWithOutOfRangeSpanId() { "out-of-range traceId | 'MAX+1' | '1' | false ", "uint64 max spanId | '1' | 'MAX' | true ", "out-of-range spanId | '1' | 'MAX+1' | false ", - "leading plus traceId | '+1' | '1' | false ", - "leading plus spanId | '1' | '+1' | false ", "empty traceId | '' | '1' | false ", "empty spanId | '1' | '' | false ", "hex traceId | '0x1a' | '1' | false ", diff --git a/dd-trace-core/src/test/java/datadog/trace/core/propagation/HttpExtractorTest.java b/dd-trace-core/src/test/java/datadog/trace/core/propagation/HttpExtractorTest.java index 5dc2d7b7db1..f2b9abbd05b 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/propagation/HttpExtractorTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/propagation/HttpExtractorTest.java @@ -137,8 +137,7 @@ void extractHttpHeadersUsingStyles( "B3MULTI,TRACECONTEXT with traceState p | [B3MULTI, TRACECONTEXT] | | | '1' | '2' | 'W3C_TRACE_STATE_WITH_P' | '1' | 'W3C_SPAN_ID_LSTR' | 'W3C_PARENT_ID' ", "B3MULTI,DATADOG,TRACECONTEXT no traceState | [B3MULTI, DATADOG, TRACECONTEXT] | '1' | '2' | '1' | '4' | | '1' | 'W3C_SPAN_ID_LSTR' | '0000000000000002'", "DATADOG,TRACECONTEXT no p traceState | [DATADOG, TRACECONTEXT] | '1' | '2' | | | 'W3C_TRACE_STATE_NO_P' | '1' | 'W3C_SPAN_ID_LSTR' | '0000000000000002'", - "DATADOG,TRACECONTEXT malformed span id | [DATADOG, TRACECONTEXT] | '1' | 'abc' | | | 'W3C_TRACE_STATE_NO_P' | '1' | 'W3C_SPAN_ID_LSTR' | ", - "DATADOG,TRACECONTEXT signed span id | [DATADOG, TRACECONTEXT] | '1' | '+2' | | | 'W3C_TRACE_STATE_NO_P' | '1' | 'W3C_SPAN_ID_LSTR' | " + "DATADOG,TRACECONTEXT malformed span id | [DATADOG, TRACECONTEXT] | '1' | 'abc' | | | 'W3C_TRACE_STATE_NO_P' | '1' | 'W3C_SPAN_ID_LSTR' | " }) void checkW3CTraceContextOverride( List styles, From adc82087b89e46003c78908c98d40d325a7895a9 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 19 Oct 2026 13:21:05 +0200 Subject: [PATCH 2/2] chore(code): Restore Javadoc --- .../datadog/trace/api/internal/util/LongStringUtils.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/internal/util/LongStringUtils.java b/dd-trace-api/src/main/java/datadog/trace/api/internal/util/LongStringUtils.java index aa55b8943fb..4cf5c652a55 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/internal/util/LongStringUtils.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/internal/util/LongStringUtils.java @@ -73,6 +73,13 @@ private static int firstNonZeroCharacter(CharSequence s, int start) { return firstNonZero; } + /** + * Parse the decimal representation of the unsigned 64 bit long from the {@code String}. + * + * @param s String in decimal of unsigned 64-bits long. + * @return long + * @throws NumberFormatException + */ public static long parseUnsignedLong(String s) throws NumberFormatException { if (s == null) { throw new NumberFormatException("s can't be null");