-
Notifications
You must be signed in to change notification settings - Fork 644
feat(gql): Gate GraphQL data collection behind data_collection option #6889
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9562fff
a6055cd
058e048
4225d03
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| from sentry_sdk.utils import ( | ||
| ensure_integration_enabled, | ||
| event_from_exception, | ||
| has_data_collection_enabled, | ||
| parse_version, | ||
| ) | ||
|
|
||
|
|
@@ -54,12 +55,18 @@ | |
|
|
||
|
|
||
| def _data_from_document(document: "DocumentNode") -> "EventDataType": | ||
| client_options = sentry_sdk.get_client().options | ||
| try: | ||
| operation_ast = get_operation_ast(document) | ||
| data: "EventDataType" = {"query": print_ast(document)} | ||
|
|
||
| if operation_ast is not None: | ||
| data["variables"] = operation_ast.variable_definitions | ||
| if has_data_collection_enabled(client_options): | ||
| if client_options["data_collection"]["graphql"]["variables"]: | ||
| data["variables"] = operation_ast.variable_definitions | ||
| elif should_send_default_pii(): | ||
| data["variables"] = operation_ast.variable_definitions | ||
|
|
||
| if operation_ast.name is not None: | ||
| data["operationName"] = operation_ast.name.value | ||
|
|
||
|
|
@@ -147,7 +154,30 @@ | |
| } | ||
| ) | ||
|
|
||
| if should_send_default_pii(): | ||
| client_options = sentry_sdk.get_client().options | ||
| if has_data_collection_enabled(client_options): | ||
| if client_options["data_collection"]["graphql"]["document"]: | ||
| if GraphQLRequest is not None and isinstance( | ||
| document_or_request, GraphQLRequest | ||
| ): | ||
| # In v4.0.0, gql moved to using GraphQLRequest instead of | ||
| # DocumentNode in execute | ||
| # https://github.com/graphql-python/gql/pull/556 | ||
| document = document_or_request.document | ||
| else: | ||
| document = document_or_request | ||
|
|
||
| request["data"] = _data_from_document(document) | ||
| contexts = event.setdefault("contexts", {}) | ||
| response = contexts.setdefault("response", {}) | ||
| response.update( | ||
|
Check warning on line 173 in sentry_sdk/integrations/gql.py
|
||
| { | ||
| "data": {"errors": errors}, | ||
| "type": response, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The Suggested FixThe value for the Prompt for AI AgentAlso affects:
Did we get this right? 👍 / 👎 to inform future reviews. |
||
| } | ||
| ) | ||
|
|
||
| elif should_send_default_pii(): | ||
| if GraphQLRequest is not None and isinstance( | ||
| document_or_request, GraphQLRequest | ||
| ): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variables require document collection
Medium Severity
graphql.variablesis only consulted inside_data_from_document, which runs only whengraphql.documentis enabled. Withdocumentoff andvariableson, variable data is never attached. The Strawberry integration and its tests treat these flags as independent under the samedata_collectioncontract.Additional Locations (1)
sentry_sdk/integrations/gql.py#L63-L68Reviewed by Cursor Bugbot for commit 058e048. Configure here.