feat(data-collection): Change stack_frame_variables to KeyValueCollection - #3066
feat(data-collection): Change stack_frame_variables to KeyValueCollection#3066sl0thentr0py wants to merge 1 commit into
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| data_collection.graphql.variables = false | ||
| data_collection.database_query_data = false | ||
| data_collection.queues = false | ||
| data_collection.stack_frame_variables = configuration.include_local_variables | ||
| data_collection.frame_context_lines = configuration.context_lines | ||
| data_collection | ||
| end |
There was a problem hiding this comment.
Bug: Setting config.include_local_variables = true before config.send_default_pii causes the local variable setting to be silently ignored due to the order of operations during initialization.
Severity: MEDIUM
Suggested Fix
Reinstate the logic in the DataCollection.backfill method to propagate the value from the deprecated configuration.include_local_variables setting to the new data_collection.stack_frame_variables instance variable. This ensures that the setting is preserved regardless of the configuration order. Also, add a test case that covers setting both include_local_variables and send_default_pii in the same configuration block to prevent regressions.
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-ruby/lib/sentry/data_collection.rb#L146-L151
Potential issue: When a user sets `config.include_local_variables = true` before
`config.send_default_pii` in the `Sentry.init` block, the setting for local variables is
lost. This happens because the setter for `send_default_pii` calls
`DataCollection.backfill`, which creates a new `DataCollection` instance. The new code
removed a line that previously propagated the `include_local_variables` value to this
new instance. As a result, `stack_frame_variables` defaults to `:off`, and no local
variables are captured, silently ignoring the user's explicit configuration.
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.
Reviewed by Cursor Bugbot for commit 69a2c7f. Configure here.
| data_collection.graphql.variables = false | ||
| data_collection.database_query_data = false | ||
| data_collection.queues = false | ||
| data_collection.stack_frame_variables = configuration.include_local_variables |
There was a problem hiding this comment.
Backfill drops local variables setting
Medium Severity
Setting include_local_variables no longer survives a later send_default_pii= assignment. backfill replaces data_collection without copying that flag, so collect_stack_frame_variables? stays off and local variables are not captured.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 69a2c7f. Configure here.
1 participant
Issues