Reuses configuration from the given propagation type; fixes gh-1846

spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/web/SkipPatternConfiguration.java

@@ -200,6 +200,7 @@ class SkipPatternConfiguration {
 
 		@Bean
 		@ConditionalOnManagementPort(ManagementPortType.SAME)
+		@ConditionalOnBean(WebEndpointProperties.class)
 		SingleSkipPattern skipPatternForActuatorEndpointsSamePort(Environment environment,
 				final ServerProperties serverProperties, final WebEndpointProperties webEndpointProperties,
 				final EndpointsSupplier<ExposableWebEndpoint> endpointsSupplier) {
@@ -211,6 +212,7 @@ class SkipPatternConfiguration {
 		@ConditionalOnManagementPort(ManagementPortType.DIFFERENT)
 		@ConditionalOnProperty(name = "management.server.servlet.context-path", havingValue = "/",
 				matchIfMissing = true)
+		@ConditionalOnBean(WebEndpointProperties.class)
 		SingleSkipPattern skipPatternForActuatorEndpointsDifferentPort(Environment environment,
 				final WebEndpointProperties webEndpointProperties,
 				ObjectProvider<ManagementServerProperties> managementServerProperties,

spring-cloud-sleuth-autoconfigure/src/test/java/org/springframework/cloud/sleuth/autoconfig/brave/baggage/CompositePropagationFactoryTests.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright 2013-2021 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.sleuth.autoconfig.brave.baggage;
+
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Mono;
+import reactor.util.context.ContextView;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
+import org.springframework.boot.test.autoconfigure.web.reactive.WebFluxTest;
+import org.springframework.cloud.sleuth.DisableWebFluxSecurity;
+import org.springframework.cloud.sleuth.TraceContext;
+import org.springframework.cloud.sleuth.autoconfig.brave.BraveAutoConfiguration;
+import org.springframework.cloud.sleuth.autoconfig.instrument.reactor.TraceReactorAutoConfiguration;
+import org.springframework.cloud.sleuth.autoconfig.instrument.web.TraceWebAutoConfiguration;
+import org.springframework.context.annotation.Import;
+import org.springframework.test.web.reactive.server.WebTestClient;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+import static org.hamcrest.core.IsEqual.equalTo;
+
+@WebFluxTest(controllers = TracingResource.class, properties = "spring.main.web-application-type=reactive")
+@ImportAutoConfiguration({ BraveAutoConfiguration.class, TraceWebAutoConfiguration.class,
+		TraceReactorAutoConfiguration.class })
+@Import(TracingResource.class)
+@DisableWebFluxSecurity
+public class CompositePropagationFactoryTests {
+
+	@Autowired
+	TracingResource tracingResource;
+
+	@Test
+	void should_delegate_configuration_to_propagation_factory(@Autowired WebTestClient webTestClient) {
+		// issue 1846 - without the fix supportJoin is assumed to be false
+		// because it's not taken from the configuration but from not overridden
+		// methods from CompositePropagationFactorySupplier
+		String spanId = "a2fb4a1d1a96d312";
+		webTestClient.get().uri("/api/tracing/spanId").header("X-B3-TraceId", "463ac35c9f6413ad48485a3953bb6124")
+				.header("X-B3-SpanId", spanId).header("X-B3-ParentSpanId", "0020000000000001").header("X-B3-Flags", "1")
+				.exchange().expectStatus().isOk().expectBody(String.class)
+				.value(returnedSpanId -> returnedSpanId, equalTo(spanId));
+
+	}
+
+}
+
+@RestController
+@RequestMapping("/api/tracing")
+class TracingResource {
+
+	private static final Class<TraceContext> KEY = TraceContext.class;
+
+	@GetMapping("spanId")
+	public Mono<String> spanId() {
+		return Mono.deferContextual(view -> traceContext(view)).map(c -> c.spanId());
+
+	}
+
+	private Mono<TraceContext> traceContext(ContextView contextView) {
+		return Mono.justOrEmpty(contextView.get(KEY));
+	}
+
+}

spring-cloud-sleuth-brave/src/main/java/org/springframework/cloud/sleuth/brave/bridge/CompositePropagationFactorySupplier.java

@@ -16,6 +16,7 @@
 
 package org.springframework.cloud.sleuth.brave.bridge;
 
+import java.util.AbstractMap;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -66,32 +67,43 @@ public class CompositePropagationFactorySupplier implements PropagationFactorySu
 
 class CompositePropagationFactory extends Propagation.Factory implements Propagation<String> {
 
-	private final Map<PropagationType, Propagation<String>> mapping = new HashMap<>();
+	private final Map<PropagationType, Map.Entry<Propagation.Factory, Propagation<String>>> mapping = new HashMap<>();
 
 	private final List<PropagationType> types;
 
 	CompositePropagationFactory(BeanFactory beanFactory, BraveBaggageManager braveBaggageManager,
 			List<String> localFields, List<PropagationType> types) {
 		this.types = types;
-		this.mapping.put(PropagationType.AWS, AWSPropagation.FACTORY.get());
+		this.mapping.put(PropagationType.AWS,
+				new AbstractMap.SimpleEntry<>(AWSPropagation.FACTORY, AWSPropagation.FACTORY.get()));
 		// Note: Versions <2.2.3 use injectFormat(MULTI) for non-remote (ex
 		// spring-messaging)
 		// See #1643
-		this.mapping.put(PropagationType.B3,
-				B3Propagation.newFactoryBuilder().injectFormat(B3Propagation.Format.SINGLE_NO_PARENT).build().get());
-		this.mapping.put(PropagationType.W3C, new W3CPropagation(braveBaggageManager, localFields));
-		this.mapping.put(PropagationType.CUSTOM, new LazyPropagation(beanFactory.getBeanProvider(Propagation.class)));
+		Factory b3Factory = b3Factory();
+		this.mapping.put(PropagationType.B3, new AbstractMap.SimpleEntry<>(b3Factory, b3Factory.get()));
+		W3CPropagation w3CPropagation = new W3CPropagation(braveBaggageManager, localFields);
+		this.mapping.put(PropagationType.W3C, new AbstractMap.SimpleEntry<>(w3CPropagation, w3CPropagation.get()));
+		LazyPropagationFactory lazyPropagationFactory = new LazyPropagationFactory(
+				beanFactory.getBeanProvider(Factory.class));
+		this.mapping.put(PropagationType.CUSTOM,
+				new AbstractMap.SimpleEntry<>(lazyPropagationFactory, lazyPropagationFactory.get()));
+	}
+
+	private Factory b3Factory() {
+		return B3Propagation.newFactoryBuilder().injectFormat(B3Propagation.Format.SINGLE_NO_PARENT).build();
 	}
 
 	@Override
 	public List<String> keys() {
-		return this.types.stream().map(this.mapping::get).flatMap(p -> p.keys().stream()).collect(Collectors.toList());
+		return this.types.stream().map(this.mapping::get).flatMap(p -> p.getValue().keys().stream())
+				.collect(Collectors.toList());
 	}
 
 	@Override
 	public <R> TraceContext.Injector<R> injector(Setter<R, String> setter) {
 		return (traceContext, request) -> {
-			this.types.stream().map(this.mapping::get).forEach(p -> p.injector(setter).inject(traceContext, request));
+			this.types.stream().map(this.mapping::get)
+					.forEach(p -> p.getValue().injector(setter).inject(traceContext, request));
 		};
 	}
 
@@ -99,7 +111,11 @@ class CompositePropagationFactory extends Propagation.Factory implements Propaga
 	public <R> TraceContext.Extractor<R> extractor(Getter<R, String> getter) {
 		return request -> {
 			for (PropagationType type : this.types) {
-				Propagation<String> propagator = this.mapping.get(type);
+				Map.Entry<Factory, Propagation<String>> entry = this.mapping.get(type);
+				if (entry == null) {
+					continue;
+				}
+				Propagation<String> propagator = entry.getValue();
 				if (propagator == null || propagator == NoOpPropagation.INSTANCE) {
 					continue;
 				}
@@ -117,33 +133,112 @@ class CompositePropagationFactory extends Propagation.Factory implements Propaga
 		return StringPropagationAdapter.create(this, keyFactory);
 	}
 
+	@Override
+	public boolean supportsJoin() {
+		return this.types.stream().map(this.mapping::get).allMatch(e -> e.getKey().supportsJoin());
+	}
+
+	@Override
+	public boolean requires128BitTraceId() {
+		return this.types.stream().map(this.mapping::get).allMatch(e -> e.getKey().requires128BitTraceId());
+	}
+
+	@Override
+	public TraceContext decorate(TraceContext context) {
+		for (PropagationType type : this.types) {
+			Map.Entry<Factory, Propagation<String>> entry = this.mapping.get(type);
+			if (entry == null) {
+				continue;
+			}
+			TraceContext decorate = entry.getKey().decorate(context);
+			if (decorate != context) {
+				return decorate;
+			}
+		}
+		return super.decorate(context);
+	}
+
 	@SuppressWarnings("unchecked")
-	private static final class LazyPropagation implements Propagation<String> {
+	private static final class LazyPropagationFactory extends Propagation.Factory {
 
-		private final ObjectProvider<Propagation> delegate;
+		private final ObjectProvider<Propagation.Factory> delegate;
 
-		private LazyPropagation(ObjectProvider<Propagation> delegate) {
+		private volatile Propagation.Factory propagationFactory;
+
+		private LazyPropagationFactory(ObjectProvider<Propagation.Factory> delegate) {
 			this.delegate = delegate;
 		}
 
-		@Override
-		public List<String> keys() {
-			return this.delegate.getIfAvailable(() -> NoOpPropagation.INSTANCE).keys();
+		private Propagation.Factory propagationFactory() {
+			if (this.propagationFactory == null) {
+				this.propagationFactory = this.delegate.getIfAvailable(() -> NoOpPropagation.INSTANCE);
+			}
+			return this.propagationFactory;
 		}
 
 		@Override
-		public <R> TraceContext.Injector<R> injector(Setter<R, String> setter) {
-			return this.delegate.getIfAvailable(() -> NoOpPropagation.INSTANCE).injector(setter);
+		public <K> Propagation<K> create(KeyFactory<K> keyFactory) {
+			return propagationFactory().create(keyFactory);
 		}
 
 		@Override
-		public <R> TraceContext.Extractor<R> extractor(Getter<R, String> getter) {
-			return this.delegate.getIfAvailable(() -> NoOpPropagation.INSTANCE).extractor(getter);
+		public boolean supportsJoin() {
+			return propagationFactory().supportsJoin();
+		}
+
+		@Override
+		public boolean requires128BitTraceId() {
+			return propagationFactory().requires128BitTraceId();
+		}
+
+		@Override
+		public Propagation<String> get() {
+			return new LazyPropagation(this);
+		}
+
+		@Override
+		public TraceContext decorate(TraceContext context) {
+			return propagationFactory().decorate(context);
 		}
 
 	}
 
-	private static class NoOpPropagation implements Propagation<String> {
+	@SuppressWarnings("unchecked")
+	private static final class LazyPropagation implements Propagation<String> {
+
+		private final LazyPropagationFactory delegate;
+
+		private volatile Propagation<String> propagation;
+
+		private LazyPropagation(LazyPropagationFactory delegate) {
+			this.delegate = delegate;
+		}
+
+		private Propagation<String> propagation() {
+			if (this.propagation == null) {
+				this.propagation = this.delegate.propagationFactory().get();
+			}
+			return this.propagation;
+		}
+
+		@Override
+		public List<String> keys() {
+			return propagation().keys();
+		}
+
+		@Override
+		public <R> TraceContext.Injector<R> injector(Setter<R, String> setter) {
+			return propagation().injector(setter);
+		}
+
+		@Override
+		public <R> TraceContext.Extractor<R> extractor(Getter<R, String> getter) {
+			return propagation().extractor(getter);
+		}
+
+	}
+
+	private static class NoOpPropagation extends Propagation.Factory implements Propagation<String> {
 
 		static final NoOpPropagation INSTANCE = new NoOpPropagation();
 
@@ -164,6 +259,11 @@ class CompositePropagationFactory extends Propagation.Factory implements Propaga
 			return request -> TraceContextOrSamplingFlags.EMPTY;
 		}
 
+		@Override
+		public <K> Propagation<K> create(KeyFactory<K> keyFactory) {
+			return StringPropagationAdapter.create(this, keyFactory);
+		}
+
 	}
 
 }

spring-cloud-sleuth-brave/src/test/java/org/springframework/cloud/sleuth/brave/bridge/CompositePropagationFactorySupplierTests.java

@@ -41,6 +41,8 @@ class CompositePropagationFactorySupplierTests {
 		BeanFactory beanFactory = Mockito.mock(BeanFactory.class);
 		Mockito.when(beanFactory.getBeanProvider(BraveBaggageManager.class))
 				.thenReturn(new SimpleObjectProvider(new BraveBaggageManager()));
+		Mockito.when(beanFactory.getBeanProvider(Propagation.Factory.class))
+				.thenReturn(new SimpleObjectProvider(new CustomTracePropagation()));
 		Mockito.when(beanFactory.getBeanProvider(Propagation.class))
 				.thenReturn(new SimpleObjectProvider(new CustomTracePropagation()));
 

spring-cloud-sleuth-brave/src/test/java/org/springframework/cloud/sleuth/brave/instrument/web/BraveSpanFromContextRetrieverTests.java

@@ -35,8 +35,8 @@ class BraveSpanFromContextRetrieverTests {
 
 	StrictCurrentTraceContext traceContext = StrictCurrentTraceContext.create();
 
-	Tracing tracing = Tracing.newBuilder().currentTraceContext(this.traceContext)
-			.sampler(Sampler.ALWAYS_SAMPLE).addSpanHandler(this.spans).build();
+	Tracing tracing = Tracing.newBuilder().currentTraceContext(this.traceContext).sampler(Sampler.ALWAYS_SAMPLE)
+			.addSpanHandler(this.spans).build();
 
 	brave.Tracer tracer = this.tracing.tracer();
 
@@ -60,4 +60,5 @@ class BraveSpanFromContextRetrieverTests {
 
 		then(BraveSpan.toBrave(retriever.findSpan(Context.of(TraceContext.class, span.context())))).isEqualTo(span);
 	}
-}
+
+}

spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java

@@ -16,6 +16,7 @@
 
 package integration;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -107,16 +108,19 @@ public class MessagingApplicationTests extends AbstractIntegrationTest {
 	}
 
 	private void thenThereIsAtLeastOneTagWithKey(String key) {
-		then(this.testSpanHandler.spans.stream().map(MutableSpan::tags).flatMap(m -> m.keySet().stream())
-				.anyMatch(b -> b.equals(key))).isTrue();
+		then(spans().stream().map(MutableSpan::tags).flatMap(m -> m.keySet().stream()).anyMatch(b -> b.equals(key)))
+				.isTrue();
+	}
+
+	private List<MutableSpan> spans() {
+		return new ArrayList<>(this.testSpanHandler.spans);
 	}
 
 	private void thenAllSpansHaveTraceIdEqualTo(long traceId) {
 		String traceIdHex = Long.toHexString(traceId);
-		log.info("Stored spans: [\n"
-				+ this.testSpanHandler.spans.stream().map(MutableSpan::toString).collect(Collectors.joining("\n"))
+		log.info("Stored spans: [\n" + spans().stream().map(MutableSpan::toString).collect(Collectors.joining("\n"))
 				+ "\n]");
-		then(this.testSpanHandler.spans.stream().filter(span -> !span.traceId().equals(SpanUtil.idToHex(traceId)))
+		then(spans().stream().filter(span -> !span.traceId().equals(SpanUtil.idToHex(traceId)))
 				.collect(Collectors.toList())).describedAs("All spans have same trace id [" + traceIdHex + "]")
 						.isEmpty();
 	}
@@ -130,8 +134,8 @@ public class MessagingApplicationTests extends AbstractIntegrationTest {
 		// "http:/parent/" -> "message:messages" -> "http:/foo" (CS + CR) -> "http:/foo"
 		// (SS)
 		thenAllSpansArePresent(firstHttpSpan, eventSpans, lastHttpSpansParent, eventSentSpan, producerSpan);
-		List<MutableSpan> spans = this.testSpanHandler.spans;
-		then(spans).as("There were 7 spans").hasSize(7);
+		List<MutableSpan> spans = spans();
+		then(spans).as("There were 6 spans").hasSize(6);
 		log.info("Checking the parent child structure");
 		List<Optional<MutableSpan>> parentChild = spans.stream().filter(span -> span.parentId() != null).map(span -> {
 			Optional<MutableSpan> any = spans.stream().filter(span1 -> span1.id().equals(span.parentId())).findAny();
@@ -146,21 +150,20 @@ public class MessagingApplicationTests extends AbstractIntegrationTest {
 	}
 
 	private Optional<MutableSpan> findLastHttpSpansParent() {
-		return this.testSpanHandler.spans.stream().filter(span -> "GET /".equals(span.name()) && span.kind() != null)
-				.findFirst();
+		return spans().stream().filter(span -> "GET /".equals(span.name()) && span.kind() != null).findFirst();
 	}
 
 	private Optional<MutableSpan> findSpanWithKind(Span.Kind kind) {
-		return this.testSpanHandler.spans.stream().filter(span -> kind.equals(span.kind())).findFirst();
+		return spans().stream().filter(span -> kind.equals(span.kind())).findFirst();
 	}
 
 	private List<MutableSpan> findAllEventRelatedSpans() {
-		return this.testSpanHandler.spans.stream().filter(span -> "send".equals(span.name()) && span.parentId() != null)
+		return spans().stream().filter(span -> "send".equals(span.name()) && span.parentId() != null)
 				.collect(Collectors.toList());
 	}
 
 	private Optional<MutableSpan> findFirstHttpRequestSpan() {
-		return this.testSpanHandler.spans.stream()
+		return spans().stream()
 				// home is the name of the method
 				.filter(span -> span.tags().values().stream().anyMatch("home"::equals)).findFirst();
 	}
@@ -174,8 +177,7 @@ public class MessagingApplicationTests extends AbstractIntegrationTest {
 		log.info("Event sent span " + eventSentSpan);
 		log.info("Event received span " + eventReceivedSpan);
 		log.info("Last http span " + lastHttpSpan);
-		log.info("All found spans \n"
-				+ this.testSpanHandler.spans.stream().map(MutableSpan::toString).collect(Collectors.joining("\n")));
+		log.info("All found spans \n" + spans().stream().map(MutableSpan::toString).collect(Collectors.joining("\n")));
 		then(firstHttpSpan.isPresent()).isTrue();
 		then(eventSpans).isNotEmpty();
 		then(eventSentSpan.isPresent()).isTrue();