Skip to content

Commit 6850089

Browse files
author
Lenin Jaganathan
committed
Fix RestClientBeanPostProcessor to avoid creating new bean when interceptor already present
The RestClientBeanPostProcessor was always creating a new RestClient bean via mutate().build(), even when the OpenTelemetry interceptor was already present. This resulted in unnecessary bean creation. Fixes #15545 Signed-off-by: Lenin Jaganathan<lenin.jaganathan@gmail.com>
1 parent b25317d commit 6850089

File tree

2 files changed

+65
-12
lines changed

2 files changed

+65
-12
lines changed

instrumentation/spring/spring-boot-autoconfigure/src/main/javaSpring3/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/web/RestClientBeanPostProcessor.java

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.InstrumentationConfigUtil;
1111
import io.opentelemetry.instrumentation.spring.web.v3_1.SpringWebTelemetry;
1212
import io.opentelemetry.instrumentation.spring.web.v3_1.internal.WebTelemetryUtil;
13+
import java.util.concurrent.atomic.AtomicBoolean;
1314
import org.springframework.beans.factory.ObjectProvider;
1415
import org.springframework.beans.factory.config.BeanPostProcessor;
1516
import org.springframework.http.client.ClientHttpRequestInterceptor;
@@ -40,18 +41,26 @@ private static RestClient addRestClientInterceptorIfNotPresent(
4041
RestClient restClient, OpenTelemetry openTelemetry, InstrumentationConfig config) {
4142
ClientHttpRequestInterceptor instrumentationInterceptor = getInterceptor(openTelemetry, config);
4243

43-
return restClient
44-
.mutate()
45-
.requestInterceptors(
46-
interceptors -> {
47-
if (interceptors.stream()
48-
.noneMatch(
49-
interceptor ->
50-
interceptor.getClass() == instrumentationInterceptor.getClass())) {
51-
interceptors.add(0, instrumentationInterceptor);
52-
}
53-
})
54-
.build();
44+
AtomicBoolean shouldAddInterceptor = new AtomicBoolean(false);
45+
RestClient.Builder result =
46+
restClient
47+
.mutate()
48+
.requestInterceptors(
49+
interceptors -> {
50+
if (isInterceptorNotPresent(interceptors, instrumentationInterceptor)) {
51+
interceptors.add(0, instrumentationInterceptor);
52+
shouldAddInterceptor.set(true);
53+
}
54+
});
55+
56+
return shouldAddInterceptor.get() ? result.build() : restClient;
57+
}
58+
59+
private static boolean isInterceptorNotPresent(
60+
java.util.List<ClientHttpRequestInterceptor> interceptors,
61+
ClientHttpRequestInterceptor instrumentationInterceptor) {
62+
return interceptors.stream()
63+
.noneMatch(interceptor -> interceptor.getClass() == instrumentationInterceptor.getClass());
5564
}
5665

5766
static ClientHttpRequestInterceptor getInterceptor(

instrumentation/spring/spring-boot-autoconfigure/src/testSpring3/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/web/RestClientInstrumentationAutoConfigurationTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,4 +87,48 @@ void defaultConfiguration() {
8787
"otelRestClientBeanPostProcessor", RestClientBeanPostProcessor.class))
8888
.isNotNull());
8989
}
90+
91+
@Test
92+
void shouldNotCreateNewBeanWhenInterceptorAlreadyPresent() {
93+
contextRunner
94+
.withPropertyValues("otel.instrumentation.spring-web.enabled=true")
95+
.run(
96+
context -> {
97+
RestClientBeanPostProcessor beanPostProcessor =
98+
context.getBean(
99+
"otelRestClientBeanPostProcessor", RestClientBeanPostProcessor.class);
100+
101+
RestClient restClientWithInterceptor =
102+
RestClient.builder()
103+
.requestInterceptor(
104+
RestClientBeanPostProcessor.getInterceptor(
105+
context.getBean(OpenTelemetry.class),
106+
context.getBean(InstrumentationConfig.class)))
107+
.build();
108+
109+
RestClient processed =
110+
(RestClient)
111+
beanPostProcessor.postProcessAfterInitialization(
112+
restClientWithInterceptor, "testBean");
113+
114+
// Should return the same instance when interceptor is already present
115+
assertThat(processed).isSameAs(restClientWithInterceptor);
116+
117+
// Verify only one interceptor exists
118+
processed
119+
.mutate()
120+
.requestInterceptors(
121+
interceptors -> {
122+
long count =
123+
interceptors.stream()
124+
.filter(
125+
rti ->
126+
rti.getClass()
127+
.getName()
128+
.startsWith("io.opentelemetry.instrumentation"))
129+
.count();
130+
assertThat(count).isEqualTo(1);
131+
});
132+
});
133+
}
90134
}

0 commit comments

Comments
 (0)