Support Spring's new RestClient with auto configuration (getsentry#3198)

- Spring RestClient support

sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java

@@ -51,6 +51,7 @@
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
+import org.springframework.boot.autoconfigure.web.client.RestClientAutoConfiguration;
 import org.springframework.boot.autoconfigure.web.client.RestTemplateAutoConfiguration;
 import org.springframework.boot.autoconfigure.web.reactive.function.client.WebClientAutoConfiguration;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
@@ -65,6 +66,7 @@
 import org.springframework.graphql.execution.DataFetcherExceptionResolverAdapter;
 import org.springframework.scheduling.quartz.SchedulerFactoryBean;
 import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.web.client.RestClient;
 import org.springframework.web.client.RestTemplate;
 import org.springframework.web.reactive.function.client.WebClient;
 import org.springframework.web.servlet.HandlerExceptionResolver;
@@ -357,6 +359,17 @@ public SentrySpanRestTemplateCustomizer sentrySpanRestTemplateCustomizer(IHub hu
       }
     }
 
+    @Configuration(proxyBeanMethods = false)
+    @AutoConfigureBefore(RestClientAutoConfiguration.class)
+    @ConditionalOnClass(RestClient.class)
+    @Open
+    static class SentrySpanRestClientConfiguration {
+      @Bean
+      public SentrySpanRestClientCustomizer sentrySpanRestClientCustomizer(IHub hub) {
+        return new SentrySpanRestClientCustomizer(hub);
+      }
+    }
+
     @Configuration(proxyBeanMethods = false)
     @AutoConfigureBefore(WebClientAutoConfiguration.class)
     @ConditionalOnClass(WebClient.class)

sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java

@@ -0,0 +1,28 @@
+package io.sentry.spring.boot.jakarta;
+
+import com.jakewharton.nopen.annotation.Open;
+import io.sentry.IHub;
+import io.sentry.spring.jakarta.tracing.SentrySpanClientHttpRequestInterceptor;
+import org.jetbrains.annotations.NotNull;
+import org.springframework.boot.web.client.RestClientCustomizer;
+import org.springframework.web.client.RestClient;
+
+@Open
+class SentrySpanRestClientCustomizer implements RestClientCustomizer {
+  private final @NotNull SentrySpanClientHttpRequestInterceptor interceptor;
+
+  public SentrySpanRestClientCustomizer(final @NotNull IHub hub) {
+    this.interceptor = new SentrySpanClientHttpRequestInterceptor(hub);
+  }
+
+  @Override
+  public void customize(final @NotNull RestClient.Builder restClientBuilder) {
+    restClientBuilder.requestInterceptors(clientHttpRequestInterceptors -> {
+      // As the SentrySpanClientHttpRequestInterceptor is being created in this class, this might not work
+      // if somebody registers it from an outside.
+      if (!clientHttpRequestInterceptors.contains(interceptor)) {
+        clientHttpRequestInterceptors.add(interceptor);
+      }
+    });
+  }
+}

sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt

@@ -61,6 +61,7 @@ import org.springframework.core.Ordered
 import org.springframework.core.annotation.Order
 import org.springframework.scheduling.quartz.SchedulerFactoryBean
 import org.springframework.security.core.context.SecurityContextHolder
+import org.springframework.web.client.RestClient
 import org.springframework.web.client.RestTemplate
 import org.springframework.web.reactive.function.client.WebClient
 import org.springframework.web.servlet.HandlerExceptionResolver
@@ -638,6 +639,23 @@ class SentryAutoConfigurationTest {
             }
     }
 
