From f86fb22bda2e02999f1080463daae3c463f57490 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 3 Aug 2023 10:14:42 +0200 Subject: [PATCH] Add HTTP response code to Spring WebFlux transactions (#2870) --- CHANGELOG.md | 1 + .../webflux/AbstractSentryWebFilter.java | 16 +++++++++----- .../webflux/SentryWebFluxTracingFilterTest.kt | 2 ++ .../spring/webflux/SentryWebFilter.java | 16 +++++++++----- .../webflux/SentryWebFluxTracingFilterTest.kt | 2 ++ sentry/api/sentry.api | 1 + .../java/io/sentry/protocol/Contexts.java | 21 ++++++++++++++++++- 7 files changed, 48 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d8fb477ce..0e8616a031 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- Add HTTP response code to Spring WebFlux transactions ([#2870](https://github.com/getsentry/sentry-java/pull/2870)) - Add `sampled` to Dynamic Sampling Context ([#2869](https://github.com/getsentry/sentry-java/pull/2869)) ### Fixes diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java index 8b93ade6cf..0b6b387e45 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java @@ -110,11 +110,17 @@ private void finishTransaction(ServerWebExchange exchange, ITransaction transact transaction.setName(transactionName, TransactionNameSource.ROUTE); transaction.setOperation(TRANSACTION_OP); } - if (transaction.getStatus() == null) { - final @Nullable ServerHttpResponse response = exchange.getResponse(); - if (response != null) { - final @Nullable HttpStatusCode statusCode = response.getStatusCode(); - if (statusCode != null) { + final @Nullable ServerHttpResponse response = exchange.getResponse(); + if (response != null) { + final @Nullable HttpStatusCode statusCode = response.getStatusCode(); + if (statusCode != null) { + transaction + .getContexts() + .withResponse( + (sentryResponse) -> { + sentryResponse.setStatusCode(statusCode.value()); + }); + if (transaction.getStatus() == null) { transaction.setStatus(SpanStatus.fromHttpStatusCode(statusCode.value())); } } diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt index ad20545473..eac0b9c60f 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt @@ -115,6 +115,7 @@ class SentryWebFluxTracingFilterTest { assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK) assertThat(it.contexts.trace!!.operation).isEqualTo("http.server") assertThat(it.contexts.trace!!.origin).isEqualTo("auto.spring_jakarta.webflux") + assertThat(it.contexts.response!!.statusCode).isEqualTo(200) }, anyOrNull(), anyOrNull(), @@ -133,6 +134,7 @@ class SentryWebFluxTracingFilterTest { verify(fixture.hub).captureTransaction( check { assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.INTERNAL_ERROR) + assertThat(it.contexts.response!!.statusCode).isEqualTo(500) }, anyOrNull(), anyOrNull(), diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java index b57bf2f55b..5dedc9dbba 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java @@ -144,11 +144,17 @@ private void finishTransaction(ServerWebExchange exchange, ITransaction transact transaction.setName(transactionName, TransactionNameSource.ROUTE); transaction.setOperation(TRANSACTION_OP); } - if (transaction.getStatus() == null) { - final @Nullable ServerHttpResponse response = exchange.getResponse(); - if (response != null) { - final @Nullable Integer rawStatusCode = response.getRawStatusCode(); - if (rawStatusCode != null) { + final @Nullable ServerHttpResponse response = exchange.getResponse(); + if (response != null) { + final @Nullable Integer rawStatusCode = response.getRawStatusCode(); + if (rawStatusCode != null) { + transaction + .getContexts() + .withResponse( + (sentryResponse) -> { + sentryResponse.setStatusCode(rawStatusCode); + }); + if (transaction.getStatus() == null) { transaction.setStatus(SpanStatus.fromHttpStatusCode(rawStatusCode)); } } diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt index 38658d644e..be56a8391d 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt @@ -116,6 +116,7 @@ class SentryWebFluxTracingFilterTest { assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK) assertThat(it.contexts.trace!!.operation).isEqualTo("http.server") assertThat(it.contexts.trace!!.origin).isEqualTo("auto.spring.webflux") + assertThat(it.contexts.response!!.statusCode).isEqualTo(200) }, anyOrNull(), anyOrNull(), @@ -134,6 +135,7 @@ class SentryWebFluxTracingFilterTest { verify(fixture.hub).captureTransaction( check { assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.INTERNAL_ERROR) + assertThat(it.contexts.response!!.statusCode).isEqualTo(500) }, anyOrNull(), anyOrNull(), diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index a7f02708e3..5e6caaf2fd 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -3009,6 +3009,7 @@ public final class io/sentry/protocol/Contexts : java/util/concurrent/Concurrent public fun setResponse (Lio/sentry/protocol/Response;)V public fun setRuntime (Lio/sentry/protocol/SentryRuntime;)V public fun setTrace (Lio/sentry/SpanContext;)V + public fun withResponse (Lio/sentry/util/HintUtils$SentryConsumer;)V } public final class io/sentry/protocol/Contexts$Deserializer : io/sentry/JsonDeserializer { diff --git a/sentry/src/main/java/io/sentry/protocol/Contexts.java b/sentry/src/main/java/io/sentry/protocol/Contexts.java index ce0c3219d0..21be9fd8a5 100644 --- a/sentry/src/main/java/io/sentry/protocol/Contexts.java +++ b/sentry/src/main/java/io/sentry/protocol/Contexts.java @@ -6,6 +6,7 @@ import io.sentry.JsonSerializable; import io.sentry.ObjectWriter; import io.sentry.SpanContext; +import io.sentry.util.HintUtils; import io.sentry.util.Objects; import io.sentry.vendor.gson.stream.JsonToken; import java.io.IOException; @@ -19,6 +20,9 @@ public final class Contexts extends ConcurrentHashMap implements JsonSerializable { private static final long serialVersionUID = 252445813254943011L; + /** Response lock, Ops should be atomic */ + private final @NotNull Object responseLock = new Object(); + public Contexts() {} public Contexts(final @NotNull Contexts contexts) { @@ -115,8 +119,23 @@ public void setGpu(final @NotNull Gpu gpu) { return toContextType(Response.TYPE, Response.class); } + public void withResponse(HintUtils.SentryConsumer callback) { + synchronized (responseLock) { + final @Nullable Response response = getResponse(); + if (response != null) { + callback.accept(response); + } else { + final @NotNull Response newResponse = new Response(); + setResponse(newResponse); + callback.accept(newResponse); + } + } + } + public void setResponse(final @NotNull Response response) { - this.put(Response.TYPE, response); + synchronized (responseLock) { + this.put(Response.TYPE, response); + } } // region json