From 8ef382513c700a49f64129001aa66e3e0662f500 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 23 Jan 2025 11:58:48 +0100 Subject: [PATCH 1/5] Attach request object to event for OTel --- .../api/sentry-opentelemetry-core.api | 5 + .../OpenTelemetryAttributesExtractor.java | 88 +++++++++ .../opentelemetry/SentrySpanExporter.java | 10 +- .../OpenTelemetryAtrtibutesExtractorTest.kt | 182 ++++++++++++++++++ 4 files changed, 284 insertions(+), 1 deletion(-) create mode 100644 sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryAttributesExtractor.java create mode 100644 sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAtrtibutesExtractorTest.kt diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/api/sentry-opentelemetry-core.api b/sentry-opentelemetry/sentry-opentelemetry-core/api/sentry-opentelemetry-core.api index fd2099a7303..1e9bb60416b 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/api/sentry-opentelemetry-core.api +++ b/sentry-opentelemetry/sentry-opentelemetry-core/api/sentry-opentelemetry-core.api @@ -1,3 +1,8 @@ +public final class io/sentry/opentelemetry/OpenTelemetryAttributesExtractor { + public fun ()V + public fun extract (Lio/opentelemetry/sdk/trace/data/SpanData;Lio/sentry/ISpan;Lio/sentry/IScope;)V +} + public final class io/sentry/opentelemetry/OpenTelemetryLinkErrorEventProcessor : io/sentry/EventProcessor { public fun ()V public fun getOrder ()Ljava/lang/Long; diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryAttributesExtractor.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryAttributesExtractor.java new file mode 100644 index 00000000000..cc4fcc5da71 --- /dev/null +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryAttributesExtractor.java @@ -0,0 +1,88 @@ +package io.sentry.opentelemetry; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.sdk.trace.data.SpanData; +import io.opentelemetry.semconv.HttpAttributes; +import io.opentelemetry.semconv.ServerAttributes; +import io.opentelemetry.semconv.UrlAttributes; +import io.sentry.IScope; +import io.sentry.ISpan; +import io.sentry.protocol.Request; +import io.sentry.util.UrlUtils; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class OpenTelemetryAttributesExtractor { + + public void extract( + final @NotNull SpanData otelSpan, + final @NotNull ISpan sentrySpan, + final @NotNull IScope scope) { + final @NotNull Attributes attributes = otelSpan.getAttributes(); + addRequestAttributesToScope(attributes, scope); + } + + private void addRequestAttributesToScope(Attributes attributes, IScope scope) { + if (scope.getRequest() == null) { + scope.setRequest(new Request()); + } + final @Nullable Request request = scope.getRequest(); + if (request != null) { + final @Nullable String requestMethod = attributes.get(HttpAttributes.HTTP_REQUEST_METHOD); + if (requestMethod != null) { + request.setMethod(requestMethod); + } + + final @Nullable String urlFull = attributes.get(UrlAttributes.URL_FULL); + if (urlFull != null) { + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(urlFull); + urlDetails.applyToRequest(request); + } + + if (request.getUrl() == null) { + final String urlString = buildUrlString(attributes); + if (!urlString.isEmpty()) { + request.setUrl(urlString); + } + } + + if (request.getQueryString() == null) { + final @Nullable String query = attributes.get(UrlAttributes.URL_QUERY); + if (query != null) { + request.setQueryString(query); + } + } + } + } + + private @NotNull String buildUrlString(final @NotNull Attributes attributes) { + final @Nullable String scheme = attributes.get(UrlAttributes.URL_SCHEME); + final @Nullable String serverAddress = attributes.get(ServerAttributes.SERVER_ADDRESS); + final @Nullable Long serverPort = attributes.get(ServerAttributes.SERVER_PORT); + final @Nullable String path = attributes.get(UrlAttributes.URL_PATH); + + if (scheme == null || serverAddress == null) { + return ""; + } + + final @NotNull StringBuilder urlBuilder = new StringBuilder(); + urlBuilder.append(scheme); + urlBuilder.append("://"); + + if (serverAddress != null) { + urlBuilder.append(serverAddress); + if (serverPort != null) { + urlBuilder.append(":"); + urlBuilder.append(serverPort); + } + } + + if (path != null) { + urlBuilder.append(path); + } + + return urlBuilder.toString(); + } +} diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanExporter.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanExporter.java index 7851f07550c..4d2e7545c64 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanExporter.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanExporter.java @@ -19,6 +19,7 @@ import io.sentry.ISpan; import io.sentry.ITransaction; import io.sentry.Instrumenter; +import io.sentry.ScopeType; import io.sentry.ScopesAdapter; import io.sentry.SentryDate; import io.sentry.SentryInstantDate; @@ -50,6 +51,8 @@ public final class SentrySpanExporter implements SpanExporter { private final @NotNull SentryWeakSpanStorage spanStorage = SentryWeakSpanStorage.getInstance(); private final @NotNull SpanDescriptionExtractor spanDescriptionExtractor = new SpanDescriptionExtractor(); + private final @NotNull OpenTelemetryAttributesExtractor attributesExtractor = + new OpenTelemetryAttributesExtractor(); private final @NotNull IScopes scopes; private final @NotNull List attributeKeysToRemove = @@ -267,8 +270,10 @@ private void transferSpanDetails( spanStorage.getSentrySpan(span.getSpanContext()); final @Nullable IScopes scopesMaybe = sentrySpanMaybe != null ? sentrySpanMaybe.getScopes() : null; - final @NotNull IScopes scopesToUse = + final @NotNull IScopes scopesToUseBeforeForking = scopesMaybe == null ? ScopesAdapter.getInstance() : scopesMaybe; + final @NotNull IScopes scopesToUse = + scopesToUseBeforeForking.forkedCurrentScope("SentrySpanExporter.createTransaction"); final @NotNull OtelSpanInfo spanInfo = spanDescriptionExtractor.extractSpanInfo(span, sentrySpanMaybe); @@ -331,6 +336,9 @@ private void transferSpanDetails( setOtelSpanKind(span, sentryTransaction); transferSpanDetails(sentrySpanMaybe, sentryTransaction); + scopesToUse.configureScope( + ScopeType.CURRENT, scope -> attributesExtractor.extract(span, sentryTransaction, scope)); + return sentryTransaction; } diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAtrtibutesExtractorTest.kt b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAtrtibutesExtractorTest.kt new file mode 100644 index 00000000000..39756b7de1a --- /dev/null +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAtrtibutesExtractorTest.kt @@ -0,0 +1,182 @@ +package io.sentry.opentelemetry + +import io.opentelemetry.api.common.AttributeKey +import io.opentelemetry.sdk.internal.AttributesMap +import io.opentelemetry.sdk.trace.data.SpanData +import io.opentelemetry.semconv.ServerAttributes +import io.opentelemetry.semconv.UrlAttributes +import io.sentry.ISpan +import io.sentry.Scope +import io.sentry.SentryOptions +import io.sentry.protocol.Request +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +class OpenTelemetryAtrtibutesExtractorTest { + + private class Fixture { + val spanData = mock() + val attributes = AttributesMap.create(100, 100) + val sentrySpan = mock() + val options = SentryOptions.empty() + val scope = Scope(options) + + init { + whenever(spanData.attributes).thenReturn(attributes) + } + } + + private val fixture = Fixture() + + @Test + fun `sets URL based on OTel attributes`() { + givenAttributes( + mapOf( + UrlAttributes.URL_SCHEME to "https", + UrlAttributes.URL_PATH to "/path/to/123", + UrlAttributes.URL_QUERY to "q=123456&b=X", + ServerAttributes.SERVER_ADDRESS to "io.sentry", + ServerAttributes.SERVER_PORT to 8081L + ) + ) + + whenExtractingAttributes() + + thenRequestIsSet() + thenUrlIsSetTo("https://io.sentry:8081/path/to/123") + thenQueryIsSetTo("q=123456&b=X") + } + + @Test + fun `when there is an existing request on scope it is filled with more details`() { + fixture.scope.request = Request().also { it.bodySize = 123L } + givenAttributes( + mapOf( + UrlAttributes.URL_SCHEME to "https", + UrlAttributes.URL_PATH to "/path/to/123", + UrlAttributes.URL_QUERY to "q=123456&b=X", + ServerAttributes.SERVER_ADDRESS to "io.sentry", + ServerAttributes.SERVER_PORT to 8081L + ) + ) + + whenExtractingAttributes() + + thenRequestIsSet() + thenUrlIsSetTo("https://io.sentry:8081/path/to/123") + thenQueryIsSetTo("q=123456&b=X") + assertEquals(123L, fixture.scope.request!!.bodySize) + } + + @Test + fun `when there is an existing request with url on scope it is kept`() { + fixture.scope.request = Request().also { + it.url = "http://docs.sentry.io:3000/platform" + it.queryString = "s=abc" + } + givenAttributes( + mapOf( + UrlAttributes.URL_SCHEME to "https", + UrlAttributes.URL_PATH to "/path/to/123", + UrlAttributes.URL_QUERY to "q=123456&b=X", + ServerAttributes.SERVER_ADDRESS to "io.sentry", + ServerAttributes.SERVER_PORT to 8081L + ) + ) + + whenExtractingAttributes() + + thenRequestIsSet() + thenUrlIsSetTo("http://docs.sentry.io:3000/platform") + thenQueryIsSetTo("s=abc") + } + + @Test + fun `sets URL based on OTel attributes without port`() { + givenAttributes( + mapOf( + UrlAttributes.URL_SCHEME to "https", + UrlAttributes.URL_PATH to "/path/to/123", + ServerAttributes.SERVER_ADDRESS to "io.sentry" + ) + ) + + whenExtractingAttributes() + + thenRequestIsSet() + thenUrlIsSetTo("https://io.sentry/path/to/123") + } + + @Test + fun `sets URL based on OTel attributes without path`() { + givenAttributes( + mapOf( + UrlAttributes.URL_SCHEME to "https", + ServerAttributes.SERVER_ADDRESS to "io.sentry" + ) + ) + + whenExtractingAttributes() + + thenRequestIsSet() + thenUrlIsSetTo("https://io.sentry") + } + + @Test + fun `does not set URL if server address is missing`() { + givenAttributes( + mapOf( + UrlAttributes.URL_SCHEME to "https" + ) + ) + + whenExtractingAttributes() + + thenRequestIsSet() + thenUrlIsNotSet() + } + + @Test + fun `does not set URL if scheme is missing`() { + givenAttributes( + mapOf( + ServerAttributes.SERVER_ADDRESS to "io.sentry" + ) + ) + + whenExtractingAttributes() + + thenRequestIsSet() + thenUrlIsNotSet() + } + + private fun givenAttributes(map: Map, Any>) { + map.forEach { k, v -> + fixture.attributes.put(k, v) + } + } + + private fun whenExtractingAttributes() { + OpenTelemetryAttributesExtractor().extract(fixture.spanData, fixture.sentrySpan, fixture.scope) + } + + private fun thenRequestIsSet() { + assertNotNull(fixture.scope.request) + } + + private fun thenUrlIsSetTo(expected: String) { + assertEquals(expected, fixture.scope.request!!.url) + } + + private fun thenUrlIsNotSet() { + assertNull(fixture.scope.request!!.url) + } + + private fun thenQueryIsSetTo(expected: String) { + assertEquals(expected, fixture.scope.request!!.queryString) + } +} From d3cab7c0de04d1d6b401f5ce8a74e4db8981c1b9 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 23 Jan 2025 15:25:27 +0100 Subject: [PATCH 2/5] fix test name --- ...esExtractorTest.kt => OpenTelemetryAttributesExtractorTest.kt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/{OpenTelemetryAtrtibutesExtractorTest.kt => OpenTelemetryAttributesExtractorTest.kt} (100%) diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAtrtibutesExtractorTest.kt b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt similarity index 100% rename from sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAtrtibutesExtractorTest.kt rename to sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt From ecd604030fab91657ee1042c7afa0033ff03558a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 23 Jan 2025 16:14:11 +0100 Subject: [PATCH 3/5] rename test class --- .../src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt index 39756b7de1a..1ba77f2454e 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt @@ -16,7 +16,7 @@ import kotlin.test.assertEquals import kotlin.test.assertNotNull import kotlin.test.assertNull -class OpenTelemetryAtrtibutesExtractorTest { +class OpenTelemetryAttributesExtractorTest { private class Fixture { val spanData = mock() From 6e0b24dbef365f04f1a17c997f2ee927e5a3a8eb Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 23 Jan 2025 16:16:37 +0100 Subject: [PATCH 4/5] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e4603cd4fa..f6c034762cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixes - Avoid logging an error when a float is passed in the manifest ([#4031](https://github.com/getsentry/sentry-java/pull/4031)) +- Add `request` details to transactions created through OpenTelemetry ([#4098](https://github.com/getsentry/sentry-java/pull/4098)) + - We now add HTTP request method and URL where Sentry expects it to display it in Sentry UI ## 8.0.0 From 28a7e5661a050531a0bf1b8fbcc16f809d859e14 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 24 Jan 2025 06:51:37 +0100 Subject: [PATCH 5/5] do not override existing url on request even with full url --- .../OpenTelemetryAttributesExtractor.java | 10 ++++++---- .../OpenTelemetryAttributesExtractorTest.kt | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryAttributesExtractor.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryAttributesExtractor.java index cc4fcc5da71..431b4d274e3 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryAttributesExtractor.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryAttributesExtractor.java @@ -35,10 +35,12 @@ private void addRequestAttributesToScope(Attributes attributes, IScope scope) { request.setMethod(requestMethod); } - final @Nullable String urlFull = attributes.get(UrlAttributes.URL_FULL); - if (urlFull != null) { - final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(urlFull); - urlDetails.applyToRequest(request); + if (request.getUrl() == null) { + final @Nullable String urlFull = attributes.get(UrlAttributes.URL_FULL); + if (urlFull != null) { + final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(urlFull); + urlDetails.applyToRequest(request); + } } if (request.getUrl() == null) { diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt index 1ba77f2454e..f962cfa594d 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt @@ -95,6 +95,25 @@ class OpenTelemetryAttributesExtractorTest { thenQueryIsSetTo("s=abc") } + @Test + fun `when there is an existing request with url on scope it is kept with URL_FULL`() { + fixture.scope.request = Request().also { + it.url = "http://docs.sentry.io:3000/platform" + it.queryString = "s=abc" + } + givenAttributes( + mapOf( + UrlAttributes.URL_FULL to "https://io.sentry:8081/path/to/123?q=123456&b=X" + ) + ) + + whenExtractingAttributes() + + thenRequestIsSet() + thenUrlIsSetTo("http://docs.sentry.io:3000/platform") + thenQueryIsSetTo("s=abc") + } + @Test fun `sets URL based on OTel attributes without port`() { givenAttributes(