diff --git a/sentry-samples/sentry-samples-spring-boot-jakarta/build.gradle.kts b/sentry-samples/sentry-samples-spring-boot-jakarta/build.gradle.kts index 320a9cc2512..11ff67adc0d 100644 --- a/sentry-samples/sentry-samples-spring-boot-jakarta/build.gradle.kts +++ b/sentry-samples/sentry-samples-spring-boot-jakarta/build.gradle.kts @@ -73,6 +73,10 @@ dependencies { // OpenFeature SDK implementation(libs.openfeature) + // okhttp client instrumentation + implementation(projects.sentryOkhttp) + implementation(libs.okhttp) + // database query tracing implementation(projects.sentryJdbc) runtimeOnly(libs.hsqldb) diff --git a/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/SentryDemoApplication.java b/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/SentryDemoApplication.java index e818cbe42ff..b9df7f2d206 100644 --- a/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/SentryDemoApplication.java +++ b/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/SentryDemoApplication.java @@ -4,6 +4,7 @@ import io.sentry.samples.spring.boot.jakarta.quartz.SampleJob; import java.util.Collections; +import okhttp3.OkHttpClient; import org.quartz.JobDetail; import org.quartz.SimpleTrigger; import org.springframework.boot.SpringApplication; @@ -42,6 +43,12 @@ RestClient restClient(RestClient.Builder builder) { return builder.build(); } + @Bean + OkHttpClient okHttpClient() { + // automatically instrumented by Sentry via sentry.clients.ok-http-enabled=true + return new OkHttpClient.Builder().build(); + } + @Bean public JobDetailFactoryBean jobDetail() { JobDetailFactoryBean jobDetailFactory = new JobDetailFactoryBean(); diff --git a/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/TodoController.java b/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/TodoController.java index 987d516936b..79c2e600046 100644 --- a/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/TodoController.java +++ b/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/TodoController.java @@ -1,6 +1,12 @@ package io.sentry.samples.spring.boot.jakarta; +import com.fasterxml.jackson.databind.ObjectMapper; import io.sentry.reactor.SentryReactorUtils; +import java.io.IOException; +import java.io.UncheckedIOException; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.Response; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RestController; @@ -16,11 +22,20 @@ public class TodoController { private final RestTemplate restTemplate; private final WebClient webClient; private final RestClient restClient; + private final OkHttpClient okHttpClient; + private final ObjectMapper objectMapper; - public TodoController(RestTemplate restTemplate, WebClient webClient, RestClient restClient) { + public TodoController( + RestTemplate restTemplate, + WebClient webClient, + RestClient restClient, + OkHttpClient okHttpClient, + ObjectMapper objectMapper) { this.restTemplate = restTemplate; this.webClient = webClient; this.restClient = restClient; + this.okHttpClient = okHttpClient; + this.objectMapper = objectMapper; } @GetMapping("/todo/{id}") @@ -54,4 +69,15 @@ Todo todoRestClient(@PathVariable Long id) { .retrieve() .body(Todo.class); } + + @GetMapping("/todo-okhttp/{id}") + Todo todoOkHttp(@PathVariable Long id) { + final Request request = + new Request.Builder().url("https://jsonplaceholder.typicode.com/todos/" + id).build(); + try (Response response = okHttpClient.newCall(request).execute()) { + return objectMapper.readValue(response.body().byteStream(), Todo.class); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } } diff --git a/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/resources/application.properties index 20f9463aabc..b8e39526452 100644 --- a/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-jakarta/src/main/resources/application.properties @@ -37,6 +37,8 @@ spring.quartz.job-store-type=memory # Cache tracing sentry.enable-cache-tracing=true +# Automatically instrument Spring-managed OkHttpClient beans +sentry.clients.ok-http-enabled=true spring.cache.cache-names=todos spring.cache.caffeine.spec=maximumSize=500,expireAfterAccess=600s diff --git a/sentry-samples/sentry-samples-spring-boot-jakarta/src/test/kotlin/io/sentry/systemtest/TodoSystemTest.kt b/sentry-samples/sentry-samples-spring-boot-jakarta/src/test/kotlin/io/sentry/systemtest/TodoSystemTest.kt index d34485e1388..66600e9753d 100644 --- a/sentry-samples/sentry-samples-spring-boot-jakarta/src/test/kotlin/io/sentry/systemtest/TodoSystemTest.kt +++ b/sentry-samples/sentry-samples-spring-boot-jakarta/src/test/kotlin/io/sentry/systemtest/TodoSystemTest.kt @@ -58,4 +58,20 @@ class TodoSystemTest { ) } } + + @Test + fun `get todo okhttp works`() { + val restClient = testHelper.restClient + restClient.getTodoOkHttp(1L) + assertEquals(200, restClient.lastKnownStatusCode) + + testHelper.ensureTransactionReceived { transaction, envelopeHeader -> + transaction.transaction == "GET /todo-okhttp/{id}" && + testHelper.doesTransactionContainSpanWithOpAndDescription( + transaction, + "http.client", + "GET https://jsonplaceholder.typicode.com/todos/1", + ) + } + } } diff --git a/sentry-samples/sentry-samples-spring-boot/build.gradle.kts b/sentry-samples/sentry-samples-spring-boot/build.gradle.kts index 0c8d2dc28e7..14119290732 100644 --- a/sentry-samples/sentry-samples-spring-boot/build.gradle.kts +++ b/sentry-samples/sentry-samples-spring-boot/build.gradle.kts @@ -77,6 +77,10 @@ dependencies { implementation(projects.sentryQuartz) implementation(projects.sentryAsyncProfiler) + // okhttp client instrumentation + implementation(projects.sentryOkhttp) + implementation(libs.okhttp) + // database query tracing implementation(projects.sentryJdbc) runtimeOnly(libs.hsqldb) diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/SentryDemoApplication.java b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/SentryDemoApplication.java index a08770b1029..a8bca03a99c 100644 --- a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/SentryDemoApplication.java +++ b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/SentryDemoApplication.java @@ -4,6 +4,7 @@ import io.sentry.samples.spring.boot.quartz.SampleJob; import java.util.Collections; +import okhttp3.OkHttpClient; import org.quartz.JobDetail; import org.quartz.SimpleTrigger; import org.springframework.boot.SpringApplication; @@ -36,6 +37,12 @@ WebClient webClient(WebClient.Builder builder) { return builder.build(); } + @Bean + OkHttpClient okHttpClient() { + // automatically instrumented by Sentry via sentry.clients.ok-http-enabled=true + return new OkHttpClient.Builder().build(); + } + @Bean public JobDetailFactoryBean jobDetail() { JobDetailFactoryBean jobDetailFactory = new JobDetailFactoryBean(); diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/TodoController.java b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/TodoController.java index 10d97bbfa7a..80996e57be6 100644 --- a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/TodoController.java +++ b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/TodoController.java @@ -1,5 +1,11 @@ package io.sentry.samples.spring.boot; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; +import java.io.UncheckedIOException; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.Response; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RestController; @@ -10,10 +16,18 @@ public class TodoController { private final RestTemplate restTemplate; private final WebClient webClient; + private final OkHttpClient okHttpClient; + private final ObjectMapper objectMapper; - public TodoController(RestTemplate restTemplate, WebClient webClient) { + public TodoController( + RestTemplate restTemplate, + WebClient webClient, + OkHttpClient okHttpClient, + ObjectMapper objectMapper) { this.restTemplate = restTemplate; this.webClient = webClient; + this.okHttpClient = okHttpClient; + this.objectMapper = objectMapper; } @GetMapping("/todo/{id}") @@ -31,4 +45,15 @@ Todo todoWebClient(@PathVariable Long id) { .bodyToMono(Todo.class) .block(); } + + @GetMapping("/todo-okhttp/{id}") + Todo todoOkHttp(@PathVariable Long id) { + final Request request = + new Request.Builder().url("https://jsonplaceholder.typicode.com/todos/" + id).build(); + try (Response response = okHttpClient.newCall(request).execute()) { + return objectMapper.readValue(response.body().byteStream(), Todo.class); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } } diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties index 4e97e7a1eb8..821564f1cd1 100644 --- a/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties @@ -22,6 +22,8 @@ sentry.profile-lifecycle=TRACE # Cache tracing sentry.enable-cache-tracing=true +# Automatically instrument Spring-managed OkHttpClient beans +sentry.clients.ok-http-enabled=true spring.cache.cache-names=todos spring.cache.caffeine.spec=maximumSize=500,expireAfterAccess=600s diff --git a/sentry-samples/sentry-samples-spring-boot/src/test/kotlin/io/sentry/systemtest/TodoSystemTest.kt b/sentry-samples/sentry-samples-spring-boot/src/test/kotlin/io/sentry/systemtest/TodoSystemTest.kt index 965e6953b06..220c175f0db 100644 --- a/sentry-samples/sentry-samples-spring-boot/src/test/kotlin/io/sentry/systemtest/TodoSystemTest.kt +++ b/sentry-samples/sentry-samples-spring-boot/src/test/kotlin/io/sentry/systemtest/TodoSystemTest.kt @@ -43,4 +43,20 @@ class TodoSystemTest { ) } } + + @Test + fun `get todo okhttp works`() { + val restClient = testHelper.restClient + restClient.getTodoOkHttp(1L) + assertEquals(200, restClient.lastKnownStatusCode) + + testHelper.ensureTransactionReceived { transaction, envelopeHeader -> + transaction.transaction == "GET /todo-okhttp/{id}" && + testHelper.doesTransactionContainSpanWithOpAndDescription( + transaction, + "http.client", + "GET https://jsonplaceholder.typicode.com/todos/1", + ) + } + } } diff --git a/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api b/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api index 52633965cda..57ea90133ba 100644 --- a/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api +++ b/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api @@ -41,6 +41,7 @@ public class io/sentry/spring/boot/jakarta/SentryProfilerAutoConfiguration { public class io/sentry/spring/boot/jakarta/SentryProperties : io/sentry/SentryOptions { public fun ()V + public fun getClients ()Lio/sentry/spring/boot/jakarta/SentryProperties$Clients; public fun getExceptionResolverOrder ()I public fun getGraphql ()Lio/sentry/spring/boot/jakarta/SentryProperties$Graphql; public fun getLogging ()Lio/sentry/spring/boot/jakarta/SentryProperties$Logging; @@ -49,6 +50,7 @@ public class io/sentry/spring/boot/jakarta/SentryProperties : io/sentry/SentryOp public fun isEnableAotCompatibility ()Z public fun isKeepTransactionsOpenForAsyncResponses ()Z public fun isUseGitCommitIdAsRelease ()Z + public fun setClients (Lio/sentry/spring/boot/jakarta/SentryProperties$Clients;)V public fun setEnableAotCompatibility (Z)V public fun setExceptionResolverOrder (I)V public fun setGraphql (Lio/sentry/spring/boot/jakarta/SentryProperties$Graphql;)V @@ -59,6 +61,12 @@ public class io/sentry/spring/boot/jakarta/SentryProperties : io/sentry/SentryOp public fun setUserFilterOrder (Ljava/lang/Integer;)V } +public class io/sentry/spring/boot/jakarta/SentryProperties$Clients { + public fun ()V + public fun isOkHttpEnabled ()Z + public fun setOkHttpEnabled (Z)V +} + public class io/sentry/spring/boot/jakarta/SentryProperties$Graphql { public fun ()V public fun getIgnoredErrorTypes ()Ljava/util/List; diff --git a/sentry-spring-boot-jakarta/build.gradle.kts b/sentry-spring-boot-jakarta/build.gradle.kts index 6cb88afc7d7..5f2bbe87b34 100644 --- a/sentry-spring-boot-jakarta/build.gradle.kts +++ b/sentry-spring-boot-jakarta/build.gradle.kts @@ -35,6 +35,8 @@ dependencies { api(projects.sentrySpringJakarta) compileOnly(projects.sentryLogback) compileOnly(projects.sentryLog4j2) + compileOnly(projects.sentryOkhttp) + compileOnly(libs.okhttp) compileOnly(projects.sentryApacheHttpClient5) compileOnly(libs.log4j.api) compileOnly(libs.log4j.core) @@ -70,10 +72,9 @@ dependencies { // tests testImplementation(projects.sentryLogback) + testImplementation(projects.sentryOkhttp) testImplementation(projects.sentryLog4j2) testImplementation(projects.sentryApacheHttpClient5) - testImplementation(libs.log4j.api) - testImplementation(libs.log4j.core) testImplementation(projects.sentryGraphql) testImplementation(projects.sentryGraphql22) testImplementation(projects.sentryKafka) @@ -88,8 +89,11 @@ dependencies { testImplementation(platform(SpringBootPlugin.BOM_COORDINATES)) testImplementation(libs.context.propagation) testImplementation(libs.kotlin.test.junit) + testImplementation(libs.google.truth) testImplementation(libs.mockito.kotlin) testImplementation(libs.okhttp) + testImplementation(libs.log4j.api) + testImplementation(libs.log4j.core) testImplementation(libs.okhttp.mockwebserver) testImplementation(libs.otel) testImplementation(libs.otel.extension.autoconfigure.spi) diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java index e1f8b026274..eab02ccb6f9 100644 --- a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java +++ b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java @@ -276,6 +276,28 @@ static class SentryKafkaQueueConfiguration { } } + @Configuration(proxyBeanMethods = false) + @ConditionalOnClass( + name = { + "okhttp3.OkHttpClient", + "io.sentry.okhttp.SentryOkHttpInterceptor", + "io.sentry.okhttp.SentryOkHttpEventListener" + }) + @ConditionalOnProperty(name = "sentry.clients.ok-http-enabled", havingValue = "true") + @ConditionalOnMissingClass({ + "io.sentry.opentelemetry.SentryAutoConfigurationCustomizerProvider", + "io.sentry.opentelemetry.agent.AgentMarker" + }) + @Open + static class SentryOkHttpConfiguration { + + @Bean + public static @NotNull SentryOkHttpClientBeanPostProcessor + sentryOkHttpClientBeanPostProcessor() { + return new SentryOkHttpClientBeanPostProcessor(); + } + } + @Configuration(proxyBeanMethods = false) @ConditionalOnClass(ProceedingJoinPoint.class) @ConditionalOnProperty( diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryOkHttpClientBeanPostProcessor.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryOkHttpClientBeanPostProcessor.java new file mode 100644 index 00000000000..8409240d7fe --- /dev/null +++ b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryOkHttpClientBeanPostProcessor.java @@ -0,0 +1,88 @@ +package io.sentry.spring.boot.jakarta; + +import io.sentry.ScopesAdapter; +import io.sentry.SentryLevel; +import io.sentry.okhttp.SentryOkHttpEventListener; +import io.sentry.okhttp.SentryOkHttpInterceptor; +import okhttp3.Call; +import okhttp3.EventListener; +import okhttp3.Interceptor; +import okhttp3.OkHttpClient; +import org.jetbrains.annotations.NotNull; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.config.BeanPostProcessor; +import org.springframework.core.Ordered; +import org.springframework.core.PriorityOrdered; + +final class SentryOkHttpClientBeanPostProcessor implements BeanPostProcessor, PriorityOrdered { + + @Override + public @NotNull Object postProcessAfterInitialization( + final @NotNull Object bean, final @NotNull String beanName) throws BeansException { + if (!(bean instanceof OkHttpClient)) { + return bean; + } + + final @NotNull OkHttpClient client = (OkHttpClient) bean; + if (client.getClass() != OkHttpClient.class) { + ScopesAdapter.getInstance() + .getOptions() + .getLogger() + .log( + SentryLevel.WARNING, + "Sentry OkHttp auto-instrumentation skipped for bean '%s' (%s) because replacing " + + "an OkHttpClient subclass would not preserve its type. Configure Sentry " + + "instrumentation manually for this client.", + beanName, + client.getClass().getName()); + return client; + } + + final boolean addInterceptor = !hasSentryInterceptor(client); + final boolean wrapEventListener = + !(client.eventListenerFactory() instanceof SentryEventListenerFactory); + if (!addInterceptor && !wrapEventListener) { + return client; + } + + final @NotNull OkHttpClient.Builder builder = client.newBuilder(); + if (addInterceptor) { + builder.addInterceptor(new SentryOkHttpInterceptor()); + } + if (wrapEventListener) { + builder.eventListenerFactory(new SentryEventListenerFactory(client.eventListenerFactory())); + } + return builder.build(); + } + + private static boolean hasSentryInterceptor(final @NotNull OkHttpClient client) { + for (final @NotNull Interceptor interceptor : client.interceptors()) { + if (interceptor instanceof SentryOkHttpInterceptor) { + return true; + } + } + return false; + } + + @Override + public int getOrder() { + return Ordered.LOWEST_PRECEDENCE; + } + + private static final class SentryEventListenerFactory implements EventListener.Factory { + private final @NotNull EventListener.Factory delegate; + + private SentryEventListenerFactory(final @NotNull EventListener.Factory delegate) { + this.delegate = delegate; + } + + @Override + public @NotNull EventListener create(final @NotNull Call call) { + final @NotNull EventListener original = delegate.create(call); + if (original instanceof SentryOkHttpEventListener) { + return original; + } + return new SentryOkHttpEventListener(ScopesAdapter.getInstance(), original); + } + } +} diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java index 7813c2e5512..485d5838d2b 100644 --- a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java +++ b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/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,28 @@ 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 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; + } + } + @Open public static class Logging { /** Enable/Disable logging auto-configuration. */ diff --git a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryOkHttpAutoConfigurationTest.kt b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryOkHttpAutoConfigurationTest.kt new file mode 100644 index 00000000000..7560d50fd46 --- /dev/null +++ b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryOkHttpAutoConfigurationTest.kt @@ -0,0 +1,322 @@ +package io.sentry.spring.boot.jakarta + +import com.google.common.truth.Truth.assertThat +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 +import okhttp3.Call +import okhttp3.EventListener +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import okhttp3.Request +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import org.springframework.boot.autoconfigure.EnableAutoConfiguration +import org.springframework.boot.test.context.FilteredClassLoader +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration + +class SentryOkHttpAutoConfigurationTest { + + private val baseContextRunner = + ApplicationContextRunner() + .withUserConfiguration(TestApplication::class.java, NoOpTransportConfiguration::class.java) + .withPropertyValues( + "sentry.shutdownTimeoutMillis=0", + "sentry.sessionFlushTimeoutMillis=0", + "sentry.flushTimeoutMillis=0", + "sentry.send-modules=false", + "sentry.enable-backpressure-handling=false", + "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(), 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`() { + 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()).hasSize(1) + assertThat(client.interceptors.last()).isInstanceOf(SentryOkHttpInterceptor::class.java) + + val call = mock() + whenever(call.request()).thenReturn(Request.Builder().url("https://example.com").build()) + val listener = client.eventListenerFactory.create(call) + assertThat(listener).isInstanceOf(SentryOkHttpEventListener::class.java) + + listener.callStart(call) + assertThat(context.getBean(RecordingEventListener::class.java).callStarted.get()).isTrue() + listener.callEnd(call) + } + } + + @Test + fun `instruments every Spring managed OkHttpClient`() { + noOtelContextRunner + .withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(MultipleOkHttpClientsConfiguration::class.java) + .run { context -> + val clients = context.getBeansOfType(OkHttpClient::class.java) + + assertThat(clients).hasSize(2) + clients.values.forEach { client -> + assertThat(client.interceptors.filterIsInstance()).hasSize(1) + assertThat(client.interceptors.last()).isInstanceOf(SentryOkHttpInterceptor::class.java) + } + } + } + + @Test + fun `forwards canceled before callStart to the original event listener`() { + noOtelContextRunner + .withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(OkHttpClientConfiguration::class.java) + .run { context -> + val client = context.getBean(OkHttpClient::class.java) + + // OkHttp creates the listener in the Call constructor and reports canceled() even if + // the call is never executed, i.e. before callStart() ever happens + client.newCall(Request.Builder().url("https://example.com").build()).cancel() + + assertThat(context.getBean(RecordingEventListener::class.java).callCanceled.get()).isTrue() + } + } + + @Test + fun `does not wrap a Sentry listener created by the original factory`() { + val sentryListener = SentryOkHttpEventListener() + val client = OkHttpClient.Builder().eventListenerFactory { sentryListener }.build() + + noOtelContextRunner + .withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withBean("okHttpClient", OkHttpClient::class.java, { client }) + .run { context -> + val instrumented = context.getBean(OkHttpClient::class.java) + + assertThat(instrumented.eventListenerFactory.create(mock())) + .isSameInstanceAs(sentryListener) + assertThat(instrumented.interceptors.filterIsInstance()).hasSize(1) + } + } + + @Test + fun `does not duplicate an existing Sentry interceptor`() { + noOtelContextRunner + .withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(ManuallyInstrumentedOkHttpClientConfiguration::class.java) + .run { context -> + val client = context.getBean(OkHttpClient::class.java) + + assertThat(client.interceptors.filterIsInstance()).hasSize(1) + assertThat(client.eventListenerFactory.create(mock())) + .isInstanceOf(SentryOkHttpEventListener::class.java) + } + } + + @Test + fun `post processor is idempotent`() { + val processor = SentryOkHttpClientBeanPostProcessor() + val firstResult = + processor.postProcessAfterInitialization(OkHttpClient(), "okHttpClient") as OkHttpClient + val secondResult = + processor.postProcessAfterInitialization(firstResult, "okHttpClient") as OkHttpClient + + assertThat(secondResult).isSameInstanceAs(firstResult) + assertThat(secondResult.interceptors.filterIsInstance()).hasSize(1) + } + + @Test + fun `does not replace unrelated beans`() { + val processor = SentryOkHttpClientBeanPostProcessor() + val bean = Any() + + assertThat(processor.postProcessAfterInitialization(bean, "bean")).isSameInstanceAs(bean) + } + + @Test + fun `does not replace OkHttpClient subclasses`() { + val processor = SentryOkHttpClientBeanPostProcessor() + val client = CustomOkHttpClient() + + assertThat(processor.postProcessAfterInitialization(client, "okHttpClient")) + .isSameInstanceAs(client) + 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()).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) + .run { context -> + val client = context.getBean(OkHttpClient::class.java) + + assertThat(client.interceptors.filterIsInstance()).isEmpty() + assertThat(client.eventListenerFactory.create(mock())) + .isNotInstanceOf(SentryOkHttpEventListener::class.java) + } + } + + @Test + fun `does not instrument OkHttpClient without a dsn`() { + noOtelContextRunner.withUserConfiguration(OkHttpClientConfiguration::class.java).run { context + -> + val client = context.getBean(OkHttpClient::class.java) + + assertThat(client.interceptors.filterIsInstance()).isEmpty() + assertThat(client.eventListenerFactory.create(mock())) + .isNotInstanceOf(SentryOkHttpEventListener::class.java) + } + } + + @Test + fun `does not create a default OkHttpClient`() { + 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, + SentryAutoConfigurationCustomizerProvider::class.java, + AgentMarker::class.java, + ) + ) + .withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(OkHttpClientConfiguration::class.java) + .run { context -> + val client = context.getBean(OkHttpClient::class.java) + assertThat(client.interceptors).hasSize(1) + assertThat(client.interceptors.first().javaClass.name) + .isEqualTo(OkHttpClientConfiguration.ExistingInterceptor::class.java.name) + } + } + + @Configuration(proxyBeanMethods = false) @EnableAutoConfiguration open class TestApplication + + @Configuration(proxyBeanMethods = false) + open class NoOpTransportConfiguration { + @Bean open fun noOpTransportFactory(): ITransportFactory = NoOpTransportFactory.getInstance() + } + + @Configuration(proxyBeanMethods = false) + open class OkHttpClientConfiguration { + @Bean open fun existingInterceptor(): Interceptor = ExistingInterceptor() + + @Bean open fun recordingEventListener(): RecordingEventListener = RecordingEventListener() + + @Bean + open fun okHttpClient( + existingInterceptor: Interceptor, + recordingEventListener: RecordingEventListener, + ): OkHttpClient = + OkHttpClient.Builder() + .connectTimeout(1234, TimeUnit.MILLISECONDS) + .addInterceptor(existingInterceptor) + .eventListener(recordingEventListener) + .build() + + class ExistingInterceptor : Interceptor { + override fun intercept(chain: Interceptor.Chain) = chain.proceed(chain.request()) + } + } + + @Configuration(proxyBeanMethods = false) + open class MultipleOkHttpClientsConfiguration { + @Bean open fun firstOkHttpClient(): OkHttpClient = OkHttpClient() + + @Bean open fun secondOkHttpClient(): OkHttpClient = OkHttpClient() + } + + @Configuration(proxyBeanMethods = false) + open class ManuallyInstrumentedOkHttpClientConfiguration { + @Bean + open fun okHttpClient(): OkHttpClient = + OkHttpClient.Builder().addInterceptor(SentryOkHttpInterceptor()).build() + } + + class CustomOkHttpClient : OkHttpClient() + + class RecordingEventListener : EventListener() { + val callStarted = AtomicBoolean(false) + val callCanceled = AtomicBoolean(false) + + override fun callStart(call: Call) { + callStarted.set(true) + } + + override fun canceled(call: Call) { + callCanceled.set(true) + } + } +} diff --git a/sentry-spring-boot/api/sentry-spring-boot.api b/sentry-spring-boot/api/sentry-spring-boot.api index ef726c4fc25..fe13689faf5 100644 --- a/sentry-spring-boot/api/sentry-spring-boot.api +++ b/sentry-spring-boot/api/sentry-spring-boot.api @@ -30,12 +30,14 @@ public class io/sentry/spring/boot/SentryProfilerAutoConfiguration { public class io/sentry/spring/boot/SentryProperties : io/sentry/SentryOptions { public fun ()V + public fun getClients ()Lio/sentry/spring/boot/SentryProperties$Clients; public fun getExceptionResolverOrder ()I public fun getGraphql ()Lio/sentry/spring/boot/SentryProperties$Graphql; public fun getLogging ()Lio/sentry/spring/boot/SentryProperties$Logging; public fun getUserFilterOrder ()Ljava/lang/Integer; public fun isKeepTransactionsOpenForAsyncResponses ()Z public fun isUseGitCommitIdAsRelease ()Z + public fun setClients (Lio/sentry/spring/boot/SentryProperties$Clients;)V public fun setExceptionResolverOrder (I)V public fun setGraphql (Lio/sentry/spring/boot/SentryProperties$Graphql;)V public fun setKeepTransactionsOpenForAsyncResponses (Z)V @@ -44,6 +46,12 @@ public class io/sentry/spring/boot/SentryProperties : io/sentry/SentryOptions { public fun setUserFilterOrder (Ljava/lang/Integer;)V } +public class io/sentry/spring/boot/SentryProperties$Clients { + public fun ()V + public fun isOkHttpEnabled ()Z + public fun setOkHttpEnabled (Z)V +} + public class io/sentry/spring/boot/SentryProperties$Graphql { public fun ()V public fun getIgnoredErrorTypes ()Ljava/util/List; diff --git a/sentry-spring-boot/build.gradle.kts b/sentry-spring-boot/build.gradle.kts index 947eaf9b03a..27cb2d7805c 100644 --- a/sentry-spring-boot/build.gradle.kts +++ b/sentry-spring-boot/build.gradle.kts @@ -29,6 +29,8 @@ dependencies { api(projects.sentry) api(projects.sentrySpring) compileOnly(projects.sentryLogback) + compileOnly(projects.sentryOkhttp) + compileOnly(libs.okhttp) compileOnly(projects.sentryApacheHttpClient5) compileOnly(libs.jetbrains.annotations) compileOnly(libs.nopen.annotations) @@ -58,12 +60,14 @@ dependencies { // tests testImplementation(projects.sentryLogback) + testImplementation(projects.sentryOkhttp) testImplementation(projects.sentryQuartz) testImplementation(projects.sentryApacheHttpClient5) testImplementation(projects.sentryKafka) testImplementation(projects.sentryTestSupport) testImplementation(kotlin(Config.kotlinStdLib)) testImplementation(libs.kotlin.test.junit) + testImplementation(libs.google.truth) testImplementation(libs.mockito.kotlin) testImplementation(libs.okhttp) testImplementation(libs.okhttp.mockwebserver) diff --git a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index f89f5c5bb31..0ed4bed32eb 100644 --- a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -259,6 +259,28 @@ static class SentryKafkaQueueConfiguration { } } + @Configuration(proxyBeanMethods = false) + @ConditionalOnClass( + name = { + "okhttp3.OkHttpClient", + "io.sentry.okhttp.SentryOkHttpInterceptor", + "io.sentry.okhttp.SentryOkHttpEventListener" + }) + @ConditionalOnProperty(name = "sentry.clients.ok-http-enabled", havingValue = "true") + @ConditionalOnMissingClass({ + "io.sentry.opentelemetry.SentryAutoConfigurationCustomizerProvider", + "io.sentry.opentelemetry.agent.AgentMarker" + }) + @Open + static class SentryOkHttpConfiguration { + + @Bean + public static @NotNull SentryOkHttpClientBeanPostProcessor + sentryOkHttpClientBeanPostProcessor() { + return new SentryOkHttpClientBeanPostProcessor(); + } + } + @Configuration(proxyBeanMethods = false) @ConditionalOnClass(ProceedingJoinPoint.class) @ConditionalOnProperty( diff --git a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryOkHttpClientBeanPostProcessor.java b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryOkHttpClientBeanPostProcessor.java new file mode 100644 index 00000000000..81a2aa9856b --- /dev/null +++ b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryOkHttpClientBeanPostProcessor.java @@ -0,0 +1,88 @@ +package io.sentry.spring.boot; + +import io.sentry.ScopesAdapter; +import io.sentry.SentryLevel; +import io.sentry.okhttp.SentryOkHttpEventListener; +import io.sentry.okhttp.SentryOkHttpInterceptor; +import okhttp3.Call; +import okhttp3.EventListener; +import okhttp3.Interceptor; +import okhttp3.OkHttpClient; +import org.jetbrains.annotations.NotNull; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.config.BeanPostProcessor; +import org.springframework.core.Ordered; +import org.springframework.core.PriorityOrdered; + +final class SentryOkHttpClientBeanPostProcessor implements BeanPostProcessor, PriorityOrdered { + + @Override + public @NotNull Object postProcessAfterInitialization( + final @NotNull Object bean, final @NotNull String beanName) throws BeansException { + if (!(bean instanceof OkHttpClient)) { + return bean; + } + + final @NotNull OkHttpClient client = (OkHttpClient) bean; + if (client.getClass() != OkHttpClient.class) { + ScopesAdapter.getInstance() + .getOptions() + .getLogger() + .log( + SentryLevel.WARNING, + "Sentry OkHttp auto-instrumentation skipped for bean '%s' (%s) because replacing " + + "an OkHttpClient subclass would not preserve its type. Configure Sentry " + + "instrumentation manually for this client.", + beanName, + client.getClass().getName()); + return client; + } + + final boolean addInterceptor = !hasSentryInterceptor(client); + final boolean wrapEventListener = + !(client.eventListenerFactory() instanceof SentryEventListenerFactory); + if (!addInterceptor && !wrapEventListener) { + return client; + } + + final @NotNull OkHttpClient.Builder builder = client.newBuilder(); + if (addInterceptor) { + builder.addInterceptor(new SentryOkHttpInterceptor()); + } + if (wrapEventListener) { + builder.eventListenerFactory(new SentryEventListenerFactory(client.eventListenerFactory())); + } + return builder.build(); + } + + private static boolean hasSentryInterceptor(final @NotNull OkHttpClient client) { + for (final @NotNull Interceptor interceptor : client.interceptors()) { + if (interceptor instanceof SentryOkHttpInterceptor) { + return true; + } + } + return false; + } + + @Override + public int getOrder() { + return Ordered.LOWEST_PRECEDENCE; + } + + private static final class SentryEventListenerFactory implements EventListener.Factory { + private final @NotNull EventListener.Factory delegate; + + private SentryEventListenerFactory(final @NotNull EventListener.Factory delegate) { + this.delegate = delegate; + } + + @Override + public @NotNull EventListener create(final @NotNull Call call) { + final @NotNull EventListener original = delegate.create(call); + if (original instanceof SentryOkHttpEventListener) { + return original; + } + return new SentryOkHttpEventListener(ScopesAdapter.getInstance(), original); + } + } +} diff --git a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java index f959fc930ba..879b4e2ada6 100644 --- a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java +++ b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java @@ -37,6 +37,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; } @@ -98,6 +101,28 @@ 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 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; + } + } + @Open public static class Logging { /** Enable/Disable logging auto-configuration. */ diff --git a/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryOkHttpAutoConfigurationTest.kt b/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryOkHttpAutoConfigurationTest.kt new file mode 100644 index 00000000000..295438318eb --- /dev/null +++ b/sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryOkHttpAutoConfigurationTest.kt @@ -0,0 +1,322 @@ +package io.sentry.spring.boot + +import com.google.common.truth.Truth.assertThat +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 +import okhttp3.Call +import okhttp3.EventListener +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import okhttp3.Request +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import org.springframework.boot.autoconfigure.EnableAutoConfiguration +import org.springframework.boot.test.context.FilteredClassLoader +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration + +class SentryOkHttpAutoConfigurationTest { + + private val baseContextRunner = + ApplicationContextRunner() + .withUserConfiguration(TestApplication::class.java, NoOpTransportConfiguration::class.java) + .withPropertyValues( + "sentry.shutdownTimeoutMillis=0", + "sentry.sessionFlushTimeoutMillis=0", + "sentry.flushTimeoutMillis=0", + "sentry.send-modules=false", + "sentry.enable-backpressure-handling=false", + "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(), 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`() { + 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()).hasSize(1) + assertThat(client.interceptors.last()).isInstanceOf(SentryOkHttpInterceptor::class.java) + + val call = mock() + whenever(call.request()).thenReturn(Request.Builder().url("https://example.com").build()) + val listener = client.eventListenerFactory.create(call) + assertThat(listener).isInstanceOf(SentryOkHttpEventListener::class.java) + + listener.callStart(call) + assertThat(context.getBean(RecordingEventListener::class.java).callStarted.get()).isTrue() + listener.callEnd(call) + } + } + + @Test + fun `instruments every Spring managed OkHttpClient`() { + noOtelContextRunner + .withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(MultipleOkHttpClientsConfiguration::class.java) + .run { context -> + val clients = context.getBeansOfType(OkHttpClient::class.java) + + assertThat(clients).hasSize(2) + clients.values.forEach { client -> + assertThat(client.interceptors.filterIsInstance()).hasSize(1) + assertThat(client.interceptors.last()).isInstanceOf(SentryOkHttpInterceptor::class.java) + } + } + } + + @Test + fun `forwards canceled before callStart to the original event listener`() { + noOtelContextRunner + .withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(OkHttpClientConfiguration::class.java) + .run { context -> + val client = context.getBean(OkHttpClient::class.java) + + // OkHttp creates the listener in the Call constructor and reports canceled() even if + // the call is never executed, i.e. before callStart() ever happens + client.newCall(Request.Builder().url("https://example.com").build()).cancel() + + assertThat(context.getBean(RecordingEventListener::class.java).callCanceled.get()).isTrue() + } + } + + @Test + fun `does not wrap a Sentry listener created by the original factory`() { + val sentryListener = SentryOkHttpEventListener() + val client = OkHttpClient.Builder().eventListenerFactory { sentryListener }.build() + + noOtelContextRunner + .withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withBean("okHttpClient", OkHttpClient::class.java, { client }) + .run { context -> + val instrumented = context.getBean(OkHttpClient::class.java) + + assertThat(instrumented.eventListenerFactory.create(mock())) + .isSameInstanceAs(sentryListener) + assertThat(instrumented.interceptors.filterIsInstance()).hasSize(1) + } + } + + @Test + fun `does not duplicate an existing Sentry interceptor`() { + noOtelContextRunner + .withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(ManuallyInstrumentedOkHttpClientConfiguration::class.java) + .run { context -> + val client = context.getBean(OkHttpClient::class.java) + + assertThat(client.interceptors.filterIsInstance()).hasSize(1) + assertThat(client.eventListenerFactory.create(mock())) + .isInstanceOf(SentryOkHttpEventListener::class.java) + } + } + + @Test + fun `post processor is idempotent`() { + val processor = SentryOkHttpClientBeanPostProcessor() + val firstResult = + processor.postProcessAfterInitialization(OkHttpClient(), "okHttpClient") as OkHttpClient + val secondResult = + processor.postProcessAfterInitialization(firstResult, "okHttpClient") as OkHttpClient + + assertThat(secondResult).isSameInstanceAs(firstResult) + assertThat(secondResult.interceptors.filterIsInstance()).hasSize(1) + } + + @Test + fun `does not replace unrelated beans`() { + val processor = SentryOkHttpClientBeanPostProcessor() + val bean = Any() + + assertThat(processor.postProcessAfterInitialization(bean, "bean")).isSameInstanceAs(bean) + } + + @Test + fun `does not replace OkHttpClient subclasses`() { + val processor = SentryOkHttpClientBeanPostProcessor() + val client = CustomOkHttpClient() + + assertThat(processor.postProcessAfterInitialization(client, "okHttpClient")) + .isSameInstanceAs(client) + 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()).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) + .run { context -> + val client = context.getBean(OkHttpClient::class.java) + + assertThat(client.interceptors.filterIsInstance()).isEmpty() + assertThat(client.eventListenerFactory.create(mock())) + .isNotInstanceOf(SentryOkHttpEventListener::class.java) + } + } + + @Test + fun `does not instrument OkHttpClient without a dsn`() { + noOtelContextRunner.withUserConfiguration(OkHttpClientConfiguration::class.java).run { context + -> + val client = context.getBean(OkHttpClient::class.java) + + assertThat(client.interceptors.filterIsInstance()).isEmpty() + assertThat(client.eventListenerFactory.create(mock())) + .isNotInstanceOf(SentryOkHttpEventListener::class.java) + } + } + + @Test + fun `does not create a default OkHttpClient`() { + 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, + SentryAutoConfigurationCustomizerProvider::class.java, + AgentMarker::class.java, + ) + ) + .withPropertyValues("sentry.dsn=http://key@localhost/proj") + .withUserConfiguration(OkHttpClientConfiguration::class.java) + .run { context -> + val client = context.getBean(OkHttpClient::class.java) + assertThat(client.interceptors).hasSize(1) + assertThat(client.interceptors.first().javaClass.name) + .isEqualTo(OkHttpClientConfiguration.ExistingInterceptor::class.java.name) + } + } + + @Configuration(proxyBeanMethods = false) @EnableAutoConfiguration open class TestApplication + + @Configuration(proxyBeanMethods = false) + open class NoOpTransportConfiguration { + @Bean open fun noOpTransportFactory(): ITransportFactory = NoOpTransportFactory.getInstance() + } + + @Configuration(proxyBeanMethods = false) + open class OkHttpClientConfiguration { + @Bean open fun existingInterceptor(): Interceptor = ExistingInterceptor() + + @Bean open fun recordingEventListener(): RecordingEventListener = RecordingEventListener() + + @Bean + open fun okHttpClient( + existingInterceptor: Interceptor, + recordingEventListener: RecordingEventListener, + ): OkHttpClient = + OkHttpClient.Builder() + .connectTimeout(1234, TimeUnit.MILLISECONDS) + .addInterceptor(existingInterceptor) + .eventListener(recordingEventListener) + .build() + + class ExistingInterceptor : Interceptor { + override fun intercept(chain: Interceptor.Chain) = chain.proceed(chain.request()) + } + } + + @Configuration(proxyBeanMethods = false) + open class MultipleOkHttpClientsConfiguration { + @Bean open fun firstOkHttpClient(): OkHttpClient = OkHttpClient() + + @Bean open fun secondOkHttpClient(): OkHttpClient = OkHttpClient() + } + + @Configuration(proxyBeanMethods = false) + open class ManuallyInstrumentedOkHttpClientConfiguration { + @Bean + open fun okHttpClient(): OkHttpClient = + OkHttpClient.Builder().addInterceptor(SentryOkHttpInterceptor()).build() + } + + class CustomOkHttpClient : OkHttpClient() + + class RecordingEventListener : EventListener() { + val callStarted = AtomicBoolean(false) + val callCanceled = AtomicBoolean(false) + + override fun callStart(call: Call) { + callStarted.set(true) + } + + override fun canceled(call: Call) { + callCanceled.set(true) + } + } +}