From 07f4df2459d71acc07af9241e01b9433fc601a7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?N=C3=A1ndor=20Holozsny=C3=A1k?= Date: Thu, 8 Feb 2024 08:02:12 +0100 Subject: [PATCH 1/5] Support Spring's new RestClient with auto configuration (#3198) - Spring RestClient support --- .../boot/jakarta/SentryAutoConfiguration.java | 13 + .../SentrySpanRestClientCustomizer.java | 28 ++ .../jakarta/SentryAutoConfigurationTest.kt | 18 ++ .../SentrySpanRestClientCustomizerTest.kt | 288 ++++++++++++++++++ 4 files changed, 347 insertions(+) create mode 100644 sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java create mode 100644 sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt 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 20cbeeeeda..fbf3162280 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 @@ -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) diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java new file mode 100644 index 0000000000..b0189c037c --- /dev/null +++ b/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); + } + }); + } +} diff --git a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt index d469dbc281..4462514b01 100644 --- a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt +++ b/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") diff --git a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt new file mode 100644 index 0000000000..a27b94c35b --- /dev/null +++ b/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() + 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 { + 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 { + 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 { + 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 { + assertEquals("http", it.type) + assertEquals(fixture.url, it.data["url"]) + assertEquals("GET", it.data["method"]) + }, + anyOrNull() + ) + } +} From fbceb0a2c4850f6981849ab27bbd67eec1c14adb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?N=C3=A1ndor=20Holozsny=C3=A1k?= Date: Mon, 4 Mar 2024 11:45:35 +0100 Subject: [PATCH 2/5] Support Spring's new RestClient with auto configuration (#3198) - Test when HTTP call fails is fixed with disabling retry mechanism in the Apache Http Client --- .../SentrySpanRestClientCustomizerTest.kt | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt index a27b94c35b..cb4078da8a 100644 --- a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt +++ b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt @@ -4,12 +4,14 @@ import io.sentry.* import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy +import org.apache.hc.client5.http.impl.classic.HttpClients 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 java.time.Duration import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -32,7 +34,12 @@ class SentrySpanRestClientCustomizerTest { 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 { + fun getSut( + isTransactionActive: Boolean, + status: HttpStatus = HttpStatus.OK, + socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN, + includeMockServerInTracingOrigins: Boolean = true + ): RestClient.Builder { customizer.customize(restClientBuilder) if (includeMockServerInTracingOrigins) { @@ -55,9 +62,13 @@ class SentrySpanRestClientCustomizerTest { whenever(hub.span).thenReturn(transaction) } - return restClientBuilder.apply{ - val requestFactory = HttpComponentsClientHttpRequestFactory() - requestFactory.setConnectTimeout(2) + return restClientBuilder.apply { + val httpClient = HttpClients.custom() + .disableAutomaticRetries() // Required to not make another request automatically + .build() + val requestFactory = HttpComponentsClientHttpRequestFactory(httpClient) + requestFactory.setConnectTimeout(Duration.ofSeconds(2)) + requestFactory.setConnectionRequestTimeout(Duration.ofSeconds(2)) it.requestFactory(requestFactory) } } @@ -89,7 +100,10 @@ class SentrySpanRestClientCustomizerTest { val sut = fixture.getSut(isTransactionActive = true) val headers = HttpHeaders() headers.add("baggage", "thirdPartyBaggage=someValue") - headers.add("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue") + headers.add( + "baggage", + "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue" + ) sut.build() .get() @@ -158,7 +172,7 @@ class SentrySpanRestClientCustomizerTest { .get() .uri(fixture.url) .retrieve() - } catch (e: Throwable) { + } catch (_: Throwable) { } fixture.mockServer.takeRequest() assertThat(fixture.transaction.spans).hasSize(1) @@ -185,7 +199,10 @@ class SentrySpanRestClientCustomizerTest { val sut = fixture.getSut(isTransactionActive = false) val headers = HttpHeaders() headers.add("baggage", "thirdPartyBaggage=someValue") - headers.add("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue") + headers.add( + "baggage", + "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue" + ) sut.build() .get() @@ -248,7 +265,7 @@ class SentrySpanRestClientCustomizerTest { @Test fun `when transaction is not active adds breadcrumb when http calls succeeds`() { - fixture.getSut(isTransactionActive = false) .build() + fixture.getSut(isTransactionActive = false).build() .post() .uri(fixture.url) .body("content") From 4193d50188b686de4b2fa3ddfdb1d83496d540ca Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 20 Mar 2024 09:01:24 +0100 Subject: [PATCH 3/5] apply code formatter etc. --- .../SentrySpanRestClientCustomizer.java | 16 ++++++++------- .../SentrySpanRestClientCustomizerTest.kt | 20 +++++++++++++++++-- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java index b0189c037c..873b51c633 100644 --- a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java +++ b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java @@ -17,12 +17,14 @@ public SentrySpanRestClientCustomizer(final @NotNull IHub 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); - } - }); + 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); + } + }); } } diff --git a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt index cb4078da8a..f9f55ffede 100644 --- a/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt +++ b/sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt @@ -1,12 +1,28 @@ package io.sentry.spring.boot.jakarta -import io.sentry.* +import io.sentry.BaggageHeader +import io.sentry.Breadcrumb +import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback +import io.sentry.SentryOptions +import io.sentry.SentryTraceHeader +import io.sentry.SentryTracer +import io.sentry.SpanStatus +import io.sentry.TracesSamplingDecision +import io.sentry.TransactionContext import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy import org.apache.hc.client5.http.impl.classic.HttpClients import org.assertj.core.api.Assertions.assertThat -import org.mockito.kotlin.* +import org.mockito.Mockito.doAnswer +import org.mockito.Mockito.mock +import org.mockito.Mockito.verify +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.check +import org.mockito.kotlin.whenever import org.springframework.http.HttpHeaders import org.springframework.http.HttpStatus import org.springframework.http.client.HttpComponentsClientHttpRequestFactory From 3df266679c4be7252741585fcba1752d51fcd1ad Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 21 Mar 2024 14:45:02 +0100 Subject: [PATCH 4/5] Use separate trace origin; Add to Spring Boot 3 sample --- .../spring/boot/jakarta/SentryDemoApplication.java | 6 ++++++ .../spring/boot/jakarta/TodoController.java | 14 +++++++++++++- .../jakarta/SentrySpanRestClientCustomizer.java | 2 +- .../api/sentry-spring-jakarta.api | 1 + .../SentrySpanClientHttpRequestInterceptor.java | 12 ++++++++++-- 5 files changed, 31 insertions(+), 4 deletions(-) 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 74b1ac3f71..8050cb8e74 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 @@ -14,6 +14,7 @@ import org.springframework.scheduling.quartz.CronTriggerFactoryBean; import org.springframework.scheduling.quartz.JobDetailFactoryBean; import org.springframework.scheduling.quartz.SimpleTriggerFactoryBean; +import org.springframework.web.client.RestClient; import org.springframework.web.client.RestTemplate; import org.springframework.web.reactive.function.client.WebClient; @@ -34,6 +35,11 @@ WebClient webClient(WebClient.Builder builder) { return builder.build(); } + @Bean + RestClient restClient(RestClient.Builder builder) { + return 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 4043cac8d3..88a6b11d4a 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 @@ -4,6 +4,7 @@ import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.client.RestClient; import org.springframework.web.client.RestTemplate; import org.springframework.web.reactive.function.client.WebClient; import reactor.core.publisher.Hooks; @@ -14,10 +15,12 @@ public class TodoController { private final RestTemplate restTemplate; private final WebClient webClient; + private final RestClient restClient; - public TodoController(RestTemplate restTemplate, WebClient webClient) { + public TodoController(RestTemplate restTemplate, WebClient webClient, RestClient restClient) { this.restTemplate = restTemplate; this.webClient = webClient; + this.restClient = restClient; } @GetMapping("/todo/{id}") @@ -42,4 +45,13 @@ Todo todoWebClient(@PathVariable Long id) { .map(response -> response))) .block(); } + + @GetMapping("/todo-restclient/{id}") + Todo todoRestClient(@PathVariable Long id) { + return restClient + .get() + .uri("https://jsonplaceholder.typicode.com/todos/{id}", id) + .retrieve() + .body(Todo.class); + } } diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java index 873b51c633..243a4d1f50 100644 --- a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java +++ b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizer.java @@ -12,7 +12,7 @@ class SentrySpanRestClientCustomizer implements RestClientCustomizer { private final @NotNull SentrySpanClientHttpRequestInterceptor interceptor; public SentrySpanRestClientCustomizer(final @NotNull IHub hub) { - this.interceptor = new SentrySpanClientHttpRequestInterceptor(hub); + this.interceptor = new SentrySpanClientHttpRequestInterceptor(hub, false); } @Override diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api index 536c9e9af8..cebbd26b4f 100644 --- a/sentry-spring-jakarta/api/sentry-spring-jakarta.api +++ b/sentry-spring-jakarta/api/sentry-spring-jakarta.api @@ -213,6 +213,7 @@ public class io/sentry/spring/jakarta/tracing/SentrySpanAdvice : org/aopalliance public class io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor : org/springframework/http/client/ClientHttpRequestInterceptor { public fun (Lio/sentry/IHub;)V + public fun (Lio/sentry/IHub;Z)V public fun intercept (Lorg/springframework/http/HttpRequest;[BLorg/springframework/http/client/ClientHttpRequestExecution;)Lorg/springframework/http/client/ClientHttpResponse; } diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java index d190a0427d..59c1926ff3 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -26,11 +26,19 @@ @Open public class SentrySpanClientHttpRequestInterceptor implements ClientHttpRequestInterceptor { - private static final String TRACE_ORIGIN = "auto.http.spring_jakarta.resttemplate"; + private static final String TRACE_ORIGIN_REST_TEMPLATE = "auto.http.spring_jakarta.resttemplate"; + private static final String TRACE_ORIGIN_REST_CLIENT = "auto.http.spring_jakarta.restclient"; private final @NotNull IHub hub; + private final @NotNull String traceOrigin; public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { + this(hub, true); + } + + public SentrySpanClientHttpRequestInterceptor( + final @NotNull IHub hub, final @NotNull boolean isRestTemplate) { this.hub = Objects.requireNonNull(hub, "hub is required"); + this.traceOrigin = isRestTemplate ? TRACE_ORIGIN_REST_TEMPLATE : TRACE_ORIGIN_REST_CLIENT; } @Override @@ -49,7 +57,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { } final ISpan span = activeSpan.startChild("http.client"); - span.getSpanContext().setOrigin(TRACE_ORIGIN); + span.getSpanContext().setOrigin(traceOrigin); final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(request.getURI().toString()); From 2a8c28049b5ce3740190de420c01046cc2f5ff84 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 21 Mar 2024 14:46:56 +0100 Subject: [PATCH 5/5] changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d081ca5826..3b578ed17f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Features + +- Add support for Spring Rest Client ([#3199](https://github.com/getsentry/sentry-java/pull/3199)) + ## 7.6.0 ### Features