From cd3b3d468abcdc135f0e41d923f6b0832d719ce1 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 28 Sep 2022 16:47:34 +0200 Subject: [PATCH 1/2] Prefer WebClient to RestTemplate for Zipkin's Sender Previously, a Webclient-based sender was only for reactive web applications, falling back to a RestTemplate-based sender in all other cases. With this commit we now prefer to use WebClient if it is available, irrespective of the web application type. The assumption is that if the user has WebClient on the classpath, it's either a reactive web application, or it's a servlet web application or non-web application but WebClient is preferred. See gh-32529 --- .../autoconfigure/tracing/zipkin/ZipkinConfigurations.java | 7 +++---- .../ZipkinConfigurationsSenderConfigurationTests.java | 4 +++- 2 files changed, 6 insertions(+), 5 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 ec291e2c95..be3c1292f8 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 @@ -28,7 +28,6 @@ import zipkin2.reporter.urlconnection.URLConnectionSender; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; -import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.web.client.RestTemplateBuilder; import org.springframework.context.annotation.Bean; @@ -46,8 +45,8 @@ import org.springframework.web.reactive.function.client.WebClient; class ZipkinConfigurations { @Configuration(proxyBeanMethods = false) - @Import({ UrlConnectionSenderConfiguration.class, RestTemplateSenderConfiguration.class, - WebClientSenderConfiguration.class }) + @Import({ UrlConnectionSenderConfiguration.class, WebClientSenderConfiguration.class, + RestTemplateSenderConfiguration.class }) static class SenderConfiguration { } @@ -85,7 +84,7 @@ class ZipkinConfigurations { } @Configuration(proxyBeanMethods = false) - @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.REACTIVE) + @ConditionalOnClass(WebClient.class) @EnableConfigurationProperties(ZipkinProperties.class) static class WebClientSenderConfiguration { 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 8cf20f96a6..3a3c5d28a7 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 @@ -77,7 +77,9 @@ class ZipkinConfigurationsSenderConfigurationTests { @Test void shouldUseRestTemplateSenderIfUrlConnectionSenderIsNotAvailableAndWebAppIsNotReactive() { this.contextRunner.withUserConfiguration(RestTemplateConfiguration.class) - .withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> { + .withClassLoader( + new FilteredClassLoader("zipkin2.reporter.urlconnection", "org.springframework.web.reactive")) + .run((context) -> { assertThat(context).doesNotHaveBean(URLConnectionSender.class); assertThat(context).hasSingleBean(Sender.class); assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class); From 8c74b628888d78630e6ceeda8d83a9b92f9e1f87 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 28 Sep 2022 19:07:27 +0100 Subject: [PATCH 2/2] Polish "Prefer WebClient to RestTemplate for Zipkin's Sender" See gh-32529 --- ...onfigurationsSenderConfigurationTests.java | 66 +++++++++++++++---- 1 file changed, 54 insertions(+), 12 deletions(-) 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 3a3c5d28a7..727a6b854d 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 @@ -25,6 +25,7 @@ import org.springframework.boot.autoconfigure.AutoConfigurations; 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; @@ -46,6 +47,9 @@ class ZipkinConfigurationsSenderConfigurationTests { private final ReactiveWebApplicationContextRunner reactiveContextRunner = new ReactiveWebApplicationContextRunner() .withConfiguration(AutoConfigurations.of(SenderConfiguration.class)); + private final WebApplicationContextRunner servletContextRunner = new WebApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(SenderConfiguration.class)); + @Test void shouldSupplyBeans() { this.contextRunner.run((context) -> { @@ -56,8 +60,8 @@ class ZipkinConfigurationsSenderConfigurationTests { } @Test - void shouldUseWebClientSenderIfWebApplicationIsReactive() { - this.reactiveContextRunner.withUserConfiguration(WebClientConfiguration.class) + void shouldPreferWebClientSenderIfWebApplicationIsReactiveAndUrlSenderIsNotAvailable() { + this.reactiveContextRunner.withUserConfiguration(RestTemplateConfiguration.class, WebClientConfiguration.class) .withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> { assertThat(context).doesNotHaveBean(URLConnectionSender.class); assertThat(context).hasSingleBean(Sender.class); @@ -66,26 +70,64 @@ class ZipkinConfigurationsSenderConfigurationTests { } @Test - void shouldNotUseWebClientSenderIfNoBuilderIsAvailable() { - this.reactiveContextRunner.run((context) -> { - assertThat(context).doesNotHaveBean(ZipkinWebClientSender.class); - assertThat(context).hasSingleBean(Sender.class); - assertThat(context).hasSingleBean(URLConnectionSender.class); - }); + void shouldPreferWebClientSenderIfWebApplicationIsServletAndUrlSenderIsNotAvailable() { + this.servletContextRunner.withUserConfiguration(RestTemplateConfiguration.class, WebClientConfiguration.class) + .withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> { + assertThat(context).doesNotHaveBean(URLConnectionSender.class); + assertThat(context).hasSingleBean(Sender.class); + assertThat(context).hasSingleBean(ZipkinWebClientSender.class); + }); + } + + @Test + void shouldPreferWebClientInNonWebApplicationAndUrlConnectionSenderIsNotAvailable() { + this.contextRunner.withUserConfiguration(RestTemplateConfiguration.class, WebClientConfiguration.class) + .withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> { + assertThat(context).doesNotHaveBean(URLConnectionSender.class); + assertThat(context).hasSingleBean(Sender.class); + assertThat(context).hasSingleBean(ZipkinWebClientSender.class); + }); } @Test - void shouldUseRestTemplateSenderIfUrlConnectionSenderIsNotAvailableAndWebAppIsNotReactive() { + void willUseRestTemplateInNonWebApplicationIfUrlConnectionSenderIsNotAvailable() { this.contextRunner.withUserConfiguration(RestTemplateConfiguration.class) - .withClassLoader( - new FilteredClassLoader("zipkin2.reporter.urlconnection", "org.springframework.web.reactive")) - .run((context) -> { + .withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> { assertThat(context).doesNotHaveBean(URLConnectionSender.class); assertThat(context).hasSingleBean(Sender.class); assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class); }); } + @Test + void willUseRestTemplateInServletWebApplicationIfUrlConnectionSenderIsNotAvailable() { + this.servletContextRunner.withUserConfiguration(RestTemplateConfiguration.class) + .withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> { + assertThat(context).doesNotHaveBean(URLConnectionSender.class); + assertThat(context).hasSingleBean(Sender.class); + assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class); + }); + } + + @Test + void willUseRestTemplateInReactiveWebApplicationIfUrlConnectionSenderIsNotAvailable() { + this.reactiveContextRunner.withUserConfiguration(RestTemplateConfiguration.class) + .withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> { + assertThat(context).doesNotHaveBean(URLConnectionSender.class); + assertThat(context).hasSingleBean(Sender.class); + assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class); + }); + } + + @Test + void shouldNotUseWebClientSenderIfNoBuilderIsAvailable() { + this.reactiveContextRunner.run((context) -> { + assertThat(context).doesNotHaveBean(ZipkinWebClientSender.class); + assertThat(context).hasSingleBean(Sender.class); + assertThat(context).hasSingleBean(URLConnectionSender.class); + }); + } + @Test void shouldBackOffOnCustomBeans() { this.contextRunner.withUserConfiguration(CustomConfiguration.class).run((context) -> {