diff --git a/benchmarks/pom.xml b/benchmarks/pom.xml index 87c2b141a..8c3d93aaf 100644 --- a/benchmarks/pom.xml +++ b/benchmarks/pom.xml @@ -34,7 +34,7 @@ 1.8 1.8 2.1.10.RELEASE - 5.11.0 + 5.11.1 3.11.0 diff --git a/pom.xml b/pom.xml index d527d8009..dca7321ea 100644 --- a/pom.xml +++ b/pom.xml @@ -264,7 +264,7 @@ Fishtown.SR4 2.1.6.BUILD-SNAPSHOT 2.1.6.BUILD-SNAPSHOT - 5.11.0 + 5.11.1 2.1.2.RELEASE diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/Slf4jScopeDecorator.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/Slf4jScopeDecorator.java index c707d2fd4..f4bce750c 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/Slf4jScopeDecorator.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/Slf4jScopeDecorator.java @@ -22,6 +22,7 @@ import java.util.TreeSet; import brave.baggage.BaggageField; import brave.baggage.BaggageFields; +import brave.baggage.CorrelationField; import brave.baggage.CorrelationScopeDecorator; import brave.context.slf4j.MDCScopeDecorator; import brave.propagation.CurrentTraceContext.Scope; @@ -45,34 +46,43 @@ final class Slf4jScopeDecorator implements ScopeDecorator { // Backward compatibility for all logging patterns private static final ScopeDecorator LEGACY_IDS = MDCScopeDecorator.newBuilder() - .clear().addField(BaggageFields.TRACE_ID, "X-B3-TraceId") - .addField(BaggageFields.PARENT_ID, "X-B3-ParentSpanId") - .addField(BaggageFields.SPAN_ID, "X-B3-SpanId") - .addField(BaggageFields.SAMPLED, "X-Span-Export").build(); + .clear() + .addField(CorrelationField.newBuilder(BaggageFields.TRACE_ID) + .name("X-B3-TraceId").build()) + .addField(CorrelationField.newBuilder(BaggageFields.PARENT_ID) + .name("X-B3-ParentSpanId").build()) + .addField(CorrelationField.newBuilder(BaggageFields.SPAN_ID) + .name("X-B3-SpanId").build()) + .addField(CorrelationField.newBuilder(BaggageFields.SAMPLED) + .name("X-Span-Export").build()) + .build(); private final ScopeDecorator delegate; Slf4jScopeDecorator(SleuthProperties sleuthProperties, SleuthSlf4jProperties sleuthSlf4jProperties) { CorrelationScopeDecorator.Builder builder = MDCScopeDecorator.newBuilder().clear() - .addField(BaggageFields.TRACE_ID).addField(BaggageFields.PARENT_ID) - .addField(BaggageFields.SPAN_ID) - .addField(BaggageFields.SAMPLED, "spanExportable"); + .addField(CorrelationField.create(BaggageFields.TRACE_ID)) + .addField(CorrelationField.create(BaggageFields.PARENT_ID)) + .addField(CorrelationField.create(BaggageFields.SPAN_ID)) + .addField(CorrelationField.newBuilder(BaggageFields.SAMPLED) + .name("spanExportable").build()); Set whitelist = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); whitelist.addAll(sleuthSlf4jProperties.getWhitelistedMdcKeys()); + // Note: we are adding all the keys as-is because correlation context doesn't + // prefix, only ExtraFieldPropagation does Set retained = new LinkedHashSet<>(); retained.addAll(sleuthProperties.getBaggageKeys()); retained.addAll(sleuthProperties.getPropagationKeys()); + retained.retainAll(whitelist); + // For backwards compatibility set all fields dirty, so that any changes made by + // MDC directly are reverted. for (String name : retained) { - if (whitelist.contains(name)) { - // Until we move off ExtraFieldPropagation onto BaggagePropagation, - // manually create the fields... - builder.addField(BaggageField.create(name)); - builder.addDirtyName(name); - } + builder.addField(CorrelationField.newBuilder(BaggageField.create(name)) + .dirty().build()); } this.delegate = builder.build(); 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 9cada2dc8..3f8fda3cc 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 @@ -18,8 +18,10 @@ package org.springframework.cloud.sleuth.log; import brave.Span; import brave.Tracer; +import brave.baggage.CorrelationField; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.ExtraFieldPropagation; +import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -40,7 +42,7 @@ import static org.assertj.core.api.Assertions.assertThat; */ @RunWith(SpringRunner.class) @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE, properties = { - "spring.sleuth.baggage-keys=my-baggage", + "spring.sleuth.baggage-keys=my-baggage,my-baggage-two", "spring.sleuth.propagation-keys=my-propagation", "spring.sleuth.log.slf4j.whitelisted-mdc-keys=my-baggage,my-propagation" }) @SpringBootConfiguration @@ -148,4 +150,54 @@ public class Slf4JSpanLoggerTest { assertThat(MDC.get("traceId")).isEqualTo("A"); } + // #1416 + @Test + public void should_clear_any_mdc_entries_when_their_keys_are_whitelisted() + throws Exception { + + Scope scope = this.slf4jScopeDecorator.decorateScope(this.span.context(), () -> { + }); + + MDC.put("my-baggage", "A"); + MDC.put("my-propagation", "B"); + + assertThat(MDC.get("my-baggage")).isEqualTo("A"); + assertThat(MDC.get("my-propagation")).isEqualTo("B"); + + scope.close(); + + assertThat(MDC.get("my-baggage")).isNullOrEmpty(); + assertThat(MDC.get("my-propagation")).isNullOrEmpty(); + } + + @Test + public void should_only_include_whitelist() { + assertThat(this.slf4jScopeDecorator).extracting("delegate.fields") + .asInstanceOf(InstanceOfAssertFactories.array(CorrelationField[].class)) + // my-baggage-two is baggage not in the whitelist + .extracting(CorrelationField::name).containsExactly("traceId", "parentId", + "spanId", "spanExportable", "my-baggage", "my-propagation"); + } + + @Test + public void should_pick_previous_mdc_entries_when_their_keys_are_whitelisted() { + + MDC.put("my-baggage", "A1"); + MDC.put("my-propagation", "B1"); + + Scope scope = this.slf4jScopeDecorator.decorateScope(this.span.context(), () -> { + }); + + MDC.put("my-baggage", "A2"); + MDC.put("my-propagation", "B2"); + + assertThat(MDC.get("my-baggage")).isEqualTo("A2"); + assertThat(MDC.get("my-propagation")).isEqualTo("B2"); + + scope.close(); + + assertThat(MDC.get("my-baggage")).isEqualTo("A1"); + assertThat(MDC.get("my-propagation")).isEqualTo("B1"); + } + } diff --git a/spring-cloud-sleuth-dependencies/pom.xml b/spring-cloud-sleuth-dependencies/pom.xml index 05dfa27b8..154b7c4c6 100644 --- a/spring-cloud-sleuth-dependencies/pom.xml +++ b/spring-cloud-sleuth-dependencies/pom.xml @@ -31,7 +31,7 @@ spring-cloud-sleuth-dependencies Spring Cloud Sleuth Dependencies - 5.11.0 + 5.11.1 0.33.13 3.0.1