diff --git a/CHANGELOG.md b/CHANGELOG.md index 52517b2d66e..68eb9a2ec21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ ### Features +- Add HTTP server request headers from OpenTelemetry span attributes to sentry `request` in payload ([#4102](https://github.com/getsentry/sentry-java/pull/4102)) + - You have to explicitly enable each header by adding it to the [OpenTelemetry config](https://opentelemetry.io/docs/zero-code/java/agent/instrumentation/http/#capturing-http-request-and-response-headers) + - Please only enable headers you actually want to send to Sentry. Some may contain sensitive data like PII, cookies, tokens etc. + - We are no longer adding request/response headers to `contexts/otel/attributes` of the event. - The `ignoredErrors` option is now configurable via the manifest property `io.sentry.traces.ignored-errors` ([#4178](https://github.com/getsentry/sentry-java/pull/4178)) - A list of active Spring profiles is attached to payloads sent to Sentry (errors, traces, etc.) and displayed in the UI when using our Spring or Spring Boot integrations ([#4147](https://github.com/getsentry/sentry-java/pull/4147)) - This consists of an empty list when only the default profile is active 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 739fc7eb1a3..f20ea0ab864 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/api/sentry-opentelemetry-core.api +++ b/sentry-opentelemetry/sentry-opentelemetry-core/api/sentry-opentelemetry-core.api @@ -1,7 +1,7 @@ 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 fun extractUrl (Lio/opentelemetry/api/common/Attributes;)Ljava/lang/String; + public fun extract (Lio/opentelemetry/sdk/trace/data/SpanData;Lio/sentry/IScope;Lio/sentry/SentryOptions;)V + public fun extractUrl (Lio/opentelemetry/api/common/Attributes;Lio/sentry/SentryOptions;)Ljava/lang/String; } public final class io/sentry/opentelemetry/OpenTelemetryLinkErrorEventProcessor : io/sentry/EventProcessor { 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 65757765184..87088ae2377 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 @@ -6,9 +6,16 @@ import io.opentelemetry.semconv.ServerAttributes; import io.opentelemetry.semconv.UrlAttributes; import io.sentry.IScope; -import io.sentry.ISpan; +import io.sentry.SentryLevel; +import io.sentry.SentryOptions; import io.sentry.protocol.Request; +import io.sentry.util.HttpUtils; +import io.sentry.util.StringUtils; import io.sentry.util.UrlUtils; +import java.net.URL; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -16,16 +23,22 @@ @ApiStatus.Internal public final class OpenTelemetryAttributesExtractor { + private static final String HTTP_REQUEST_HEADER_PREFIX = "http.request.header."; + public void extract( final @NotNull SpanData otelSpan, - final @NotNull ISpan sentrySpan, - final @NotNull IScope scope) { + final @NotNull IScope scope, + final @NotNull SentryOptions options) { final @NotNull Attributes attributes = otelSpan.getAttributes(); - addRequestAttributesToScope(attributes, scope); + if (attributes.get(HttpAttributes.HTTP_REQUEST_METHOD) != null) { + addRequestAttributesToScope(attributes, scope, options); + } } private void addRequestAttributesToScope( - final @NotNull Attributes attributes, final @NotNull IScope scope) { + final @NotNull Attributes attributes, + final @NotNull IScope scope, + final @NotNull SentryOptions options) { if (scope.getRequest() == null) { scope.setRequest(new Request()); } @@ -37,7 +50,7 @@ private void addRequestAttributesToScope( } if (request.getUrl() == null) { - final @Nullable String url = extractUrl(attributes); + final @Nullable String url = extractUrl(attributes, options); if (url != null) { final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(url); urlDetails.applyToRequest(request); @@ -50,16 +63,56 @@ private void addRequestAttributesToScope( request.setQueryString(query); } } + + if (request.getHeaders() == null) { + Map headers = collectHeaders(attributes, options); + if (!headers.isEmpty()) { + request.setHeaders(headers); + } + } } } - public @Nullable String extractUrl(final @NotNull Attributes attributes) { + @SuppressWarnings("unchecked") + private static Map collectHeaders( + final @NotNull Attributes attributes, final @NotNull SentryOptions options) { + Map headers = new HashMap<>(); + + attributes.forEach( + (key, value) -> { + final @NotNull String attributeKeyAsString = key.getKey(); + if (attributeKeyAsString.startsWith(HTTP_REQUEST_HEADER_PREFIX)) { + final @NotNull String headerName = + StringUtils.removePrefix(attributeKeyAsString, HTTP_REQUEST_HEADER_PREFIX); + if (options.isSendDefaultPii() || !HttpUtils.containsSensitiveHeader(headerName)) { + if (value instanceof List) { + try { + final @NotNull List headerValues = (List) value; + headers.put( + headerName, + toString( + HttpUtils.filterOutSecurityCookiesFromHeader( + headerValues, headerName, null))); + } catch (Throwable t) { + options + .getLogger() + .log(SentryLevel.WARNING, "Expected a List as header", t); + } + } + } + } + }); + return headers; + } + + public @Nullable String extractUrl( + final @NotNull Attributes attributes, final @NotNull SentryOptions options) { final @Nullable String urlFull = attributes.get(UrlAttributes.URL_FULL); if (urlFull != null) { return urlFull; } - final String urlString = buildUrlString(attributes); + final String urlString = buildUrlString(attributes, options); if (!urlString.isEmpty()) { return urlString; } @@ -67,7 +120,12 @@ private void addRequestAttributesToScope( return null; } - private @NotNull String buildUrlString(final @NotNull Attributes attributes) { + private static @Nullable String toString(final @Nullable List list) { + return list != null ? String.join(",", list) : null; + } + + private @NotNull String buildUrlString( + final @NotNull Attributes attributes, final @NotNull SentryOptions options) { 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); @@ -77,22 +135,18 @@ private void addRequestAttributesToScope( 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); + try { + final @NotNull String pathToUse = path == null ? "" : path; + if (serverPort == null) { + return new URL(scheme, serverAddress, pathToUse).toString(); + } else { + return new URL(scheme, serverAddress, serverPort.intValue(), pathToUse).toString(); } + } catch (Throwable t) { + options + .getLogger() + .log(SentryLevel.WARNING, "Unable to combine URL span attributes into one.", t); + return ""; } - - if (path != null) { - urlBuilder.append(path); - } - - return urlBuilder.toString(); } } diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java index c36f1829926..5e59358b09c 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java @@ -17,6 +17,7 @@ import io.sentry.ScopesAdapter; import io.sentry.Sentry; import io.sentry.SentryLevel; +import io.sentry.SentryOptions; import io.sentry.SentryTraceHeader; import io.sentry.exception.InvalidSentryTraceHeaderException; import io.sentry.util.TracingUtils; @@ -76,7 +77,7 @@ public void inject(final Context context, final C carrier, final TextMapSett return; } - final @Nullable String url = getUrl(sentrySpan); + final @Nullable String url = getUrl(sentrySpan, scopes.getOptions()); final @Nullable TracingUtils.TracingHeaders tracingHeaders = url == null ? TracingUtils.trace(scopes, Collections.emptyList(), sentrySpan) @@ -92,12 +93,13 @@ public void inject(final Context context, final C carrier, final TextMapSett } } - private @Nullable String getUrl(final @NotNull IOtelSpanWrapper sentrySpan) { + private @Nullable String getUrl( + final @NotNull IOtelSpanWrapper sentrySpan, final @NotNull SentryOptions options) { final @Nullable Attributes attributes = sentrySpan.getOpenTelemetrySpanAttributes(); if (attributes == null) { return null; } - return attributesExtractor.extractUrl(attributes); + return attributesExtractor.extractUrl(attributes, options); } @Override 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 693b94fe38d..6db21ff7979 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 @@ -68,6 +68,9 @@ public final class SentrySpanExporter implements SpanExporter { InternalSemanticAttributes.PARENT_SAMPLED.getKey(), ProcessIncubatingAttributes.PROCESS_COMMAND_ARGS.getKey() // can be very long ); + + private final @NotNull List attributeToRemoveByPrefix = + Arrays.asList("http.request.header.", "http.response.header."); private static final @NotNull Long SPAN_TIMEOUT = DateUtils.secondsToNanos(5 * 60); public static final String TRACE_ORIGIN = "auto.opentelemetry"; @@ -338,7 +341,8 @@ private void transferSpanDetails( transferSpanDetails(sentrySpanMaybe, sentryTransaction); scopesToUse.configureScope( - ScopeType.CURRENT, scope -> attributesExtractor.extract(span, sentryTransaction, scope)); + ScopeType.CURRENT, + scope -> attributesExtractor.extract(span, scope, scopesToUse.getOptions())); return sentryTransaction; } @@ -488,7 +492,17 @@ private SpanStatus mapOtelStatus( } private boolean shouldRemoveAttribute(final @NotNull String key) { - return attributeKeysToRemove.contains(key); + if (attributeKeysToRemove.contains(key)) { + return true; + } + + for (String prefix : attributeToRemoveByPrefix) { + if (key.startsWith(prefix)) { + return true; + } + } + + return false; } private void setOtelInstrumentationInfo( 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 80631406713..1227509e0d0 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt @@ -3,9 +3,9 @@ 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.HttpAttributes 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 @@ -13,6 +13,7 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull @@ -21,7 +22,6 @@ class OpenTelemetryAttributesExtractorTest { private class Fixture { val spanData = mock() val attributes = AttributesMap.create(100, 100) - val sentrySpan = mock() val options = SentryOptions.empty() val scope = Scope(options) @@ -36,6 +36,7 @@ class OpenTelemetryAttributesExtractorTest { fun `sets URL based on OTel attributes`() { givenAttributes( mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", UrlAttributes.URL_SCHEME to "https", UrlAttributes.URL_PATH to "/path/to/123", UrlAttributes.URL_QUERY to "q=123456&b=X", @@ -56,6 +57,7 @@ class OpenTelemetryAttributesExtractorTest { fixture.scope.request = Request().also { it.bodySize = 123L } givenAttributes( mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", UrlAttributes.URL_SCHEME to "https", UrlAttributes.URL_PATH to "/path/to/123", UrlAttributes.URL_QUERY to "q=123456&b=X", @@ -80,6 +82,7 @@ class OpenTelemetryAttributesExtractorTest { } givenAttributes( mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", UrlAttributes.URL_SCHEME to "https", UrlAttributes.URL_PATH to "/path/to/123", UrlAttributes.URL_QUERY to "q=123456&b=X", @@ -118,6 +121,7 @@ class OpenTelemetryAttributesExtractorTest { fun `sets URL based on OTel attributes without port`() { givenAttributes( mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", UrlAttributes.URL_SCHEME to "https", UrlAttributes.URL_PATH to "/path/to/123", ServerAttributes.SERVER_ADDRESS to "io.sentry" @@ -134,6 +138,7 @@ class OpenTelemetryAttributesExtractorTest { fun `sets URL based on OTel attributes without path`() { givenAttributes( mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", UrlAttributes.URL_SCHEME to "https", ServerAttributes.SERVER_ADDRESS to "io.sentry" ) @@ -149,6 +154,7 @@ class OpenTelemetryAttributesExtractorTest { fun `does not set URL if server address is missing`() { givenAttributes( mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", UrlAttributes.URL_SCHEME to "https" ) ) @@ -163,6 +169,7 @@ class OpenTelemetryAttributesExtractorTest { fun `does not set URL if scheme is missing`() { givenAttributes( mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", ServerAttributes.SERVER_ADDRESS to "io.sentry" ) ) @@ -285,6 +292,49 @@ class OpenTelemetryAttributesExtractorTest { assertEquals("https://sentry.io:8082", url) } + @Test + fun `sets server request headers based on OTel attributes and merges list of values`() { + givenAttributes( + mapOf( + HttpAttributes.HTTP_REQUEST_METHOD to "GET", + AttributeKey.stringArrayKey("http.request.header.baggage") to listOf("sentry-environment=production,sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-sampled=true,sentry-trace_id=df71f5972f754b4c85af13ff5c07017d", "another-baggage=abc,more=def"), + AttributeKey.stringArrayKey("http.request.header.sentry-trace") to listOf("f9118105af4a2d42b4124532cd176588-4542d085bb0b4de5"), + AttributeKey.stringArrayKey("http.response.header.some-header") to listOf("some-value") + ) + ) + + whenExtractingAttributes() + + thenRequestIsSet() + thenHeaderIsPresentOnRequest("baggage", "sentry-environment=production,sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-sampled=true,sentry-trace_id=df71f5972f754b4c85af13ff5c07017d,another-baggage=abc,more=def") + thenHeaderIsPresentOnRequest("sentry-trace", "f9118105af4a2d42b4124532cd176588-4542d085bb0b4de5") + thenHeaderIsNotPresentOnRequest("some-header") + } + + @Test + fun `if there are no header attributes does not set headers on request`() { + givenAttributes(mapOf(HttpAttributes.HTTP_REQUEST_METHOD to "GET")) + + whenExtractingAttributes() + + thenRequestIsSet() + assertNull(fixture.scope.request!!.headers) + } + + @Test + fun `if there is no request method attribute does not set request on scope`() { + givenAttributes( + mapOf( + UrlAttributes.URL_SCHEME to "https", + ServerAttributes.SERVER_ADDRESS to "io.sentry" + ) + ) + + whenExtractingAttributes() + + thenRequestIsNotSet() + } + private fun givenAttributes(map: Map, Any>) { map.forEach { k, v -> fixture.attributes.put(k, v) @@ -292,17 +342,21 @@ class OpenTelemetryAttributesExtractorTest { } private fun whenExtractingAttributes() { - OpenTelemetryAttributesExtractor().extract(fixture.spanData, fixture.sentrySpan, fixture.scope) + OpenTelemetryAttributesExtractor().extract(fixture.spanData, fixture.scope, fixture.options) } private fun whenExtractingUrl(): String? { - return OpenTelemetryAttributesExtractor().extractUrl(fixture.attributes) + return OpenTelemetryAttributesExtractor().extractUrl(fixture.attributes, fixture.options) } private fun thenRequestIsSet() { assertNotNull(fixture.scope.request) } + private fun thenRequestIsNotSet() { + assertNull(fixture.scope.request) + } + private fun thenUrlIsSetTo(expected: String) { assertEquals(expected, fixture.scope.request!!.url) } @@ -314,4 +368,12 @@ class OpenTelemetryAttributesExtractorTest { private fun thenQueryIsSetTo(expected: String) { assertEquals(expected, fixture.scope.request!!.queryString) } + + private fun thenHeaderIsPresentOnRequest(headerName: String, expectedValue: String) { + assertEquals(expectedValue, fixture.scope.request!!.headers!!.get(headerName)) + } + + private fun thenHeaderIsNotPresentOnRequest(headerName: String) { + assertFalse(fixture.scope.request!!.headers!!.containsKey(headerName)) + } }