From 9c7a845677d49a4479ecdd84d0a6e0a6d20d6e4d Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 17 Jul 2026 15:16:35 +0200 Subject: [PATCH] feat(graphql): Apply Data Collection options Control GraphQL documents and variables through the new Data Collection policies across GraphQL and Apollo integrations. Preserve sendDefaultPii and maxRequestBodySize behavior when Data Collection is absent. Co-Authored-By: Claude --- sentry-apollo-3/api/sentry-apollo-3.api | 2 + .../apollo3/SentryApollo3HttpInterceptor.kt | 7 +- .../apollo3/SentryApollo3Interceptor.kt | 24 +-- .../apollo3/SentryApolloBuilderExtensions.kt | 2 +- .../SentryApollo3InterceptorClientErrors.kt | 56 +++++++ ...ntryApollo3InterceptorWithVariablesTest.kt | 26 +++- .../apollo4/SentryApollo4HttpInterceptor.kt | 7 +- .../apollo4/SentryApollo4Interceptor.kt | 12 +- .../apollo4/SentryApolloBuilderExtensions.kt | 2 +- ...pollo4BuilderExtensionsClientErrorsTest.kt | 56 +++++++ .../SentryApollo4BuilderExtensionsTest.kt | 27 +++- .../sentry/apollo/SentryApolloInterceptor.kt | 4 +- .../apollo/SentryApolloInterceptorTest.kt | 18 +++ .../io/sentry/graphql/ExceptionReporter.java | 29 +++- .../sentry/graphql/ExceptionReporterTest.kt | 140 ++++++++++++++++++ sentry/api/sentry.api | 8 + .../io/sentry/DataCollectionResolver.java | 32 +++- .../java/io/sentry/util/GraphqlUtils.java | 53 +++++++ .../io/sentry/DataCollectionResolverTest.kt | 75 ++++++++-- 19 files changed, 535 insertions(+), 45 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/util/GraphqlUtils.java diff --git a/sentry-apollo-3/api/sentry-apollo-3.api b/sentry-apollo-3/api/sentry-apollo-3.api index e106585156f..9df63356733 100644 --- a/sentry-apollo-3/api/sentry-apollo-3.api +++ b/sentry-apollo-3/api/sentry-apollo-3.api @@ -35,6 +35,8 @@ public final class io/sentry/apollo3/SentryApollo3HttpInterceptor$Companion { public final class io/sentry/apollo3/SentryApollo3Interceptor : com/apollographql/apollo3/interceptor/ApolloInterceptor { public fun ()V + public fun (Lio/sentry/IScopes;)V + public synthetic fun (Lio/sentry/IScopes;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun intercept (Lcom/apollographql/apollo3/api/ApolloRequest;Lcom/apollographql/apollo3/interceptor/ApolloInterceptorChain;)Lkotlinx/coroutines/flow/Flow; } diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index 8337eeb7b15..450681de94b 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt @@ -27,6 +27,7 @@ import io.sentry.exception.ExceptionMechanismException import io.sentry.protocol.Mechanism import io.sentry.protocol.Request import io.sentry.protocol.Response +import io.sentry.util.GraphqlUtils import io.sentry.util.HttpUtils import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion import io.sentry.util.Platform @@ -174,7 +175,9 @@ constructor( operationId?.let { setData("operationId", it) } - variables?.let { setData("variables", it) } + if (scopes.options.dataCollectionResolver.isGraphqlVariablesWithLegacyAlways) { + variables?.let { setData("variables", it) } + } setData(HTTP_METHOD_KEY, method.uppercase()) } } @@ -366,7 +369,7 @@ constructor( try { it.writeTo(buffer) - data = buffer.readUtf8() + data = GraphqlUtils.filterRequestBody(buffer.readUtf8(), scopes.options) } catch (e: Throwable) { scopes.options.logger.log(SentryLevel.ERROR, "Error reading the request body.", e) // continue because the response body alone can already give some insights diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3Interceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3Interceptor.kt index ea0fa1fa18e..b58a2551566 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3Interceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3Interceptor.kt @@ -10,12 +10,16 @@ import com.apollographql.apollo3.api.Subscription import com.apollographql.apollo3.api.variables import com.apollographql.apollo3.interceptor.ApolloInterceptor import com.apollographql.apollo3.interceptor.ApolloInterceptorChain +import io.sentry.IScopes +import io.sentry.ScopesAdapter import io.sentry.apollo3.SentryApollo3HttpInterceptor.Companion.SENTRY_APOLLO_3_OPERATION_TYPE import io.sentry.apollo3.SentryApollo3HttpInterceptor.Companion.SENTRY_APOLLO_3_VARIABLES import io.sentry.vendor.Base64 import kotlinx.coroutines.flow.Flow -class SentryApollo3Interceptor : ApolloInterceptor { +class SentryApollo3Interceptor +@JvmOverloads +constructor(private val scopes: IScopes = ScopesAdapter.getInstance()) : ApolloInterceptor { override fun intercept( request: ApolloRequest, chain: ApolloInterceptorChain, @@ -28,14 +32,16 @@ class SentryApollo3Interceptor : ApolloInterceptor { Base64.encodeToString(operationType(request).toByteArray(), Base64.NO_WRAP), ) - request.scalarAdapters?.let { - builder.addHttpHeader( - SENTRY_APOLLO_3_VARIABLES, - Base64.encodeToString( - request.operation.variables(it).valueMap.toString().toByteArray(), - Base64.NO_WRAP, - ), - ) + if (scopes.options.dataCollectionResolver.isGraphqlVariablesWithLegacyAlways) { + request.scalarAdapters?.let { + builder.addHttpHeader( + SENTRY_APOLLO_3_VARIABLES, + Base64.encodeToString( + request.operation.variables(it).valueMap.toString().toByteArray(), + Base64.NO_WRAP, + ), + ) + } } return chain.proceed(builder.build()) } diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApolloBuilderExtensions.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApolloBuilderExtensions.kt index b5498a31316..076cfea521d 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApolloBuilderExtensions.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApolloBuilderExtensions.kt @@ -13,7 +13,7 @@ fun ApolloClient.Builder.sentryTracing( failedRequestTargets: List = listOf(DEFAULT_PROPAGATION_TARGETS), beforeSpan: SentryApollo3HttpInterceptor.BeforeSpanCallback? = null, ): ApolloClient.Builder { - addInterceptor(SentryApollo3Interceptor()) + addInterceptor(SentryApollo3Interceptor(scopes)) addHttpInterceptor( SentryApollo3HttpInterceptor( scopes = scopes, diff --git a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorClientErrors.kt b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorClientErrors.kt index 78be36f83b0..2b4eed0aa4c 100644 --- a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorClientErrors.kt +++ b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorClientErrors.kt @@ -72,6 +72,7 @@ class SentryApollo3InterceptorClientErrors { responseBody: String = responseBodyOk, sendDefaultPii: Boolean = false, socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN, + configureOptions: SentryOptions.() -> Unit = {}, ): ApolloClient { SentryIntegrationPackageStorage.getInstance().clearStorage() @@ -83,6 +84,7 @@ class SentryApollo3InterceptorClientErrors { dsn = "https://key@sentry.io/proj" sdkVersion = SdkVersion("test", "1.2.3") isSendDefaultPii = sendDefaultPii + configureOptions() } ) } @@ -266,6 +268,60 @@ class SentryApollo3InterceptorClientErrors { ) } + @Test + fun `data collection can disable the GraphQL document independently`() { + val sut = + fixture.getSut(responseBody = fixture.responseBodyNotOk) { + dataCollection.graphql.setDocument(false) + } + executeQuery(sut) + + verify(fixture.scopes) + .captureEvent( + check { + val body = it.request!!.data as String + assertFalse(body.contains("\"query\"")) + assertTrue(body.contains("\"variables\"")) + }, + any(), + ) + } + + @Test + fun `data collection can disable GraphQL variables independently`() { + val sut = + fixture.getSut(responseBody = fixture.responseBodyNotOk) { + dataCollection.graphql.setVariables(false) + } + executeQuery(sut) + + verify(fixture.scopes) + .captureEvent( + check { + val body = it.request!!.data as String + assertTrue(body.contains("\"query\"")) + assertFalse(body.contains("\"variables\"")) + }, + any(), + ) + } + + @Test + fun `data collection can disable the GraphQL request body`() { + val sut = + fixture.getSut(responseBody = fixture.responseBodyNotOk) { + dataCollection.graphql.setDocument(false) + dataCollection.graphql.setVariables(false) + } + executeQuery(sut) + + verify(fixture.scopes) + .captureEvent( + check { assertNull(it.request!!.data) }, + any(), + ) + } + @Test fun `capture errors with more request context if sendDefaultPii is enabled`() { val sut = fixture.getSut(responseBody = fixture.responseBodyNotOk, sendDefaultPii = true) diff --git a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorWithVariablesTest.kt b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorWithVariablesTest.kt index 9d0028b5db7..a77c3b6ecd4 100644 --- a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorWithVariablesTest.kt +++ b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorWithVariablesTest.kt @@ -55,9 +55,9 @@ class SentryApollo3InterceptorWithVariablesTest { }""", socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN, beforeSpan: BeforeSpanCallback? = null, + options: SentryOptions = SentryOptions().apply { dsn = "http://key@localhost/proj" }, ): ApolloClient { - whenever(scopes.options) - .thenReturn(SentryOptions().apply { dsn = "http://key@localhost/proj" }) + whenever(scopes.options).thenReturn(options) server.enqueue( MockResponse() @@ -91,6 +91,28 @@ class SentryApollo3InterceptorWithVariablesTest { ) } + @Test + fun `does not attach GraphQL variables when data collection disables them`() { + val options = + SentryOptions().apply { + dsn = "http://key@localhost/proj" + dataCollection.graphql.setVariables(false) + } + + executeQuery(fixture.getSut(options = options)) + + verify(fixture.scopes) + .captureTransaction( + check { + assertNull(it.spans.first().data?.get("variables")) + assertNotNull(it.spans.first().data?.get("operationId")) + }, + anyOrNull(), + anyOrNull(), + anyOrNull(), + ) + } + @Test fun `creates a span around the failed request`() { executeQuery(fixture.getSut(httpStatusCode = 403)) diff --git a/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt b/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt index fcf50564e5a..697ef81e571 100644 --- a/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt +++ b/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt @@ -25,6 +25,7 @@ import io.sentry.exception.ExceptionMechanismException import io.sentry.protocol.Mechanism import io.sentry.protocol.Request import io.sentry.protocol.Response +import io.sentry.util.GraphqlUtils import io.sentry.util.HttpUtils import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion import io.sentry.util.Platform @@ -173,7 +174,9 @@ constructor( operationId?.let { setData("operationId", it) } - variables?.let { setData("variables", it) } + if (scopes.options.dataCollectionResolver.isGraphqlVariablesWithLegacyAlways) { + variables?.let { setData("variables", it) } + } setData(HTTP_METHOD_KEY, method.uppercase(Locale.ROOT)) } } @@ -365,7 +368,7 @@ constructor( try { it.writeTo(buffer) - data = buffer.readUtf8() + data = GraphqlUtils.filterRequestBody(buffer.readUtf8(), scopes.options) } catch (e: Throwable) { scopes.options.logger.log(SentryLevel.ERROR, "Error reading the request body.", e) // continue because the response body alone can already give some insights diff --git a/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4Interceptor.kt b/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4Interceptor.kt index 5e0b882aad6..2481e2893d4 100644 --- a/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4Interceptor.kt +++ b/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4Interceptor.kt @@ -35,11 +35,13 @@ constructor(@ApiStatus.Internal private val scopes: IScopes = ScopesAdapter.getI .addHttpHeader(OPERATION_NAME_HEADER_NAME, encodeHeaderValue(request.operation.name())) .addHttpHeader(OPERATION_TYPE_HEADER_NAME, encodeHeaderValue(operationType(request))) - request.scalarAdapters?.let { - builder.addHttpHeader( - VARIABLES_HEADER_NAME, - encodeHeaderValue(request.operation.variables(it).valueMap.toString()), - ) + if (scopes.options.dataCollectionResolver.isGraphqlVariablesWithLegacyAlways) { + request.scalarAdapters?.let { + builder.addHttpHeader( + VARIABLES_HEADER_NAME, + encodeHeaderValue(request.operation.variables(it).valueMap.toString()), + ) + } } return chain.proceed(builder.build()) diff --git a/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApolloBuilderExtensions.kt b/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApolloBuilderExtensions.kt index 61ff468d265..51383d33ed7 100644 --- a/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApolloBuilderExtensions.kt +++ b/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApolloBuilderExtensions.kt @@ -13,7 +13,7 @@ fun ApolloClient.Builder.sentryTracing( failedRequestTargets: List = listOf(DEFAULT_PROPAGATION_TARGETS), beforeSpan: SentryApollo4HttpInterceptor.BeforeSpanCallback? = null, ): ApolloClient.Builder { - addInterceptor(SentryApollo4Interceptor()) + addInterceptor(SentryApollo4Interceptor(scopes)) addHttpInterceptor( SentryApollo4HttpInterceptor( scopes = scopes, diff --git a/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4BuilderExtensionsClientErrorsTest.kt b/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4BuilderExtensionsClientErrorsTest.kt index 0572e4f1323..fe870a8f9f1 100644 --- a/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4BuilderExtensionsClientErrorsTest.kt +++ b/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4BuilderExtensionsClientErrorsTest.kt @@ -86,6 +86,7 @@ abstract class SentryApollo4BuilderExtensionsClientErrorsTest( responseBody: String = responseBodyOk, sendDefaultPii: Boolean = false, socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN, + configureOptions: SentryOptions.() -> Unit = {}, ): ApolloClient { SentryIntegrationPackageStorage.getInstance().clearStorage() @@ -97,6 +98,7 @@ abstract class SentryApollo4BuilderExtensionsClientErrorsTest( dsn = "https://key@sentry.io/proj" sdkVersion = SdkVersion("test", "1.2.3") isSendDefaultPii = sendDefaultPii + configureOptions() } ) } @@ -280,6 +282,60 @@ abstract class SentryApollo4BuilderExtensionsClientErrorsTest( ) } + @Test + fun `data collection can disable the GraphQL document independently`() { + val sut = + fixture.getSut(responseBody = fixture.responseBodyNotOk) { + dataCollection.graphql.setDocument(false) + } + executeQuery(sut) + + verify(fixture.scopes) + .captureEvent( + check { + val body = it.request!!.data as String + assertFalse(body.contains("\"query\"")) + assertTrue(body.contains("\"variables\"")) + }, + any(), + ) + } + + @Test + fun `data collection can disable GraphQL variables independently`() { + val sut = + fixture.getSut(responseBody = fixture.responseBodyNotOk) { + dataCollection.graphql.setVariables(false) + } + executeQuery(sut) + + verify(fixture.scopes) + .captureEvent( + check { + val body = it.request!!.data as String + assertTrue(body.contains("\"query\"")) + assertFalse(body.contains("\"variables\"")) + }, + any(), + ) + } + + @Test + fun `data collection can disable the GraphQL request body`() { + val sut = + fixture.getSut(responseBody = fixture.responseBodyNotOk) { + dataCollection.graphql.setDocument(false) + dataCollection.graphql.setVariables(false) + } + executeQuery(sut) + + verify(fixture.scopes) + .captureEvent( + check { assertNull(it.request!!.data) }, + any(), + ) + } + @Test fun `capture errors with more request context if sendDefaultPii is enabled`() { val sut = fixture.getSut(responseBody = fixture.responseBodyNotOk, sendDefaultPii = true) diff --git a/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4BuilderExtensionsTest.kt b/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4BuilderExtensionsTest.kt index 2c5b23adc4f..654ff307eba 100644 --- a/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4BuilderExtensionsTest.kt +++ b/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4BuilderExtensionsTest.kt @@ -23,6 +23,7 @@ import kotlin.reflect.KSuspendFunction1 import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertNull import kotlin.test.assertTrue import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking @@ -69,9 +70,9 @@ abstract class SentryApollo4BuilderExtensionsTest( }""", socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN, beforeSpan: BeforeSpanCallback? = null, + options: SentryOptions = SentryOptions().apply { dsn = "http://key@localhost/proj" }, ): ApolloClient { - whenever(scopes.options) - .thenReturn(SentryOptions().apply { dsn = "http://key@localhost/proj" }) + whenever(scopes.options).thenReturn(options) server.enqueue( MockResponse() @@ -105,6 +106,28 @@ abstract class SentryApollo4BuilderExtensionsTest( ) } + @Test + fun `does not attach GraphQL variables when data collection disables them`() { + val options = + SentryOptions().apply { + dsn = "http://key@localhost/proj" + dataCollection.graphql.setVariables(false) + } + + executeQuery(fixture.getSut(options = options)) + + verify(fixture.scopes) + .captureTransaction( + check { + assertNull(it.spans.first().data?.get("variables")) + assertNotNull(it.spans.first().data?.get("operationId")) + }, + anyOrNull(), + anyOrNull(), + anyOrNull(), + ) + } + @Test fun `creates span around failed request`() { executeQuery(fixture.getSut(httpStatusCode = 403)) diff --git a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt index e496d1055f3..b4fc25e7be2 100644 --- a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt +++ b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt @@ -74,7 +74,9 @@ class SentryApolloInterceptor( val requestWithHeader = request.toBuilder().requestHeaders(headers).build() span.setData("operationId", requestWithHeader.operation.operationId()) - span.setData("variables", requestWithHeader.operation.variables().valueMap().toString()) + if (scopes.options.dataCollectionResolver.isGraphqlVariablesWithLegacyAlways) { + span.setData("variables", requestWithHeader.operation.variables().valueMap().toString()) + } chain.proceedAsync( requestWithHeader, diff --git a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt index aaf9b30b7f3..d43fe40c9e4 100644 --- a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt +++ b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt @@ -121,6 +121,24 @@ class SentryApolloInterceptorTest { ) } + @Test + fun `does not attach GraphQL variables when data collection disables them`() { + fixture.options.dataCollection.graphql.setVariables(false) + + executeQuery() + + verify(fixture.scopes) + .captureTransaction( + check { + assertNull(it.spans.first().data?.get("variables")) + assertNotNull(it.spans.first().data?.get("operationId")) + }, + anyOrNull(), + anyOrNull(), + anyOrNull(), + ) + } + @Test fun `creates a span around the failed request`() { executeQuery(fixture.getSut(httpStatusCode = 403)) diff --git a/sentry-graphql-core/src/main/java/io/sentry/graphql/ExceptionReporter.java b/sentry-graphql-core/src/main/java/io/sentry/graphql/ExceptionReporter.java index 9bca0955e40..d53a6376e01 100644 --- a/sentry-graphql-core/src/main/java/io/sentry/graphql/ExceptionReporter.java +++ b/sentry-graphql-core/src/main/java/io/sentry/graphql/ExceptionReporter.java @@ -45,7 +45,7 @@ public void captureThrowable( final @NotNull Hint hint = new Hint(); setRequestDetailsOnEvent(scopes, exceptionDetails, event); - if (result != null && isAllowedToAttachBody(scopes)) { + if (result != null && isAllowedToAttachResponseBody(scopes)) { final @NotNull Response response = new Response(); final @NotNull Map responseBody = result.toSpecification(); response.setData(responseBody); @@ -55,7 +55,13 @@ public void captureThrowable( scopes.captureEvent(event, hint); } - private boolean isAllowedToAttachBody(final @NotNull IScopes scopes) { + private boolean isAllowedToAttachRequestBody(final @NotNull IScopes scopes) { + final @NotNull SentryOptions options = scopes.getOptions(); + return options.getDataCollectionResolver().isGraphqlDocumentWithLegacyBodyGate() + || options.getDataCollectionResolver().isGraphqlVariablesWithLegacyBodyGate(); + } + + private boolean isAllowedToAttachResponseBody(final @NotNull IScopes scopes) { final @NotNull SentryOptions options = scopes.getOptions(); return options.isSendDefaultPii() && !SentryOptions.RequestSize.NONE.equals(options.getMaxRequestBodySize()); @@ -80,20 +86,27 @@ private void setDetailsOnRequest( final @NotNull Request request) { request.setApiTarget("graphql"); - if (isAllowedToAttachBody(scopes) + if (isAllowedToAttachRequestBody(scopes) && (exceptionDetails.isSubscription() || captureRequestBodyForNonSubscriptions)) { final @NotNull Map data = new HashMap<>(); + final @NotNull SentryOptions options = scopes.getOptions(); - data.put("query", exceptionDetails.getQuery()); + if (options.getDataCollectionResolver().isGraphqlDocumentWithLegacyBodyGate()) { + data.put("query", exceptionDetails.getQuery()); + } - final @Nullable Map variables = exceptionDetails.getVariables(); - if (variables != null && !variables.isEmpty()) { - data.put("variables", variables); + if (options.getDataCollectionResolver().isGraphqlVariablesWithLegacyBodyGate()) { + final @Nullable Map variables = exceptionDetails.getVariables(); + if (variables != null && !variables.isEmpty()) { + data.put("variables", variables); + } } // for Spring HTTP this will be replaced by RequestBodyExtractingEventProcessor // for non subscription (websocket) errors - request.setData(data); + if (!data.isEmpty()) { + request.setData(data); + } } } diff --git a/sentry-graphql-core/src/test/kotlin/io/sentry/graphql/ExceptionReporterTest.kt b/sentry-graphql-core/src/test/kotlin/io/sentry/graphql/ExceptionReporterTest.kt index 759591d323c..316edf53ff4 100644 --- a/sentry-graphql-core/src/test/kotlin/io/sentry/graphql/ExceptionReporterTest.kt +++ b/sentry-graphql-core/src/test/kotlin/io/sentry/graphql/ExceptionReporterTest.kt @@ -14,6 +14,7 @@ import graphql.schema.GraphQLSchema import io.sentry.Hint import io.sentry.IScope import io.sentry.IScopes +import io.sentry.KeyValueCollectionBehavior import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.SentryOptions @@ -221,6 +222,37 @@ class ExceptionReporterTest { ) } + @Test + fun `data collection ignores the legacy max request body size option`() { + val options = + SentryOptions().also { + it.maxRequestBodySize = SentryOptions.RequestSize.NONE + it.dataCollection.graphql.setDocument(true) + it.dataCollection.graphql.setVariables(true) + } + val exceptionReporter = fixture.getSut(options) + + exceptionReporter.captureThrowable( + fixture.exception, + ExceptionReporter.ExceptionDetails( + fixture.scopes, + fixture.instrumentationExecutionParameters, + false, + ), + fixture.executionResult, + ) + + verify(fixture.scopes) + .captureEvent( + org.mockito.kotlin.check { + val data = it.request!!.data as Map + assertEquals(fixture.query, data["query"]) + assertEquals(fixture.variables, data["variables"]) + }, + any(), + ) + } + @Test fun `does not attach query or variables if sendDefaultPii is false`() { val exceptionReporter = @@ -254,6 +286,114 @@ class ExceptionReporterTest { ) } + @Test + fun `data collection can disable the query independently`() { + val options = fixture.defaultOptions + options.dataCollection.graphql.setDocument(false) + val exceptionReporter = fixture.getSut(options) + + exceptionReporter.captureThrowable( + fixture.exception, + ExceptionReporter.ExceptionDetails( + fixture.scopes, + fixture.instrumentationExecutionParameters, + false, + ), + fixture.executionResult, + ) + + verify(fixture.scopes) + .captureEvent( + org.mockito.kotlin.check { + val data = it.request!!.data as Map + assertNull(data["query"]) + assertEquals(fixture.variables, data["variables"]) + }, + any(), + ) + } + + @Test + fun `data collection can disable variables independently`() { + val options = fixture.defaultOptions + options.dataCollection.graphql.setVariables(false) + val exceptionReporter = fixture.getSut(options) + + exceptionReporter.captureThrowable( + fixture.exception, + ExceptionReporter.ExceptionDetails( + fixture.scopes, + fixture.instrumentationExecutionParameters, + false, + ), + fixture.executionResult, + ) + + verify(fixture.scopes) + .captureEvent( + org.mockito.kotlin.check { + val data = it.request!!.data as Map + assertEquals(fixture.query, data["query"]) + assertNull(data["variables"]) + }, + any(), + ) + } + + @Test + fun `data collection can disable both query and variables`() { + val options = fixture.defaultOptions + options.dataCollection.graphql.setDocument(false) + options.dataCollection.graphql.setVariables(false) + val exceptionReporter = fixture.getSut(options) + + exceptionReporter.captureThrowable( + fixture.exception, + ExceptionReporter.ExceptionDetails( + fixture.scopes, + fixture.instrumentationExecutionParameters, + false, + ), + fixture.executionResult, + ) + + verify(fixture.scopes) + .captureEvent( + org.mockito.kotlin.check { assertNull(it.request!!.data) }, + any(), + ) + } + + @Test + fun `data collection namespace defaults enable query and variables`() { + val options = + SentryOptions().also { + it.maxRequestBodySize = SentryOptions.RequestSize.ALWAYS + it.dataCollection.cookies = KeyValueCollectionBehavior.off() + } + val exceptionReporter = fixture.getSut(options) + + exceptionReporter.captureThrowable( + fixture.exception, + ExceptionReporter.ExceptionDetails( + fixture.scopes, + fixture.instrumentationExecutionParameters, + false, + ), + fixture.executionResult, + ) + + verify(fixture.scopes) + .captureEvent( + org.mockito.kotlin.check { + val data = it.request!!.data as Map + assertEquals(fixture.query, data["query"]) + assertEquals(fixture.variables, data["variables"]) + }, + any(), + ) + } + @Test fun `attaches query and variables if spring and subscription`() { val exceptionReporter = fixture.getSut(captureRequestBodyForNonSubscriptions = false) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index bfdc8ff97fe..4ab82709159 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -425,7 +425,11 @@ public final class io/sentry/DataCollectionResolver { public fun isDataCollectionConfigured ()Z public fun isDatabaseQueryData ()Z public fun isGraphqlDocument ()Z + public fun isGraphqlDocumentWithLegacyAlways ()Z + public fun isGraphqlDocumentWithLegacyBodyGate ()Z public fun isGraphqlVariables ()Z + public fun isGraphqlVariablesWithLegacyAlways ()Z + public fun isGraphqlVariablesWithLegacyBodyGate ()Z public fun isIncomingRequestBody ()Z public fun isIncomingResponseBody ()Z public fun isOutgoingRequestBody ()Z @@ -7765,6 +7769,10 @@ public final class io/sentry/util/FileUtils { public static fun readText (Ljava/io/File;)Ljava/lang/String; } +public final class io/sentry/util/GraphqlUtils { + public static fun filterRequestBody (Ljava/lang/String;Lio/sentry/SentryOptions;)Ljava/lang/String; +} + public final class io/sentry/util/HintUtils { public static fun createWithTypeCheckHint (Ljava/lang/Object;)Lio/sentry/Hint; public static fun getEventDropReason (Lio/sentry/Hint;)Lio/sentry/hints/EventDropReason; diff --git a/sentry/src/main/java/io/sentry/DataCollectionResolver.java b/sentry/src/main/java/io/sentry/DataCollectionResolver.java index 3063268f0f7..cdfb0649188 100644 --- a/sentry/src/main/java/io/sentry/DataCollectionResolver.java +++ b/sentry/src/main/java/io/sentry/DataCollectionResolver.java @@ -35,10 +35,30 @@ public boolean isGraphqlDocument() { return explicitOrSendDefaultPii(options.getDataCollection().getGraphql().getDocument(), true); } + public boolean isGraphqlDocumentWithLegacyBodyGate() { + return explicitOrDefault( + options.getDataCollection().getGraphql().getDocument(), true, isLegacyGraphqlBodyEnabled()); + } + + public boolean isGraphqlDocumentWithLegacyAlways() { + return explicitOrDefault(options.getDataCollection().getGraphql().getDocument(), true, true); + } + public boolean isGraphqlVariables() { return explicitOrSendDefaultPii(options.getDataCollection().getGraphql().getVariables(), true); } + public boolean isGraphqlVariablesWithLegacyBodyGate() { + return explicitOrDefault( + options.getDataCollection().getGraphql().getVariables(), + true, + isLegacyGraphqlBodyEnabled()); + } + + public boolean isGraphqlVariablesWithLegacyAlways() { + return explicitOrDefault(options.getDataCollection().getGraphql().getVariables(), true, true); + } + public @NotNull KeyValueCollectionBehavior getCookies() { final @NotNull DataCollection dataCollection = options.getDataCollection(); final @Nullable KeyValueCollectionBehavior cookies = dataCollection.getCookies(); @@ -80,12 +100,22 @@ public boolean isOutgoingResponseBody() { return isHttpBodyEnabled(HttpBodyType.OUTGOING_RESPONSE, options.isSendDefaultPii()); } + private boolean isLegacyGraphqlBodyEnabled() { + return options.isSendDefaultPii() + && !SentryOptions.RequestSize.NONE.equals(options.getMaxRequestBodySize()); + } + private boolean explicitOrSendDefaultPii( final @Nullable Boolean explicit, final boolean defaultValue) { + return explicitOrDefault(explicit, defaultValue, options.isSendDefaultPii()); + } + + private boolean explicitOrDefault( + final @Nullable Boolean explicit, final boolean defaultValue, final boolean legacyFallback) { if (explicit != null) { return explicit; } - return isDataCollectionConfigured() ? defaultValue : options.isSendDefaultPii(); + return isDataCollectionConfigured() ? defaultValue : legacyFallback; } private @NotNull KeyValueCollectionBehavior explicitOrEmptyDenyList( diff --git a/sentry/src/main/java/io/sentry/util/GraphqlUtils.java b/sentry/src/main/java/io/sentry/util/GraphqlUtils.java new file mode 100644 index 00000000000..30c164e3a00 --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/GraphqlUtils.java @@ -0,0 +1,53 @@ +package io.sentry.util; + +import io.sentry.DataCollectionResolver; +import io.sentry.JsonObjectReader; +import io.sentry.SentryLevel; +import io.sentry.SentryOptions; +import java.io.StringReader; +import java.util.LinkedHashMap; +import java.util.Map; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class GraphqlUtils { + + private GraphqlUtils() {} + + public static @Nullable String filterRequestBody( + final @NotNull String body, final @NotNull SentryOptions options) { + final @NotNull DataCollectionResolver resolver = options.getDataCollectionResolver(); + final boolean includeDocument = resolver.isGraphqlDocumentWithLegacyAlways(); + final boolean includeVariables = resolver.isGraphqlVariablesWithLegacyAlways(); + + if (includeDocument && includeVariables) { + return body; + } + if (!includeDocument && !includeVariables) { + return null; + } + + try (JsonObjectReader reader = new JsonObjectReader(new StringReader(body))) { + final @Nullable Object value = reader.nextObjectOrNull(); + if (!(value instanceof Map)) { + return null; + } + + @SuppressWarnings("unchecked") + final @NotNull Map requestBody = (Map) value; + final @NotNull Map filtered = new LinkedHashMap<>(requestBody); + if (!includeDocument) { + filtered.remove("query"); + } + if (!includeVariables) { + filtered.remove("variables"); + } + return options.getSerializer().serialize(filtered); + } catch (Throwable e) { + options.getLogger().log(SentryLevel.ERROR, "Failed to filter GraphQL request body.", e); + return null; + } + } +} diff --git a/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt b/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt index 73f1ba93dc9..47f637f0a08 100644 --- a/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt +++ b/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt @@ -89,6 +89,70 @@ class DataCollectionResolverTest { assertThat(options.dataCollectionResolver.isGraphqlDocument).isFalse() } + @Test + fun `GraphQL variables fall back to sendDefaultPii and override takes precedence`() { + val options = SentryOptions().apply { isSendDefaultPii = true } + + assertThat(options.dataCollectionResolver.isGraphqlVariables).isTrue() + + options.dataCollection.graphql.setVariables(false) + + assertThat(options.dataCollectionResolver.isGraphqlVariables).isFalse() + } + + @Test + fun `GraphQL legacy body variants preserve the legacy size gate when namespace is absent`() { + val options = + SentryOptions().apply { + isSendDefaultPii = true + maxRequestBodySize = SentryOptions.RequestSize.NONE + } + + assertThat(options.dataCollectionResolver.isGraphqlDocumentWithLegacyBodyGate).isFalse() + assertThat(options.dataCollectionResolver.isGraphqlVariablesWithLegacyBodyGate).isFalse() + + options.maxRequestBodySize = SentryOptions.RequestSize.SMALL + + assertThat(options.dataCollectionResolver.isGraphqlDocumentWithLegacyBodyGate).isTrue() + assertThat(options.dataCollectionResolver.isGraphqlVariablesWithLegacyBodyGate).isTrue() + } + + @Test + fun `GraphQL legacy body variants ignore the size option when namespace is explicit`() { + val options = + SentryOptions().apply { + isSendDefaultPii = false + maxRequestBodySize = SentryOptions.RequestSize.NONE + dataCollection.graphql.setDocument(true) + dataCollection.graphql.setVariables(true) + } + + assertThat(options.dataCollectionResolver.isGraphqlDocumentWithLegacyBodyGate).isTrue() + assertThat(options.dataCollectionResolver.isGraphqlVariablesWithLegacyBodyGate).isTrue() + } + + @Test + fun `GraphQL document legacy always variant preserves collection when namespace is absent`() { + val options = SentryOptions().apply { isSendDefaultPii = false } + + assertThat(options.dataCollectionResolver.isGraphqlDocumentWithLegacyAlways).isTrue() + + options.dataCollection.graphql.setDocument(false) + + assertThat(options.dataCollectionResolver.isGraphqlDocumentWithLegacyAlways).isFalse() + } + + @Test + fun `GraphQL variables legacy always variant preserves collection when namespace is absent`() { + val options = SentryOptions().apply { isSendDefaultPii = false } + + assertThat(options.dataCollectionResolver.isGraphqlVariablesWithLegacyAlways).isTrue() + + options.dataCollection.graphql.setVariables(false) + + assertThat(options.dataCollectionResolver.isGraphqlVariablesWithLegacyAlways).isFalse() + } + @Test fun `cookies are off when unset and sendDefaultPii is false`() { val options = SentryOptions() @@ -217,15 +281,4 @@ class DataCollectionResolverTest { assertThat(options.dataCollectionResolver.isIncomingResponseBody).isFalse() assertThat(options.dataCollectionResolver.isOutgoingResponseBody).isTrue() } - - @Test - fun `GraphQL variables fall back to sendDefaultPii and override takes precedence`() { - val options = SentryOptions().apply { isSendDefaultPii = true } - - assertThat(options.dataCollectionResolver.isGraphqlVariables).isTrue() - - options.dataCollection.graphql.setVariables(false) - - assertThat(options.dataCollectionResolver.isGraphqlVariables).isFalse() - } }