Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/Make ZipkinHttpClientSender the default BytesMessageSender #43085

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+81 −76
Open

Refactor/Make ZipkinHttpClientSender the default BytesMessageSender #43085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
da1e1fd
<modify>: switch `HttpClientSenderConfiguration` and `UrlConnectionSe…
wickdynex Nov 7, 2024
019a04a
<update>: update the unit tests.
wickdynex Nov 7, 2024
7302188
<modify>: update new unit tests.
wickdynex Nov 8, 2024
4e3e37b
<modify>: adjust configuration class filter order.
wickdynex Nov 8, 2024
2049c17
<refactor>: use `.class` to replace `string`.
wickdynex Nov 9, 2024
f4db7df
<modify>: adjust filter's order.
wickdynex Nov 9, 2024
e9e6287
<adjust>: run format test command to be passed.
wickdynex Nov 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 45 additions & 44 deletions ...a/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,37 +52,60 @@
*
* @author Moritz Halbritter
* @author Stefan Bratanov
* @author Wick Dynex
*/
class ZipkinConfigurations {

@Configuration(proxyBeanMethods = false)
@Import({ UrlConnectionSenderConfiguration.class, WebClientSenderConfiguration.class,
RestTemplateSenderConfiguration.class, HttpClientSenderConfiguration.class })
@Import({ HttpClientSenderConfiguration.class, WebClientSenderConfiguration.class,
RestTemplateSenderConfiguration.class, UrlConnectionSenderConfiguration.class })
static class SenderConfiguration {

}

@Configuration(proxyBeanMethods = false)
@ConditionalOnClass(URLConnectionSender.class)
@ConditionalOnClass(HttpClient.class)
@EnableConfigurationProperties(ZipkinProperties.class)
static class UrlConnectionSenderConfiguration {
static class HttpClientSenderConfiguration {

@Bean
@ConditionalOnMissingBean(BytesMessageSender.class)
URLConnectionSender urlConnectionSender(ZipkinProperties properties, Encoding encoding,
ZipkinHttpClientSender httpClientSender(ZipkinProperties properties, Encoding encoding,
ObjectProvider<ZipkinHttpClientBuilderCustomizer> customizers,
ObjectProvider<ZipkinConnectionDetails> connectionDetailsProvider,
ObjectProvider<HttpEndpointSupplier.Factory> endpointSupplierFactoryProvider) {
ZipkinConnectionDetails connectionDetails = connectionDetailsProvider
.getIfAvailable(() -> new PropertiesZipkinConnectionDetails(properties));
HttpEndpointSupplier.Factory endpointSupplierFactory = endpointSupplierFactoryProvider
.getIfAvailable(HttpEndpointSuppliers::constantFactory);
URLConnectionSender.Builder builder = URLConnectionSender.newBuilder();
builder.connectTimeout((int) properties.getConnectTimeout().toMillis());
builder.readTimeout((int) properties.getReadTimeout().toMillis());
builder.endpointSupplierFactory(endpointSupplierFactory);
builder.endpoint(connectionDetails.getSpanEndpoint());
builder.encoding(encoding);
return builder.build();
Builder httpClientBuilder = HttpClient.newBuilder().connectTimeout(properties.getConnectTimeout());
customizers.orderedStream().forEach((customizer) -> customizer.customize(httpClientBuilder));
return new ZipkinHttpClientSender(encoding, endpointSupplierFactory, connectionDetails.getSpanEndpoint(),
httpClientBuilder.build(), properties.getReadTimeout());
}

}

@Configuration(proxyBeanMethods = false)
@ConditionalOnClass(WebClient.class)
@EnableConfigurationProperties(ZipkinProperties.class)
static class WebClientSenderConfiguration {

@Bean
@ConditionalOnMissingBean(BytesMessageSender.class)
@SuppressWarnings({ "deprecation", "removal" })
ZipkinWebClientSender webClientSender(ZipkinProperties properties, Encoding encoding,
ObjectProvider<ZipkinWebClientBuilderCustomizer> customizers,
ObjectProvider<ZipkinConnectionDetails> connectionDetailsProvider,
ObjectProvider<HttpEndpointSupplier.Factory> endpointSupplierFactoryProvider) {
ZipkinConnectionDetails connectionDetails = connectionDetailsProvider
.getIfAvailable(() -> new PropertiesZipkinConnectionDetails(properties));
HttpEndpointSupplier.Factory endpointSupplierFactory = endpointSupplierFactoryProvider
.getIfAvailable(HttpEndpointSuppliers::constantFactory);
WebClient.Builder builder = WebClient.builder();
customizers.orderedStream().forEach((customizer) -> customizer.customize(builder));
return new ZipkinWebClientSender(encoding, endpointSupplierFactory, connectionDetails.getSpanEndpoint(),
builder.build(), properties.getConnectTimeout().plus(properties.getReadTimeout()));
}

}
Expand Down Expand Up @@ -126,48 +149,26 @@ private RestTemplateBuilder applyCustomizers(RestTemplateBuilder restTemplateBui
}

@Configuration(proxyBeanMethods = false)
@ConditionalOnClass(WebClient.class)
@EnableConfigurationProperties(ZipkinProperties.class)
static class WebClientSenderConfiguration {

@Bean
@ConditionalOnMissingBean(BytesMessageSender.class)
@SuppressWarnings({ "deprecation", "removal" })
ZipkinWebClientSender webClientSender(ZipkinProperties properties, Encoding encoding,
ObjectProvider<ZipkinWebClientBuilderCustomizer> customizers,
ObjectProvider<ZipkinConnectionDetails> connectionDetailsProvider,
ObjectProvider<HttpEndpointSupplier.Factory> endpointSupplierFactoryProvider) {
ZipkinConnectionDetails connectionDetails = connectionDetailsProvider
.getIfAvailable(() -> new PropertiesZipkinConnectionDetails(properties));
HttpEndpointSupplier.Factory endpointSupplierFactory = endpointSupplierFactoryProvider
.getIfAvailable(HttpEndpointSuppliers::constantFactory);
WebClient.Builder builder = WebClient.builder();
customizers.orderedStream().forEach((customizer) -> customizer.customize(builder));
return new ZipkinWebClientSender(encoding, endpointSupplierFactory, connectionDetails.getSpanEndpoint(),
builder.build(), properties.getConnectTimeout().plus(properties.getReadTimeout()));
}

}

@Configuration(proxyBeanMethods = false)
@ConditionalOnClass(HttpClient.class)
@ConditionalOnClass(URLConnectionSender.class)
@EnableConfigurationProperties(ZipkinProperties.class)
static class HttpClientSenderConfiguration {
static class UrlConnectionSenderConfiguration {

@Bean
@ConditionalOnMissingBean(BytesMessageSender.class)
ZipkinHttpClientSender httpClientSender(ZipkinProperties properties, Encoding encoding,
ObjectProvider<ZipkinHttpClientBuilderCustomizer> customizers,
URLConnectionSender urlConnectionSender(ZipkinProperties properties, Encoding encoding,
ObjectProvider<ZipkinConnectionDetails> connectionDetailsProvider,
ObjectProvider<HttpEndpointSupplier.Factory> endpointSupplierFactoryProvider) {
ZipkinConnectionDetails connectionDetails = connectionDetailsProvider
.getIfAvailable(() -> new PropertiesZipkinConnectionDetails(properties));
HttpEndpointSupplier.Factory endpointSupplierFactory = endpointSupplierFactoryProvider
.getIfAvailable(HttpEndpointSuppliers::constantFactory);
Builder httpClientBuilder = HttpClient.newBuilder().connectTimeout(properties.getConnectTimeout());
customizers.orderedStream().forEach((customizer) -> customizer.customize(httpClientBuilder));
return new ZipkinHttpClientSender(encoding, endpointSupplierFactory, connectionDetails.getSpanEndpoint(),
httpClientBuilder.build(), properties.getReadTimeout());
URLConnectionSender.Builder builder = URLConnectionSender.newBuilder();
builder.connectTimeout((int) properties.getConnectTimeout().toMillis());
builder.readTimeout((int) properties.getReadTimeout().toMillis());
builder.endpointSupplierFactory(endpointSupplierFactory);
builder.endpoint(connectionDetails.getSpanEndpoint());
builder.encoding(encoding);
return builder.build();
}

}
Expand Down
68 changes: 36 additions & 32 deletions ...ot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsSenderConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.boot.actuate.autoconfigure.tracing.zipkin;

import java.io.IOException;
import java.net.http.HttpClient;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.concurrent.TimeUnit;
Expand All @@ -31,6 +32,7 @@
import zipkin2.reporter.urlconnection.URLConnectionSender;

import org.springframework.boot.actuate.autoconfigure.tracing.zipkin.ZipkinConfigurations.SenderConfiguration;
import org.springframework.boot.actuate.autoconfigure.tracing.zipkin.ZipkinConfigurations.UrlConnectionSenderConfiguration;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.FilteredClassLoader;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
Expand All @@ -39,6 +41,7 @@
import org.springframework.boot.web.client.RestTemplateBuilder;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.reactive.function.client.WebClient;

import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -49,6 +52,7 @@
* Tests for {@link SenderConfiguration}.
*
* @author Moritz Halbritter
* @author Wick Dynex
*/
@SuppressWarnings({ "deprecation", "removal" })
class ZipkinConfigurationsSenderConfigurationTests {
Expand All @@ -63,34 +67,33 @@ class ZipkinConfigurationsSenderConfigurationTests {
.withConfiguration(AutoConfigurations.of(DefaultEncodingConfiguration.class, SenderConfiguration.class));

@Test
void shouldSupplyBeans() {
void shouldSupplyDefaultHttpClientSenderBeans() {
this.contextRunner.run((context) -> {
assertThat(context).hasSingleBean(BytesMessageSender.class);
assertThat(context).hasSingleBean(URLConnectionSender.class);
assertThat(context).hasSingleBean(ZipkinHttpClientSender.class);
assertThat(context).doesNotHaveBean(ZipkinRestTemplateSender.class);
assertThat(context).doesNotHaveBean(ZipkinWebClientSenderTests.class);
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
});
}

@Test
void shouldUseHttpClientIfUrlSenderIsNotAvailable() {
this.contextRunner.withUserConfiguration(HttpClientConfiguration.class)
.withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection", "org.springframework.web.client",
"org.springframework.web.reactive.function.client"))
void shouldUseUrlSenderIfHttpSenderIsNotAvailable() {
this.contextRunner.withUserConfiguration(UrlConnectionSenderConfiguration.class)
.withClassLoader(new FilteredClassLoader(HttpClient.class, WebClient.class, RestTemplate.class))
.run((context) -> {
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
assertThat(context).doesNotHaveBean(ZipkinHttpClientSender.class);
assertThat(context).hasSingleBean(BytesMessageSender.class);
assertThat(context).hasSingleBean(ZipkinHttpClientSender.class);
then(context.getBean(ZipkinHttpClientBuilderCustomizer.class)).should()
.customize(ArgumentMatchers.any());
assertThat(context).hasSingleBean(URLConnectionSender.class);
});
}

@Test
void shouldPreferWebClientSenderIfWebApplicationIsReactiveAndUrlSenderIsNotAvailable() {
void shouldPreferWebClientSenderIfWebApplicationIsReactiveAndHttpClientSenderIsNotAvailable() {
this.reactiveContextRunner.withUserConfiguration(RestTemplateConfiguration.class, WebClientConfiguration.class)
.withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection"))
.withClassLoader(new FilteredClassLoader(HttpClient.class))
.run((context) -> {
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
assertThat(context).doesNotHaveBean(ZipkinHttpClientSender.class);
assertThat(context).hasSingleBean(BytesMessageSender.class);
assertThat(context).hasSingleBean(ZipkinWebClientSender.class);
then(context.getBean(ZipkinWebClientBuilderCustomizer.class)).should()
Expand All @@ -99,55 +102,55 @@ void shouldPreferWebClientSenderIfWebApplicationIsReactiveAndUrlSenderIsNotAvail
}

@Test
void shouldPreferWebClientSenderIfWebApplicationIsServletAndUrlSenderIsNotAvailable() {
void shouldPreferWebClientSenderIfWebApplicationIsServletAndHttpClientSenderIsNotAvailable() {
this.servletContextRunner.withUserConfiguration(RestTemplateConfiguration.class, WebClientConfiguration.class)
.withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection"))
.withClassLoader(new FilteredClassLoader(HttpClient.class))
.run((context) -> {
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
assertThat(context).doesNotHaveBean(ZipkinHttpClientSender.class);
assertThat(context).hasSingleBean(BytesMessageSender.class);
assertThat(context).hasSingleBean(ZipkinWebClientSender.class);
});
}

@Test
void shouldPreferWebClientInNonWebApplicationAndUrlConnectionSenderIsNotAvailable() {
void shouldPreferWebClientInNonWebApplicationAndHttpClientSenderIsNotAvailable() {
this.contextRunner.withUserConfiguration(RestTemplateConfiguration.class, WebClientConfiguration.class)
.withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection"))
.withClassLoader(new FilteredClassLoader(HttpClient.class))
.run((context) -> {
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
assertThat(context).doesNotHaveBean(ZipkinHttpClientSender.class);
assertThat(context).hasSingleBean(BytesMessageSender.class);
assertThat(context).hasSingleBean(ZipkinWebClientSender.class);
});
}

@Test
void willUseRestTemplateInNonWebApplicationIfUrlConnectionSenderAndWebClientAreNotAvailable() {
void willUseRestTemplateInNonWebApplicationIfSenderAndWebClientAreNotAvailable() {
this.contextRunner.withUserConfiguration(RestTemplateConfiguration.class)
.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class))
.withClassLoader(new FilteredClassLoader(HttpClient.class, WebClient.class))
.run((context) -> {
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
assertThat(context).doesNotHaveBean(HttpClient.class);
assertThat(context).hasSingleBean(BytesMessageSender.class);
assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class);
});
}

@Test
void willUseRestTemplateInServletWebApplicationIfUrlConnectionSenderAndWebClientNotAvailable() {
void willUseRestTemplateInServletWebApplicationIfHttpClientSenderAndWebClientNotAvailable() {
this.servletContextRunner.withUserConfiguration(RestTemplateConfiguration.class)
.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class))
.withClassLoader(new FilteredClassLoader(HttpClient.class, WebClient.class))
.run((context) -> {
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
assertThat(context).doesNotHaveBean(ZipkinHttpClientSender.class);
assertThat(context).hasSingleBean(BytesMessageSender.class);
assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class);
});
}

@Test
void willUseRestTemplateInReactiveWebApplicationIfUrlConnectionSenderAndWebClientAreNotAvailable() {
void willUseRestTemplateInReactiveWebApplicationIfHttpClientSenderAndWebClientAreNotAvailable() {
this.reactiveContextRunner.withUserConfiguration(RestTemplateConfiguration.class)
.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class))
.withClassLoader(new FilteredClassLoader(HttpClient.class, WebClient.class))
.run((context) -> {
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
assertThat(context).doesNotHaveBean(ZipkinHttpClientSender.class);
assertThat(context).hasSingleBean(BytesMessageSender.class);
assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class);
});
Expand All @@ -158,7 +161,7 @@ void shouldNotUseWebClientSenderIfNoBuilderIsAvailable() {
this.reactiveContextRunner.run((context) -> {
assertThat(context).doesNotHaveBean(ZipkinWebClientSender.class);
assertThat(context).hasSingleBean(BytesMessageSender.class);
assertThat(context).hasSingleBean(URLConnectionSender.class);
assertThat(context).hasSingleBean(ZipkinHttpClientSender.class);
});
}

Expand All @@ -177,7 +180,7 @@ void shouldApplyZipkinRestTemplateBuilderCustomizers() throws IOException {
this.reactiveContextRunner
.withPropertyValues("management.zipkin.tracing.endpoint=" + mockWebServer.url("/"))
.withUserConfiguration(RestTemplateConfiguration.class)
.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class))
.withClassLoader(new FilteredClassLoader(HttpClient.class, WebClient.class))
.run((context) -> {
assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class);
ZipkinRestTemplateSender sender = context.getBean(ZipkinRestTemplateSender.class);
Expand All @@ -192,6 +195,7 @@ void shouldApplyZipkinRestTemplateBuilderCustomizers() throws IOException {
@Test
void shouldUseCustomHttpEndpointSupplierFactory() {
this.contextRunner.withUserConfiguration(CustomHttpEndpointSupplierFactoryConfiguration.class)
.withClassLoader(new FilteredClassLoader(HttpClient.class, WebClient.class, RestTemplate.class))
.run((context) -> assertThat(context.getBean(URLConnectionSender.class))
.extracting("delegate.endpointSupplier")
.isInstanceOf(CustomHttpEndpointSupplier.class));
Expand All @@ -200,7 +204,7 @@ void shouldUseCustomHttpEndpointSupplierFactory() {
@Test
void shouldUseCustomHttpEndpointSupplierFactoryWhenReactive() {
this.reactiveContextRunner.withUserConfiguration(WebClientConfiguration.class)
.withClassLoader(new FilteredClassLoader(URLConnectionSender.class))
.withClassLoader(new FilteredClassLoader(HttpClient.class))
.withUserConfiguration(CustomHttpEndpointSupplierFactoryConfiguration.class)
.run((context) -> assertThat(context.getBean(ZipkinWebClientSender.class)).extracting("endpointSupplier")
.isInstanceOf(CustomHttpEndpointSupplier.class));
Expand All @@ -209,7 +213,7 @@ void shouldUseCustomHttpEndpointSupplierFactoryWhenReactive() {
@Test
void shouldUseCustomHttpEndpointSupplierFactoryWhenRestTemplate() {
this.contextRunner.withUserConfiguration(RestTemplateConfiguration.class)
.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class))
.withClassLoader(new FilteredClassLoader(HttpClient.class, WebClient.class))
.withUserConfiguration(CustomHttpEndpointSupplierFactoryConfiguration.class)
.run((context) -> assertThat(context.getBean(ZipkinRestTemplateSender.class)).extracting("endpointSupplier")
.isInstanceOf(CustomHttpEndpointSupplier.class));
Expand Down