From d078667b251a0450941f3accbb6e73d9484f3d5e Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 1 Feb 2024 12:22:35 +1100 Subject: [PATCH 1/2] remove baggage propagation from xray propagator --- .../awsxray/propagator/AwsXrayPropagator.java | 72 +++--------- .../propagator/AwsXrayPropagatorTest.java | 106 ------------------ 2 files changed, 13 insertions(+), 165 deletions(-) diff --git a/aws-xray-propagator/src/main/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagator.java b/aws-xray-propagator/src/main/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagator.java index ef7a4cf8a..086bacd45 100644 --- a/aws-xray-propagator/src/main/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagator.java +++ b/aws-xray-propagator/src/main/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagator.java @@ -5,9 +5,6 @@ package io.opentelemetry.contrib.awsxray.propagator; -import io.opentelemetry.api.baggage.Baggage; -import io.opentelemetry.api.baggage.BaggageBuilder; -import io.opentelemetry.api.baggage.BaggageEntry; import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; @@ -21,7 +18,6 @@ import io.opentelemetry.context.propagation.TextMapSetter; import java.util.Collections; import java.util.List; -import java.util.function.BiConsumer; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -112,49 +108,19 @@ public void inject(Context context, @Nullable C carrier, TextMapSetter se char samplingFlag = spanContext.isSampled() ? IS_SAMPLED : NOT_SAMPLED; // TODO: Add OT trace state to the X-Ray trace header - StringBuilder traceHeader = new StringBuilder(); - traceHeader - .append(TRACE_ID_KEY) - .append(KV_DELIMITER) - .append(xrayTraceId) - .append(TRACE_HEADER_DELIMITER) - .append(PARENT_ID_KEY) - .append(KV_DELIMITER) - .append(parentId) - .append(TRACE_HEADER_DELIMITER) - .append(SAMPLED_FLAG_KEY) - .append(KV_DELIMITER) - .append(samplingFlag); - - Baggage baggage = Baggage.fromContext(context); - // Truncate baggage to 256 chars per X-Ray spec. - baggage.forEach( - new BiConsumer() { - - private int baggageWrittenBytes; - - @Override - public void accept(String key, BaggageEntry entry) { - if (key.equals(TRACE_ID_KEY) - || key.equals(PARENT_ID_KEY) - || key.equals(SAMPLED_FLAG_KEY)) { - return; - } - // Size is key/value pair, excludes delimiter. - int size = key.length() + entry.getValue().length() + 1; - if (baggageWrittenBytes + size > 256) { - return; - } - traceHeader - .append(TRACE_HEADER_DELIMITER) - .append(key) - .append(KV_DELIMITER) - .append(entry.getValue()); - baggageWrittenBytes += size; - } - }); - - setter.set(carrier, TRACE_HEADER_KEY, traceHeader.toString()); + String traceHeader = TRACE_ID_KEY + + KV_DELIMITER + + xrayTraceId + + TRACE_HEADER_DELIMITER + + PARENT_ID_KEY + + KV_DELIMITER + + parentId + + TRACE_HEADER_DELIMITER + + SAMPLED_FLAG_KEY + + KV_DELIMITER + + samplingFlag; + + setter.set(carrier, TRACE_HEADER_KEY, traceHeader); } @Override @@ -180,9 +146,6 @@ private static Context getContextFromHeader( String spanId = SpanId.getInvalid(); Boolean isSampled = false; - BaggageBuilder baggage = null; - int baggageReadBytes = 0; - int pos = 0; while (pos < traceHeader.length()) { int delimiterIndex = traceHeader.indexOf(TRACE_HEADER_DELIMITER, pos); @@ -210,12 +173,6 @@ private static Context getContextFromHeader( spanId = parseSpanId(value); } else if (trimmedPart.startsWith(SAMPLED_FLAG_KEY)) { isSampled = parseTraceFlag(value); - } else if (baggageReadBytes + trimmedPart.length() <= 256) { - if (baggage == null) { - baggage = Baggage.builder(); - } - baggage.put(trimmedPart.substring(0, equalsIndex), value); - baggageReadBytes += trimmedPart.length(); } } if (isSampled == null) { @@ -241,9 +198,6 @@ private static Context getContextFromHeader( if (spanContext.isValid()) { context = context.with(Span.wrap(spanContext)); } - if (baggage != null) { - context = context.with(baggage.build()); - } return context; } diff --git a/aws-xray-propagator/src/test/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagatorTest.java b/aws-xray-propagator/src/test/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagatorTest.java index 2637e78e5..f94da194a 100644 --- a/aws-xray-propagator/src/test/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagatorTest.java +++ b/aws-xray-propagator/src/test/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagatorTest.java @@ -8,7 +8,6 @@ import static io.opentelemetry.contrib.awsxray.propagator.AwsXrayPropagator.TRACE_HEADER_KEY; import static org.assertj.core.api.Assertions.assertThat; -import io.opentelemetry.api.baggage.Baggage; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.TraceFlags; @@ -21,8 +20,6 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; import javax.annotation.Nullable; import org.junit.jupiter.api.Test; @@ -89,63 +86,6 @@ void inject_NotSampledContext() { "Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=0"); } - @Test - void inject_WithBaggage() { - Map carrier = new LinkedHashMap<>(); - subject.inject( - withSpanContext( - SpanContext.create( - TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault()), - Context.current()) - .with( - Baggage.builder() - .put("cat", "meow") - .put("dog", "bark") - .put("Root", "ignored") - .put("Parent", "ignored") - .put("Sampled", "ignored") - .build()), - carrier, - SETTER); - - assertThat(carrier) - .containsEntry( - TRACE_HEADER_KEY, - "Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=0;" - + "cat=meow;dog=bark"); - } - - @Test - void inject_WithBaggage_LimitTruncates() { - Map carrier = new LinkedHashMap<>(); - // Limit is 256 characters for all baggage. We add a 254-character key/value pair and a - // 3 character key value pair. - String key1 = Stream.generate(() -> "a").limit(252).collect(Collectors.joining()); - String value1 = "a"; // 252 + 1 (=) + 1 = 254 - - String key2 = "b"; - String value2 = "b"; // 1 + 1 (=) + 1 = 3 - - Baggage baggage = Baggage.builder().put(key1, value1).put(key2, value2).build(); - - subject.inject( - withSpanContext( - SpanContext.create( - TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault()), - Context.current()) - .with(baggage), - carrier, - SETTER); - - assertThat(carrier) - .containsEntry( - TRACE_HEADER_KEY, - "Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=0;" - + key1 - + '=' - + value1); - } - @Test void inject_WithTraceState() { Map carrier = new LinkedHashMap<>(); @@ -232,52 +172,6 @@ void extract_DifferentPartOrder() { TRACE_ID, SPAN_ID, TraceFlags.getSampled(), TraceState.getDefault())); } - @Test - void extract_AdditionalFields() { - Map carrier = new LinkedHashMap<>(); - carrier.put( - TRACE_HEADER_KEY, - "Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=1;Foo=Bar"); - - Context context = subject.extract(Context.current(), carrier, GETTER); - assertThat(getSpanContext(context)) - .isEqualTo( - SpanContext.createFromRemoteParent( - TRACE_ID, SPAN_ID, TraceFlags.getSampled(), TraceState.getDefault())); - assertThat(Baggage.fromContext(context).getEntryValue("Foo")).isEqualTo("Bar"); - } - - @Test - void extract_Baggage_LimitTruncates() { - // Limit is 256 characters for all baggage. We add a 254-character key/value pair and a - // 3 character key value pair. - String key1 = Stream.generate(() -> "a").limit(252).collect(Collectors.joining()); - String value1 = "a"; // 252 + 1 (=) + 1 = 254 - - String key2 = "b"; - String value2 = "b"; // 1 + 1 (=) + 1 = 3 - - Map carrier = new LinkedHashMap<>(); - carrier.put( - TRACE_HEADER_KEY, - "Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=1;" - + key1 - + '=' - + value1 - + ';' - + key2 - + '=' - + value2); - - Context context = subject.extract(Context.current(), carrier, GETTER); - assertThat(getSpanContext(context)) - .isEqualTo( - SpanContext.createFromRemoteParent( - TRACE_ID, SPAN_ID, TraceFlags.getSampled(), TraceState.getDefault())); - assertThat(Baggage.fromContext(context).getEntryValue(key1)).isEqualTo(value1); - assertThat(Baggage.fromContext(context).getEntryValue(key2)).isNull(); - } - @Test void extract_EmptyHeaderValue() { Map invalidHeaders = new LinkedHashMap<>(); From 4dde536328a50b617e96a33dd3bee72e69c17931 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 1 Feb 2024 13:39:44 +1100 Subject: [PATCH 2/2] spotless --- .../awsxray/propagator/AwsXrayPropagator.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/aws-xray-propagator/src/main/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagator.java b/aws-xray-propagator/src/main/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagator.java index 086bacd45..83bf4bb69 100644 --- a/aws-xray-propagator/src/main/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagator.java +++ b/aws-xray-propagator/src/main/java/io/opentelemetry/contrib/awsxray/propagator/AwsXrayPropagator.java @@ -108,17 +108,18 @@ public void inject(Context context, @Nullable C carrier, TextMapSetter se char samplingFlag = spanContext.isSampled() ? IS_SAMPLED : NOT_SAMPLED; // TODO: Add OT trace state to the X-Ray trace header - String traceHeader = TRACE_ID_KEY + - KV_DELIMITER + - xrayTraceId + - TRACE_HEADER_DELIMITER + - PARENT_ID_KEY + - KV_DELIMITER + - parentId + - TRACE_HEADER_DELIMITER + - SAMPLED_FLAG_KEY + - KV_DELIMITER + - samplingFlag; + String traceHeader = + TRACE_ID_KEY + + KV_DELIMITER + + xrayTraceId + + TRACE_HEADER_DELIMITER + + PARENT_ID_KEY + + KV_DELIMITER + + parentId + + TRACE_HEADER_DELIMITER + + SAMPLED_FLAG_KEY + + KV_DELIMITER + + samplingFlag; setter.set(carrier, TRACE_HEADER_KEY, traceHeader); }