implement check before building interceptor if already exists

Author: jaydeluca

instrumentation/spring/spring-boot-autoconfigure/src/main/javaSpring4/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/web/RestClientBeanPostProcessorSpring4.java

@@ -10,6 +10,8 @@
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.InstrumentationConfigUtil;
 import io.opentelemetry.instrumentation.spring.web.v3_1.SpringWebTelemetry;
 import io.opentelemetry.instrumentation.spring.web.v3_1.internal.WebTelemetryUtil;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.beans.factory.config.BeanPostProcessor;
 import org.springframework.http.client.ClientHttpRequestInterceptor;
@@ -40,18 +42,26 @@ private static RestClient addRestClientInterceptorIfNotPresent(
       RestClient restClient, OpenTelemetry openTelemetry, InstrumentationConfig config) {
     ClientHttpRequestInterceptor instrumentationInterceptor = getInterceptor(openTelemetry, config);
 
-    return restClient
-        .mutate()
-        .requestInterceptors(
-            interceptors -> {
-              if (interceptors.stream()
-                  .noneMatch(
-                      interceptor ->
-                          interceptor.getClass() == instrumentationInterceptor.getClass())) {
-                interceptors.add(0, instrumentationInterceptor);
-              }
-            })
-        .build();
+    AtomicBoolean shouldAddInterceptor = new AtomicBoolean(false);
+    RestClient.Builder result =
+        restClient
+            .mutate()
+            .requestInterceptors(
+                interceptors -> {
+                  if (isInterceptorNotPresent(interceptors, instrumentationInterceptor)) {
+                    interceptors.add(0, instrumentationInterceptor);
+                    shouldAddInterceptor.set(true);
+                  }
+                });
+
+    return shouldAddInterceptor.get() ? result.build() : restClient;
+  }
+
+  private static boolean isInterceptorNotPresent(
+      List<ClientHttpRequestInterceptor> interceptors,
+      ClientHttpRequestInterceptor instrumentationInterceptor) {
+    return interceptors.stream()
+        .noneMatch(interceptor -> interceptor.getClass() == instrumentationInterceptor.getClass());
   }
 
   static ClientHttpRequestInterceptor getInterceptor(

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

@@ -5,8 +5,11 @@
 
 package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.web;
 
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.instrumentation.api.incubator.config.internal.InstrumentationConfig;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.AbstractRestClientInstrumentationAutoConfigurationTest;
 import org.springframework.boot.autoconfigure.AutoConfigurations;
+import org.springframework.http.client.ClientHttpRequestInterceptor;
 
 class RestClientInstrumentationAutoConfigurationTest
     extends AbstractRestClientInstrumentationAutoConfigurationTest {
@@ -20,4 +23,10 @@ protected AutoConfigurations autoConfigurations() {
   protected Class<?> postProcessorClass() {
     return RestClientBeanPostProcessor.class;
   }
+
+  @Override
+  protected ClientHttpRequestInterceptor getInterceptor(
+      OpenTelemetry openTelemetry, InstrumentationConfig config) {
+    return RestClientBeanPostProcessorSpring4.getInterceptor(openTelemetry, config);
+  }
 }

instrumentation/spring/spring-boot-autoconfigure/src/testSpring4/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/web/RestClientInstrumentationSpringBoot4AutoConfigurationTest.java

@@ -5,8 +5,11 @@
 
 package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.web;
 
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.instrumentation.api.incubator.config.internal.InstrumentationConfig;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.AbstractRestClientInstrumentationAutoConfigurationTest;
 import org.springframework.boot.autoconfigure.AutoConfigurations;
+import org.springframework.http.client.ClientHttpRequestInterceptor;
 
 class RestClientInstrumentationSpringBoot4AutoConfigurationTest
     extends AbstractRestClientInstrumentationAutoConfigurationTest {
@@ -20,4 +23,10 @@ protected AutoConfigurations autoConfigurations() {
   protected Class<?> postProcessorClass() {
     return RestClientBeanPostProcessorSpring4.class;
   }
+
+  @Override
+  protected ClientHttpRequestInterceptor getInterceptor(
+      OpenTelemetry openTelemetry, InstrumentationConfig config) {
+    return RestClientBeanPostProcessorSpring4.getInterceptor(openTelemetry, config);
+  }
 }

instrumentation/spring/spring-boot-autoconfigure/testing/build.gradle.kts

@@ -18,13 +18,3 @@ dependencies {
   compileOnly("io.opentelemetry:opentelemetry-sdk-testing")
   compileOnly("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi")
 }
-
-tasks {
-  compileJava {
-    // RestClient from spring-boot-restclient:4.0.0 uses Spring Framework 7.0.1
-    // which has @Deprecated annotations with Java 9+ attributes (since, forRemoval)
-    sourceCompatibility = "17"
-    targetCompatibility = "17"
-    options.release.set(17)
-  }
-}

instrumentation/spring/spring-boot-autoconfigure/testing/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/AbstractRestClientInstrumentationAutoConfigurationTest.java

@@ -13,8 +13,10 @@
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.ConfigPropertiesBridge;
 import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
 import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.config.BeanPostProcessor;
 import org.springframework.boot.autoconfigure.AutoConfigurations;
 import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.http.client.ClientHttpRequestInterceptor;
 import org.springframework.web.client.RestClient;
 
 public abstract class AbstractRestClientInstrumentationAutoConfigurationTest {
@@ -23,6 +25,9 @@ public abstract class AbstractRestClientInstrumentationAutoConfigurationTest {
 
   protected abstract Class<?> postProcessorClass();
 
+  protected abstract ClientHttpRequestInterceptor getInterceptor(
+      OpenTelemetry openTelemetry, InstrumentationConfig config);
+
   private final ApplicationContextRunner contextRunner =
       new ApplicationContextRunner()
           .withBean(OpenTelemetry.class, OpenTelemetry::noop)
@@ -84,4 +89,45 @@ void defaultConfiguration() {
             assertThat(context.getBean("otelRestClientBeanPostProcessor", postProcessorClass()))
                 .isNotNull());
   }
+
+  @Test
+  void shouldNotCreateNewBeanWhenInterceptorAlreadyPresent() {
+    contextRunner.run(
+        context -> {
+          Object beanPostProcessor =
+              context.getBean("otelRestClientBeanPostProcessor", postProcessorClass());
+
+          RestClient restClientWithInterceptor =
+              RestClient.builder()
+                  .requestInterceptor(
+                      getInterceptor(
+                          context.getBean(OpenTelemetry.class),
+                          context.getBean(InstrumentationConfig.class)))
+                  .build();
+
+          RestClient processed =
+              (RestClient)
+                  ((BeanPostProcessor) beanPostProcessor)
+                      .postProcessAfterInitialization(restClientWithInterceptor, "testBean");
+
+          // Should return the same instance when interceptor is already present
+          assertThat(processed).isSameAs(restClientWithInterceptor);
+
+          // Verify only one interceptor exists
+          processed
+              .mutate()
+              .requestInterceptors(
+                  interceptors -> {
+                    long count =
+                        interceptors.stream()
+                            .filter(
+                                rti ->
+                                    rti.getClass()
+                                        .getName()
+                                        .startsWith("io.opentelemetry.instrumentation"))
+                            .count();
+                    assertThat(count).isEqualTo(1);
+                  });
+        });
+  }
 }

smoke-tests-otel-starter/spring-boot-4/src/test/java/io/opentelemetry/spring/smoketest/OtelSpringStarterSmokeTest.java

@@ -61,8 +61,6 @@ void restTemplateCall(String path) {
 
   @Test
   void restClientBuilder() {
-    testing.clearAllExportedData();
-
     RestClient restClient = restClientBuilder.baseUrl("http://localhost:" + port).build();
     restClient.get().uri(OtelSpringStarterSmokeTestController.PING).retrieve().body(String.class);
 

smoke-tests-otel-starter/spring-boot-common/src/main/java/io/opentelemetry/spring/smoketest/AbstractOtelSpringStarterSmokeTest.java

@@ -294,8 +294,6 @@ private static boolean isFlightRecorderAvailable() {
 
   @Test
   void databaseQuery() {
-    testing.clearAllExportedData();
-
     testing.runWithSpan(
         "server",
         () -> {
@@ -317,8 +315,6 @@ void databaseQuery() {
 
   @Test
   void restTemplate() {
-    testing.clearAllExportedData();
-
     restTemplateCall(OtelSpringStarterSmokeTestController.PING);
 
     testing.waitAndAssertTraces(
@@ -332,8 +328,6 @@ void restTemplate() {
 
   @Test
   void shouldRedactSomeUrlParameters() {
-    testing.clearAllExportedData();
-
     restTemplateCall("/test?X-Goog-Signature=39Up9jzHkxhuIhFE9594DJxe7w6cIRCg0V6ICGS0");
 
     testing.waitAndAssertTraces(