From 4267202f8fb279a2e7e4f6579e999c450fbcddae Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Fri, 2 Jul 2021 11:35:47 +0200 Subject: [PATCH 1/5] Fix #3364 Signed-off-by: Francesco Guardiani --- .../CompositeTextMapPropagatorBuilder.java | 69 +++++++++++++++++++ .../propagation/MultiTextMapPropagator.java | 24 +++++-- .../propagation/TextMapPropagator.java | 7 ++ .../MultiTextMapPropagatorTest.java | 29 ++++++++ 4 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java diff --git a/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java new file mode 100644 index 00000000000..5a26ed91217 --- /dev/null +++ b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java @@ -0,0 +1,69 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.context.propagation; + +import java.util.ArrayList; +import java.util.List; + +/** + * A builder for configuring an {@link TextMapPropagator} specifying which propagators should be used for extracting the context and which should be used for injecting the context. + */ +public final class CompositeTextMapPropagatorBuilder { + + private final List extractors; + private final List injectors; + + /** + * Package protected to disallow direct initialization. + * + * @see TextMapPropagator#builder() + */ + CompositeTextMapPropagatorBuilder() { + this.extractors = new ArrayList<>(); + this.injectors = new ArrayList<>(); + } + + /** + * Add a {@link TextMapPropagator} to be used only to extract the context. + */ + public CompositeTextMapPropagatorBuilder extractor(TextMapPropagator propagator) { + this.extractors.add(propagator); + return this; + } + + /** + * Add a {@link TextMapPropagator} to be used only to inject the context. + */ + public CompositeTextMapPropagatorBuilder injector(TextMapPropagator propagator) { + this.injectors.add(propagator); + return this; + } + + /** + * Add a {@link TextMapPropagator} to be used both to extract and inject the context. + */ + public CompositeTextMapPropagatorBuilder propagator(TextMapPropagator propagator) { + this.injectors.add(propagator); + this.extractors.add(propagator); + return this; + } + + /** + * Returns the built {@link TextMapPropagator} + * + * @see CompositeTextMapPropagatorBuilder + */ + public TextMapPropagator build() { + if (this.injectors.isEmpty()) { + this.injectors.add(TextMapPropagator.noop()); + } + if (this.extractors.isEmpty()) { + this.extractors.add(TextMapPropagator.noop()); + } + + return new MultiTextMapPropagator(this.extractors, this.injectors); + } +} diff --git a/context/src/main/java/io/opentelemetry/context/propagation/MultiTextMapPropagator.java b/context/src/main/java/io/opentelemetry/context/propagation/MultiTextMapPropagator.java index ce2cecb165c..f113e5dce35 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/MultiTextMapPropagator.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/MultiTextMapPropagator.java @@ -16,7 +16,8 @@ import javax.annotation.Nullable; final class MultiTextMapPropagator implements TextMapPropagator { - private final TextMapPropagator[] textPropagators; + private final TextMapPropagator[] extractors; + private final TextMapPropagator[] injectors; private final Collection allFields; MultiTextMapPropagator(TextMapPropagator... textPropagators) { @@ -24,9 +25,20 @@ final class MultiTextMapPropagator implements TextMapPropagator { } MultiTextMapPropagator(List textPropagators) { - this.textPropagators = new TextMapPropagator[textPropagators.size()]; - textPropagators.toArray(this.textPropagators); - this.allFields = Collections.unmodifiableList(getAllFields(this.textPropagators)); + this.extractors = new TextMapPropagator[textPropagators.size()]; + textPropagators.toArray(this.extractors); + this.injectors = this.extractors; // No need to copy + this.allFields = Collections.unmodifiableList(getAllFields(this.injectors)); + } + + MultiTextMapPropagator(List extractors, List injectors) { + this.extractors = new TextMapPropagator[extractors.size()]; + extractors.toArray(this.extractors); + + this.injectors = new TextMapPropagator[injectors.size()]; + injectors.toArray(this.injectors); + + this.allFields = Collections.unmodifiableList(getAllFields(this.injectors)); } @Override @@ -48,7 +60,7 @@ public void inject(Context context, @Nullable C carrier, TextMapSetter se if (context == null || setter == null) { return; } - for (TextMapPropagator textPropagator : textPropagators) { + for (TextMapPropagator textPropagator : injectors) { textPropagator.inject(context, carrier, setter); } } @@ -61,7 +73,7 @@ public Context extract(Context context, @Nullable C carrier, TextMapGetter carrier = new HashMap<>(); + + TextMapPropagator prop = TextMapPropagator.builder() + .injector(propagator1) + .extractor(propagator2) + .propagator(propagator3) + .build(); + + Context context1 = mock(Context.class); + Context context2 = mock(Context.class); + Context expectedContext = mock(Context.class); + + when(propagator2.extract(context1, carrier, getter)).thenReturn(context2); + when(propagator3.extract(context2, carrier, getter)).thenReturn(expectedContext); + + assertThat(prop.extract(context1, carrier, getter)).isEqualTo(expectedContext); + + Context context = mock(Context.class); + TextMapSetter> setter = Map::put; + + prop.inject(context, carrier, setter); + verify(propagator1).inject(context, carrier, setter); + verify(propagator2, never()).inject(context, carrier, setter); + verify(propagator3).inject(context, carrier, setter); + } } From 3e706c557b32bbc3183e39c4c33196df16d0078d Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Mon, 5 Jul 2021 09:53:19 +0200 Subject: [PATCH 2/5] Fix checkstyle lint Signed-off-by: Francesco Guardiani --- .../propagation/CompositeTextMapPropagatorBuilder.java | 5 +++-- .../opentelemetry/context/propagation/TextMapPropagator.java | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java index 5a26ed91217..698f180c284 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java @@ -9,7 +9,8 @@ import java.util.List; /** - * A builder for configuring an {@link TextMapPropagator} specifying which propagators should be used for extracting the context and which should be used for injecting the context. + * A builder for configuring an {@link TextMapPropagator} specifying which propagators should be + * used for extracting the context and which should be used for injecting the context. */ public final class CompositeTextMapPropagatorBuilder { @@ -52,7 +53,7 @@ public CompositeTextMapPropagatorBuilder propagator(TextMapPropagator propagator } /** - * Returns the built {@link TextMapPropagator} + * Returns the built {@link TextMapPropagator}. * * @see CompositeTextMapPropagatorBuilder */ diff --git a/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java b/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java index 860603a9b1e..6cb9a0b966d 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java @@ -44,7 +44,8 @@ public interface TextMapPropagator { /** - * Returns a composite text map propagator builder that allows to specify specific propagators as extractors and specific propagators as injectors. + * Returns a composite text map propagator builder that allows to specify specific propagators + * as extractors and specific propagators as injectors. */ static CompositeTextMapPropagatorBuilder builder() { return new CompositeTextMapPropagatorBuilder(); From 5b151446b44cfac0014e2d65c93ce1357c3369ef Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Mon, 5 Jul 2021 10:01:46 +0200 Subject: [PATCH 3/5] Resolve PR comments Signed-off-by: Francesco Guardiani --- ...r.java => CompositeTextMapPropagator.java} | 8 +-- .../CompositeTextMapPropagatorBuilder.java | 20 ++++--- .../propagation/TextMapPropagator.java | 2 +- ...va => CompositeTextMapPropagatorTest.java} | 55 +++++++++++++------ 4 files changed, 55 insertions(+), 30 deletions(-) rename context/src/main/java/io/opentelemetry/context/propagation/{MultiTextMapPropagator.java => CompositeTextMapPropagator.java} (87%) rename context/src/test/java/io/opentelemetry/context/propagation/{MultiTextMapPropagatorTest.java => CompositeTextMapPropagatorTest.java} (78%) diff --git a/context/src/main/java/io/opentelemetry/context/propagation/MultiTextMapPropagator.java b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagator.java similarity index 87% rename from context/src/main/java/io/opentelemetry/context/propagation/MultiTextMapPropagator.java rename to context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagator.java index f113e5dce35..e0a17b6be0c 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/MultiTextMapPropagator.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagator.java @@ -15,23 +15,23 @@ import java.util.Set; import javax.annotation.Nullable; -final class MultiTextMapPropagator implements TextMapPropagator { +final class CompositeTextMapPropagator implements TextMapPropagator { private final TextMapPropagator[] extractors; private final TextMapPropagator[] injectors; private final Collection allFields; - MultiTextMapPropagator(TextMapPropagator... textPropagators) { + CompositeTextMapPropagator(TextMapPropagator... textPropagators) { this(Arrays.asList(textPropagators)); } - MultiTextMapPropagator(List textPropagators) { + CompositeTextMapPropagator(List textPropagators) { this.extractors = new TextMapPropagator[textPropagators.size()]; textPropagators.toArray(this.extractors); this.injectors = this.extractors; // No need to copy this.allFields = Collections.unmodifiableList(getAllFields(this.injectors)); } - MultiTextMapPropagator(List extractors, List injectors) { + CompositeTextMapPropagator(List extractors, List injectors) { this.extractors = new TextMapPropagator[extractors.size()]; extractors.toArray(this.extractors); diff --git a/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java index 698f180c284..754be2722f2 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java @@ -7,9 +7,10 @@ import java.util.ArrayList; import java.util.List; +import java.util.Objects; /** - * A builder for configuring an {@link TextMapPropagator} specifying which propagators should be + * A builder for configuring a {@link TextMapPropagator} specifying which propagators should be * used for extracting the context and which should be used for injecting the context. */ public final class CompositeTextMapPropagatorBuilder { @@ -28,25 +29,28 @@ public final class CompositeTextMapPropagatorBuilder { } /** - * Add a {@link TextMapPropagator} to be used only to extract the context. + * Adds a {@link TextMapPropagator} to be used only when extracting context. */ - public CompositeTextMapPropagatorBuilder extractor(TextMapPropagator propagator) { + public CompositeTextMapPropagatorBuilder addExtractor(TextMapPropagator propagator) { + Objects.requireNonNull(propagator, "propagator"); this.extractors.add(propagator); return this; } /** - * Add a {@link TextMapPropagator} to be used only to inject the context. + * Adds a {@link TextMapPropagator} to be used only when injecting context. */ - public CompositeTextMapPropagatorBuilder injector(TextMapPropagator propagator) { + public CompositeTextMapPropagatorBuilder addInjector(TextMapPropagator propagator) { + Objects.requireNonNull(propagator, "propagator"); this.injectors.add(propagator); return this; } /** - * Add a {@link TextMapPropagator} to be used both to extract and inject the context. + * Adds a {@link TextMapPropagator} to be used both when extracting and injecting context. */ - public CompositeTextMapPropagatorBuilder propagator(TextMapPropagator propagator) { + public CompositeTextMapPropagatorBuilder addPropagator(TextMapPropagator propagator) { + Objects.requireNonNull(propagator, "propagator"); this.injectors.add(propagator); this.extractors.add(propagator); return this; @@ -65,6 +69,6 @@ public TextMapPropagator build() { this.extractors.add(TextMapPropagator.noop()); } - return new MultiTextMapPropagator(this.extractors, this.injectors); + return new CompositeTextMapPropagator(this.extractors, this.injectors); } } diff --git a/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java b/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java index 6cb9a0b966d..6716001f137 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java @@ -80,7 +80,7 @@ static TextMapPropagator composite(Iterable propagators) { if (propagatorsList.size() == 1) { return propagatorsList.get(0); } - return new MultiTextMapPropagator(propagatorsList); + return new CompositeTextMapPropagator(propagatorsList); } /** Returns a {@link TextMapPropagator} which does no injection or extraction. */ diff --git a/context/src/test/java/io/opentelemetry/context/propagation/MultiTextMapPropagatorTest.java b/context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java similarity index 78% rename from context/src/test/java/io/opentelemetry/context/propagation/MultiTextMapPropagatorTest.java rename to context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java index 46fe360129c..e0024d10693 100644 --- a/context/src/test/java/io/opentelemetry/context/propagation/MultiTextMapPropagatorTest.java +++ b/context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java @@ -28,7 +28,7 @@ import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) -class MultiTextMapPropagatorTest { +class CompositeTextMapPropagatorTest { private static final ContextKey KEY = ContextKey.named("key"); @@ -51,17 +51,17 @@ public String get(Map carrier, String key) { }; @Test - void addPropagator_null() { + void new_composite_with_null_list() { assertThrows( NullPointerException.class, - () -> new MultiTextMapPropagator((List) null)); + () -> new CompositeTextMapPropagator((List) null)); } @Test void fields() { when(propagator1.fields()).thenReturn(Arrays.asList("foo", "bar")); when(propagator2.fields()).thenReturn(Arrays.asList("hello", "world")); - TextMapPropagator prop = new MultiTextMapPropagator(propagator1, propagator2); + TextMapPropagator prop = new CompositeTextMapPropagator(propagator1, propagator2); Collection fields = prop.fields(); assertThat(fields).containsExactly("foo", "bar", "hello", "world"); @@ -71,7 +71,7 @@ void fields() { void fields_duplicates() { when(propagator1.fields()).thenReturn(Arrays.asList("foo", "bar", "foo")); when(propagator2.fields()).thenReturn(Arrays.asList("hello", "world", "world", "bar")); - TextMapPropagator prop = new MultiTextMapPropagator(propagator1, propagator2); + TextMapPropagator prop = new CompositeTextMapPropagator(propagator1, propagator2); Collection fields = prop.fields(); assertThat(fields).containsExactly("foo", "bar", "hello", "world"); @@ -81,7 +81,7 @@ void fields_duplicates() { void fields_readOnly() { when(propagator1.fields()).thenReturn(Arrays.asList("rubber", "baby")); when(propagator2.fields()).thenReturn(Arrays.asList("buggy", "bumpers")); - TextMapPropagator prop = new MultiTextMapPropagator(propagator1, propagator2); + TextMapPropagator prop = new CompositeTextMapPropagator(propagator1, propagator2); Collection fields = prop.fields(); assertThrows(UnsupportedOperationException.class, () -> fields.add("hi")); } @@ -92,7 +92,7 @@ void inject_allDelegated() { Context context = mock(Context.class); TextMapSetter> setter = Map::put; - TextMapPropagator prop = new MultiTextMapPropagator(propagator1, propagator2, propagator3); + TextMapPropagator prop = new CompositeTextMapPropagator(propagator1, propagator2, propagator3); prop.inject(context, carrier, setter); verify(propagator1).inject(context, carrier, setter); verify(propagator2).inject(context, carrier, setter); @@ -104,7 +104,7 @@ void extract_noPropagators() { Map carrier = new HashMap<>(); Context context = mock(Context.class); - TextMapPropagator prop = new MultiTextMapPropagator(); + TextMapPropagator prop = new CompositeTextMapPropagator(); Context resContext = prop.extract(context, carrier, getter); assertThat(context).isSameAs(resContext); } @@ -112,7 +112,7 @@ void extract_noPropagators() { @Test void extract_found_all() { Map carrier = new HashMap<>(); - TextMapPropagator prop = new MultiTextMapPropagator(propagator1, propagator2, propagator3); + TextMapPropagator prop = new CompositeTextMapPropagator(propagator1, propagator2, propagator3); Context context1 = mock(Context.class); Context context2 = mock(Context.class); Context context3 = mock(Context.class); @@ -132,7 +132,7 @@ void extract_notFound() { when(propagator1.extract(context, carrier, getter)).thenReturn(context); when(propagator2.extract(context, carrier, getter)).thenReturn(context); - TextMapPropagator prop = new MultiTextMapPropagator(propagator1, propagator2); + TextMapPropagator prop = new CompositeTextMapPropagator(propagator1, propagator2); Context result = prop.extract(context, carrier, getter); assertThat(result).isSameAs(context); @@ -141,7 +141,7 @@ void extract_notFound() { @Test void extract_nullContext() { assertThat( - new MultiTextMapPropagator(propagator1, propagator2) + new CompositeTextMapPropagator(propagator1, propagator2) .extract(null, Collections.emptyMap(), getter)) .isSameAs(Context.root()); } @@ -150,7 +150,7 @@ void extract_nullContext() { void extract_nullGetter() { Context context = Context.current().with(KEY, "treasure"); assertThat( - new MultiTextMapPropagator(propagator1, propagator2) + new CompositeTextMapPropagator(propagator1, propagator2) .extract(context, Collections.emptyMap(), null)) .isSameAs(context); } @@ -158,7 +158,7 @@ void extract_nullGetter() { @Test void inject_nullContext() { Map carrier = new LinkedHashMap<>(); - new MultiTextMapPropagator(propagator1, propagator2).inject(null, carrier, Map::put); + new CompositeTextMapPropagator(propagator1, propagator2).inject(null, carrier, Map::put); assertThat(carrier).isEmpty(); } @@ -166,7 +166,7 @@ void inject_nullContext() { void inject_nullSetter() { Map carrier = new LinkedHashMap<>(); Context context = Context.current().with(KEY, "treasure"); - new MultiTextMapPropagator(propagator1, propagator2).inject(context, carrier, null); + new CompositeTextMapPropagator(propagator1, propagator2).inject(context, carrier, null); assertThat(carrier).isEmpty(); } @@ -175,9 +175,9 @@ void different_inject_and_extract() { Map carrier = new HashMap<>(); TextMapPropagator prop = TextMapPropagator.builder() - .injector(propagator1) - .extractor(propagator2) - .propagator(propagator3) + .addInjector(propagator1) + .addExtractor(propagator2) + .addPropagator(propagator3) .build(); Context context1 = mock(Context.class); @@ -197,4 +197,25 @@ void different_inject_and_extract() { verify(propagator2, never()).inject(context, carrier, setter); verify(propagator3).inject(context, carrier, setter); } + + @Test + void addPropagator_null() { + assertThrows( + NullPointerException.class, + () -> TextMapPropagator.builder().addPropagator(null)); + } + + @Test + void addExtractor_null() { + assertThrows( + NullPointerException.class, + () -> TextMapPropagator.builder().addExtractor(null)); + } + + @Test + void addInjector_null() { + assertThrows( + NullPointerException.class, + () -> TextMapPropagator.builder().addInjector(null)); + } } From fe812216669d8d3e6e66b04e24244b826b07762d Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Mon, 5 Jul 2021 10:11:22 +0200 Subject: [PATCH 4/5] More suggestions Signed-off-by: Francesco Guardiani --- .../CompositeTextMapPropagator.java | 3 +- .../CompositeTextMapPropagatorBuilder.java | 23 +++++--------- .../propagation/TextMapPropagator.java | 4 +-- .../CompositeTextMapPropagatorTest.java | 31 ++++++++++--------- 4 files changed, 28 insertions(+), 33 deletions(-) diff --git a/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagator.java b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagator.java index e0a17b6be0c..01c315465e9 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagator.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagator.java @@ -31,7 +31,8 @@ final class CompositeTextMapPropagator implements TextMapPropagator { this.allFields = Collections.unmodifiableList(getAllFields(this.injectors)); } - CompositeTextMapPropagator(List extractors, List injectors) { + CompositeTextMapPropagator( + List extractors, List injectors) { this.extractors = new TextMapPropagator[extractors.size()]; extractors.toArray(this.extractors); diff --git a/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java index 754be2722f2..51627451057 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorBuilder.java @@ -10,8 +10,8 @@ import java.util.Objects; /** - * A builder for configuring a {@link TextMapPropagator} specifying which propagators should be - * used for extracting the context and which should be used for injecting the context. + * A builder for configuring a {@link TextMapPropagator} specifying which propagators should be used + * for extracting the context and which should be used for injecting the context. */ public final class CompositeTextMapPropagatorBuilder { @@ -28,27 +28,21 @@ public final class CompositeTextMapPropagatorBuilder { this.injectors = new ArrayList<>(); } - /** - * Adds a {@link TextMapPropagator} to be used only when extracting context. - */ + /** Adds a {@link TextMapPropagator} to be used only when extracting context. */ public CompositeTextMapPropagatorBuilder addExtractor(TextMapPropagator propagator) { Objects.requireNonNull(propagator, "propagator"); this.extractors.add(propagator); return this; } - /** - * Adds a {@link TextMapPropagator} to be used only when injecting context. - */ + /** Adds a {@link TextMapPropagator} to be used only when injecting context. */ public CompositeTextMapPropagatorBuilder addInjector(TextMapPropagator propagator) { Objects.requireNonNull(propagator, "propagator"); this.injectors.add(propagator); return this; } - /** - * Adds a {@link TextMapPropagator} to be used both when extracting and injecting context. - */ + /** Adds a {@link TextMapPropagator} to be used both when extracting and injecting context. */ public CompositeTextMapPropagatorBuilder addPropagator(TextMapPropagator propagator) { Objects.requireNonNull(propagator, "propagator"); this.injectors.add(propagator); @@ -62,11 +56,8 @@ public CompositeTextMapPropagatorBuilder addPropagator(TextMapPropagator propaga * @see CompositeTextMapPropagatorBuilder */ public TextMapPropagator build() { - if (this.injectors.isEmpty()) { - this.injectors.add(TextMapPropagator.noop()); - } - if (this.extractors.isEmpty()) { - this.extractors.add(TextMapPropagator.noop()); + if (this.injectors.isEmpty() && this.extractors.isEmpty()) { + return NoopTextMapPropagator.getInstance(); } return new CompositeTextMapPropagator(this.extractors, this.injectors); diff --git a/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java b/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java index 6716001f137..e8e454ed079 100644 --- a/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java +++ b/context/src/main/java/io/opentelemetry/context/propagation/TextMapPropagator.java @@ -44,8 +44,8 @@ public interface TextMapPropagator { /** - * Returns a composite text map propagator builder that allows to specify specific propagators - * as extractors and specific propagators as injectors. + * Returns a composite text map propagator builder that allows to specify specific propagators as + * extractors and specific propagators as injectors. */ static CompositeTextMapPropagatorBuilder builder() { return new CompositeTextMapPropagatorBuilder(); diff --git a/context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java b/context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java index e0024d10693..b90c0c34bb9 100644 --- a/context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java +++ b/context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java @@ -174,11 +174,12 @@ void inject_nullSetter() { void different_inject_and_extract() { Map carrier = new HashMap<>(); - TextMapPropagator prop = TextMapPropagator.builder() - .addInjector(propagator1) - .addExtractor(propagator2) - .addPropagator(propagator3) - .build(); + TextMapPropagator prop = + TextMapPropagator.builder() + .addInjector(propagator1) + .addExtractor(propagator2) + .addPropagator(propagator3) + .build(); Context context1 = mock(Context.class); Context context2 = mock(Context.class); @@ -200,22 +201,24 @@ void different_inject_and_extract() { @Test void addPropagator_null() { - assertThrows( - NullPointerException.class, - () -> TextMapPropagator.builder().addPropagator(null)); + assertThrows(NullPointerException.class, () -> TextMapPropagator.builder().addPropagator(null)); } @Test void addExtractor_null() { - assertThrows( - NullPointerException.class, - () -> TextMapPropagator.builder().addExtractor(null)); + assertThrows(NullPointerException.class, () -> TextMapPropagator.builder().addExtractor(null)); } @Test void addInjector_null() { - assertThrows( - NullPointerException.class, - () -> TextMapPropagator.builder().addInjector(null)); + assertThrows(NullPointerException.class, () -> TextMapPropagator.builder().addInjector(null)); } + + @Test + void empty_builder_returns_noop() { + assertThat( + TextMapPropagator.builder().build() + ).isSameAs(NoopTextMapPropagator.getInstance()); + } + } From 27b4edac012fad99190c67ca6d449e9e1c200a60 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Mon, 5 Jul 2021 10:18:09 +0200 Subject: [PATCH 5/5] Add method change doc Signed-off-by: Francesco Guardiani --- .../propagation/CompositeTextMapPropagatorTest.java | 5 +---- .../current_vs_latest/opentelemetry-context.txt | 11 ++++++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java b/context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java index b90c0c34bb9..1282d8cec4f 100644 --- a/context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java +++ b/context/src/test/java/io/opentelemetry/context/propagation/CompositeTextMapPropagatorTest.java @@ -216,9 +216,6 @@ void addInjector_null() { @Test void empty_builder_returns_noop() { - assertThat( - TextMapPropagator.builder().build() - ).isSameAs(NoopTextMapPropagator.getInstance()); + assertThat(TextMapPropagator.builder().build()).isSameAs(NoopTextMapPropagator.getInstance()); } - } diff --git a/docs/apidiffs/current_vs_latest/opentelemetry-context.txt b/docs/apidiffs/current_vs_latest/opentelemetry-context.txt index df26146497b..e5663e12404 100644 --- a/docs/apidiffs/current_vs_latest/opentelemetry-context.txt +++ b/docs/apidiffs/current_vs_latest/opentelemetry-context.txt @@ -1,2 +1,11 @@ Comparing source compatibility of against -No changes. \ No newline at end of file ++++ NEW CLASS: PUBLIC(+) FINAL(+) io.opentelemetry.context.propagation.CompositeTextMapPropagatorBuilder (not serializable) + +++ CLASS FILE FORMAT VERSION: 52.0 <- n.a. + +++ NEW SUPERCLASS: java.lang.Object + +++ NEW METHOD: PUBLIC(+) io.opentelemetry.context.propagation.CompositeTextMapPropagatorBuilder addExtractor(io.opentelemetry.context.propagation.TextMapPropagator) + +++ NEW METHOD: PUBLIC(+) io.opentelemetry.context.propagation.CompositeTextMapPropagatorBuilder addInjector(io.opentelemetry.context.propagation.TextMapPropagator) + +++ NEW METHOD: PUBLIC(+) io.opentelemetry.context.propagation.CompositeTextMapPropagatorBuilder addPropagator(io.opentelemetry.context.propagation.TextMapPropagator) + +++ NEW METHOD: PUBLIC(+) io.opentelemetry.context.propagation.TextMapPropagator build() +***! MODIFIED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.context.propagation.TextMapPropagator (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 + +++! NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.context.propagation.CompositeTextMapPropagatorBuilder builder()