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..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 @@ -81,85 +81,6 @@ private static int firstNonZeroCharacter(CharSequence s, int start) { * @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,