feat(gql): Gate GraphQL data collection behind data_collection option - #6889
feat(gql): Gate GraphQL data collection behind data_collection option#6889ericapisani wants to merge 4 commits into
Conversation
…ption The graphene integration now honors the experimental structured data_collection configuration when deciding whether to attach GraphQL-specific data. When data_collection is set, graphql.document controls whether request.api_target is marked as graphql on error events and whether the graphql.document attribute is attached to query/mutation spans (both streamed and transaction-embedded). When data_collection is not configured, behavior falls back to the existing send_default_pii gating, and data_collection takes precedence when both options are set. Adds tests covering the new gating for the event processor and both span paths, including precedence over send_default_pii.
… option Apply the data_collection option to the Strawberry integration so that graphql.document, request data, and variables are only collected when explicitly enabled. send_default_pii is used as a fallback only when data_collection is not configured. GraphQL operation names are now always captured when available, independent of document or PII settings.
Attach the GraphQL document and variable definitions to error events based on the structured data_collection configuration (graphql.document / graphql.variables), falling back to send_default_pii when data_collection is not set.
Codecov Results 📊✅ 99362 passed | ⏭️ 6806 skipped | Total: 106168 | Pass Rate: 93.59% | Execution Time: 359m 54s 📊 Comparison with Base Branch
All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 2570 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 89.54% 89.55% +0.01%
==========================================
Files 193 193 —
Lines 24588 24603 +15
Branches 8744 8756 +12
==========================================
+ Hits 22018 22033 +15
- Misses 2570 2570 —
- Partials 1408 1409 +1Generated by Codecov Action |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| response.update( | ||
| { | ||
| "data": {"errors": errors}, | ||
| "type": response, |
There was a problem hiding this comment.
Bug: The response context dictionary is updated with a self-reference ("type": response), which will cause a ValueError during JSON serialization, preventing event capture.
Severity: CRITICAL
Suggested Fix
The value for the "type" key in the response context dictionary should be a string literal, such as "response", instead of the dictionary object itself. This will prevent the circular reference and allow the event to be serialized correctly.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sentry_sdk/integrations/gql.py#L176
Potential issue: In the `processor` function, when GraphQL data collection is enabled
and an error occurs, the `response` context is updated with a self-referential key:
`response.update(..., "type": response)`. This creates a circular reference within the
dictionary. When the Sentry SDK attempts to serialize the event to JSON using
`json.dumps()`, it will encounter this circular reference and raise a `ValueError`,
causing the event capture to fail. This affects any user who enables the new GraphQL
document collection feature.
Also affects:
sentry_sdk/integrations/gql.py:180~183
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 058e048. Configure here.
| else: | ||
| document = document_or_request | ||
|
|
||
| request["data"] = _data_from_document(document) |
There was a problem hiding this comment.
Variables require document collection
Medium Severity
graphql.variables is only consulted inside _data_from_document, which runs only when graphql.document is enabled. With document off and variables on, variable data is never attached. The Strawberry integration and its tests treat these flags as independent under the same data_collection contract.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 058e048. Configure here.
2 participants
Attach the GraphQL document and variable definitions to error events based on the structured data_collection configuration (graphql.document / graphql.variables), falling back to send_default_pii when data_collection is not set.
Refs PY-2585
Refs #6745