From 79213ca1d7418b57a590754e47090621300c42a0 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 7 Nov 2024 12:11:33 +1100 Subject: [PATCH 1/8] adds getList() method to TextMapGetter, with default method --- .../context/propagation/TextMapGetter.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java b/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java index f160b7857ab..77af56cad7b 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java @@ -5,6 +5,8 @@ package io.opentelemetry.context.propagation; +import java.util.Collections; +import java.util.List; import javax.annotation.Nullable; /** @@ -33,4 +35,20 @@ public interface TextMapGetter { */ @Nullable String get(@Nullable C carrier, String key); + + /** + * If implemented, returns all values for a given {@code key}, in order. + * + *

The default method returns the first value of the given propagation {@code key} as a + * singleton list, or returns an empty list. + * + * @param carrier carrier of propagation fields, such as an http request. + * @param key the key of the field. + * @return the first value of the given propagation {@code key} as a singleton list, or returns an + * empty list. + */ + default List getList(@Nullable C carrier, String key) { + String first = get(carrier, key); + return first == null ? Collections.emptyList() : Collections.singletonList(first); + } } From 46fa12d97e1850c01d2a89cbf8dbcf89f9c84882 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Thu, 7 Nov 2024 15:18:10 +1100 Subject: [PATCH 2/8] changes W3CBaggagePropagator to handle multiple headers for baggage, using new getList() method on the TextMapGetter. Includes tests. --- .../propagation/W3CBaggagePropagator.java | 31 ++++--- .../propagation/W3CBaggagePropagatorTest.java | 90 +++++++++++++++++++ .../context/propagation/TextMapGetter.java | 2 +- 3 files changed, 112 insertions(+), 11 deletions(-) diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java index 150be6e4fdd..e7361bd0683 100644 --- a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java @@ -86,6 +86,12 @@ private static String encodeValue(String value) { return URL_ESCAPER.escape(value); } + /** + * @param context the {@code Context} used to store the extracted value. + * @param carrier holds propagation fields. For example, an outgoing message or http request. + * @param getter invoked for each propagation key to get data from the carrier. + * @return the extracted context + */ @Override public Context extract(Context context, @Nullable C carrier, TextMapGetter getter) { if (context == null) { @@ -95,21 +101,26 @@ public Context extract(Context context, @Nullable C carrier, TextMapGetter baggageHeaders = getter.getList(carrier, FIELD); + if (baggageHeaders == null || baggageHeaders.isEmpty()) { return context; } + boolean extracted = false; BaggageBuilder baggageBuilder = Baggage.builder(); - try { - extractEntries(baggageHeader, baggageBuilder); - } catch (RuntimeException e) { - return context; + for (String header : baggageHeaders) { + if (header.isEmpty()) { + continue; + } + + try { + extractEntries(header, baggageBuilder); + extracted = true; + } catch (RuntimeException expected) { + // invalid baggage header, continue + } } - return context.with(baggageBuilder.build()); + return extracted ? context.with(baggageBuilder.build()) : context; } private static void extractEntries(String baggageHeader, BaggageBuilder baggageBuilder) { diff --git a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java index e775ed04f1e..d2f05dcef43 100644 --- a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java +++ b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java @@ -8,6 +8,7 @@ import static java.util.Collections.singletonMap; import static org.assertj.core.api.Assertions.assertThat; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import io.opentelemetry.api.baggage.Baggage; import io.opentelemetry.api.baggage.BaggageEntryMetadata; @@ -16,6 +17,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import javax.annotation.Nullable; import org.junit.jupiter.api.Test; @@ -36,6 +38,27 @@ public String get(Map carrier, String key) { } }; + private static final TextMapGetter>> multiGetter = + new TextMapGetter>>() { + @Override + public Iterable keys(Map> carrier) { + return carrier.keySet(); + } + + @Nullable + @Override + public String get(Map> carrier, String key) { + return carrier.getOrDefault(key, Collections.emptyList()).stream() + .findFirst() + .orElse(null); + } + + @Override + public List getList(Map> carrier, String key) { + return carrier.getOrDefault(key, Collections.emptyList()); + } + }; + @Test void fields() { assertThat(W3CBaggagePropagator.getInstance().fields()).containsExactly("baggage"); @@ -421,6 +444,73 @@ void extract_nullGetter() { .isSameAs(context); } + @Test + void extract_multiple_headers() { + W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); + + Context result = + propagator.extract( + Context.root(), + ImmutableMap.of("baggage", ImmutableList.of("k1=v1", "k2=v2")), + multiGetter); + + Baggage expectedBaggage = Baggage.builder().put("k1", "v1").put("k2", "v2").build(); + assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage); + } + + @Test + void extract_multiple_headers_duplicate_key() { + W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); + + Context result = + propagator.extract( + Context.root(), + ImmutableMap.of("baggage", ImmutableList.of("k1=v1", "k1=v2")), + multiGetter); + + Baggage expectedBaggage = Baggage.builder().put("k1", "v2").build(); + assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage); + } + + @Test + void extract_multiple_headers_mixed_duplicates_non_duplicates() { + W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); + + Context result = + propagator.extract( + Context.root(), + ImmutableMap.of("baggage", ImmutableList.of("k1=v1,k2=v0", "k2=v2,k3=v3")), + multiGetter); + + Baggage expectedBaggage = + Baggage.builder().put("k1", "v1").put("k2", "v2").put("k3", "v3").build(); + assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage); + } + + @Test + void extract_multiple_headers_all_empty() { + W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); + + Context result = + propagator.extract( + Context.root(), ImmutableMap.of("baggage", ImmutableList.of("", "")), multiGetter); + + Baggage expectedBaggage = Baggage.builder().build(); + assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage); + } + + @Test + void extract_multiple_headers_some_empty() { + W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); + + Context result = + propagator.extract( + Context.root(), ImmutableMap.of("baggage", ImmutableList.of("", "k=v")), multiGetter); + + Baggage expectedBaggage = Baggage.builder().put("k", "v").build(); + assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage); + } + @Test void inject_noBaggage() { W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); diff --git a/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java b/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java index 77af56cad7b..de8012f00b9 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java @@ -37,7 +37,7 @@ public interface TextMapGetter { String get(@Nullable C carrier, String key); /** - * If implemented, returns all values for a given {@code key}, in order. + * If implemented, returns all values for a given {@code key} in order, or returns an empty list. * *

The default method returns the first value of the given propagation {@code key} as a * singleton list, or returns an empty list. From bb5f97bc26e709d65675d5ce07ee537a2d412f16 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Fri, 15 Nov 2024 14:12:38 +1100 Subject: [PATCH 3/8] fix checkstyle --- .../api/baggage/propagation/W3CBaggagePropagator.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java index e7361bd0683..dfffb518e6b 100644 --- a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java @@ -86,12 +86,6 @@ private static String encodeValue(String value) { return URL_ESCAPER.escape(value); } - /** - * @param context the {@code Context} used to store the extracted value. - * @param carrier holds propagation fields. For example, an outgoing message or http request. - * @param getter invoked for each propagation key to get data from the carrier. - * @return the extracted context - */ @Override public Context extract(Context context, @Nullable C carrier, TextMapGetter getter) { if (context == null) { From db8e4d4da75dd0d6725025196aa21978b6981835 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Fri, 15 Nov 2024 15:01:11 +1100 Subject: [PATCH 4/8] refactors the PR so that we have a separate internal ExtendedTextMapGetter, as per review. Also changes the return type of the method to Iterator as per review, and renames the method to getAll(). Tests are also adjusted --- .../propagation/W3CBaggagePropagator.java | 38 ++++++++++++++++-- .../propagation/W3CBaggagePropagatorTest.java | 11 +++-- .../propagation/ExtendedTextMapGetter.java | 40 +++++++++++++++++++ .../context/propagation/TextMapGetter.java | 18 --------- 4 files changed, 82 insertions(+), 25 deletions(-) create mode 100644 context/src/main/java/io/opentelemetry/context/internal/propagation/ExtendedTextMapGetter.java diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java index dfffb518e6b..425f9451dc8 100644 --- a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java @@ -13,10 +13,12 @@ import io.opentelemetry.api.internal.PercentEscaper; import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.context.Context; +import io.opentelemetry.context.internal.propagation.ExtendedTextMapGetter; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.context.propagation.TextMapSetter; import java.util.Collection; +import java.util.Iterator; import java.util.List; import javax.annotation.Nullable; @@ -95,14 +97,43 @@ public Context extract(Context context, @Nullable C carrier, TextMapGetter baggageHeaders = getter.getList(carrier, FIELD); - if (baggageHeaders == null || baggageHeaders.isEmpty()) { + if (getter instanceof ExtendedTextMapGetter) { + return extractMulti(context, carrier, (ExtendedTextMapGetter) getter); + } + return extractSingle(context, carrier, getter); + } + + private static Context extractSingle( + Context context, @Nullable C carrier, TextMapGetter getter) { + String baggageHeader = getter.get(carrier, FIELD); + if (baggageHeader == null) { + return context; + } + if (baggageHeader.isEmpty()) { + return context; + } + + BaggageBuilder baggageBuilder = Baggage.builder(); + try { + extractEntries(baggageHeader, baggageBuilder); + } catch (RuntimeException e) { + return context; + } + return context.with(baggageBuilder.build()); + } + + private static Context extractMulti( + Context context, @Nullable C carrier, ExtendedTextMapGetter getter) { + Iterator baggageHeaders = getter.getAll(carrier, FIELD); + if (baggageHeaders == null) { return context; } boolean extracted = false; BaggageBuilder baggageBuilder = Baggage.builder(); - for (String header : baggageHeaders) { + + while (baggageHeaders.hasNext()) { + String header = baggageHeaders.next(); if (header.isEmpty()) { continue; } @@ -114,6 +145,7 @@ public Context extract(Context context, @Nullable C carrier, TextMapGetter carrier, String key) { } }; - private static final TextMapGetter>> multiGetter = - new TextMapGetter>>() { + private static final ExtendedTextMapGetter>> multiGetter = + new ExtendedTextMapGetter>>() { @Override public Iterable keys(Map> carrier) { return carrier.keySet(); @@ -54,8 +56,9 @@ public String get(Map> carrier, String key) { } @Override - public List getList(Map> carrier, String key) { - return carrier.getOrDefault(key, Collections.emptyList()); + public Iterator getAll(Map> carrier, String key) { + List values = carrier.get(key); + return values == null ? Collections.emptyIterator() : values.iterator(); } }; diff --git a/context/src/main/java/io/opentelemetry/context/internal/propagation/ExtendedTextMapGetter.java b/context/src/main/java/io/opentelemetry/context/internal/propagation/ExtendedTextMapGetter.java new file mode 100644 index 00000000000..ea84460e611 --- /dev/null +++ b/context/src/main/java/io/opentelemetry/context/internal/propagation/ExtendedTextMapGetter.java @@ -0,0 +1,40 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.context.internal.propagation; + +import io.opentelemetry.context.propagation.TextMapGetter; +import java.util.Collections; +import java.util.Iterator; +import javax.annotation.Nullable; + +/** + * Extends {@link TextMapGetter} to return possibly multiple values for a given key. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. + * + * @param carrier of propagation fields, such as an http request. + */ +public interface ExtendedTextMapGetter extends TextMapGetter { + /** + * If implemented, returns all values for a given {@code key} in order, or returns an empty list. + * + *

The default method returns the first value of the given propagation {@code key} as a + * singleton list, or returns an empty list. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. + * + * @param carrier carrier of propagation fields, such as an http request. + * @param key the key of the field. + * @return all values for a given {@code key} in order, or returns an empty list. Default method + * wraps {@code get()} as an {@link Iterator}. + */ + default Iterator getAll(@Nullable C carrier, String key) { + String first = get(carrier, key); + return Collections.singleton(first).iterator(); + } +} diff --git a/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java b/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java index de8012f00b9..f160b7857ab 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java @@ -5,8 +5,6 @@ package io.opentelemetry.context.propagation; -import java.util.Collections; -import java.util.List; import javax.annotation.Nullable; /** @@ -35,20 +33,4 @@ public interface TextMapGetter { */ @Nullable String get(@Nullable C carrier, String key); - - /** - * If implemented, returns all values for a given {@code key} in order, or returns an empty list. - * - *

The default method returns the first value of the given propagation {@code key} as a - * singleton list, or returns an empty list. - * - * @param carrier carrier of propagation fields, such as an http request. - * @param key the key of the field. - * @return the first value of the given propagation {@code key} as a singleton list, or returns an - * empty list. - */ - default List getList(@Nullable C carrier, String key) { - String first = get(carrier, key); - return first == null ? Collections.emptyList() : Collections.singletonList(first); - } } From 28c8491bcd32e2eab1bae3409d957b53b0434c4b Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 26 Nov 2024 11:18:20 +1100 Subject: [PATCH 5/8] implement jack's suggestion of moving the ExtendedTextMapGetter from context.internal.propagation package into context.propagation.internal package --- .../api/baggage/propagation/W3CBaggagePropagator.java | 2 +- .../api/baggage/propagation/W3CBaggagePropagatorTest.java | 2 +- .../internal}/ExtendedTextMapGetter.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename context/src/main/java/io/opentelemetry/context/{internal/propagation => propagation/internal}/ExtendedTextMapGetter.java (96%) diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java index 425f9451dc8..e03f8e9078f 100644 --- a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java @@ -13,10 +13,10 @@ import io.opentelemetry.api.internal.PercentEscaper; import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.context.Context; -import io.opentelemetry.context.internal.propagation.ExtendedTextMapGetter; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.context.propagation.TextMapSetter; +import io.opentelemetry.context.propagation.internal.ExtendedTextMapGetter; import java.util.Collection; import java.util.Iterator; import java.util.List; diff --git a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java index 2de4cae87e9..321e64e0a69 100644 --- a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java +++ b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java @@ -13,8 +13,8 @@ import io.opentelemetry.api.baggage.Baggage; import io.opentelemetry.api.baggage.BaggageEntryMetadata; import io.opentelemetry.context.Context; -import io.opentelemetry.context.internal.propagation.ExtendedTextMapGetter; import io.opentelemetry.context.propagation.TextMapGetter; +import io.opentelemetry.context.propagation.internal.ExtendedTextMapGetter; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; diff --git a/context/src/main/java/io/opentelemetry/context/internal/propagation/ExtendedTextMapGetter.java b/context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java similarity index 96% rename from context/src/main/java/io/opentelemetry/context/internal/propagation/ExtendedTextMapGetter.java rename to context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java index ea84460e611..79ffea344d3 100644 --- a/context/src/main/java/io/opentelemetry/context/internal/propagation/ExtendedTextMapGetter.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.context.internal.propagation; +package io.opentelemetry.context.propagation.internal; import io.opentelemetry.context.propagation.TextMapGetter; import java.util.Collections; From 15d429d7e3e4329d5c37015f1b1e8c517e41d396 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 26 Nov 2024 15:32:47 +1100 Subject: [PATCH 6/8] Change behaviour of default getAll() method, so if get() returns null, it returns an empty iterator. Includes tests to verify the behaviour. --- .../internal/ExtendedTextMapGetter.java | 3 + .../internal/ExtendedTextMapGetterTest.java | 70 +++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 context/src/test/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetterTest.java diff --git a/context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java b/context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java index 79ffea344d3..da12a610d48 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java @@ -35,6 +35,9 @@ public interface ExtendedTextMapGetter extends TextMapGetter { */ default Iterator getAll(@Nullable C carrier, String key) { String first = get(carrier, key); + if (first == null) { + return Collections.emptyIterator(); + } return Collections.singleton(first).iterator(); } } diff --git a/context/src/test/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetterTest.java b/context/src/test/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetterTest.java new file mode 100644 index 00000000000..4b505dc8d8f --- /dev/null +++ b/context/src/test/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetterTest.java @@ -0,0 +1,70 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.context.propagation.internal; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import javax.annotation.Nullable; +import org.junit.jupiter.api.Test; + +public class ExtendedTextMapGetterTest { + + final ExtendedTextMapGetter nullGet = + new ExtendedTextMapGetter() { + @Override + public Iterable keys(Void carrier) { + return ImmutableList.of("key"); + } + + @Nullable + @Override + public String get(@Nullable Void carrier, String key) { + return null; + } + }; + + final ExtendedTextMapGetter nonNullGet = + new ExtendedTextMapGetter() { + @Override + public Iterable keys(Void carrier) { + return ImmutableList.of("key"); + } + + @Override + public String get(@Nullable Void carrier, String key) { + return "123"; + } + }; + + @Test + void extendedTextMapGetterdefaultMethod_returnsEmpty() { + Iterator result = nullGet.getAll(null, "key"); + assertThat(result).isNotNull(); + List values = iterToList(result); + assertThat(values).isEqualTo(Collections.emptyList()); + } + + @Test + void extendedTextMapGetterdefaultMethod_returnsSingleVal() { + Iterator result = nonNullGet.getAll(null, "key"); + assertThat(result).isNotNull(); + List values = iterToList(result); + assertThat(values).isEqualTo(Collections.singletonList("123")); + } + + private static List iterToList(Iterator iter) { + List list = new ArrayList<>(); + while (iter.hasNext()) { + list.add(iter.next()); + } + return list; + } +} From 54474fddba67e8ce8e928f3e0d68f99733165208 Mon Sep 17 00:00:00 2001 From: James Moessis Date: Tue, 26 Nov 2024 15:33:52 +1100 Subject: [PATCH 7/8] add tests for extracting multiple baggage headers, when some are invalid baggage syntax --- .../propagation/W3CBaggagePropagatorTest.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java index 321e64e0a69..fb0c342affc 100644 --- a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java +++ b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java @@ -514,6 +514,34 @@ void extract_multiple_headers_some_empty() { assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage); } + @Test + void extract_multiple_headers_all_invalid() { + W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); + + Context result = + propagator.extract( + Context.root(), + ImmutableMap.of("baggage", ImmutableList.of("!@#$%^", "key=va%lue")), + multiGetter); + + Baggage expectedBaggage = Baggage.builder().build(); + assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage); + } + + @Test + void extract_multiple_headers_some_invalid() { + W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); + + Context result = + propagator.extract( + Context.root(), + ImmutableMap.of("baggage", ImmutableList.of("k1=v1", "key=va%lue", "k2=v2")), + multiGetter); + + Baggage expectedBaggage = Baggage.builder().put("k1", "v1").put("k2", "v2").build(); + assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage); + } + @Test void inject_noBaggage() { W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); From c95cec8c3b9ea2f3d984e62e140c7947da0f9a13 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 5 Dec 2024 13:01:21 -0600 Subject: [PATCH 8/8] PR feedback --- .../context/propagation/internal/ExtendedTextMapGetter.java | 5 +++-- .../propagation/internal/ExtendedTextMapGetterTest.java | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java b/context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java index da12a610d48..d64604757db 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetter.java @@ -13,8 +13,9 @@ /** * Extends {@link TextMapGetter} to return possibly multiple values for a given key. * - *

This class is internal and is hence not for public use. Its APIs are unstable and can change - * at any time. + *

This class is internal and experimental. Its APIs are unstable and can change at any time. Its + * APIs (or a version of them) may be promoted to the public stable API in the future, but no + * guarantees are made. * * @param carrier of propagation fields, such as an http request. */ diff --git a/context/src/test/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetterTest.java b/context/src/test/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetterTest.java index 4b505dc8d8f..f8c35ea2ed9 100644 --- a/context/src/test/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetterTest.java +++ b/context/src/test/java/io/opentelemetry/context/propagation/internal/ExtendedTextMapGetterTest.java @@ -15,7 +15,7 @@ import javax.annotation.Nullable; import org.junit.jupiter.api.Test; -public class ExtendedTextMapGetterTest { +class ExtendedTextMapGetterTest { final ExtendedTextMapGetter nullGet = new ExtendedTextMapGetter() {