From 71ad8ff646500814e08fef38aa7509ad1681bd35 Mon Sep 17 00:00:00 2001 From: Jakub Wach Date: Fri, 2 Apr 2021 10:52:07 +0200 Subject: [PATCH] X-Ray traceid extraction - 64bit support (#3087) * X-Ray traceid extraction - 64bit support * perf improvement - separate methods depending on traceid length * code review * code review - scare-comment for internal util * added new unit tests * Update api/all/src/main/java/io/opentelemetry/api/internal/StringUtils.java Co-authored-by: Anuraag Agrawal --- .../api/internal/StringUtils.java | 68 +++++++++++++- .../api/internal/StringUtilsTest.java | 27 ++++++ .../extension/aws/AwsXrayPropagator.java | 40 ++++++++- .../extension/aws/AwsXrayPropagatorTest.java | 90 +++++++++++++++++++ .../B3PropagatorExtractorSingleHeader.java | 1 + .../extension/trace/propagation/Common.java | 1 + .../trace/propagation/JaegerPropagator.java | 1 + .../trace/propagation/OtTracePropagator.java | 1 + .../trace/propagation/StringUtils.java | 82 ----------------- .../trace/propagation/B3PropagatorTest.java | 1 + .../trace/propagation/StringUtilsTest.java | 40 --------- 11 files changed, 225 insertions(+), 127 deletions(-) delete mode 100644 extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/StringUtils.java delete mode 100644 extensions/trace-propagators/src/test/java/io/opentelemetry/extension/trace/propagation/StringUtilsTest.java diff --git a/api/all/src/main/java/io/opentelemetry/api/internal/StringUtils.java b/api/all/src/main/java/io/opentelemetry/api/internal/StringUtils.java index 4dc89177a3..0af3b5cdb1 100644 --- a/api/all/src/main/java/io/opentelemetry/api/internal/StringUtils.java +++ b/api/all/src/main/java/io/opentelemetry/api/internal/StringUtils.java @@ -5,12 +5,78 @@ package io.opentelemetry.api.internal; +import java.util.Objects; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; -/** Internal utility methods for working with attribute keys, attribute values, and metric names. */ +/** + * Utilities for working with strings. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. + */ @Immutable public final class StringUtils { + /** + * Returns {@code true} if the given string is null or is the empty string. + * + *

This method was copied verbatim from Guava library method + * com.google.common.base.Strings#isNullOrEmpty(java.lang.String). + * + * @param string a string reference to check + * @return {@code true} if the string is null or is the empty string + */ + public static boolean isNullOrEmpty(@Nullable String string) { + return string == null || string.isEmpty(); + } + + /** + * Pads a given string on the left with leading 0's up the length. + * + * @param value the string to pad + * @param minLength the minimum length the resulting padded string must have. Can be zero or + * negative, in which case the input string is always returned. + * @return the padded string + */ + public static String padLeft(String value, int minLength) { + return padStart(value, minLength, '0'); + } + + /** + * Returns a string, of length at least {@code minLength}, consisting of {@code string} prepended + * with as many copies of {@code padChar} as are necessary to reach that length. For example, + * + *

+ * + *

See {@link java.util.Formatter} for a richer set of formatting capabilities. + * + *

This method was copied almost verbatim from Guava library method + * com.google.common.base.Strings#padStart(java.lang.String, int, char). + * + * @param string the string which should appear at the end of the result + * @param minLength the minimum length the resulting string must have. Can be zero or negative, in + * which case the input string is always returned. + * @param padChar the character to insert at the beginning of the result until the minimum length + * is reached + * @return the padded string + */ + private static String padStart(String string, int minLength, char padChar) { + Objects.requireNonNull(string); + if (string.length() >= minLength) { + return string; + } + StringBuilder sb = new StringBuilder(minLength); + for (int i = string.length(); i < minLength; i++) { + sb.append(padChar); + } + sb.append(string); + return sb.toString(); + } + /** * Determines whether the {@code String} contains only printable characters. * diff --git a/api/all/src/test/java/io/opentelemetry/api/internal/StringUtilsTest.java b/api/all/src/test/java/io/opentelemetry/api/internal/StringUtilsTest.java index d9a885fc9f..cc214bc323 100644 --- a/api/all/src/test/java/io/opentelemetry/api/internal/StringUtilsTest.java +++ b/api/all/src/test/java/io/opentelemetry/api/internal/StringUtilsTest.java @@ -6,6 +6,7 @@ package io.opentelemetry.api.internal; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import org.junit.jupiter.api.Test; @@ -18,4 +19,30 @@ class StringUtilsTest { assertThat(StringUtils.isPrintableString("\u0002ab")).isFalse(); assertThat(StringUtils.isPrintableString("\u0127ab")).isFalse(); } + + @Test + void isNullOrEmpty() { + assertThat(StringUtils.isNullOrEmpty("")).isTrue(); + assertThat(StringUtils.isNullOrEmpty(null)).isTrue(); + assertThat(StringUtils.isNullOrEmpty("hello")).isFalse(); + assertThat(StringUtils.isNullOrEmpty(" ")).isFalse(); + } + + @Test + void padLeft() { + assertThat(StringUtils.padLeft("value", 10)).isEqualTo("00000value"); + } + + @Test + void padLeft_throws_for_null_value() { + assertThatThrownBy(() -> StringUtils.padLeft(null, 10)) + .isInstanceOf(NullPointerException.class); + } + + @Test + void padLeft_length_does_not_exceed_length() { + assertThat(StringUtils.padLeft("value", 3)).isEqualTo("value"); + assertThat(StringUtils.padLeft("value", -10)).isEqualTo("value"); + assertThat(StringUtils.padLeft("value", 0)).isEqualTo("value"); + } } diff --git a/extensions/aws/src/main/java/io/opentelemetry/extension/aws/AwsXrayPropagator.java b/extensions/aws/src/main/java/io/opentelemetry/extension/aws/AwsXrayPropagator.java index e86133d8ef..928cc68af9 100644 --- a/extensions/aws/src/main/java/io/opentelemetry/extension/aws/AwsXrayPropagator.java +++ b/extensions/aws/src/main/java/io/opentelemetry/extension/aws/AwsXrayPropagator.java @@ -5,6 +5,7 @@ package io.opentelemetry.extension.aws; +import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.SpanId; @@ -193,16 +194,19 @@ public final class AwsXrayPropagator implements TextMapPropagator { } return SpanContext.createFromRemoteParent( - traceId, + StringUtils.padLeft(traceId, TraceId.getLength()), spanId, isSampled ? TraceFlags.getSampled() : TraceFlags.getDefault(), TraceState.getDefault()); } private static String parseTraceId(String xrayTraceId) { - if (xrayTraceId.length() != TRACE_ID_LENGTH) { - return TraceId.getInvalid(); - } + return (xrayTraceId.length() == TRACE_ID_LENGTH + ? parseSpecTraceId(xrayTraceId) + : parseShortTraceId(xrayTraceId)); + } + + private static String parseSpecTraceId(String xrayTraceId) { // Check version trace id version if (!xrayTraceId.startsWith(TRACE_ID_VERSION)) { @@ -223,6 +227,34 @@ public final class AwsXrayPropagator implements TextMapPropagator { return epochPart + uniquePart; } + private static String parseShortTraceId(String xrayTraceId) { + if (xrayTraceId.length() > TRACE_ID_LENGTH) { + return TraceId.getInvalid(); + } + + // Check version trace id version + if (!xrayTraceId.startsWith(TRACE_ID_VERSION)) { + return TraceId.getInvalid(); + } + + // Check delimiters + int firstDelimiter = xrayTraceId.indexOf(TRACE_ID_DELIMITER); + // we don't allow the epoch part to be missing completely + int secondDelimiter = xrayTraceId.indexOf(TRACE_ID_DELIMITER, firstDelimiter + 2); + if (firstDelimiter != TRACE_ID_DELIMITER_INDEX_1 + || secondDelimiter == -1 + || secondDelimiter > TRACE_ID_DELIMITER_INDEX_2) { + return TraceId.getInvalid(); + } + + String epochPart = xrayTraceId.substring(firstDelimiter + 1, secondDelimiter); + String uniquePart = xrayTraceId.substring(secondDelimiter + 1, secondDelimiter + 25); + + // X-Ray trace id format is 1-{at most 8 digit hex}-{24 digit hex} + // epoch part can have leading 0s truncated + return epochPart + uniquePart; + } + private static String parseSpanId(String xrayParentId) { if (xrayParentId.length() != PARENT_ID_LENGTH) { return SpanId.getInvalid(); diff --git a/extensions/aws/src/test/java/io/opentelemetry/extension/aws/AwsXrayPropagatorTest.java b/extensions/aws/src/test/java/io/opentelemetry/extension/aws/AwsXrayPropagatorTest.java index 12718f0f80..ed63dbd00b 100644 --- a/extensions/aws/src/test/java/io/opentelemetry/extension/aws/AwsXrayPropagatorTest.java +++ b/extensions/aws/src/test/java/io/opentelemetry/extension/aws/AwsXrayPropagatorTest.java @@ -276,6 +276,96 @@ class AwsXrayPropagatorTest { assertThat(xrayPropagator.extract(context, Collections.emptyMap(), null)).isSameAs(context); } + @Test + void extract_EpochPart_ZeroedSingleDigit() { + Map carrier = new LinkedHashMap<>(); + carrier.put( + TRACE_HEADER_KEY, + "Root=1-0-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=1;Foo=Bar"); + + assertThat(getSpanContext(xrayPropagator.extract(Context.current(), carrier, getter))) + .isEqualTo( + SpanContext.createFromRemoteParent( + "00000000d188f8fa79d48a391a778fa6", + SPAN_ID, + TraceFlags.getSampled(), + TraceState.getDefault())); + } + + @Test + void extract_EpochPart_TwoChars() { + Map carrier = new LinkedHashMap<>(); + carrier.put( + TRACE_HEADER_KEY, + "Root=1-1a-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=1;Foo=Bar"); + + assertThat(getSpanContext(xrayPropagator.extract(Context.current(), carrier, getter))) + .isEqualTo( + SpanContext.createFromRemoteParent( + "0000001ad188f8fa79d48a391a778fa6", + SPAN_ID, + TraceFlags.getSampled(), + TraceState.getDefault())); + } + + @Test + void extract_EpochPart_Zeroed() { + Map carrier = new LinkedHashMap<>(); + carrier.put( + TRACE_HEADER_KEY, + "Root=1-00000000-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=1;Foo=Bar"); + + assertThat(getSpanContext(xrayPropagator.extract(Context.current(), carrier, getter))) + .isEqualTo( + SpanContext.createFromRemoteParent( + "00000000d188f8fa79d48a391a778fa6", + SPAN_ID, + TraceFlags.getSampled(), + TraceState.getDefault())); + } + + @Test + void extract_InvalidTraceId_EpochPart_TooLong() { + Map invalidHeaders = new LinkedHashMap<>(); + invalidHeaders.put( + TRACE_HEADER_KEY, + "Root=1-8a3c60f711-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=0"); + + assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter))) + .isSameAs(SpanContext.getInvalid()); + } + + @Test + void extract_InvalidTraceId_EpochPart_Empty() { + Map invalidHeaders = new LinkedHashMap<>(); + invalidHeaders.put( + TRACE_HEADER_KEY, "Root=1--d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=0"); + + assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter))) + .isSameAs(SpanContext.getInvalid()); + } + + @Test + void extract_InvalidTraceId_EpochPart_Missing() { + Map invalidHeaders = new LinkedHashMap<>(); + invalidHeaders.put( + TRACE_HEADER_KEY, "Root=1-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=0"); + + assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter))) + .isSameAs(SpanContext.getInvalid()); + } + + @Test + void extract_InvalidTraceId_WrongVersion() { + Map carrier = new LinkedHashMap<>(); + carrier.put( + TRACE_HEADER_KEY, + "Root=2-1a2a3a4a-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=1;Foo=Bar"); + + assertThat(getSpanContext(xrayPropagator.extract(Context.current(), carrier, getter))) + .isSameAs(SpanContext.getInvalid()); + } + private static Context withSpanContext(SpanContext spanContext, Context context) { return context.with(Span.wrap(spanContext)); } diff --git a/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/B3PropagatorExtractorSingleHeader.java b/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/B3PropagatorExtractorSingleHeader.java index 4e8f7b8a76..bfd24b9768 100644 --- a/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/B3PropagatorExtractorSingleHeader.java +++ b/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/B3PropagatorExtractorSingleHeader.java @@ -5,6 +5,7 @@ package io.opentelemetry.extension.trace.propagation; +import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapGetter; diff --git a/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/Common.java b/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/Common.java index 02ca37bc31..6eefa04269 100644 --- a/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/Common.java +++ b/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/Common.java @@ -5,6 +5,7 @@ package io.opentelemetry.extension.trace.propagation; +import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.SpanId; import io.opentelemetry.api.trace.TraceFlags; diff --git a/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/JaegerPropagator.java b/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/JaegerPropagator.java index efd649efa2..37ea3b4680 100644 --- a/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/JaegerPropagator.java +++ b/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/JaegerPropagator.java @@ -7,6 +7,7 @@ package io.opentelemetry.extension.trace.propagation; import io.opentelemetry.api.baggage.Baggage; import io.opentelemetry.api.baggage.BaggageBuilder; +import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.SpanId; diff --git a/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/OtTracePropagator.java b/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/OtTracePropagator.java index b4806c371e..c5ca2f04e1 100644 --- a/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/OtTracePropagator.java +++ b/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/OtTracePropagator.java @@ -9,6 +9,7 @@ import static io.opentelemetry.extension.trace.propagation.Common.MAX_TRACE_ID_L import io.opentelemetry.api.baggage.Baggage; import io.opentelemetry.api.baggage.BaggageBuilder; +import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.TraceId; diff --git a/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/StringUtils.java b/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/StringUtils.java deleted file mode 100644 index 4cb1eb4ace..0000000000 --- a/extensions/trace-propagators/src/main/java/io/opentelemetry/extension/trace/propagation/StringUtils.java +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -// Includes work from: -/* - * Copyright 2010, Guava Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.extension.trace.propagation; - -import java.util.Objects; -import javax.annotation.Nullable; -import javax.annotation.concurrent.Immutable; - -/** Utilities for working with strings. */ -@Immutable -final class StringUtils { - - /** - * Returns a string, of length at least {@code minLength}, consisting of {@code string} prepended - * with as many copies of {@code padChar} as are necessary to reach that length. For example, - * - *

- * - *

See {@link java.util.Formatter} for a richer set of formatting capabilities. - * - *

This method was copied almost verbatim from Guava library method - * com.google.common.base.Strings#padStart(java.lang.String, int, char). - * - * @param string the string which should appear at the end of the result - * @param minLength the minimum length the resulting string must have. Can be zero or negative, in - * which case the input string is always returned. - * @param padChar the character to insert at the beginning of the result until the minimum length - * is reached - * @return the padded string - */ - static String padStart(String string, int minLength, char padChar) { - Objects.requireNonNull(string); - if (string.length() >= minLength) { - return string; - } - StringBuilder sb = new StringBuilder(minLength); - for (int i = string.length(); i < minLength; i++) { - sb.append(padChar); - } - sb.append(string); - return sb.toString(); - } - - /** - * Returns {@code true} if the given string is null or is the empty string. - * - *

This method was copied verbatim from Guava library method - * com.google.common.base.Strings#isNullOrEmpty(java.lang.String). - * - * @param string a string reference to check - * @return {@code true} if the string is null or is the empty string - */ - static boolean isNullOrEmpty(@Nullable String string) { - return string == null || string.isEmpty(); - } - - /** - * Pads a given string on the left with leading 0's up the length. - * - * @param value the string to pad - * @param minLength the minimum length the resulting padded string must have. Can be zero or - * negative, in which case the input string is always returned. - * @return the padded string - */ - static String padLeft(String value, int minLength) { - return padStart(value, minLength, '0'); - } - - private StringUtils() {} -} diff --git a/extensions/trace-propagators/src/test/java/io/opentelemetry/extension/trace/propagation/B3PropagatorTest.java b/extensions/trace-propagators/src/test/java/io/opentelemetry/extension/trace/propagation/B3PropagatorTest.java index aa95b9f635..8db8c0098b 100644 --- a/extensions/trace-propagators/src/test/java/io/opentelemetry/extension/trace/propagation/B3PropagatorTest.java +++ b/extensions/trace-propagators/src/test/java/io/opentelemetry/extension/trace/propagation/B3PropagatorTest.java @@ -9,6 +9,7 @@ import static io.opentelemetry.extension.trace.propagation.B3Propagator.DEBUG_CO import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; +import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.SpanId; diff --git a/extensions/trace-propagators/src/test/java/io/opentelemetry/extension/trace/propagation/StringUtilsTest.java b/extensions/trace-propagators/src/test/java/io/opentelemetry/extension/trace/propagation/StringUtilsTest.java deleted file mode 100644 index 8650cbc8d7..0000000000 --- a/extensions/trace-propagators/src/test/java/io/opentelemetry/extension/trace/propagation/StringUtilsTest.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.extension.trace.propagation; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -import org.junit.jupiter.api.Test; - -class StringUtilsTest { - - @Test - void isNullOrEmpty() { - assertThat(StringUtils.isNullOrEmpty("")).isTrue(); - assertThat(StringUtils.isNullOrEmpty(null)).isTrue(); - assertThat(StringUtils.isNullOrEmpty("hello")).isFalse(); - assertThat(StringUtils.isNullOrEmpty(" ")).isFalse(); - } - - @Test - void padLeft() { - assertThat(StringUtils.padLeft("value", 10)).isEqualTo("00000value"); - } - - @Test - void padLeft_throws_for_null_value() { - assertThatThrownBy(() -> StringUtils.padLeft(null, 10)) - .isInstanceOf(NullPointerException.class); - } - - @Test - void padLeft_length_does_not_exceed_length() { - assertThat(StringUtils.padLeft("value", 3)).isEqualTo("value"); - assertThat(StringUtils.padLeft("value", -10)).isEqualTo("value"); - assertThat(StringUtils.padLeft("value", 0)).isEqualTo("value"); - } -}