From acf85adafd573479be02a5b9adb0b1108b331771 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 23 Sep 2022 13:40:35 +0200 Subject: [PATCH] W3CPropagation allows not having baggage without this change you will always parse baggage fields with this change you can create a W3Propagation instance that doesn't use any baggage --- .../tracing/brave/bridge/W3CPropagation.java | 41 ++-- .../brave/bridge/W3CPropagationTest.java | 213 +++++++++++------- 2 files changed, 156 insertions(+), 98 deletions(-) diff --git a/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/main/java/io/micrometer/tracing/brave/bridge/W3CPropagation.java b/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/main/java/io/micrometer/tracing/brave/bridge/W3CPropagation.java index 32467e75..bb1d02ff 100755 --- a/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/main/java/io/micrometer/tracing/brave/bridge/W3CPropagation.java +++ b/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/main/java/io/micrometer/tracing/brave/bridge/W3CPropagation.java @@ -16,13 +16,12 @@ package io.micrometer.tracing.brave.bridge; -import brave.baggage.BaggageField; -import brave.baggage.BaggagePropagation; import brave.internal.baggage.BaggageFields; import brave.internal.propagation.StringPropagationAdapter; import brave.propagation.Propagation; import brave.propagation.TraceContext; import brave.propagation.TraceContextOrSamplingFlags; +import io.micrometer.common.lang.Nullable; import io.micrometer.common.util.StringUtils; import io.micrometer.common.util.internal.logging.InternalLogger; import io.micrometer.common.util.internal.logging.InternalLoggerFactory; @@ -96,15 +95,30 @@ public class W3CPropagation extends Propagation.Factory implements Propagation localFields) { this.baggagePropagator = new W3CBaggagePropagator(braveBaggageManager, localFields); this.braveBaggageManager = braveBaggageManager; } + /** + * Creates an instance of {@link W3CPropagation} without baggage support. + */ + public W3CPropagation() { + this.baggagePropagator = null; + this.braveBaggageManager = null; + } + private static boolean isTraceIdValid(CharSequence traceId) { return (traceId.length() == TRACE_ID_HEX_SIZE) && !INVALID_TRACE_ID.contentEquals(traceId) && EncodingUtils.isValidBase16String(traceId); @@ -190,12 +204,14 @@ public TraceContext.Injector injector(Setter setter) { copyTraceFlagsHexTo(chars, TRACE_OPTION_OFFSET, context); setter.put(carrier, TRACE_PARENT, new String(chars, 0, TRACEPARENT_HEADER_SIZE)); addTraceState(setter, context, carrier); - this.baggagePropagator.injector(setter).inject(context, carrier); + if (this.baggagePropagator != null) { + this.baggagePropagator.injector(setter).inject(context, carrier); + } }; } private void addTraceState(Setter setter, TraceContext context, R carrier) { - if (carrier != null) { + if (carrier != null && this.braveBaggageManager != null) { BaggageInScope baggage = this.braveBaggageManager.getBaggage(BraveTraceContext.fromBrave(context), TRACE_STATE); if (baggage == null) { @@ -240,7 +256,11 @@ public TraceContext.Extractor extractor(Getter getter) { return withBaggage(TraceContextOrSamplingFlags.EMPTY, carrier, getter); } String traceStateHeader = getter.get(carrier, TRACE_STATE); - return withBaggage(context(contextFromParentHeader, traceStateHeader), carrier, getter); + TraceContextOrSamplingFlags context = context(contextFromParentHeader, traceStateHeader); + if (this.baggagePropagator == null || this.braveBaggageManager == null) { + return context; + } + return withBaggage(context, carrier, getter); }; } @@ -294,8 +314,6 @@ class W3CBaggagePropagator { private static final String TRACE_STATE = "tracestate"; - private static final BaggageField TRACE_STATE_BAGGAGE = BaggageField.create(TRACE_STATE); - private static final String FIELD = "baggage"; private static final List FIELDS = singletonList(FIELD); @@ -309,15 +327,6 @@ class W3CBaggagePropagator { this.localFields = localFields; } - private BaggagePropagation.FactoryBuilder factory() { - return BaggagePropagation.newFactoryBuilder(new Propagation.Factory() { - @Override - public Propagation create(Propagation.KeyFactory keyFactory) { - return null; - } - }); - } - public List keys() { return FIELDS; } diff --git a/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/test/java/io/micrometer/tracing/brave/bridge/W3CPropagationTest.java b/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/test/java/io/micrometer/tracing/brave/bridge/W3CPropagationTest.java index b0b2f84b..206ce84f 100644 --- a/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/test/java/io/micrometer/tracing/brave/bridge/W3CPropagationTest.java +++ b/micrometer-tracing-bridges/micrometer-tracing-bridge-brave/src/test/java/io/micrometer/tracing/brave/bridge/W3CPropagationTest.java @@ -23,8 +23,11 @@ import brave.propagation.TraceContextOrSamplingFlags; import io.micrometer.tracing.internal.EncodingUtils; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import java.util.*; +import java.util.function.Supplier; import java.util.stream.Collectors; import static io.micrometer.tracing.brave.bridge.W3CPropagation.TRACE_PARENT; @@ -54,13 +57,12 @@ class W3CPropagationTest { private static final String TRACESTATE_HEADER = "sappp=CwAAmEnGj0gThK52TCXZ270X8nBhc3Nwb3J0LWFwcABQT1NU"; - private final W3CPropagation w3CPropagation = new W3CPropagation(new BraveBaggageManager(), new ArrayList<>()); - - @Test - void inject_NullCarrierUsage() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void inject_NullCarrierUsage(W3CPropagationType propagationType) { final Map carrier = new LinkedHashMap<>(); TraceContext traceContext = sampledTraceContext().build(); - w3CPropagation.injector((ignored, key, value) -> carrier.put(key, value)).inject(traceContext, null); + propagationType.get().injector((ignored, key, value) -> carrier.put(key, value)).inject(traceContext, null); assertThat(carrier).containsExactly(entry(TRACE_PARENT, TRACEPARENT_HEADER_SAMPLED)); } @@ -75,11 +77,12 @@ private TraceContext.Builder sampledTraceContext(String traceIdHigh, String trac .spanId(EncodingUtils.longFromBase16String(spanId)); } - @Test - void inject_SampledContext() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void inject_SampledContext(W3CPropagationType propagationType) { Map carrier = new LinkedHashMap<>(); TraceContext traceContext = sampledTraceContext().build(); - w3CPropagation.injector((ignored, key, value) -> carrier.put(key, value)).inject(traceContext, carrier); + propagationType.get().injector((ignored, key, value) -> carrier.put(key, value)).inject(traceContext, carrier); assertThat(carrier).containsExactly(entry(TRACE_PARENT, TRACEPARENT_HEADER_SAMPLED)); } @@ -92,75 +95,86 @@ void inject_tracestate() { .addExtra(BaggageFields.newFactory(Arrays.asList(traceStateField, mybaggageField), 2).create()).build(); traceStateField.updateValue(traceContext, TRACESTATE_HEADER); mybaggageField.updateValue(traceContext, "mybaggagevalue"); - w3CPropagation.injector((ignored, key, value) -> carrier.put(key, value)).inject(traceContext, carrier); + W3CPropagationType.WITH_BAGGAGE.get().injector((ignored, key, value) -> carrier.put(key, value)) + .inject(traceContext, carrier); assertThat(carrier).containsEntry("baggage", "mybaggage=mybaggagevalue").containsEntry("tracestate", "sappp=CwAAmEnGj0gThK52TCXZ270X8nBhc3Nwb3J0LWFwcABQT1NU"); } - @Test - void inject_NotSampledContext() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void inject_NotSampledContext(W3CPropagationType propagationType) { Map carrier = new LinkedHashMap<>(); TraceContext traceContext = notSampledTraceContext().build(); - w3CPropagation.injector((ignored, key, value) -> carrier.put(key, value)).inject(traceContext, carrier); + propagationType.get().injector((ignored, key, value) -> carrier.put(key, value)).inject(traceContext, carrier); assertThat(carrier).containsExactly(entry(TRACE_PARENT, TRACEPARENT_HEADER_NOT_SAMPLED)); } /** * see: gh-1809 */ - @Test - void inject_traceIdShouldBePaddedWithZeros() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void inject_traceIdShouldBePaddedWithZeros(W3CPropagationType propagationType) { Map carrier = new LinkedHashMap<>(); TraceContext traceContext = sampledTraceContext("0000000000000000", "123456789abcdef0", "123456789abcdef1") .build(); - w3CPropagation.injector((ignored, key, value) -> carrier.put(key, value)).inject(traceContext, carrier); + propagationType.get().injector((ignored, key, value) -> carrier.put(key, value)).inject(traceContext, carrier); assertThat(carrier) .containsExactly(entry(TRACE_PARENT, "00-0000000000000000123456789abcdef0-123456789abcdef1-01")); } - @Test - void extract_Nothing() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_Nothing(W3CPropagationType propagationType) { // Context remains untouched. - assertThat(w3CPropagation.extractor(getter).extract(Collections.emptyMap())) + assertThat(propagationType.get().extractor(getter).extract(Collections.emptyMap())) .isSameAs(TraceContextOrSamplingFlags.EMPTY); } - @Test - void extract_SampledContext() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_SampledContext(W3CPropagationType propagationType) { Map carrier = new LinkedHashMap<>(); carrier.put(TRACE_PARENT, TRACEPARENT_HEADER_SAMPLED); - assertThat(w3CPropagation.extractor(getter).extract(carrier).context()).isEqualTo(sharedTraceContext().build()); + assertThat(propagationType.get().extractor(getter).extract(carrier).context()) + .isEqualTo(sharedTraceContext().build()); } - @Test - void extract_NullCarrier() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_NullCarrier(W3CPropagationType propagationType) { Map carrier = new LinkedHashMap<>(); carrier.put(TRACE_PARENT, TRACEPARENT_HEADER_SAMPLED); - assertThat(w3CPropagation.extractor((request, key) -> carrier.get(key)).extract(null).context()) + assertThat(propagationType.get().extractor((request, key) -> carrier.get(key)).extract(null).context()) .isEqualTo(sharedTraceContext().build()); } - @Test - void extract_NotSampledContext() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_NotSampledContext(W3CPropagationType propagationType) { Map carrier = new LinkedHashMap<>(); carrier.put(TRACE_PARENT, TRACEPARENT_HEADER_NOT_SAMPLED); - assertThat(w3CPropagation.extractor(getter).extract(carrier).context()) + assertThat(propagationType.get().extractor(getter).extract(carrier).context()) .isEqualTo(notSampledTraceContext().shared(true).build()); } - @Test - void extract_NotSampledContext_NextVersion() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_NotSampledContext_NextVersion(W3CPropagationType propagationType) { Map carrier = new LinkedHashMap<>(); carrier.put(TRACE_PARENT, "01-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-00-02"); - assertThat(w3CPropagation.extractor(getter).extract(carrier).context()).isEqualTo(sharedTraceContext().build()); + assertThat(propagationType.get().extractor(getter).extract(carrier).context()) + .isEqualTo(sharedTraceContext().build()); } - @Test - void extract_NotSampledContext_EmptyTraceState() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_NotSampledContext_EmptyTraceState(W3CPropagationType propagationType) { Map carrier = new LinkedHashMap<>(); carrier.put(TRACE_PARENT, TRACEPARENT_HEADER_NOT_SAMPLED); carrier.put(TRACE_STATE, ""); - assertThat(w3CPropagation.extractor(getter).extract(carrier).context()) + assertThat(propagationType.get().extractor(getter).extract(carrier).context()) .isEqualTo(notSampledTraceContext().shared(true).build()); } @@ -168,12 +182,13 @@ private TraceContext.Builder notSampledTraceContext() { return sampledTraceContext().sampled(false); } - @Test - void extract_NotSampledContext_TraceStateWithSpaces() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_NotSampledContext_TraceStateWithSpaces(W3CPropagationType propagationType) { Map carrier = new LinkedHashMap<>(); carrier.put(TRACE_PARENT, TRACEPARENT_HEADER_NOT_SAMPLED); carrier.put(TRACE_STATE, TRACESTATE_NOT_DEFAULT_ENCODING_WITH_SPACES); - assertThat(w3CPropagation.extractor(getter).extract(carrier).context()) + assertThat(propagationType.get().extractor(getter).extract(carrier).context()) .isEqualTo(sharedTraceContext().sampled(false).build()); } @@ -184,7 +199,7 @@ void extract_tracestate_shouldNotBePartOfBaggage() { carrier.put(TRACE_STATE, TRACESTATE_HEADER); carrier.put("baggage", "mybaggage=mybaggagevalue"); - TraceContext context = w3CPropagation.extractor(getter).extract(carrier).context(); + TraceContext context = W3CPropagationType.WITH_BAGGAGE.get().extractor(getter).extract(carrier).context(); Map baggageEntries = baggageEntries(context); assertThat(baggageEntries).doesNotContainKey(TRACE_STATE).containsEntry("mybaggage", "mybaggagevalue"); @@ -199,68 +214,76 @@ private Map baggageEntries(TraceContext flags) { .collect(Collectors.toMap(e -> e.getKey().name(), AbstractMap.SimpleEntry::getValue, (o, o2) -> o2)); } - @Test - void extract_EmptyHeader() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_EmptyHeader(W3CPropagationType propagationType) { Map invalidHeaders = new LinkedHashMap<>(); invalidHeaders.put(TRACE_PARENT, ""); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders)) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders)) .isSameAs(TraceContextOrSamplingFlags.EMPTY); } - @Test - void extract_InvalidTraceId() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidTraceId(W3CPropagationType propagationType) { Map invalidHeaders = new LinkedHashMap<>(); invalidHeaders.put(TRACE_PARENT, "00-" + "abcdefghijklmnopabcdefghijklmnop" + "-" + SPAN_ID_BASE16 + "-01"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders)) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders)) .isSameAs(TraceContextOrSamplingFlags.EMPTY); } - @Test - void extract_InvalidTraceId_Size() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidTraceId_Size(W3CPropagationType propagationType) { Map invalidHeaders = new LinkedHashMap<>(); invalidHeaders.put(TRACE_PARENT, "00-" + TRACE_ID_BASE16 + "00-" + SPAN_ID_BASE16 + "-01"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders)) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders)) .isSameAs(TraceContextOrSamplingFlags.EMPTY); } - @Test - void extract_InvalidSpanId() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidSpanId(W3CPropagationType propagationType) { Map invalidHeaders = new HashMap<>(); invalidHeaders.put(TRACE_PARENT, "00-" + TRACE_ID_BASE16 + "-" + "abcdefghijklmnop" + "-01"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders)) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders)) .isSameAs(TraceContextOrSamplingFlags.EMPTY); } - @Test - void extract_InvalidSpanId_Size() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidSpanId_Size(W3CPropagationType propagationType) { Map invalidHeaders = new HashMap<>(); invalidHeaders.put(TRACE_PARENT, "00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "00-01"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders)) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders)) .isSameAs(TraceContextOrSamplingFlags.EMPTY); } - @Test - void extract_InvalidTraceFlags() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidTraceFlags(W3CPropagationType propagationType) { Map invalidHeaders = new HashMap<>(); invalidHeaders.put(TRACE_PARENT, "00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-gh"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders)) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders)) .isSameAs(TraceContextOrSamplingFlags.EMPTY); } - @Test - void extract_InvalidTraceFlags_Size() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidTraceFlags_Size(W3CPropagationType propagationType) { Map invalidHeaders = new HashMap<>(); invalidHeaders.put(TRACE_PARENT, "00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-0100"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders)) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders)) .isSameAs(TraceContextOrSamplingFlags.EMPTY); } - @Test - void extract_InvalidTracestate_EntriesDelimiter() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidTracestate_EntriesDelimiter(W3CPropagationType propagationType) { Map invalidHeaders = new HashMap<>(); invalidHeaders.put(TRACE_PARENT, "00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-01"); invalidHeaders.put(TRACE_STATE, "foo=bar;test=test"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders).context()) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders).context()) .isEqualTo(sharedTraceContext().build()); } @@ -268,51 +291,57 @@ private TraceContext.Builder sharedTraceContext() { return sampledTraceContext().shared(true); } - @Test - void extract_InvalidTracestate_KeyValueDelimiter() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidTracestate_KeyValueDelimiter(W3CPropagationType propagationType) { Map invalidHeaders = new HashMap<>(); invalidHeaders.put(TRACE_PARENT, "00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-01"); invalidHeaders.put(TRACE_STATE, "foo=bar,test-test"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders).context()) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders).context()) .isEqualTo(sharedTraceContext().build()); } - @Test - void extract_InvalidTracestate_OneString() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidTracestate_OneString(W3CPropagationType propagationType) { Map invalidHeaders = new HashMap<>(); invalidHeaders.put(TRACE_PARENT, "00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-01"); invalidHeaders.put(TRACE_STATE, "test-test"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders).context()) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders).context()) .isEqualTo(sampledTraceContext().shared(true).build()); } - @Test - void extract_InvalidVersion_ff() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidVersion_ff(W3CPropagationType propagationType) { Map invalidHeaders = new HashMap<>(); invalidHeaders.put(TRACE_PARENT, "ff-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-01"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders)) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders)) .isSameAs(TraceContextOrSamplingFlags.EMPTY); } - @Test - void extract_InvalidTraceparent_extraTrailing() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_InvalidTraceparent_extraTrailing(W3CPropagationType propagationType) { Map invalidHeaders = new HashMap<>(); invalidHeaders.put(TRACE_PARENT, "00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-00-01"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders)) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders)) .isSameAs(TraceContextOrSamplingFlags.EMPTY); } - @Test - void extract_ValidTraceparent_nextVersion_extraTrailing() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_ValidTraceparent_nextVersion_extraTrailing(W3CPropagationType propagationType) { Map invalidHeaders = new HashMap<>(); invalidHeaders.put(TRACE_PARENT, "01-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-00-01"); - assertThat(w3CPropagation.extractor(getter).extract(invalidHeaders).context()) + assertThat(propagationType.get().extractor(getter).extract(invalidHeaders).context()) .isEqualTo(sharedTraceContext().build()); } - @Test - void fieldsList() { - assertThat(w3CPropagation.keys()).containsExactly(TRACE_PARENT, TRACE_STATE); + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void fieldsList(W3CPropagationType propagationType) { + assertThat(propagationType.get().keys()).containsExactly(TRACE_PARENT, TRACE_STATE); } @Test @@ -321,10 +350,30 @@ void headerNames() { assertThat(TRACE_STATE).isEqualTo("tracestate"); } - @Test - void extract_emptyCarrier() { + @ParameterizedTest + @EnumSource(W3CPropagationType.class) + void extract_emptyCarrier(W3CPropagationType propagationType) { Map emptyHeaders = new HashMap<>(); - assertThat(w3CPropagation.extractor(getter).extract(emptyHeaders)).isSameAs(TraceContextOrSamplingFlags.EMPTY); + assertThat(propagationType.get().extractor(getter).extract(emptyHeaders)) + .isSameAs(TraceContextOrSamplingFlags.EMPTY); + } + + enum W3CPropagationType implements Supplier { + + WITH_BAGGAGE { + @Override + public W3CPropagation get() { + return new W3CPropagation(new BraveBaggageManager(), new ArrayList<>()); + } + }, + + WITHOUT_BAGGAGE { + @Override + public W3CPropagation get() { + return new W3CPropagation(); + } + } + } }