From 936dca635b1a4e1073bd733697d098b14d1b1616 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 13 Mar 2025 14:03:49 +0100 Subject: [PATCH 1/6] Assume http.client for span op if not a root span --- .../SpanDescriptionExtractor.java | 37 +- .../kotlin/SpanDescriptionExtractorTest.kt | 344 ++++++++++++++++++ 2 files changed, 362 insertions(+), 19 deletions(-) create mode 100644 sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SpanDescriptionExtractorTest.kt diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SpanDescriptionExtractor.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SpanDescriptionExtractor.java index 2047bd37f8..b66555d68c 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SpanDescriptionExtractor.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SpanDescriptionExtractor.java @@ -18,23 +18,16 @@ public final class SpanDescriptionExtractor { @SuppressWarnings("deprecation") public @NotNull OtelSpanInfo extractSpanInfo( final @NotNull SpanData otelSpan, final @Nullable IOtelSpanWrapper sentrySpan) { - if (!isInternalSpanKind(otelSpan)) { - final @NotNull Attributes attributes = otelSpan.getAttributes(); - - final @Nullable String httpMethod = attributes.get(HttpAttributes.HTTP_REQUEST_METHOD); - if (httpMethod != null) { - return descriptionForHttpMethod(otelSpan, httpMethod); - } + final @NotNull Attributes attributes = otelSpan.getAttributes(); - final @Nullable String httpRequestMethod = attributes.get(HttpAttributes.HTTP_REQUEST_METHOD); - if (httpRequestMethod != null) { - return descriptionForHttpMethod(otelSpan, httpRequestMethod); - } + final @Nullable String httpMethod = attributes.get(HttpAttributes.HTTP_REQUEST_METHOD); + if (httpMethod != null) { + return descriptionForHttpMethod(otelSpan, httpMethod); + } - final @Nullable String dbSystem = attributes.get(DbIncubatingAttributes.DB_SYSTEM); - if (dbSystem != null) { - return descriptionForDbSystem(otelSpan); - } + final @Nullable String dbSystem = attributes.get(DbIncubatingAttributes.DB_SYSTEM); + if (dbSystem != null) { + return descriptionForDbSystem(otelSpan); } final @NotNull String name = otelSpan.getName(); @@ -44,10 +37,6 @@ public final class SpanDescriptionExtractor { return new OtelSpanInfo(name, description, TransactionNameSource.CUSTOM); } - private boolean isInternalSpanKind(final @NotNull SpanData otelSpan) { - return SpanKind.INTERNAL.equals(otelSpan.getKind()); - } - @SuppressWarnings("deprecation") private OtelSpanInfo descriptionForHttpMethod( final @NotNull SpanData otelSpan, final @NotNull String httpMethod) { @@ -60,6 +49,12 @@ private OtelSpanInfo descriptionForHttpMethod( opBuilder.append(".client"); } else if (SpanKind.SERVER.equals(kind)) { opBuilder.append(".server"); + } else { + // we cannot be certain that a root span is a server span as it might simply be a client span + // without parent + if (!isRootSpan(otelSpan)) { + opBuilder.append(".client"); + } } final @Nullable String httpTarget = attributes.get(HttpIncubatingAttributes.HTTP_TARGET); final @Nullable String httpRoute = attributes.get(HttpAttributes.HTTP_ROUTE); @@ -92,6 +87,10 @@ private OtelSpanInfo descriptionForHttpMethod( return new OtelSpanInfo(op, description, transactionNameSource); } + private static boolean isRootSpan(SpanData otelSpan) { + return !otelSpan.getParentSpanContext().isValid() || otelSpan.getParentSpanContext().isRemote(); + } + @SuppressWarnings("deprecation") private OtelSpanInfo descriptionForDbSystem(final @NotNull SpanData otelSpan) { final @NotNull Attributes attributes = otelSpan.getAttributes(); diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SpanDescriptionExtractorTest.kt b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SpanDescriptionExtractorTest.kt new file mode 100644 index 0000000000..6d33a7df7c --- /dev/null +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SpanDescriptionExtractorTest.kt @@ -0,0 +1,344 @@ +package io.sentry.opentelemetry + +import io.opentelemetry.api.common.AttributeKey +import io.opentelemetry.api.trace.SpanContext +import io.opentelemetry.api.trace.SpanKind +import io.opentelemetry.api.trace.TraceFlags +import io.opentelemetry.api.trace.TraceState +import io.opentelemetry.sdk.internal.AttributesMap +import io.opentelemetry.sdk.trace.data.SpanData +import io.opentelemetry.semconv.HttpAttributes +import io.opentelemetry.semconv.UrlAttributes +import io.opentelemetry.semconv.incubating.DbIncubatingAttributes +import io.opentelemetry.semconv.incubating.HttpIncubatingAttributes +import io.sentry.protocol.TransactionNameSource +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class SpanDescriptionExtractorTest { + + private class Fixture { + val sentrySpan = mock() + val otelSpan = mock() + val attributes = AttributesMap.create(100, 100) + var parentSpanContext = SpanContext.getInvalid() + var spanKind = SpanKind.INTERNAL + var spanName: String? = null + var spanDescription: String? = null + + fun setup() { + whenever(otelSpan.attributes).thenReturn(attributes) + whenever(otelSpan.parentSpanContext).thenReturn(parentSpanContext) + whenever(otelSpan.kind).thenReturn(spanKind) + spanName?.let { + whenever(otelSpan.name).thenReturn(it) + } + spanDescription?.let { + whenever(sentrySpan.description).thenReturn(it) + } + } + } + + private val fixture = Fixture() + + @Test + fun `sets op to http server for kind SERVER`() { + givenSpanKind(SpanKind.SERVER) + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("http.server", info.op) + assertNull(info.description) + assertEquals(TransactionNameSource.CUSTOM, info.transactionNameSource) + } + + @Test + fun `sets op to http client for kind CLIENT`() { + givenSpanKind(SpanKind.CLIENT) + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("http.client", info.op) + assertNull(info.description) + assertEquals(TransactionNameSource.CUSTOM, info.transactionNameSource) + } + + @Test + fun `sets op to http without server for root span with http GET`() { + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("http", info.op) + assertNull(info.description) + assertEquals(TransactionNameSource.CUSTOM, info.transactionNameSource) + } + + @Test + fun `sets op to http without server for non root span with remote parent with http GET`() { + givenParentContext(createSpanContext(true)) + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("http", info.op) + assertNull(info.description) + assertEquals(TransactionNameSource.CUSTOM, info.transactionNameSource) + } + + @Test + fun `sets op to http client for non root span with http GET`() { + givenParentContext(createSpanContext(false)) + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("http.client", info.op) + assertNull(info.description) + assertEquals(TransactionNameSource.CUSTOM, info.transactionNameSource) + } + + @Test + fun `uses URL_FULL for description`() { + givenSpanKind(SpanKind.SERVER) + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", + UrlAttributes.URL_FULL to "https://sentry.io/some/path?q=1#top" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("http.server", info.op) + assertEquals("GET https://sentry.io/some/path?q=1#top", info.description) + assertEquals(TransactionNameSource.URL, info.transactionNameSource) + } + + @Test + fun `uses URL_PATH for description`() { + givenSpanKind(SpanKind.SERVER) + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", + UrlAttributes.URL_PATH to "/some/path" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("http.server", info.op) + assertEquals("GET /some/path", info.description) + assertEquals(TransactionNameSource.URL, info.transactionNameSource) + } + + @Test + fun `uses HTTP_TARGET for description`() { + givenSpanKind(SpanKind.SERVER) + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", + HttpAttributes.HTTP_ROUTE to "/some/{id}", + HttpIncubatingAttributes.HTTP_TARGET to "some/path?q=1#top", + UrlAttributes.URL_PATH to "/some/path" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("http.server", info.op) + assertEquals("GET /some/{id}", info.description) + assertEquals(TransactionNameSource.ROUTE, info.transactionNameSource) + } + + @Test + fun `uses span name as description fallback`() { + givenSpanKind(SpanKind.SERVER) + givenSpanName("span name") + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("http.server", info.op) + assertEquals("span name", info.description) + assertEquals(TransactionNameSource.CUSTOM, info.transactionNameSource) + } + + @Test + fun `no description if no span name as fallback`() { + givenSpanKind(SpanKind.SERVER) + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("http.server", info.op) + assertNull(info.description) + assertEquals(TransactionNameSource.CUSTOM, info.transactionNameSource) + } + + @Test + fun `sets op to db for span with db system and query text`() { + givenAttributes( + mapOf( + DbIncubatingAttributes.DB_SYSTEM to "some", + DbIncubatingAttributes.DB_QUERY_TEXT to "SELECT * FROM tbl" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("db", info.op) + assertEquals("SELECT * FROM tbl", info.description) + assertEquals(TransactionNameSource.TASK, info.transactionNameSource) + } + + @Test + fun `sets op to db for span with db system and statement`() { + givenAttributes( + mapOf( + DbIncubatingAttributes.DB_SYSTEM to "some", + DbIncubatingAttributes.DB_STATEMENT to "SELECT * FROM tbl" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("db", info.op) + assertEquals("SELECT * FROM tbl", info.description) + assertEquals(TransactionNameSource.TASK, info.transactionNameSource) + } + + @Test + fun `sets op to db for span with db system`() { + givenAttributes( + mapOf( + DbIncubatingAttributes.DB_SYSTEM to "some" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("db", info.op) + assertNull(info.description) + assertEquals(TransactionNameSource.TASK, info.transactionNameSource) + } + + @Test + fun `sets op to db for span with db system fallback to span name as description`() { + givenSpanName("span name") + givenAttributes( + mapOf( + DbIncubatingAttributes.DB_SYSTEM to "some" + ) + ) + + val info = whenExtractingSpanInfo() + + assertEquals("db", info.op) + assertEquals("span name", info.description) + assertEquals(TransactionNameSource.TASK, info.transactionNameSource) + } + + @Test + fun `uses span name as op and description if no relevant attributes`() { + givenSpanName("span name") + givenAttributes(emptyMap()) + + val info = whenExtractingSpanInfo() + + assertEquals("span name", info.op) + assertEquals("span name", info.description) + assertEquals(TransactionNameSource.CUSTOM, info.transactionNameSource) + } + + @Test + fun `uses existing sentry span description as description`() { + givenSpanName("span name") + givenSentrySpanDescription("span description") + givenAttributes(emptyMap()) + + val info = whenExtractingSpanInfo() + + assertEquals("span name", info.op) + assertEquals("span description", info.description) + assertEquals(TransactionNameSource.CUSTOM, info.transactionNameSource) + } + + private fun createSpanContext(isRemote: Boolean, traceId: String = "f9118105af4a2d42b4124532cd1065ff", spanId: String = "424cffc8f94feeee"): SpanContext { + if (isRemote) { + return SpanContext.createFromRemoteParent( + traceId, + spanId, + TraceFlags.getSampled(), + TraceState.getDefault() + ) + } else { + return SpanContext.create( + traceId, + spanId, + TraceFlags.getSampled(), + TraceState.getDefault() + ) + } + } + + private fun givenAttributes(map: Map, Any>) { + map.forEach { k, v -> + fixture.attributes.put(k, v) + } + } + + private fun whenExtractingSpanInfo(): OtelSpanInfo { + fixture.setup() + return SpanDescriptionExtractor().extractSpanInfo(fixture.otelSpan, fixture.sentrySpan) + } + + private fun givenParentContext(parentContext: SpanContext) { + fixture.parentSpanContext = parentContext + } + + private fun givenSpanName(name: String) { + fixture.spanName = name + } + + private fun givenSentrySpanDescription(description: String) { + fixture.spanDescription = description + } + + private fun givenSpanKind(spanKind: SpanKind) { + fixture.spanKind = spanKind + } +} From 7435268a4136dcafda919dd8d635a74d5926e84f Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 13 Mar 2025 14:11:46 +0100 Subject: [PATCH 2/6] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3e26188c6..d78021f995 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - Pass OpenTelemetry span attributes into TracesSampler callback ([#4253](https://github.com/getsentry/sentry-java/pull/4253)) - `SamplingContext` now has a `getAttribute` method that grants access to OpenTelemetry span attributes via their String key (e.g. `http.request.method`) - Fix AbstractMethodError when using SentryTraced for Jetpack Compose ([#4255](https://github.com/getsentry/sentry-java/pull/4255)) +- Assume `http.client` for span `op` if not a root span ([#4257](https://github.com/getsentry/sentry-java/pull/4257)) ### Features From 2beca0a3dab56be77d8cb1b105bdd46e711bcc04 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 17 Mar 2025 14:30:11 +0100 Subject: [PATCH 3/6] Use SpringServletTransactionNameProvider as fallback --- .../boot/jakarta/SentryAutoConfiguration.java | 7 ++- .../spring/boot/SentryAutoConfiguration.java | 7 ++- .../api/sentry-spring-jakarta.api | 14 +++++ .../CombinedTransactionNameProvider.java | 58 ++++++++++++++++++ .../jakarta/tracing/SentryTracingFilter.java | 8 ++- .../tracing/TransactionNameProvider.java | 8 +++ .../tracing/TransactionNameWithSource.java | 27 ++++++++ .../tracing/SentryTracingFilterTest.kt | 6 ++ sentry-spring/api/sentry-spring.api | 14 +++++ .../CombinedTransactionNameProvider.java | 61 +++++++++++++++++++ .../spring/tracing/SentryTracingFilter.java | 8 ++- .../tracing/TransactionNameProvider.java | 8 +++ .../tracing/TransactionNameWithSource.java | 27 ++++++++ .../spring/tracing/SentryTracingFilterTest.kt | 1 + 14 files changed, 246 insertions(+), 8 deletions(-) create mode 100644 sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java create mode 100644 sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/TransactionNameWithSource.java create mode 100644 sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java create mode 100644 sentry-spring/src/main/java/io/sentry/spring/tracing/TransactionNameWithSource.java 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 4ed9acf1c0..9dc865ade7 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 @@ -32,6 +32,7 @@ import io.sentry.spring.jakarta.exception.SentryExceptionParameterAdviceConfiguration; import io.sentry.spring.jakarta.opentelemetry.SentryOpenTelemetryAgentWithoutAutoInitConfiguration; import io.sentry.spring.jakarta.opentelemetry.SentryOpenTelemetryNoAgentConfiguration; +import io.sentry.spring.jakarta.tracing.CombinedTransactionNameProvider; import io.sentry.spring.jakarta.tracing.SentryAdviceConfiguration; import io.sentry.spring.jakarta.tracing.SentrySpanPointcutConfiguration; import io.sentry.spring.jakarta.tracing.SentryTracingFilter; @@ -42,6 +43,7 @@ import io.sentry.transport.ITransportGate; import io.sentry.transport.apache.ApacheHttpClientTransportFactory; import jakarta.servlet.http.HttpServletRequest; +import java.util.Arrays; import java.util.List; import java.util.Optional; import org.aspectj.lang.ProceedingJoinPoint; @@ -342,7 +344,10 @@ static class SentryMvcModeConfig { @Bean @ConditionalOnMissingBean(TransactionNameProvider.class) public @NotNull TransactionNameProvider transactionNameProvider() { - return new SpringMvcTransactionNameProvider(); + return new CombinedTransactionNameProvider( + Arrays.asList( + new SpringMvcTransactionNameProvider(), + new SpringServletTransactionNameProvider())); } } 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 98cdc46487..cf55d5b1be 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 @@ -32,6 +32,7 @@ import io.sentry.spring.exception.SentryExceptionParameterAdviceConfiguration; import io.sentry.spring.opentelemetry.SentryOpenTelemetryAgentWithoutAutoInitConfiguration; import io.sentry.spring.opentelemetry.SentryOpenTelemetryNoAgentConfiguration; +import io.sentry.spring.tracing.CombinedTransactionNameProvider; import io.sentry.spring.tracing.SentryAdviceConfiguration; import io.sentry.spring.tracing.SentrySpanPointcutConfiguration; import io.sentry.spring.tracing.SentryTracingFilter; @@ -41,6 +42,7 @@ import io.sentry.spring.tracing.TransactionNameProvider; import io.sentry.transport.ITransportGate; import io.sentry.transport.apache.ApacheHttpClientTransportFactory; +import java.util.Arrays; import java.util.List; import java.util.Optional; import javax.servlet.http.HttpServletRequest; @@ -327,7 +329,10 @@ static class SentryMvcModeConfig { @Bean @ConditionalOnMissingBean(TransactionNameProvider.class) public @NotNull TransactionNameProvider transactionNameProvider() { - return new SpringMvcTransactionNameProvider(); + return new CombinedTransactionNameProvider( + Arrays.asList( + new SpringMvcTransactionNameProvider(), + new SpringServletTransactionNameProvider())); } } diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api index 4140cd0a59..3c1db200cb 100644 --- a/sentry-spring-jakarta/api/sentry-spring-jakarta.api +++ b/sentry-spring-jakarta/api/sentry-spring-jakarta.api @@ -220,6 +220,13 @@ public class io/sentry/spring/jakarta/opentelemetry/SentryOpenTelemetryNoAgentCo public fun sentryOpenTelemetryOptionsConfiguration ()Lio/sentry/Sentry$OptionsConfiguration; } +public final class io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider : io/sentry/spring/jakarta/tracing/TransactionNameProvider { + public fun (Ljava/util/List;)V + public fun provideTransactionName (Ljakarta/servlet/http/HttpServletRequest;)Ljava/lang/String; + public fun provideTransactionNameAndSource (Ljakarta/servlet/http/HttpServletRequest;)Lio/sentry/spring/jakarta/tracing/TransactionNameWithSource; + public fun provideTransactionSource ()Lio/sentry/protocol/TransactionNameSource; +} + public class io/sentry/spring/jakarta/tracing/SentryAdviceConfiguration { public fun ()V public fun sentrySpanAdvice ()Lorg/aopalliance/aop/Advice; @@ -300,9 +307,16 @@ public final class io/sentry/spring/jakarta/tracing/SpringServletTransactionName public abstract interface class io/sentry/spring/jakarta/tracing/TransactionNameProvider { public abstract fun provideTransactionName (Ljakarta/servlet/http/HttpServletRequest;)Ljava/lang/String; + public fun provideTransactionNameAndSource (Ljakarta/servlet/http/HttpServletRequest;)Lio/sentry/spring/jakarta/tracing/TransactionNameWithSource; public fun provideTransactionSource ()Lio/sentry/protocol/TransactionNameSource; } +public final class io/sentry/spring/jakarta/tracing/TransactionNameWithSource { + public fun (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;)V + public fun getTransactionName ()Ljava/lang/String; + public fun getTransactionNameSource ()Lio/sentry/protocol/TransactionNameSource; +} + public abstract class io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter : org/springframework/web/server/WebFilter { public static final field SENTRY_HUB_KEY Ljava/lang/String; public static final field SENTRY_SCOPES_KEY Ljava/lang/String; diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java new file mode 100644 index 0000000000..daa751c768 --- /dev/null +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java @@ -0,0 +1,58 @@ +package io.sentry.spring.jakarta.tracing; + +import io.sentry.protocol.TransactionNameSource; +import jakarta.servlet.http.HttpServletRequest; +import java.util.List; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Resolves transaction name using {@link HttpServletRequest#getMethod()} and templated route that + * handled the request. To return correct transaction name, it must be used after request is + * processed by {@link org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping} + * where {@link org.springframework.web.servlet.HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE} is + * set. + */ +@ApiStatus.Internal +public final class CombinedTransactionNameProvider implements TransactionNameProvider { + + private final @NotNull List providers; + + public CombinedTransactionNameProvider(final @NotNull List providers) { + this.providers = providers; + } + + @Override + public @Nullable String provideTransactionName(@NotNull HttpServletRequest request) { + for (TransactionNameProvider provider : providers) { + String transactionName = provider.provideTransactionName(request); + if (transactionName != null) { + return transactionName; + } + } + + return null; + } + + @Override + @ApiStatus.Internal + public @NotNull TransactionNameSource provideTransactionSource() { + return TransactionNameSource.CUSTOM; + } + + @ApiStatus.Internal + @Override + public @NotNull TransactionNameWithSource provideTransactionNameAndSource( + @NotNull HttpServletRequest request) { + for (TransactionNameProvider provider : providers) { + String transactionName = provider.provideTransactionName(request); + if (transactionName != null) { + final @NotNull TransactionNameSource source = provider.provideTransactionSource(); + return new TransactionNameWithSource(transactionName, source); + } + } + + return new TransactionNameWithSource(null, TransactionNameSource.CUSTOM); + } +} diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java index f740ce4e61..d059338c0d 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java @@ -145,9 +145,11 @@ private void doFilterWithTransaction( } finally { if (shouldFinishTransaction(httpRequest) && transaction != null) { // after all filters run, templated path pattern is available in request attribute - final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); - final TransactionNameSource transactionNameSource = - transactionNameProvider.provideTransactionSource(); + final @NotNull TransactionNameWithSource transactionNameWithSource = + transactionNameProvider.provideTransactionNameAndSource(httpRequest); + final @Nullable String transactionName = transactionNameWithSource.getTransactionName(); + final @NotNull TransactionNameSource transactionNameSource = + transactionNameWithSource.getTransactionNameSource(); // if transaction name is not resolved, the request has not been processed by a controller // and we should not report it to Sentry if (transactionName != null) { diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/TransactionNameProvider.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/TransactionNameProvider.java index ccf59f7ded..7c0eac8152 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/TransactionNameProvider.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/TransactionNameProvider.java @@ -27,4 +27,12 @@ public interface TransactionNameProvider { default TransactionNameSource provideTransactionSource() { return TransactionNameSource.CUSTOM; } + + @NotNull + @ApiStatus.Internal + default TransactionNameWithSource provideTransactionNameAndSource( + final @NotNull HttpServletRequest request) { + return new TransactionNameWithSource( + provideTransactionName(request), provideTransactionSource()); + } } diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/TransactionNameWithSource.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/TransactionNameWithSource.java new file mode 100644 index 0000000000..f913b895cf --- /dev/null +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/TransactionNameWithSource.java @@ -0,0 +1,27 @@ +package io.sentry.spring.jakarta.tracing; + +import io.sentry.protocol.TransactionNameSource; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class TransactionNameWithSource { + private final @Nullable String transactionName; + private final @NotNull TransactionNameSource transactionNameSource; + + public TransactionNameWithSource( + final @Nullable String transactionName, + final @NotNull TransactionNameSource transactionNameSource) { + this.transactionName = transactionName; + this.transactionNameSource = transactionNameSource; + } + + public @Nullable String getTransactionName() { + return transactionName; + } + + public @NotNull TransactionNameSource getTransactionNameSource() { + return transactionNameSource; + } +} diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt index 14bf096903..31188a2017 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt @@ -65,6 +65,12 @@ class SentryTracingFilterTest { request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}") whenever(transactionNameProvider.provideTransactionName(request)).thenReturn("POST /product/{id}") whenever(transactionNameProvider.provideTransactionSource()).thenReturn(TransactionNameSource.CUSTOM) + whenever(transactionNameProvider.provideTransactionNameAndSource(request)).thenReturn( + TransactionNameWithSource( + "POST /product/{id}", + TransactionNameSource.CUSTOM + ) + ) if (sentryTraceHeader != null) { request.addHeader("sentry-trace", sentryTraceHeader) whenever(scopes.startTransaction(any(), check { it.isBindToScope })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, scopes) } diff --git a/sentry-spring/api/sentry-spring.api b/sentry-spring/api/sentry-spring.api index 4743c32284..467d96beec 100644 --- a/sentry-spring/api/sentry-spring.api +++ b/sentry-spring/api/sentry-spring.api @@ -212,6 +212,13 @@ public class io/sentry/spring/opentelemetry/SentryOpenTelemetryNoAgentConfigurat public fun sentryOpenTelemetryOptionsConfiguration ()Lio/sentry/Sentry$OptionsConfiguration; } +public final class io/sentry/spring/tracing/CombinedTransactionNameProvider : io/sentry/spring/tracing/TransactionNameProvider { + public fun (Ljava/util/List;)V + public fun provideTransactionName (Ljavax/servlet/http/HttpServletRequest;)Ljava/lang/String; + public fun provideTransactionNameAndSource (Ljavax/servlet/http/HttpServletRequest;)Lio/sentry/spring/tracing/TransactionNameWithSource; + public fun provideTransactionSource ()Lio/sentry/protocol/TransactionNameSource; +} + public class io/sentry/spring/tracing/SentryAdviceConfiguration { public fun ()V public fun sentrySpanAdvice ()Lorg/aopalliance/aop/Advice; @@ -291,9 +298,16 @@ public final class io/sentry/spring/tracing/SpringServletTransactionNameProvider public abstract interface class io/sentry/spring/tracing/TransactionNameProvider { public abstract fun provideTransactionName (Ljavax/servlet/http/HttpServletRequest;)Ljava/lang/String; + public fun provideTransactionNameAndSource (Ljavax/servlet/http/HttpServletRequest;)Lio/sentry/spring/tracing/TransactionNameWithSource; public fun provideTransactionSource ()Lio/sentry/protocol/TransactionNameSource; } +public final class io/sentry/spring/tracing/TransactionNameWithSource { + public fun (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;)V + public fun getTransactionName ()Ljava/lang/String; + public fun getTransactionNameSource ()Lio/sentry/protocol/TransactionNameSource; +} + public class io/sentry/spring/webflux/SentryRequestResolver { public fun (Lio/sentry/IScopes;)V public fun resolveSentryRequest (Lorg/springframework/http/server/reactive/ServerHttpRequest;)Lio/sentry/protocol/Request; diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java new file mode 100644 index 0000000000..a3150609da --- /dev/null +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java @@ -0,0 +1,61 @@ +package io.sentry.spring.tracing; + +import io.sentry.protocol.TransactionNameSource; +import java.util.List; +import javax.servlet.http.HttpServletRequest; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Resolves transaction name using {@link HttpServletRequest#getMethod()} and templated route that + * handled the request. To return correct transaction name, it must be used after request is + * processed by {@link org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping} + * where {@link org.springframework.web.servlet.HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE} is + * set. + */ +@ApiStatus.Internal +public final class CombinedTransactionNameProvider implements TransactionNameProvider { + + private final @NotNull List providers; + private @Nullable TransactionNameSource source = null; + + public CombinedTransactionNameProvider(final @NotNull List providers) { + this.providers = providers; + } + + @Override + public @Nullable String provideTransactionName(final @NotNull HttpServletRequest request) { + for (TransactionNameProvider provider : providers) { + String transactionName = provider.provideTransactionName(request); + if (transactionName != null) { + source = provider.provideTransactionSource(); + return transactionName; + } + } + + return null; + } + + @Override + @ApiStatus.Internal + public @NotNull TransactionNameSource provideTransactionSource() { + final @Nullable TransactionNameSource tmpSource = source; + return tmpSource == null ? TransactionNameSource.CUSTOM : tmpSource; + } + + @ApiStatus.Internal + @Override + public @NotNull TransactionNameWithSource provideTransactionNameAndSource( + @NotNull HttpServletRequest request) { + for (TransactionNameProvider provider : providers) { + String transactionName = provider.provideTransactionName(request); + if (transactionName != null) { + final @NotNull TransactionNameSource source = provider.provideTransactionSource(); + return new TransactionNameWithSource(transactionName, source); + } + } + + return new TransactionNameWithSource(null, TransactionNameSource.CUSTOM); + } +} diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java index 6c83eb2fb4..d720e4429d 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java @@ -144,9 +144,11 @@ private void doFilterWithTransaction( } finally { if (shouldFinishTransaction(httpRequest) && transaction != null) { // after all filters run, templated path pattern is available in request attribute - final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); - final TransactionNameSource transactionNameSource = - transactionNameProvider.provideTransactionSource(); + final @NotNull TransactionNameWithSource transactionNameWithSource = + transactionNameProvider.provideTransactionNameAndSource(httpRequest); + final @Nullable String transactionName = transactionNameWithSource.getTransactionName(); + final @NotNull TransactionNameSource transactionNameSource = + transactionNameWithSource.getTransactionNameSource(); // if transaction name is not resolved, the request has not been processed by a controller // and we should not report it to Sentry if (transactionName != null) { diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/TransactionNameProvider.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/TransactionNameProvider.java index b8e57c2267..9371cc601e 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/TransactionNameProvider.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/TransactionNameProvider.java @@ -27,4 +27,12 @@ public interface TransactionNameProvider { default TransactionNameSource provideTransactionSource() { return TransactionNameSource.CUSTOM; } + + @NotNull + @ApiStatus.Internal + default TransactionNameWithSource provideTransactionNameAndSource( + final @NotNull HttpServletRequest request) { + return new TransactionNameWithSource( + provideTransactionName(request), provideTransactionSource()); + } } diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/TransactionNameWithSource.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/TransactionNameWithSource.java new file mode 100644 index 0000000000..1b786b6a0e --- /dev/null +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/TransactionNameWithSource.java @@ -0,0 +1,27 @@ +package io.sentry.spring.tracing; + +import io.sentry.protocol.TransactionNameSource; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class TransactionNameWithSource { + private final @Nullable String transactionName; + private final @NotNull TransactionNameSource transactionNameSource; + + public TransactionNameWithSource( + final @Nullable String transactionName, + final @NotNull TransactionNameSource transactionNameSource) { + this.transactionName = transactionName; + this.transactionNameSource = transactionNameSource; + } + + public @Nullable String getTransactionName() { + return transactionName; + } + + public @NotNull TransactionNameSource getTransactionNameSource() { + return transactionNameSource; + } +} diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt index 9270d9b0c8..2377f54f42 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt @@ -65,6 +65,7 @@ class SentryTracingFilterTest { request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}") whenever(transactionNameProvider.provideTransactionName(request)).thenReturn("POST /product/{id}") whenever(transactionNameProvider.provideTransactionSource()).thenReturn(TransactionNameSource.CUSTOM) + whenever(transactionNameProvider.provideTransactionNameAndSource(request)).thenReturn(TransactionNameWithSource("POST /product/{id}", TransactionNameSource.CUSTOM)) if (sentryTraceHeader != null) { request.addHeader("sentry-trace", sentryTraceHeader) whenever(scopes.startTransaction(any(), check { it.isBindToScope })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, scopes) } From 0d27f8b2bdfc6759f932c83bb2d5830d640d4ac8 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 17 Mar 2025 14:44:20 +0100 Subject: [PATCH 4/6] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e81bd10e0..7df5afff11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Reduce excessive CPU usage when serializing breadcrumbs to disk for ANRs ([#4181](https://github.com/getsentry/sentry-java/pull/4181)) - Ensure app start type is set, even when ActivityLifecycleIntegration is not running ([#4250](https://github.com/getsentry/sentry-java/pull/4250)) +- Use `SpringServletTransactionNameProvider` as fallback for Spring WebMVC ([#4263](https://github.com/getsentry/sentry-java/pull/4263)) + - In certain cases the SDK was not able to provide a transaction name automatically and thus did not finish the transaction for the request. + - We now first try `SpringMvcTransactionNameProvider` which would provide the route as transaction name. + - If that does not return anything, we try `SpringServletTransactionNameProvider` next, which returns the URL of the request. ### Behavioral Changes From c771cbe03096c3f2bad50e3129a072e0115c9d27 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 18 Mar 2025 06:02:15 +0100 Subject: [PATCH 5/6] review changes --- .../tracing/CombinedTransactionNameProvider.java | 7 ++----- .../tracing/CombinedTransactionNameProvider.java | 12 +++--------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java index daa751c768..9008d9bca1 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java @@ -8,11 +8,8 @@ import org.jetbrains.annotations.Nullable; /** - * Resolves transaction name using {@link HttpServletRequest#getMethod()} and templated route that - * handled the request. To return correct transaction name, it must be used after request is - * processed by {@link org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping} - * where {@link org.springframework.web.servlet.HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE} is - * set. + * Resolves transaction name using other transaction name providers by invoking them in order. + * If a provider returns no transaction name, the next one is invoked. */ @ApiStatus.Internal public final class CombinedTransactionNameProvider implements TransactionNameProvider { diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java index a3150609da..e138587765 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java @@ -8,17 +8,13 @@ import org.jetbrains.annotations.Nullable; /** - * Resolves transaction name using {@link HttpServletRequest#getMethod()} and templated route that - * handled the request. To return correct transaction name, it must be used after request is - * processed by {@link org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping} - * where {@link org.springframework.web.servlet.HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE} is - * set. + * Resolves transaction name using other transaction name providers by invoking them in order. + * If a provider returns no transaction name, the next one is invoked. */ @ApiStatus.Internal public final class CombinedTransactionNameProvider implements TransactionNameProvider { private final @NotNull List providers; - private @Nullable TransactionNameSource source = null; public CombinedTransactionNameProvider(final @NotNull List providers) { this.providers = providers; @@ -29,7 +25,6 @@ public CombinedTransactionNameProvider(final @NotNull List Date: Tue, 18 Mar 2025 05:04:04 +0000 Subject: [PATCH 6/6] Format code --- .../jakarta/tracing/CombinedTransactionNameProvider.java | 4 ++-- .../spring/tracing/CombinedTransactionNameProvider.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java index 9008d9bca1..1e142e958d 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java @@ -8,8 +8,8 @@ import org.jetbrains.annotations.Nullable; /** - * Resolves transaction name using other transaction name providers by invoking them in order. - * If a provider returns no transaction name, the next one is invoked. + * Resolves transaction name using other transaction name providers by invoking them in order. If a + * provider returns no transaction name, the next one is invoked. */ @ApiStatus.Internal public final class CombinedTransactionNameProvider implements TransactionNameProvider { diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java index e138587765..384000b36b 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java @@ -8,8 +8,8 @@ import org.jetbrains.annotations.Nullable; /** - * Resolves transaction name using other transaction name providers by invoking them in order. - * If a provider returns no transaction name, the next one is invoked. + * Resolves transaction name using other transaction name providers by invoking them in order. If a + * provider returns no transaction name, the next one is invoked. */ @ApiStatus.Internal public final class CombinedTransactionNameProvider implements TransactionNameProvider {