only auto-configure okhttp when otel is absent, add flag to enable okhttp autoconfig

Author: lbloder

sentry-spring-boot-4/api/sentry-spring-boot-4.api

@@ -34,6 +34,7 @@ public class io/sentry/spring/boot4/SentryProfilerAutoConfiguration {
 
 public class io/sentry/spring/boot4/SentryProperties : io/sentry/SentryOptions {
 	public fun <init> ()V
+	public fun getClients ()Lio/sentry/spring/boot4/SentryProperties$Clients;
 	public fun getExceptionResolverOrder ()I
 	public fun getGraphql ()Lio/sentry/spring/boot4/SentryProperties$Graphql;
 	public fun getLogging ()Lio/sentry/spring/boot4/SentryProperties$Logging;
@@ -42,6 +43,7 @@ public class io/sentry/spring/boot4/SentryProperties : io/sentry/SentryOptions {
 	public fun isEnableAotCompatibility ()Z
 	public fun isKeepTransactionsOpenForAsyncResponses ()Z
 	public fun isUseGitCommitIdAsRelease ()Z
+	public fun setClients (Lio/sentry/spring/boot4/SentryProperties$Clients;)V
 	public fun setEnableAotCompatibility (Z)V
 	public fun setExceptionResolverOrder (I)V
 	public fun setGraphql (Lio/sentry/spring/boot4/SentryProperties$Graphql;)V
@@ -52,6 +54,12 @@ public class io/sentry/spring/boot4/SentryProperties : io/sentry/SentryOptions {
 	public fun setUserFilterOrder (Ljava/lang/Integer;)V
 }
 
+public class io/sentry/spring/boot4/SentryProperties$Clients {
+	public fun <init> ()V
+	public fun isOkHttpEnabled ()Z
+	public fun setOkHttpEnabled (Z)V
+}
+
 public class io/sentry/spring/boot4/SentryProperties$Graphql {
 	public fun <init> ()V
 	public fun getIgnoredErrorTypes ()Ljava/util/List;

sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryOkHttpAutoConfiguration.java

@@ -6,6 +6,7 @@
 import okhttp3.OkHttpClient;
 import org.jetbrains.annotations.NotNull;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -18,10 +19,15 @@
   SentryOkHttpInterceptor.class,
   SentryOkHttpEventListener.class
 })
+@ConditionalOnMissingClass({
+  "io.sentry.opentelemetry.SentryAutoConfigurationCustomizerProvider",
+  "io.sentry.opentelemetry.agent.AgentMarker"
+})
 @ConditionalOnProperty(name = "sentry.dsn")
 public class SentryOkHttpAutoConfiguration {
 
   @Bean
+  @ConditionalOnProperty(name = "sentry.clients.ok-http-enabled", havingValue = "true")
   static @NotNull SentryOkHttpClientBeanPostProcessor sentryOkHttpClientBeanPostProcessor() {
     return new SentryOkHttpClientBeanPostProcessor();
   }

sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryProperties.java

@@ -47,6 +47,9 @@ public class SentryProperties extends SentryOptions {
   /** Graphql integration properties. */
   private @NotNull Graphql graphql = new Graphql();
 
+  /** Clients integration properties. */
+  private @NotNull Clients clients = new Clients();
+
   public boolean isUseGitCommitIdAsRelease() {
     return useGitCommitIdAsRelease;
   }
@@ -124,6 +127,14 @@ public void setGraphql(@NotNull Graphql graphql) {
     this.graphql = graphql;
   }
 
+  public @NotNull Clients getClients() {
+    return clients;
+  }
+
+  public void setClients(@NotNull Clients clients) {
+    this.clients = clients;
+  }
+
   @Open
   public static class Logging {
     /** Enable/Disable logging auto-configuration. */
@@ -215,4 +226,18 @@ public void setIgnoredErrorTypes(final @NotNull List<String> ignoredErrorTypes)
       this.ignoredErrorTypes = ignoredErrorTypes;
     }
   }
+
+  @Open
+  public static class Clients {
+    /** Enable automatic instrumentation of Spring-managed OkHttp clients. Disabled by default. */
+    private boolean okHttpEnabled = false;
+
+    public boolean isOkHttpEnabled() {
+      return okHttpEnabled;
+    }
+
+    public void setOkHttpEnabled(boolean okHttpEnabled) {
+      this.okHttpEnabled = okHttpEnabled;
+    }
+  }
 }

sentry-spring-boot-4/src/test/kotlin/io/sentry/spring/boot4/SentryOkHttpAutoConfigurationTest.kt

@@ -1,10 +1,13 @@
 package io.sentry.spring.boot4
 
 import com.google.common.truth.Truth.assertThat
+import io.opentelemetry.api.OpenTelemetry
 import io.sentry.ITransportFactory
 import io.sentry.NoOpTransportFactory
 import io.sentry.okhttp.SentryOkHttpEventListener
 import io.sentry.okhttp.SentryOkHttpInterceptor
+import io.sentry.opentelemetry.SentryAutoConfigurationCustomizerProvider
+import io.sentry.opentelemetry.agent.AgentMarker
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.atomic.AtomicBoolean
 import kotlin.test.Test
@@ -23,7 +26,7 @@ import org.springframework.context.annotation.Configuration
 
 class SentryOkHttpAutoConfigurationTest {
 
-  private val contextRunner =
+  private val baseContextRunner =
     ApplicationContextRunner()
       .withUserConfiguration(TestApplication::class.java, NoOpTransportConfiguration::class.java)
       .withPropertyValues(
@@ -35,15 +38,54 @@ class SentryOkHttpAutoConfigurationTest {
         "sentry.enable-spotlight=false",
       )
 
+  private val contextRunner =
+    baseContextRunner.withPropertyValues("sentry.clients.ok-http-enabled=true")
+
+  private val noOtelClassLoader =
+    FilteredClassLoader(
+      SentryAutoConfigurationCustomizerProvider::class.java,
+      AgentMarker::class.java,
+    )
+
+  private val noOtelContextRunner = contextRunner.withClassLoader(noOtelClassLoader)
+
+  @Test
+  fun `does not modify clients unless explicitly enabled`() {
+    for (properties in
+      listOf(emptyArray<String>(), arrayOf("sentry.clients.ok-http-enabled=false"))) {
+      val plainClient = OkHttpClient()
+      val manualClient =
+        OkHttpClient.Builder()
+          .addInterceptor(SentryOkHttpInterceptor())
+          .eventListener(SentryOkHttpEventListener())
+          .build()
+
+      baseContextRunner
+        .withClassLoader(noOtelClassLoader)
+        .withPropertyValues("sentry.dsn=http://key@localhost/proj", *properties)
+        .withBean("plainClient", OkHttpClient::class.java, { plainClient })
+        .withBean("manualClient", OkHttpClient::class.java, { manualClient })
+        .run { context ->
+          assertThat(context.getBean("plainClient")).isSameInstanceAs(plainClient)
+          assertThat(context.getBean("manualClient")).isSameInstanceAs(manualClient)
+          assertThat(context.getBeansOfType(SentryOkHttpClientBeanPostProcessor::class.java))
+            .isEmpty()
+          assertThat(context.getBean(SentryProperties::class.java).clients.isOkHttpEnabled)
+            .isFalse()
+        }
+    }
+  }
+
   @Test
   fun `instruments a Spring managed OkHttpClient`() {
-    contextRunner
+    noOtelContextRunner
       .withPropertyValues("sentry.dsn=http://key@localhost/proj")
       .withUserConfiguration(OkHttpClientConfiguration::class.java)
       .run { context ->
         val client = context.getBean(OkHttpClient::class.java)
         val existingInterceptor = context.getBean("existingInterceptor", Interceptor::class.java)
 
+        assertThat(context.getBean(SentryProperties::class.java).clients.isOkHttpEnabled).isTrue()
         assertThat(client.connectTimeoutMillis).isEqualTo(1234)
         assertThat(client.interceptors).contains(existingInterceptor)
         assertThat(client.interceptors.filterIsInstance<SentryOkHttpInterceptor>()).hasSize(1)
@@ -62,7 +104,7 @@ class SentryOkHttpAutoConfigurationTest {
 
   @Test
   fun `instruments every Spring managed OkHttpClient`() {
-    contextRunner
+    noOtelContextRunner
       .withPropertyValues("sentry.dsn=http://key@localhost/proj")
       .withUserConfiguration(MultipleOkHttpClientsConfiguration::class.java)
       .run { context ->
@@ -78,7 +120,7 @@ class SentryOkHttpAutoConfigurationTest {
 
   @Test
   fun `does not duplicate an existing Sentry interceptor`() {
-    contextRunner
+    noOtelContextRunner
       .withPropertyValues("sentry.dsn=http://key@localhost/proj")
       .withUserConfiguration(ManuallyInstrumentedOkHttpClientConfiguration::class.java)
       .run { context ->
@@ -120,9 +162,42 @@ class SentryOkHttpAutoConfigurationTest {
     assertThat(client.interceptors).isEmpty()
   }
 
+  @Test
+  fun `does not instrument when OpenTelemetry agent is present`() {
+    contextRunner
+      .withPropertyValues("sentry.dsn=http://key@localhost/proj")
+      .withUserConfiguration(OkHttpClientConfiguration::class.java)
+      .run { context ->
+        val client = context.getBean(OkHttpClient::class.java)
+
+        assertThat(client.interceptors.filterIsInstance<SentryOkHttpInterceptor>()).isEmpty()
+        assertThat(client.eventListenerFactory.create(mock()))
+          .isNotInstanceOf(SentryOkHttpEventListener::class.java)
+      }
+  }
+
+  @Test
+  fun `does not instrument when Sentry OpenTelemetry integration is present`() {
+    contextRunner
+      .withClassLoader(FilteredClassLoader(AgentMarker::class.java))
+      .withPropertyValues("sentry.dsn=http://key@localhost/proj")
+      .withUserConfiguration(
+        OkHttpClientConfiguration::class.java,
+        OpenTelemetryConfiguration::class.java,
+      )
+      .run { context ->
+        val client = context.getBean(OkHttpClient::class.java)
+
+        assertThat(client.interceptors.filterIsInstance<SentryOkHttpInterceptor>()).isEmpty()
+        assertThat(client.eventListenerFactory.create(mock()))
+          .isNotInstanceOf(SentryOkHttpEventListener::class.java)
+      }
+  }
+
   @Test
   fun `does not instrument OkHttpClient without a dsn`() {
-    contextRunner.withUserConfiguration(OkHttpClientConfiguration::class.java).run { context ->
+    noOtelContextRunner.withUserConfiguration(OkHttpClientConfiguration::class.java).run { context
+      ->
       val client = context.getBean(OkHttpClient::class.java)
 
       assertThat(client.interceptors.filterIsInstance<SentryOkHttpInterceptor>()).isEmpty()
@@ -133,15 +208,21 @@ class SentryOkHttpAutoConfigurationTest {
 
   @Test
   fun `does not create a default OkHttpClient`() {
-    contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj").run { context ->
+    noOtelContextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj").run { context ->
       assertThat(context.getBeansOfType(OkHttpClient::class.java)).isEmpty()
     }
   }
 
   @Test
   fun `does not instrument when sentry-okhttp is not on the classpath`() {
     contextRunner
-      .withClassLoader(FilteredClassLoader(SentryOkHttpInterceptor::class.java))
+      .withClassLoader(
+        FilteredClassLoader(
+          SentryOkHttpInterceptor::class.java,
+          SentryAutoConfigurationCustomizerProvider::class.java,
+          AgentMarker::class.java,
+        )
+      )
       .withPropertyValues("sentry.dsn=http://key@localhost/proj")
       .withUserConfiguration(OkHttpClientConfiguration::class.java)
       .run { context ->
@@ -154,6 +235,11 @@ class SentryOkHttpAutoConfigurationTest {
 
   @Configuration(proxyBeanMethods = false) @EnableAutoConfiguration open class TestApplication
 
+  @Configuration(proxyBeanMethods = false)
+  open class OpenTelemetryConfiguration {
+    @Bean open fun openTelemetry(): OpenTelemetry = OpenTelemetry.noop()
+  }
+
   @Configuration(proxyBeanMethods = false)
   open class NoOpTransportConfiguration {
     @Bean open fun noOpTransportFactory(): ITransportFactory = NoOpTransportFactory.getInstance()