Merge pull request #32528 from marcingrzejszczak

* gh-32528:
  Polish "Break cycles between Zipkin senders and HTTP client observation"
  Break cycles between Zipkin senders and HTTP client observation

Closes gh-32528

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java

@@ -25,6 +25,7 @@ import zipkin2.reporter.Sender;
 import zipkin2.reporter.brave.ZipkinSpanHandler;
 import zipkin2.reporter.urlconnection.URLConnectionSender;
 
+import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
@@ -73,12 +74,12 @@ class ZipkinConfigurations {
 
 		@Bean
 		@ConditionalOnMissingBean(Sender.class)
-		@ConditionalOnBean(RestTemplateBuilder.class)
 		ZipkinRestTemplateSender restTemplateSender(ZipkinProperties properties,
-				RestTemplateBuilder restTemplateBuilder) {
+				ObjectProvider<ZipkinRestTemplateBuilderCustomizer> customizers) {
-			RestTemplate restTemplate = restTemplateBuilder.setConnectTimeout(properties.getConnectTimeout())
+			RestTemplateBuilder restTemplateBuilder = new RestTemplateBuilder()
-					.setReadTimeout(properties.getReadTimeout()).build();
+					.setConnectTimeout(properties.getConnectTimeout()).setReadTimeout(properties.getReadTimeout());
-			return new ZipkinRestTemplateSender(properties.getEndpoint(), restTemplate);
+			customizers.orderedStream().forEach((c) -> c.customize(restTemplateBuilder));
+			return new ZipkinRestTemplateSender(properties.getEndpoint(), restTemplateBuilder.build());
 		}
 
 	}
