Skip to content

Commit

Permalink
Merge pull request #43085 from wickdynex
Browse files Browse the repository at this point in the history 
* pr/43085:
  Polish "Make ZipkinHttpClientSender the default BytesMessageSender'
  Make ZipkinHttpClientSender the default BytesMessageSender

Closes gh-43085
  • Loading branch information
@philwebb
philwebb committed Jan 13, 2025
2 parents e9abe34 + b051a74 commit 1c991a7
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 78 deletions.
91 changes: 46 additions & 45 deletions ...a/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2024 the original author or authors.
* Copyright 2012-2025 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.
Expand Down 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
70 changes: 37 additions & 33 deletions ...ot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsSenderConfigurationTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2024 the original author or authors.
* Copyright 2012-2025 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.
Expand All @@ -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 @@ -201,7 +205,7 @@ void shouldUseCustomHttpEndpointSupplierFactory() {
@SuppressWarnings("resource")
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 @@ -211,7 +215,7 @@ void shouldUseCustomHttpEndpointSupplierFactoryWhenReactive() {
@SuppressWarnings("resource")
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

0 comments on commit 1c991a7

Please sign in to comment.