+    @Test
+    fun `when tracing is enabled and RestClient is on the classpath, SentrySpanRestClientCustomizer bean is created`() {
+        contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.traces-sample-rate=1.0")
+            .run {
+                assertThat(it).hasSingleBean(SentrySpanRestClientCustomizer::class.java)
+            }
+    }
+
+    @Test
+    fun `when tracing is enabled and RestClient is not on the classpath, SentrySpanRestClientCustomizer bean is not created`() {
+        contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.traces-sample-rate=1.0")
+            .withClassLoader(FilteredClassLoader(RestClient::class.java))
+            .run {
+                assertThat(it).doesNotHaveBean(SentrySpanRestClientCustomizer::class.java)
+            }
+    }
+
     @Test
     fun `when tracing is enabled and WebClient is on the classpath, SentrySpanWebClientCustomizer bean is created`() {
         contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.traces-sample-rate=1.0")

sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt

@@ -0,0 +1,288 @@
+package io.sentry.spring.boot.jakarta
+
+import io.sentry.*
+import okhttp3.mockwebserver.MockResponse
+import okhttp3.mockwebserver.MockWebServer
+import okhttp3.mockwebserver.SocketPolicy
+import org.assertj.core.api.Assertions.assertThat
+import org.mockito.kotlin.*
+import org.springframework.http.HttpHeaders
+import org.springframework.http.HttpStatus
+import org.springframework.http.client.HttpComponentsClientHttpRequestFactory
+import org.springframework.web.client.RestClient
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNotNull
+import kotlin.test.assertTrue
+
+class SentrySpanRestClientCustomizerTest {
+    class Fixture {
+        val sentryOptions = SentryOptions()
+        val hub = mock<IHub>()
+        val restClientBuilder = RestClient.builder()
+        var mockServer = MockWebServer()
+        val transaction: SentryTracer
+        internal val customizer = SentrySpanRestClientCustomizer(hub)
+        val url = mockServer.url("/test/123").toString()
+        val scope = Scope(sentryOptions)
+
+        init {
+            whenever(hub.options).thenReturn(sentryOptions)
+            doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any())
+            transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub)
+        }
+
+        fun getSut(isTransactionActive: Boolean, status: HttpStatus = HttpStatus.OK, socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN, includeMockServerInTracingOrigins: Boolean = true): RestClient.Builder {
+            customizer.customize(restClientBuilder)
+
+            if (includeMockServerInTracingOrigins) {
+                sentryOptions.setTracePropagationTargets(listOf(mockServer.hostName))
+            } else {
+                sentryOptions.setTracePropagationTargets(listOf("other-api"))
+            }
+
+            sentryOptions.dsn = "https://key@sentry.io/proj"
+            sentryOptions.isTraceSampling = true
+
+            mockServer.enqueue(
+                MockResponse()
+                    .setBody("OK")
+                    .setSocketPolicy(socketPolicy)
+                    .setResponseCode(status.value())
+            )
+
+            if (isTransactionActive) {
+                whenever(hub.span).thenReturn(transaction)
+            }
+
+            return restClientBuilder.apply{
+                val requestFactory = HttpComponentsClientHttpRequestFactory()
+                requestFactory.setConnectTimeout(2)
+                it.requestFactory(requestFactory)
+            }
+        }
+    }
+
+    private val fixture = Fixture()
+
+    @Test
+    fun `when transaction is active, creates span around RestClient HTTP call`() {
+        val result = fixture.getSut(isTransactionActive = true).build()
+            .get().uri(fixture.url).retrieve().toEntity(String::class.java)
+
+        assertThat(result.body).isEqualTo("OK")
+        assertThat(fixture.transaction.spans).hasSize(1)
+        val span = fixture.transaction.spans.first()
+        assertThat(span.operation).isEqualTo("http.client")
+        assertThat(span.description).isEqualTo("GET ${fixture.url}")
+        assertThat(span.status).isEqualTo(SpanStatus.OK)
+
+        val recordedRequest = fixture.mockServer.takeRequest()
+        assertThat(recordedRequest.headers["sentry-trace"]!!).startsWith(fixture.transaction.spanContext.traceId.toString())
+            .endsWith("-1")
+            .doesNotContain(fixture.transaction.spanContext.spanId.toString())
+        assertThat(recordedRequest.headers["baggage"]!!).contains(fixture.transaction.spanContext.traceId.toString())
+    }
+
+    @Test
+    fun `when there is an active span, existing baggage headers are merged with sentry baggage into single header`() {
+        val sut = fixture.getSut(isTransactionActive = true)
+        val headers = HttpHeaders()
+        headers.add("baggage", "thirdPartyBaggage=someValue")
+        headers.add("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue")
+
+        sut.build()
+            .get()
+            .uri(fixture.url)
+            .httpRequest { it.headers.addAll(headers) }
+            .retrieve()
+            .toEntity(String::class.java)
+
+        val recorderRequest = fixture.mockServer.takeRequest()
+        assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
+        assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
+
+        val baggageHeaderValues = recorderRequest.headers.values(BaggageHeader.BAGGAGE_HEADER)
+        assertEquals(baggageHeaderValues.size, 1)
+        assertTrue(baggageHeaderValues[0].startsWith("thirdPartyBaggage=someValue,secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue"))
+        assertTrue(baggageHeaderValues[0].contains("sentry-public_key=key"))
+        assertTrue(baggageHeaderValues[0].contains("sentry-transaction=aTransaction"))
+        assertTrue(baggageHeaderValues[0].contains("sentry-trace_id"))
+    }
+
+    @Test
+    fun `when transaction is active and server is not listed in tracing origins, does not add sentry trace header to the request`() {
+        fixture.getSut(isTransactionActive = true, includeMockServerInTracingOrigins = false).build()
+            .get()
+            .uri(fixture.url)
+            .retrieve()
+            .toEntity(String::class.java)
+        val recordedRequest = fixture.mockServer.takeRequest()
+        assertThat(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]).isNull()
+    }
+
+    @Test
+    fun `when transaction is active and server is listed in tracing origins, adds sentry trace header to the request`() {
+        fixture.getSut(isTransactionActive = true, includeMockServerInTracingOrigins = true)
+            .build()
+            .get()
+            .uri(fixture.url)
+            .retrieve()
+            .toEntity(String::class.java)
+        val recordedRequest = fixture.mockServer.takeRequest()
+        assertThat(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]).isNotNull()
+    }
+
+    @Test
+    fun `when transaction is active and response code is not 2xx, creates span with error status around RestClient HTTP call`() {
+        try {
+            fixture.getSut(isTransactionActive = true, status = HttpStatus.INTERNAL_SERVER_ERROR)
+                .build()
+                .get()
+                .uri(fixture.url)
+                .retrieve()
+                .toEntity(String::class.java)
+        } catch (e: Throwable) {
+        }
+        assertThat(fixture.transaction.spans).hasSize(1)
+        val span = fixture.transaction.spans.first()
+        assertThat(span.operation).isEqualTo("http.client")
+        assertThat(span.description).isEqualTo("GET ${fixture.url}")
+        assertThat(span.status).isEqualTo(SpanStatus.INTERNAL_ERROR)
+    }
+
+    @Test
+    fun `when transaction is active and throws IO exception, creates span with error status around RestClient HTTP call`() {
+        try {
+            fixture.getSut(isTransactionActive = true, socketPolicy = SocketPolicy.DISCONNECT_AT_START).build()
+                .get()
+                .uri(fixture.url)
+                .retrieve()
+        } catch (e: Throwable) {
+        }
+        fixture.mockServer.takeRequest()
+        assertThat(fixture.transaction.spans).hasSize(1)
+        val span = fixture.transaction.spans.first()
+        assertThat(span.operation).isEqualTo("http.client")
+        assertThat(span.description).isEqualTo("GET ${fixture.url}")
+        assertThat(span.status).isEqualTo(SpanStatus.INTERNAL_ERROR)
+    }
+
+    @Test
+    fun `when transaction is not active, does not create span around RestClient HTTP call`() {
+        val result = fixture.getSut(isTransactionActive = false).build()
+            .get()
+            .uri(fixture.url)
+            .retrieve()
+            .toEntity(String::class.java)
+
+        assertThat(result.body).isEqualTo("OK")
+        assertThat(fixture.transaction.spans).isEmpty()
+    }
+
+    @Test
+    fun `when transaction is not active, adds tracing headers from scope`() {
+        val sut = fixture.getSut(isTransactionActive = false)
+        val headers = HttpHeaders()
+        headers.add("baggage", "thirdPartyBaggage=someValue")
+        headers.add("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue")
+
+        sut.build()
+            .get()
+            .uri(fixture.url)
+            .httpRequest { it.headers.addAll(headers) }
+            .retrieve()
+            .toEntity(String::class.java)
+
+        val recorderRequest = fixture.mockServer.takeRequest()
+        assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
+        assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
+
+        val baggageHeaderValues = recorderRequest.headers.values(BaggageHeader.BAGGAGE_HEADER)
+        assertEquals(baggageHeaderValues.size, 1)
+        assertTrue(baggageHeaderValues[0].startsWith("thirdPartyBaggage=someValue,secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue"))
+        assertTrue(baggageHeaderValues[0].contains("sentry-public_key=key"))
+        assertTrue(baggageHeaderValues[0].contains("sentry-trace_id"))
+    }
+
+    @Test
+    fun `when transaction is active adds breadcrumb when http calls succeeds`() {
+        fixture.getSut(isTransactionActive = true)
+            .build()
+            .post()
+            .uri(fixture.url)
+            .body("content")
+            .retrieve()
+            .toEntity(String::class.java)
+        verify(fixture.hub).addBreadcrumb(
+            check<Breadcrumb> {
+                assertEquals("http", it.type)
+                assertEquals(fixture.url, it.data["url"])
+                assertEquals("POST", it.data["method"])
+                assertEquals(7, it.data["request_body_size"])
+            },
+            anyOrNull()
+        )
+    }
+
+    @SuppressWarnings("SwallowedException")
+    @Test
+    fun `when transaction is active adds breadcrumb when http calls results in exception`() {
+        try {
+            fixture.getSut(isTransactionActive = true, status = HttpStatus.INTERNAL_SERVER_ERROR).build()
+                .get()
+                .uri(fixture.url)
+                .retrieve()
+                .toEntity(String::class.java)
+        } catch (e: Throwable) {
+        }
+        verify(fixture.hub).addBreadcrumb(
+            check<Breadcrumb> {
+                assertEquals("http", it.type)
+                assertEquals(fixture.url, it.data["url"])
+                assertEquals("GET", it.data["method"])
+            },
+            anyOrNull()
+        )
+    }
+
+    @Test
+    fun `when transaction is not active adds breadcrumb when http calls succeeds`() {
+        fixture.getSut(isTransactionActive = false) .build()
+            .post()
+            .uri(fixture.url)
+            .body("content")
+            .retrieve()
+            .toEntity(String::class.java)
+        verify(fixture.hub).addBreadcrumb(
+            check<Breadcrumb> {
+                assertEquals("http", it.type)
+                assertEquals(fixture.url, it.data["url"])
+                assertEquals("POST", it.data["method"])
+                assertEquals(7, it.data["request_body_size"])
+            },
+            anyOrNull()
+        )
+    }
+
+    @SuppressWarnings("SwallowedException")
+    @Test
+    fun `when transaction is not active adds breadcrumb when http calls results in exception`() {
+        try {
+            fixture.getSut(isTransactionActive = false, status = HttpStatus.INTERNAL_SERVER_ERROR).build()
+                .get()
+                .uri(fixture.url)
+                .retrieve()
+                .toEntity(String::class.java)
+        } catch (e: Throwable) {
+        }
+        verify(fixture.hub).addBreadcrumb(
+            check<Breadcrumb> {
+                assertEquals("http", it.type)
+                assertEquals(fixture.url, it.data["url"])
+                assertEquals("GET", it.data["method"])
+            },
+            anyOrNull()
+        )
+    }
+}