@@ -90,10 +91,11 @@ class ZipkinConfigurations {
 
 		@Bean
 		@ConditionalOnMissingBean(Sender.class)
-		@ConditionalOnBean(WebClient.Builder.class)
+		ZipkinWebClientSender webClientSender(ZipkinProperties properties,
-		ZipkinWebClientSender webClientSender(ZipkinProperties properties, WebClient.Builder webClientBuilder) {
+				ObjectProvider<ZipkinWebClientBuilderCustomizer> customizers) {
-			WebClient webClient = webClientBuilder.build();
+			WebClient.Builder builder = WebClient.builder();
-			return new ZipkinWebClientSender(properties.getEndpoint(), webClient);
+			customizers.orderedStream().forEach((c) -> c.customize(builder));
+			return new ZipkinWebClientSender(properties.getEndpoint(), builder.build());
 		}
 
 	}

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinRestTemplateBuilderCustomizer.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2012-2022 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.boot.actuate.autoconfigure.tracing.zipkin;
+
+import org.springframework.boot.web.client.RestTemplateBuilder;
+
+/**
+ * Callback interface that can be implemented by beans wishing to customize the
+ * {@link RestTemplateBuilder} used to send spans to Zipkin.
+ *
+ * @author Marcin Grzejszczak
+ * @since 3.0.0
+ */
+@FunctionalInterface
+public interface ZipkinRestTemplateBuilderCustomizer {
+
+	/**
+	 * Customize the rest template builder.
+	 * @param restTemplateBuilder the {@code RestTemplateBuilder} to customize
+	 */
+	void customize(RestTemplateBuilder restTemplateBuilder);
+
+}

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinWebClientBuilderCustomizer.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright 2012-2022 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.boot.actuate.autoconfigure.tracing.zipkin;
+
+import org.springframework.web.reactive.function.client.WebClient;
+import org.springframework.web.reactive.function.client.WebClient.Builder;
+
+/**
+ * Callback interface that can be implemented by beans wishing to customize the
+ * {@link Builder} used to send spans to Zipkin.
+ *
+ * @author Marcin Grzejszczak
+ * @since 3.0.0
+ */
+@FunctionalInterface
+public interface ZipkinWebClientBuilderCustomizer {
+
+	/**
+	 * Customize the web client builder.
+	 * @param webClientBuilder the {@code WebClient.Builder} to customize
+	 */
+	void customize(WebClient.Builder webClientBuilder);
+
+}

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfigurationIntegrationTests.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright 2012-2022 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.boot.actuate.autoconfigure.tracing.zipkin;
+
+import org.junit.jupiter.api.Test;
+import zipkin2.reporter.urlconnection.URLConnectionSender;
+
+import org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration;
+import org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimpleMetricsExportAutoConfiguration;
+import org.springframework.boot.actuate.autoconfigure.metrics.web.client.HttpClientMetricsAutoConfiguration;
+import org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfiguration;
+import org.springframework.boot.actuate.autoconfigure.tracing.BraveAutoConfiguration;
+import org.springframework.boot.actuate.autoconfigure.tracing.MicrometerTracingAutoConfiguration;
+import org.springframework.boot.autoconfigure.AutoConfigurations;
+import org.springframework.boot.autoconfigure.web.client.RestTemplateAutoConfiguration;
+import org.springframework.boot.autoconfigure.web.reactive.function.client.WebClientAutoConfiguration;
+import org.springframework.boot.test.context.FilteredClassLoader;
+import org.springframework.boot.test.context.assertj.ApplicationContextAssertProvider;
+import org.springframework.boot.test.context.runner.AbstractApplicationContextRunner;
+import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner;
+import org.springframework.boot.test.context.runner.WebApplicationContextRunner;
+import org.springframework.context.ConfigurableApplicationContext;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Integration tests for {@link ZipkinAutoConfiguration} and other related
+ * auto-configurations.
+ *
+ * @author Andy Wilkinson
+ */
+class ZipkinAutoConfigurationIntegrationTests {
+
+	@Test
+	void zipkinsUseOfRestTemplateDoesNotCauseACycle() {
+		configure(new WebApplicationContextRunner())
+				.withConfiguration(AutoConfigurations.of(RestTemplateAutoConfiguration.class))
+				.run((context) -> assertThat(context).hasNotFailed());
+	}
+
+	@Test
+	void zipkinsUseOfWebClientDoesNotCauseACycle() {
+		configure(new ReactiveWebApplicationContextRunner())
+				.withConfiguration(AutoConfigurations.of(WebClientAutoConfiguration.class))
+				.run((context) -> assertThat(context).hasNotFailed());
+	}
+
+	<SELF extends AbstractApplicationContextRunner<SELF, C, A>, C extends ConfigurableApplicationContext, A extends ApplicationContextAssertProvider<C>> AbstractApplicationContextRunner<SELF, C, A> configure(
+			AbstractApplicationContextRunner<SELF, ?, ?> runner) {
+		return runner
+				.withConfiguration(AutoConfigurations.of(MicrometerTracingAutoConfiguration.class,
+						ObservationAutoConfiguration.class, BraveAutoConfiguration.class, ZipkinAutoConfiguration.class,
+						HttpClientMetricsAutoConfiguration.class, MetricsAutoConfiguration.class,
+						SimpleMetricsExportAutoConfiguration.class))
+				.withClassLoader(new FilteredClassLoader(URLConnectionSender.class));
+	}
+
+}

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsSenderConfigurationTests.java

@@ -17,6 +17,7 @@
 package org.springframework.boot.actuate.autoconfigure.tracing.zipkin;
 
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentMatchers;
 import zipkin2.reporter.Sender;
 import zipkin2.reporter.urlconnection.URLConnectionSender;
 
@@ -26,12 +27,12 @@ import org.springframework.boot.test.context.FilteredClassLoader;
 import org.springframework.boot.test.context.runner.ApplicationContextRunner;
 import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner;
 import org.springframework.boot.test.context.runner.WebApplicationContextRunner;
-import org.springframework.boot.web.client.RestTemplateBuilder;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.web.reactive.function.client.WebClient;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.BDDMockito.then;
 import static org.mockito.Mockito.mock;
 
 /**
@@ -66,6 +67,8 @@ class ZipkinConfigurationsSenderConfigurationTests {
 					assertThat(context).doesNotHaveBean(URLConnectionSender.class);
 					assertThat(context).hasSingleBean(Sender.class);
 					assertThat(context).hasSingleBean(ZipkinWebClientSender.class);
+					then(context.getBean(ZipkinWebClientBuilderCustomizer.class)).should()
+							.customize(ArgumentMatchers.any());
 				});
 	}
 
@@ -90,9 +93,9 @@ class ZipkinConfigurationsSenderConfigurationTests {
 	}
 
 	@Test
-	void willUseRestTemplateInNonWebApplicationIfUrlConnectionSenderIsNotAvailable() {
+	void willUseRestTemplateInNonWebApplicationIfUrlConnectionSenderAndWebclientAreNotAvailable() {
 		this.contextRunner.withUserConfiguration(RestTemplateConfiguration.class)
-				.withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> {
+				.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class)).run((context) -> {
 					assertThat(context).doesNotHaveBean(URLConnectionSender.class);
 					assertThat(context).hasSingleBean(Sender.class);
 					assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class);
@@ -100,9 +103,9 @@ class ZipkinConfigurationsSenderConfigurationTests {
 	}
 
 	@Test
-	void willUseRestTemplateInServletWebApplicationIfUrlConnectionSenderIsNotAvailable() {
+	void willUseRestTemplateInServletWebApplicationIfUrlConnectionSenderAndWebClientNotAvailable() {
 		this.servletContextRunner.withUserConfiguration(RestTemplateConfiguration.class)
-				.withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> {
+				.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class)).run((context) -> {
 					assertThat(context).doesNotHaveBean(URLConnectionSender.class);
 					assertThat(context).hasSingleBean(Sender.class);
 					assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class);
@@ -110,9 +113,9 @@ class ZipkinConfigurationsSenderConfigurationTests {
 	}
 
 	@Test
-	void willUseRestTemplateInReactiveWebApplicationIfUrlConnectionSenderIsNotAvailable() {
+	void willUseRestTemplateInReactiveWebApplicationIfUrlConnectionSenderAndWebClientAreNotAvailable() {
 		this.reactiveContextRunner.withUserConfiguration(RestTemplateConfiguration.class)
-				.withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> {
+				.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class)).run((context) -> {
 					assertThat(context).doesNotHaveBean(URLConnectionSender.class);
 					assertThat(context).hasSingleBean(Sender.class);
 					assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class);
@@ -140,8 +143,8 @@ class ZipkinConfigurationsSenderConfigurationTests {
 	private static class RestTemplateConfiguration {
 
 		@Bean
-		RestTemplateBuilder restTemplateBuilder() {
+		ZipkinRestTemplateBuilderCustomizer restTemplateBuilder() {
-			return new RestTemplateBuilder();
+			return mock(ZipkinRestTemplateBuilderCustomizer.class);
 		}
 
 	}
@@ -150,8 +153,8 @@ class ZipkinConfigurationsSenderConfigurationTests {
 	private static class WebClientConfiguration {
 
 		@Bean
-		WebClient.Builder webClientBuilder() {
+		ZipkinWebClientBuilderCustomizer webClientBuilder() {
-			return WebClient.builder();
+			return mock(ZipkinWebClientBuilderCustomizer.class);
 		}
 
 	}