Skip to content

feat: track read options via seen logline#115610

Open
joshuarli wants to merge 6 commits into
masterfrom
feat-sentry-option-seen
Open

feat: track read options via seen logline#115610
joshuarli wants to merge 6 commits into
masterfrom
feat-sentry-option-seen

Conversation

@joshuarli
Copy link
Copy Markdown
Member

@joshuarli joshuarli commented May 14, 2026

edit: this has now been changed to emit logs instead - no databases! and we want 100% sampling so the way to not slam logs is to just keep a seen cache on the options manager to emit at most 1 logline for each newly seen option


as part of the sentry options migration i'd like to clean up unused options which seem to represent a not-insignificant portion of options registered today

according to options.html in getsentry/sentry-options#123 there's about 1,100 total registered options in getsentry, of which 100 or so unreferenced options that should be safe to remove, and ~300 more dynamically generated options that couldn't be determined via static analysis alone, rather i identified them by warming up getsentry then reading all registered options

i'm not 100% sure how to reliably determine if those dynamically generated options are referenced so i want to x-reference them against the actual options db

the current sentry_options/sentry_controloptions tables do not have something like access time

so: this introduces a new sentry_option_seen table so I can determine the set of options that are not being read at all so that they can be deleted

since options.get is pretty hot my design is essentially a tripwire based on row existence, and on startup we read all rows so we can fastpath out (1 string set membership) without touching the db if an option's already read

i think even just a few days data would be sufficient to make the call?

also added a docs/MIGRATIONS.md to help agents write migrations!

@joshuarli joshuarli requested review from a team as code owners May 14, 2026 22:05
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 14, 2026
@joshuarli joshuarli requested a review from markstory May 14, 2026 22:06
region_silo_model does not exist in sentry.db.models; the correct
decorator for region/cell silo models is cell_silo_model, matching
what Option and ControlOption use.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Comment thread tests/sentry/tasks/test_options.py Outdated
Copy link
Copy Markdown
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

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

I'm a little wary about this because it's potentially a lot of querires on postgres.

One option is if instead of writing this to a table, we just write logs? There will be a lot, but if it's only a few days it won't be too expensive, and then we can just query them from GCP. We could also sample at 1% to reduce the cost here.

The other is putting this functionality behind an option if that's possible, so we can quickly revert it if it costs a lot of load during deploys.

Comment thread src/sentry/options/manager.py Outdated
Comment thread src/sentry/options/manager.py Outdated
Comment thread src/sentry/options/manager.py Outdated
@joshuarli joshuarli changed the title feat: track read options via sentry_option_seen feat: track read options via seen logline May 14, 2026
@joshuarli joshuarli requested a review from wedamija May 14, 2026 22:50
Comment thread src/sentry/options/manager.py Outdated
Comment thread src/sentry/options/manager.py
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

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 3 potential issues.

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 4685147. Configure here.

Comment thread tests/sentry/tasks/test_options.py Outdated
Comment thread tests/sentry/tasks/test_options.py Outdated
Comment thread src/sentry/options/manager.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants