From b41ed44b60822dd02e40e37b9a9f2f691e22146e Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 28 Sep 2022 15:54:42 +0200 Subject: [PATCH] Break cycles between Zipkin senders and HTTP client observation Previously, RestTemplateBuilder and WebClient.Builder beans were used to create the HTTP client for sending out spans. Those same beans are also instrumented for observability which results in a cycle. This commit breaks the cycle by not using the application-web builders to create the RestTemplate and WebClient's used by the Zipkin senders. Instead, builders are created inline, with new callbacks being introduced to allow the user to customize these Zipkin-specific builders. See gh-32528 --- .../tracing/zipkin/ZipkinConfigurations.java | 21 +++++++++------- ...onfigurationsSenderConfigurationTests.java | 25 +++++++++++-------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java index be3c1292f83..85d90bd945c 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java @@ -16,6 +16,8 @@ package org.springframework.boot.actuate.autoconfigure.tracing.zipkin; +import java.util.List; + import io.opentelemetry.exporter.zipkin.ZipkinSpanExporter; import zipkin2.Span; import zipkin2.codec.BytesEncoder; @@ -73,12 +75,12 @@ class ZipkinConfigurations { @Bean @ConditionalOnMissingBean(Sender.class) - @ConditionalOnBean(RestTemplateBuilder.class) ZipkinRestTemplateSender restTemplateSender(ZipkinProperties properties, - RestTemplateBuilder restTemplateBuilder) { - RestTemplate restTemplate = restTemplateBuilder.setConnectTimeout(properties.getConnectTimeout()) - .setReadTimeout(properties.getReadTimeout()).build(); - return new ZipkinRestTemplateSender(properties.getEndpoint(), restTemplate); + List customizers) { + RestTemplateBuilder restTemplateBuilder = new RestTemplateBuilder() + .setConnectTimeout(properties.getConnectTimeout()).setReadTimeout(properties.getReadTimeout()); + customizers.forEach((c) -> c.customize(restTemplateBuilder)); + return new ZipkinRestTemplateSender(properties.getEndpoint(), restTemplateBuilder.build()); } } @@ -90,10 +92,11 @@ class ZipkinConfigurations { @Bean @ConditionalOnMissingBean(Sender.class) - @ConditionalOnBean(WebClient.Builder.class) - ZipkinWebClientSender webClientSender(ZipkinProperties properties, WebClient.Builder webClientBuilder) { - WebClient webClient = webClientBuilder.build(); - return new ZipkinWebClientSender(properties.getEndpoint(), webClient); + ZipkinWebClientSender webClientSender(ZipkinProperties properties, + List customizers) { + WebClient.Builder builder = WebClient.builder(); + customizers.forEach((c) -> c.customize(builder)); + return new ZipkinWebClientSender(properties.getEndpoint(), builder.build()); } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsSenderConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsSenderConfigurationTests.java index 727a6b854dc..892e3e01756 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsSenderConfigurationTests.java +++ b/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() { - return new RestTemplateBuilder(); + ZipkinRestTemplateBuilderCustomizer restTemplateBuilder() { + return mock(ZipkinRestTemplateBuilderCustomizer.class); } } @@ -150,8 +153,8 @@ class ZipkinConfigurationsSenderConfigurationTests { private static class WebClientConfiguration { @Bean - WebClient.Builder webClientBuilder() { - return WebClient.builder(); + ZipkinWebClientBuilderCustomizer webClientBuilder() { + return mock(ZipkinWebClientBuilderCustomizer.class); } }