Skip to content

Commit b41ed44

Browse files
marcingrzejszczakwilkinsona
authored andcommitted
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
1 parent 13c638b commit b41ed44

File tree

2 files changed

+26
-20
lines changed

2 files changed

+26
-20
lines changed

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.boot.actuate.autoconfigure.tracing.zipkin;
1818

19+
import java.util.List;
20+
1921
import io.opentelemetry.exporter.zipkin.ZipkinSpanExporter;
2022
import zipkin2.Span;
2123
import zipkin2.codec.BytesEncoder;
@@ -73,12 +75,12 @@ static class RestTemplateSenderConfiguration {
7375

7476
@Bean
7577
@ConditionalOnMissingBean(Sender.class)
76-
@ConditionalOnBean(RestTemplateBuilder.class)
7778
ZipkinRestTemplateSender restTemplateSender(ZipkinProperties properties,
78-
RestTemplateBuilder restTemplateBuilder) {
79-
RestTemplate restTemplate = restTemplateBuilder.setConnectTimeout(properties.getConnectTimeout())
80-
.setReadTimeout(properties.getReadTimeout()).build();
81-
return new ZipkinRestTemplateSender(properties.getEndpoint(), restTemplate);
79+
List<ZipkinRestTemplateBuilderCustomizer> customizers) {
80+
RestTemplateBuilder restTemplateBuilder = new RestTemplateBuilder()
81+
.setConnectTimeout(properties.getConnectTimeout()).setReadTimeout(properties.getReadTimeout());
82+
customizers.forEach((c) -> c.customize(restTemplateBuilder));
83+
return new ZipkinRestTemplateSender(properties.getEndpoint(), restTemplateBuilder.build());
8284
}
8385

8486
}
@@ -90,10 +92,11 @@ static class WebClientSenderConfiguration {
9092

9193
@Bean
9294
@ConditionalOnMissingBean(Sender.class)
93-
@ConditionalOnBean(WebClient.Builder.class)
94-
ZipkinWebClientSender webClientSender(ZipkinProperties properties, WebClient.Builder webClientBuilder) {
95-
WebClient webClient = webClientBuilder.build();
96-
return new ZipkinWebClientSender(properties.getEndpoint(), webClient);
95+
ZipkinWebClientSender webClientSender(ZipkinProperties properties,
96+
List<ZipkinWebClientBuilderCustomizer> customizers) {
97+
WebClient.Builder builder = WebClient.builder();
98+
customizers.forEach((c) -> c.customize(builder));
99+
return new ZipkinWebClientSender(properties.getEndpoint(), builder.build());
97100
}
98101

99102
}

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.boot.actuate.autoconfigure.tracing.zipkin;
1818

1919
import org.junit.jupiter.api.Test;
20+
import org.mockito.ArgumentMatchers;
2021
import zipkin2.reporter.Sender;
2122
import zipkin2.reporter.urlconnection.URLConnectionSender;
2223

@@ -26,12 +27,12 @@
2627
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
2728
import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner;
2829
import org.springframework.boot.test.context.runner.WebApplicationContextRunner;
29-
import org.springframework.boot.web.client.RestTemplateBuilder;
3030
import org.springframework.context.annotation.Bean;
3131
import org.springframework.context.annotation.Configuration;
3232
import org.springframework.web.reactive.function.client.WebClient;
3333

3434
import static org.assertj.core.api.Assertions.assertThat;
35+
import static org.mockito.BDDMockito.then;
3536
import static org.mockito.Mockito.mock;
3637

3738
/**
@@ -66,6 +67,8 @@ void shouldPreferWebClientSenderIfWebApplicationIsReactiveAndUrlSenderIsNotAvail
6667
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
6768
assertThat(context).hasSingleBean(Sender.class);
6869
assertThat(context).hasSingleBean(ZipkinWebClientSender.class);
70+
then(context.getBean(ZipkinWebClientBuilderCustomizer.class)).should()
71+
.customize(ArgumentMatchers.any());
6972
});
7073
}
7174

@@ -90,29 +93,29 @@ void shouldPreferWebClientInNonWebApplicationAndUrlConnectionSenderIsNotAvailabl
9093
}
9194

9295
@Test
93-
void willUseRestTemplateInNonWebApplicationIfUrlConnectionSenderIsNotAvailable() {
96+
void willUseRestTemplateInNonWebApplicationIfUrlConnectionSenderAndWebclientAreNotAvailable() {
9497
this.contextRunner.withUserConfiguration(RestTemplateConfiguration.class)
95-
.withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> {
98+
.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class)).run((context) -> {
9699
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
97100
assertThat(context).hasSingleBean(Sender.class);
98101
assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class);
99102
});
100103
}
101104

102105
@Test
103-
void willUseRestTemplateInServletWebApplicationIfUrlConnectionSenderIsNotAvailable() {
106+
void willUseRestTemplateInServletWebApplicationIfUrlConnectionSenderAndWebClientNotAvailable() {
104107
this.servletContextRunner.withUserConfiguration(RestTemplateConfiguration.class)
105-
.withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> {
108+
.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class)).run((context) -> {
106109
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
107110
assertThat(context).hasSingleBean(Sender.class);
108111
assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class);
109112
});
110113
}
111114

112115
@Test
113-
void willUseRestTemplateInReactiveWebApplicationIfUrlConnectionSenderIsNotAvailable() {
116+
void willUseRestTemplateInReactiveWebApplicationIfUrlConnectionSenderAndWebClientAreNotAvailable() {
114117
this.reactiveContextRunner.withUserConfiguration(RestTemplateConfiguration.class)
115-
.withClassLoader(new FilteredClassLoader("zipkin2.reporter.urlconnection")).run((context) -> {
118+
.withClassLoader(new FilteredClassLoader(URLConnectionSender.class, WebClient.class)).run((context) -> {
116119
assertThat(context).doesNotHaveBean(URLConnectionSender.class);
117120
assertThat(context).hasSingleBean(Sender.class);
118121
assertThat(context).hasSingleBean(ZipkinRestTemplateSender.class);
@@ -140,8 +143,8 @@ void shouldBackOffOnCustomBeans() {
140143
private static class RestTemplateConfiguration {
141144

142145
@Bean
143-
RestTemplateBuilder restTemplateBuilder() {
144-
return new RestTemplateBuilder();
146+
ZipkinRestTemplateBuilderCustomizer restTemplateBuilder() {
147+
return mock(ZipkinRestTemplateBuilderCustomizer.class);
145148
}
146149

147150
}
@@ -150,8 +153,8 @@ RestTemplateBuilder restTemplateBuilder() {
150153
private static class WebClientConfiguration {
151154

152155
@Bean
153-
WebClient.Builder webClientBuilder() {
154-
return WebClient.builder();
156+
ZipkinWebClientBuilderCustomizer webClientBuilder() {
157+
return mock(ZipkinWebClientBuilderCustomizer.class);
155158
}
156159

157160
}

0 commit comments

Comments
 (0)