Skip to content

feat(data-collection): Add base DataCollection configuration with defaults and backfill - #3022

Open
sl0thentr0py wants to merge 1 commit into
masterfrom
neel/data-collection/base
Open

feat(data-collection): Add base DataCollection configuration with defaults and backfill#3022
sl0thentr0py wants to merge 1 commit into
masterfrom
neel/data-collection/base

Conversation

@sl0thentr0py

@sl0thentr0py sl0thentr0py commented Jul 15, 2026

Copy link
Copy Markdown
Member

@linear-code

linear-code Bot commented Jul 15, 2026

Copy link
Copy Markdown

RUBY-188

RUBY-190

@sl0thentr0py
sl0thentr0py force-pushed the neel/data-collection/base branch 10 times, most recently from e46836a to 2bd8779 Compare July 22, 2026 12:12
@sl0thentr0py
sl0thentr0py force-pushed the neel/data-collection/base branch 3 times, most recently from 99ba0b8 to 370c6c1 Compare July 22, 2026 16:23
@sl0thentr0py
sl0thentr0py marked this pull request as ready for review July 23, 2026 19:22
Comment thread sentry-ruby/lib/sentry/configuration.rb

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 370c6c1. Configure here.

Comment thread sentry-ruby/lib/sentry/data_collection.rb
@dingsdax
dingsdax self-requested a review July 24, 2026 07:46

@dingsdax dingsdax left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ Ruby is just beautiful, lgtm 👍

@solnic solnic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except this one tiny formatting issue.

Comment thread sentry-ruby/lib/sentry/data_collection.rb
@sl0thentr0py
sl0thentr0py force-pushed the neel/data-collection/base branch 2 times, most recently from 199ed3f to 0b9189c Compare August 10, 2026 12:08
Comment thread sentry-ruby/lib/sentry/configuration.rb
@sl0thentr0py
sl0thentr0py force-pushed the neel/data-collection/base branch from 0b9189c to 3d85e61 Compare August 10, 2026 12:30
Comment on lines +125 to +126
# the new DataCollection defaults are already correct if pii is enabled
data_collection = new

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: When send_default_pii is set to true, the DataCollection.backfill method incorrectly ignores the include_local_variables setting, preventing local variable capture.
Severity: MEDIUM

Suggested Fix

In DataCollection.backfill, when configuration.send_default_pii is true, migrate the include_local_variables and context_lines settings from the main configuration to the new data_collection object before returning. Specifically, set data_collection.stack_frame_variables = configuration.include_local_variables and data_collection.frame_context_lines = configuration.context_lines.

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#L125-L126

Potential issue: When `send_default_pii` is set to `true`, the `DataCollection.backfill`
method returns a new `DataCollection` object without migrating the user's
`include_local_variables` setting. This happens due to an early return. As a result,
`data_collection.stack_frame_variables` defaults to `false`, even if the user explicitly
set `include_local_variables=true`. This causes local variables from exception frames to
be silently dropped, contrary to the user's configuration. This behavior is inconsistent
with the `send_default_pii=false` path, which correctly migrates the setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(pii): Add data_collection config object, accessors & defaults

3 participants