From f01a65b03590da84f43e279d5e80d551511e6927 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Mon, 6 Apr 2020 19:40:17 +0800 Subject: [PATCH] Deprecates spring.sleuth.baggage-keys property (#1605) The `spring.sleuth.baggage-keys` property assigned two headers for each baggage. This causes unnecessary overhead and can be accomplished in another way now: ```java public class MyCode { static final BaggageField USER_ID = BaggageField.create("userId"); @Autowired CurrentTraceContext currentTraceContext; @Nullable public String currentUserId() { return USER_ID.getValue(currentTraceContext.get()); } } // In your @Configuration type, if you want to map multiple prefixes... @Bean BaggagePropagationConfig userIdBaggageConfig() { return SingleBaggageField.newBuilder(MyBaggage.USER_ID) .addKeyName("baggage-userId") .addKeyName("baggage_userId") .build(); } ``` --- README.adoc | 3 - docs/src/main/asciidoc/_configprops.adoc | 1 - .../main/asciidoc/spring-cloud-sleuth.adoc | 20 ++- .../PropertyBasedBaggageConfiguration.java | 31 ++++- .../sleuth/autoconfig/SleuthProperties.java | 44 +++---- ...raceAutoConfigurationCustomizersTests.java | 2 +- ...gurationPropagationCustomizationTests.java | 4 +- .../instrument/multiple/DemoApplication.java | 6 +- .../MultipleHopsIntegrationTests.java | 39 +++--- .../opentracing/BraveTracerTest.java | 6 +- .../cloud/sleuth/log/Slf4JSpanLoggerTest.java | 114 ++++++------------ ...TagPropagationFinishedSpanHandlerTest.java | 16 +-- 12 files changed, 128 insertions(+), 158 deletions(-) diff --git a/README.adoc b/README.adoc index 9aea84d9b..7561c7e8d 100644 --- a/README.adoc +++ b/README.adoc @@ -380,9 +380,6 @@ The following listing shows integration tests that use baggage: [source,yml] ---- spring.sleuth: - baggage-keys: - - baz - - bizarrecase local-keys: - bp remote-keys: diff --git a/docs/src/main/asciidoc/_configprops.adoc b/docs/src/main/asciidoc/_configprops.adoc index b5547db82..c862a9dba 100644 --- a/docs/src/main/asciidoc/_configprops.adoc +++ b/docs/src/main/asciidoc/_configprops.adoc @@ -5,7 +5,6 @@ |spring.sleuth.async.configurer.enabled | true | Enable default AsyncConfigurer. |spring.sleuth.async.enabled | true | Enable instrumenting async related components so that the tracing information is passed between threads. |spring.sleuth.async.ignored-beans | | List of {@link java.util.concurrent.Executor} bean names that should be ignored and not wrapped in a trace representation. -|spring.sleuth.baggage-keys | | List of baggage key names that should be propagated out of process. These keys will be prefixed with `baggage` before the actual key. This property is set in order to be backward compatible with previous Sleuth versions. @see brave.propagation.ExtraFieldPropagation.FactoryBuilder#addPrefixedFields(String, java.util.Collection) |spring.sleuth.circuitbreaker.enabled | true | Enable Spring Cloud CircuitBreaker instrumentation. |spring.sleuth.enabled | true | |spring.sleuth.feign.enabled | true | Enable span information propagation when using Feign. diff --git a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc index ff8180336..fdc424840 100644 --- a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc +++ b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc @@ -87,10 +87,15 @@ Doing so forces the current span to be exportable regardless of the sampling dec In order to use the rate-limited sampler set the `spring.sleuth.sampler.rate` property to choose an amount of traces to accept on a per-second interval. The minimum number is 0 and the max is 2,147,483,647 (max int). == Baggage -With the `spring.sleuth.baggage-keys`, you set keys that get prefixed with `baggage-` for HTTP calls and `baggage_` for messaging. -You can also use the `spring.sleuth.remote-keys` property to pass a list of prefixed keys that are propagated to remote services without any prefix. -You can also use the `spring.sleuth.local-keys` property to pass a list keys that will be propagated locally but will not be propagated over the wire. -Notice that there's no `x-` in front of the header keys. +Baggage are fields that are propagated with the trace, optionally out of process. You can use +properties to define fields that have no special configuration such as name mapping: + + * `spring.sleuth.remote-keys` is a list of header names to accept and propagate to remote services. + * `spring.sleuth.local-keys` is a list of names to propagate locally + +No prefixing applies with these keys. What you set is literally what is used. + +A name set in either of these properties will result in a `BaggageField` of the same name. In order to automatically set the baggage values to Slf4j's MDC, you have to set the `spring.sleuth.log.slf4j.whitelisted-mdc-keys` property with a list of whitelisted @@ -102,6 +107,13 @@ IMPORTANT: Remember that adding entries to MDC can drastically decrease the perf If you want to add the baggage entries as tags, to make it possible to search for spans via the baggage entries, you can set the value of `spring.sleuth.propagation.tag.whitelisted-keys` with a list of whitelisted baggage keys. To disable the feature you have to pass the `spring.sleuth.propagation.tag.enabled=false` property. +=== Java configuration + +If you need to do anything more advanced than above, do not define properties and instead use a +`@Bean` config for the baggage fields you use. + * `SingleBaggageField` controls header names for one `BaggageField`. + * `SingleCorrelationField` controls the MDC name of one `BaggageField`, and whether updates flush. + == Instrumentation Spring Cloud Sleuth automatically instruments all your Spring applications, so you should not have to do anything to activate it. diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/PropertyBasedBaggageConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/PropertyBasedBaggageConfiguration.java index f09ac50d8..8838ac111 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/PropertyBasedBaggageConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/PropertyBasedBaggageConfiguration.java @@ -25,6 +25,8 @@ import brave.baggage.BaggagePropagationConfig; import brave.baggage.BaggagePropagationConfig.SingleBaggageField; import brave.baggage.CorrelationScopeConfig; import brave.baggage.CorrelationScopeConfig.SingleCorrelationField; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; @@ -41,6 +43,8 @@ import org.springframework.core.env.Environment; @ConditionalOnProperty(value = "spring.sleuth.enabled", matchIfMissing = true) public class PropertyBasedBaggageConfiguration implements BeanFactoryPostProcessor { + static final Log logger = LogFactory.getLog(PropertyBasedBaggageConfiguration.class); + @Override public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) { Environment env = beanFactory.getBean(Environment.class); @@ -70,11 +74,28 @@ public class PropertyBasedBaggageConfiguration implements BeanFactoryPostProcess baggageConfigs.add(SingleBaggageField.remote(BaggageField.create(key))); } - for (String key : collectKeysOfType(env, "baggage")) { - baggageConfigs.add(SingleBaggageField.newBuilder(BaggageField.create(key)) - .addKeyName("baggage-" + key) // for HTTP - .addKeyName("baggage_" + key) // for messaging - .build()); + Set propagationKeys = collectKeysOfType(env, "propagation"); + if (!propagationKeys.isEmpty()) { + logger.warn( + "'spring.sleuth.propagation-keys' has been renamed to 'spring.sleuth.remote-keys' and will be removed in a future release."); + for (String key : propagationKeys) { + baggageConfigs.add(SingleBaggageField.remote(BaggageField.create(key))); + } + } + + Set baggageKeys = collectKeysOfType(env, "baggage"); + if (!baggageKeys.isEmpty()) { + logger.warn( + "'spring.sleuth.baggage-keys' will be removed in a future release.\n" + + "To change header names define a @Bean of type " + + SingleBaggageField.class.getName()); + + for (String key : baggageKeys) { + baggageConfigs.add(SingleBaggageField.newBuilder(BaggageField.create(key)) + .addKeyName("baggage-" + key) // for HTTP + .addKeyName("baggage_" + key) // for messaging + .build()); + } } return baggageConfigs; } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/SleuthProperties.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/SleuthProperties.java index 82ea31021..8eded441e 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/SleuthProperties.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/SleuthProperties.java @@ -45,13 +45,12 @@ public class SleuthProperties { private boolean supportsJoin = true; /** - * List of baggage key names that should be propagated out of process. These keys will - * be prefixed with `baggage` before the actual key. This property is set in order to - * be backward compatible with previous Sleuth versions. + * Same as {@link #remoteKeys} except that this field is not propagated to remote + * services. * - * @see BaggagePropagationConfig.SingleBaggageField.Builder#addKeyName(String) + * @see BaggagePropagationConfig.SingleBaggageField#local(BaggageField) */ - private List baggageKeys = new ArrayList<>(); + private List localKeys = new ArrayList<>(); /** * List of fields that are referenced the same in-process as it is on the wire. For @@ -61,16 +60,9 @@ public class SleuthProperties { * Note: {@code fieldName} will be implicitly lower-cased. * * @see BaggagePropagationConfig.SingleBaggageField#remote(BaggageField) + * @see BaggagePropagationConfig.SingleBaggageField.Builder#addKeyName(String) */ - private List propagationKeys = new ArrayList<>(); - - /** - * Same as {@link #propagationKeys} except that this field is not propagated to remote - * services. - * - * @see BaggagePropagationConfig.SingleBaggageField#local(BaggageField) - */ - private List localKeys = new ArrayList<>(); + private List remoteKeys = new ArrayList<>(); public boolean isEnabled() { return this.enabled; @@ -96,22 +88,6 @@ public class SleuthProperties { this.supportsJoin = supportsJoin; } - public List getBaggageKeys() { - return this.baggageKeys; - } - - public void setBaggageKeys(List baggageKeys) { - this.baggageKeys = baggageKeys; - } - - public List getPropagationKeys() { - return this.propagationKeys; - } - - public void setPropagationKeys(List propagationKeys) { - this.propagationKeys = propagationKeys; - } - public List getLocalKeys() { return this.localKeys; } @@ -120,4 +96,12 @@ public class SleuthProperties { this.localKeys = localKeys; } + public List getRemoteKeys() { + return this.remoteKeys; + } + + public void setRemoteKeys(List remoteKeys) { + this.remoteKeys = remoteKeys; + } + } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationCustomizersTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationCustomizersTests.java index 471fc9718..2ba561ef4 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationCustomizersTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationCustomizersTests.java @@ -51,7 +51,7 @@ public class TraceAutoConfigurationCustomizersTests { @Test public void should_apply_customizers() { - this.contextRunner.withPropertyValues("spring.sleuth.baggage-keys=my-baggage") + this.contextRunner.withPropertyValues("spring.sleuth.remote-keys=country-code") .run((context) -> { Customizers bean = context.getBean(Customizers.class); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationPropagationCustomizationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationPropagationCustomizationTests.java index 791b7e481..fcbc6adac 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationPropagationCustomizationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfigurationPropagationCustomizationTests.java @@ -48,7 +48,7 @@ public class TraceAutoConfigurationPropagationCustomizationTests { @Test public void allowsCustomization() { - this.contextRunner.withPropertyValues("spring.sleuth.baggage-keys=my-baggage") + this.contextRunner.withPropertyValues("spring.sleuth.remote-keys=country-code") .run((context) -> { BDDAssertions.then(context.getBean(Propagation.Factory.class)) .hasFieldOrPropertyWithValue("delegate", @@ -79,7 +79,7 @@ public class TraceAutoConfigurationPropagationCustomizationTests { @Test public void allowsCustomizationOfBuilder() { - this.contextRunner.withPropertyValues("spring.sleuth.baggage-keys=my-baggage") + this.contextRunner.withPropertyValues("spring.sleuth.remote-keys=country-code") .withUserConfiguration(CustomPropagationFactoryBuilderConfig.class) .run((context) -> BDDAssertions .then(context.getBean(Propagation.Factory.class)) diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/multiple/DemoApplication.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/multiple/DemoApplication.java index 875064d23..78e7f60e9 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/multiple/DemoApplication.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/multiple/DemoApplication.java @@ -22,7 +22,6 @@ import java.util.List; import brave.Span; import brave.Tags; import brave.Tracer; -import brave.baggage.BaggageField; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -41,6 +40,8 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; +import static org.springframework.cloud.sleuth.instrument.multiple.MultipleHopsIntegrationTests.COUNTRY_CODE; + @MessagingGateway(name = "greeter") interface Sender { @@ -77,8 +78,7 @@ public class DemoApplication { this.httpSpan = this.tracer.currentSpan(); // tag what was propagated - BaggageField baz = BaggageField.getByName(httpSpan.context(), "baz"); - Tags.BAGGAGE_FIELD.tag(baz, httpSpan); + Tags.BAGGAGE_FIELD.tag(COUNTRY_CODE, httpSpan); return new Greeting(message); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/multiple/MultipleHopsIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/multiple/MultipleHopsIntegrationTests.java index 818b892da..7a6b2cef4 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/multiple/MultipleHopsIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/multiple/MultipleHopsIntegrationTests.java @@ -55,10 +55,13 @@ import static org.awaitility.Awaitility.await; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; @SpringBootTest(classes = MultipleHopsIntegrationTests.Config.class, - webEnvironment = RANDOM_PORT, properties = { "spring.sleuth.baggage-keys=baz", - "spring.sleuth.remote-keys=country-code" }) + webEnvironment = RANDOM_PORT, + properties = { "spring.sleuth.remote-keys=x-vcap-request-id,country-code", + "spring.sleuth.local-keys=bp" }) public class MultipleHopsIntegrationTests { + static final BaggageField REQUEST_ID = BaggageField.create("x-vcap-request-id"); + static final BaggageField BUSINESS_PROCESS = BaggageField.create("bp"); static final BaggageField COUNTRY_CODE = BaggageField.create("country-code"); @Autowired @@ -103,25 +106,21 @@ public class MultipleHopsIntegrationTests { @Test public void should_propagate_the_baggage() { - // TODO: make a DemoBaggage type instead of saying to use the api directly - BaggageField bar = BaggageField.create("bar"); - BaggageField baz = BaggageField.create("baz"); - // tag::baggage[] Span initialSpan = this.tracer.nextSpan().name("span").start(); + BUSINESS_PROCESS.updateValue(initialSpan.context(), "ALM"); COUNTRY_CODE.updateValue(initialSpan.context(), "FO"); - bar.updateValue(initialSpan.context(), "2"); // end::baggage[] try (SpanInScope ws = this.tracer.withSpanInScope(initialSpan)) { // tag::baggage_tag[] - Tags.BAGGAGE_FIELD.tag(COUNTRY_CODE, initialSpan); - Tags.BAGGAGE_FIELD.tag(bar, initialSpan); + Tags.BAGGAGE_FIELD.tag(BUSINESS_PROCESS, initialSpan); // end::baggage_tag[] - // set baz in a header not with the api explicitly + // set request ID in a header not with the api explicitly HttpHeaders headers = new HttpHeaders(); - headers.put("baggage-baz", Collections.singletonList("3")); + headers.put(REQUEST_ID.name(), + Collections.singletonList("f4308d05-2228-4468-80f6-92a8377ba193")); RequestEntity requestEntity = new RequestEntity(headers, HttpMethod.GET, URI.create("http://localhost:" + this.config.port + "/greeting")); this.restTemplate.exchange(requestEntity, String.class); @@ -135,23 +134,23 @@ public class MultipleHopsIntegrationTests { }); List withBagTags = this.reporter.getSpans().stream() - .filter(s -> s.tags().containsKey(COUNTRY_CODE.name())).collect(toList()); + .filter(s -> s.tags().containsKey(BUSINESS_PROCESS.name())) + .collect(toList()); // set with tag api then(withBagTags).as("only initialSpan was bag tagged").hasSize(1); - assertThat(withBagTags.get(0).tags()).containsEntry("country-code", "FO") - .containsEntry("bar", "2"); + assertThat(withBagTags.get(0).tags()).containsEntry(BUSINESS_PROCESS.name(), + "ALM"); // set with baggage api - then(this.application.allSpans()).as("All have country-code") - .allMatch(span -> "FO".equals(COUNTRY_CODE.getValue(span.context()))); - then(this.application.allSpans()).as("All have bar") - .allMatch(span -> "2".equals(bar.getValue(span.context()))); + then(this.application.allSpans()).as("All have request ID") + .allMatch(span -> "f4308d05-2228-4468-80f6-92a8377ba193" + .equals(REQUEST_ID.getValue(span.context()))); // baz is not tagged in the initial span, only downstream! - then(this.application.allSpans()).as("All downstream have baz") + then(this.application.allSpans()).as("All downstream have country-code") .filteredOn(span -> !span.equals(initialSpan)) - .allMatch(span -> "3".equals(baz.getValue(span.context()))); + .allMatch(span -> "FO".equals(COUNTRY_CODE.getValue(span.context()))); } @Configuration diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/opentracing/BraveTracerTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/opentracing/BraveTracerTest.java index de7946dc5..39de8277d 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/opentracing/BraveTracerTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/opentracing/BraveTracerTest.java @@ -55,7 +55,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen * @author Marcin Grzejszczak */ @SpringBootTest(webEnvironment = NONE, - properties = "spring.sleuth.baggage-keys=country-code,country-code") + properties = "spring.sleuth.remote-keys=country-code") public class BraveTracerTest { @Autowired @@ -100,7 +100,7 @@ public class BraveTracerTest { map.put("X-B3-TraceId", "0000000000000001"); map.put("X-B3-SpanId", "0000000000000002"); map.put("X-B3-Sampled", "1"); - map.put("baggage-country-code", "FO"); + map.put("country-code", "FO"); BraveSpanContext openTracingContext = this.opentracing .extract(Format.Builtin.HTTP_HEADERS, new TextMapAdapter(map)); @@ -147,7 +147,7 @@ public class BraveTracerTest { TextMapAdapter carrier = new TextMapAdapter(map); this.opentracing.inject(span.context(), Format.Builtin.HTTP_HEADERS, carrier); - assertThat(map).containsEntry("baggage-country-code", "FO"); + assertThat(map).containsEntry("country-code", "FO"); } void checkSpanReportedToZipkin() { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/log/Slf4JSpanLoggerTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/log/Slf4JSpanLoggerTest.java index e39342ca7..cbe2f35d2 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/log/Slf4JSpanLoggerTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/log/Slf4JSpanLoggerTest.java @@ -22,7 +22,6 @@ import brave.baggage.BaggageField; import brave.baggage.CorrelationScopeConfig.SingleCorrelationField; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.CurrentTraceContext.ScopeDecorator; -import brave.propagation.ExtraFieldPropagation; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -40,10 +39,10 @@ import static org.assertj.core.api.Assertions.assertThat; /** * @author Marcin Grzejszczak */ -@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE, properties = { - "spring.sleuth.baggage-keys=my-baggage,my-baggage-two", - "spring.sleuth.remote-keys=country-code", "spring.sleuth.local-keys=bp", - "spring.sleuth.log.slf4j.whitelisted-mdc-keys=my-baggage,country-code,bp" }) +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE, + properties = { "spring.sleuth.remote-keys=x-vcap-request-id,country-code", + "spring.sleuth.local-keys=bp", + "spring.sleuth.log.slf4j.whitelisted-mdc-keys=country-code,bp" }) @SpringBootConfiguration @EnableAutoConfiguration public class Slf4JSpanLoggerTest { @@ -55,7 +54,7 @@ public class Slf4JSpanLoggerTest { Tracer tracer; @Autowired - ScopeDecorator slf4jScopeDecorator; + ScopeDecorator scopeDecorator; Span span; @@ -67,140 +66,107 @@ public class Slf4JSpanLoggerTest { } @Test - public void should_set_entries_to_mdc_from_span() throws Exception { - Scope scope = this.slf4jScopeDecorator.decorateScope(this.span.context(), () -> { - }); - - assertThat(MDC.get("traceId")).isEqualTo(this.span.context().traceIdString()); - - scope.close(); + public void should_set_entries_to_mdc_from_span() { + // can't use NOOP as it is special cased + try (Scope scope = this.scopeDecorator.decorateScope(this.span.context(), () -> { + })) { + assertThat(MDC.get("traceId")).isEqualTo(this.span.context().traceIdString()); + } assertThat(MDC.get("traceId")).isNullOrEmpty(); } @Test - public void should_set_entries_to_mdc_from_span_with_baggage() throws Exception { - ExtraFieldPropagation.set(this.span.context(), "my-baggage", "my-value"); + public void should_set_entries_to_mdc_from_span_with_baggage() { COUNTRY_CODE.updateValue(this.span.context(), "FO"); BUSINESS_PROCESS.updateValue(this.span.context(), "ALM"); - Scope scope = this.slf4jScopeDecorator.decorateScope(this.span.context(), () -> { - }); - assertThat(MDC.get("my-baggage")).isEqualTo("my-value"); - assertThat(MDC.get(COUNTRY_CODE.name())).isEqualTo("FO"); - assertThat(MDC.get(BUSINESS_PROCESS.name())).isEqualTo("ALM"); + try (Scope scope = this.scopeDecorator.decorateScope(this.span.context(), NOOP)) { + assertThat(MDC.get(COUNTRY_CODE.name())).isEqualTo("FO"); + assertThat(MDC.get(BUSINESS_PROCESS.name())).isEqualTo("ALM"); + } - scope.close(); - - assertThat(MDC.get("my-baggage")).isNullOrEmpty(); assertThat(MDC.get(COUNTRY_CODE.name())).isNull(); assertThat(MDC.get(BUSINESS_PROCESS.name())).isNull(); } @Test - public void should_remove_entries_from_mdc_for_null_span() throws Exception { - ExtraFieldPropagation.set(this.span.context(), "my-baggage", "my-value"); + public void should_remove_entries_from_mdc_for_null_span() { COUNTRY_CODE.updateValue(this.span.context(), "FO"); - try (Scope scope1 = this.slf4jScopeDecorator.decorateScope(this.span.context(), + try (Scope scope1 = this.scopeDecorator.decorateScope(this.span.context(), NOOP)) { - assertThat(MDC.get("my-baggage")).isEqualTo("my-value"); assertThat(MDC.get(COUNTRY_CODE.name())).isEqualTo("FO"); - try (Scope scope2 = this.slf4jScopeDecorator.decorateScope(null, NOOP)) { - assertThat(MDC.get("my-baggage")).isNullOrEmpty(); + try (Scope scope2 = this.scopeDecorator.decorateScope(null, NOOP)) { assertThat(MDC.get(COUNTRY_CODE.name())).isNullOrEmpty(); } } } @Test - public void should_remove_entries_from_mdc_for_null_span_and_mdc_fields_set_directly() - throws Exception { - MDC.put("my-baggage", "my-value"); + public void should_remove_entries_from_mdc_for_null_span_and_mdc_fields_set_directly() { MDC.put(COUNTRY_CODE.name(), "FO"); // the span is holding no baggage so it clears the preceding values - try (Scope scope = this.slf4jScopeDecorator.decorateScope(this.span.context(), - NOOP)) { - assertThat(MDC.get("my-baggage")).isNullOrEmpty(); + try (Scope scope = this.scopeDecorator.decorateScope(this.span.context(), NOOP)) { assertThat(MDC.get(COUNTRY_CODE.name())).isNullOrEmpty(); } - assertThat(MDC.get("my-baggage")).isEqualTo("my-value"); assertThat(MDC.get(COUNTRY_CODE.name())).isEqualTo("FO"); - try (Scope scope = this.slf4jScopeDecorator.decorateScope(null, NOOP)) { - assertThat(MDC.get("my-baggage")).isNullOrEmpty(); + try (Scope scope = this.scopeDecorator.decorateScope(null, NOOP)) { assertThat(MDC.get(COUNTRY_CODE.name())).isNullOrEmpty(); } - assertThat(MDC.get("my-baggage")).isEqualTo("my-value"); assertThat(MDC.get(COUNTRY_CODE.name())).isEqualTo("FO"); } @Test - public void should_remove_entries_from_mdc_from_null_span() throws Exception { + public void should_remove_entries_from_mdc_from_null_span() { MDC.put("traceId", "A"); - Scope scope = this.slf4jScopeDecorator.decorateScope(null, () -> { - }); - - assertThat(MDC.get("traceId")).isNullOrEmpty(); - - scope.close(); + // can't use NOOP as it is special cased + try (Scope scope = this.scopeDecorator.decorateScope(null, () -> { + })) { + assertThat(MDC.get("traceId")).isNullOrEmpty(); + } assertThat(MDC.get("traceId")).isEqualTo("A"); } // #1416 @Test - public void should_clear_any_mdc_entries_when_their_keys_are_whitelisted() - throws Exception { + public void should_clear_any_mdc_entries_when_their_keys_are_whitelisted() { + try (Scope scope = this.scopeDecorator.decorateScope(this.span.context(), NOOP)) { + MDC.put(COUNTRY_CODE.name(), "FO"); - Scope scope = this.slf4jScopeDecorator.decorateScope(this.span.context(), () -> { - }); + assertThat(MDC.get(COUNTRY_CODE.name())).isEqualTo("FO"); + } - MDC.put("my-baggage", "A"); - MDC.put(COUNTRY_CODE.name(), "FO"); - - assertThat(MDC.get("my-baggage")).isEqualTo("A"); - assertThat(MDC.get(COUNTRY_CODE.name())).isEqualTo("FO"); - - scope.close(); - - assertThat(MDC.get("my-baggage")).isNullOrEmpty(); assertThat(MDC.get(COUNTRY_CODE.name())).isNullOrEmpty(); } @Test public void should_only_include_whitelist() { - assertThat(this.slf4jScopeDecorator).extracting("fields") + assertThat(this.scopeDecorator).extracting("fields") .asInstanceOf( InstanceOfAssertFactories.array(SingleCorrelationField[].class)) .extracting(SingleCorrelationField::name).containsOnly("traceId", - "parentId", "spanId", "spanExportable", "my-baggage", "bp", - COUNTRY_CODE.name()); // my-baggage-two is not in the whitelist + "parentId", "spanId", "spanExportable", "bp", + COUNTRY_CODE.name()); // x-vcap-request-id is not in the whitelist } @Test public void should_pick_previous_mdc_entries_when_their_keys_are_whitelisted() { - - MDC.put("my-baggage", "A1"); MDC.put(COUNTRY_CODE.name(), "FO"); - Scope scope = this.slf4jScopeDecorator.decorateScope(this.span.context(), () -> { - }); + try (Scope scope = this.scopeDecorator.decorateScope(this.span.context(), NOOP)) { + MDC.put(COUNTRY_CODE.name(), "BV"); - MDC.put("my-baggage", "A2"); - MDC.put(COUNTRY_CODE.name(), "BV"); + assertThat(MDC.get(COUNTRY_CODE.name())).isEqualTo("BV"); + } - assertThat(MDC.get("my-baggage")).isEqualTo("A2"); - assertThat(MDC.get(COUNTRY_CODE.name())).isEqualTo("BV"); - - scope.close(); - - assertThat(MDC.get("my-baggage")).isEqualTo("A1"); assertThat(MDC.get(COUNTRY_CODE.name())).isEqualTo("FO"); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/propagation/TagPropagationFinishedSpanHandlerTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/propagation/TagPropagationFinishedSpanHandlerTest.java index 048ee79d7..4cf2f8176 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/propagation/TagPropagationFinishedSpanHandlerTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/propagation/TagPropagationFinishedSpanHandlerTest.java @@ -22,7 +22,6 @@ import java.util.Map; import brave.ScopedSpan; import brave.Tracer; import brave.baggage.BaggageField; -import brave.propagation.ExtraFieldPropagation; import brave.propagation.TraceContext; import brave.sampler.Sampler; import org.junit.jupiter.api.BeforeEach; @@ -40,20 +39,15 @@ import static org.assertj.core.api.Assertions.assertThat; /** * @author Taras Danylchuk */ -@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE, properties = { - "spring.sleuth.baggage-keys=my-baggage", - "spring.sleuth.remote-keys=country-code,x-vcap-request-id", - "spring.sleuth.propagation.tag.whitelisted-keys=my-baggage,country-code" }, +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE, + properties = { "spring.sleuth.remote-keys=country-code,x-vcap-request-id", + "spring.sleuth.propagation.tag.whitelisted-keys=country-code" }, classes = TagPropagationFinishedSpanHandlerTest.TestConfiguration.class) public class TagPropagationFinishedSpanHandlerTest { static final BaggageField COUNTRY_CODE = BaggageField.create("country-code"); static final BaggageField REQUEST_ID = BaggageField.create("x-vcap-request-id"); - private static final String BAGGAGE_KEY = "my-baggage"; - - private static final String BAGGAGE_VALUE = "332323"; - @Autowired private Tracer tracer; @@ -67,7 +61,6 @@ public class TagPropagationFinishedSpanHandlerTest { this.arrayListSpanReporter.clear(); this.span = this.tracer.startScopedSpan("my-scoped-span"); TraceContext context = this.span.context(); - ExtraFieldPropagation.set(context, BAGGAGE_KEY, BAGGAGE_VALUE); COUNTRY_CODE.updateValue(context, "FO"); REQUEST_ID.updateValue(context, "f4308d05-2228-4468-80f6-92a8377ba193"); } @@ -79,8 +72,7 @@ public class TagPropagationFinishedSpanHandlerTest { List spans = this.arrayListSpanReporter.getSpans(); assertThat(spans).hasSize(1); Map tags = spans.get(0).tags(); - assertThat(tags).hasSize(2); // REQUEST_ID is not in the whitelist - assertThat(tags).containsEntry(BAGGAGE_KEY, BAGGAGE_VALUE); + assertThat(tags).hasSize(1); // REQUEST_ID is not in the whitelist assertThat(tags).containsEntry(COUNTRY_CODE.name(), "FO"); }