ref(options): migrate allocation-policy config to sentry-options#8168
Open
phacops wants to merge 12 commits into
Open
ref(options): migrate allocation-policy config to sentry-options#8168phacops wants to merge 12 commits into
phacops wants to merge 12 commits into
Conversation
Final slice of the runtime-config → sentry-options migration. The
allocation policies were the last ConfigurableComponents still reading
the legacy Redis runtime config through the base
`ConfigurableComponent.get_config_value`.
`get_config_value` now reads the centrally-managed
`configurable_component_overrides` sentry-option (values stored as
numbers, cast to each config's declared int/float type) and then the
code default — the legacy Redis runtime config is no longer consulted
for any ConfigurableComponent. This is the same behavior the
storage-routing strategies already had via a `get_config_value`
override; that override is now redundant and removed. No new schema
keys are needed: allocation-policy overrides flow through the existing
`configurable_component_overrides` dict, keyed by the same
fully-qualified config key (`{resource}.{ClassName}.{config}[.params]`).
`set_config_value` / `delete_config_value` are retained (snuba-admin
still writes them to Redis, surfaced via `get_current_configs`), so
editing moves to sentry-options-automator rather than snuba-admin,
consistent with the rest of this migration.
Tests that toggled config via `set_config_value(...)` round-trips now
use a shared `override_component_config` /
`override_component_configs` helper (moved to `tests/configs/`), which
sets the sentry-option for the duration of a context.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…indow test Addresses review feedback: after moving to override_component_configs, test_enforcement_switch had is_enforced=0 for the whole context and no longer exercised the on→off switch. Split it into two sequential override contexts — first enforced (exhaust the quota via the real `progress_bytes` profile key and assert the query is throttled), then enforcement off (assert the still-over-quota query runs unthrottled). The sliding-window usage persists in Redis across the two contexts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Addresses review feedback: test_override's second context set only referrer_concurrent_override, dropping the combined-override scenario the original had (both overrides accumulated in Redis). Use override_component_configs to keep referrer_max_threads_override and referrer_concurrent_override active together, matching the equivalent test_per_referrer test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
Update the sentry-options dependency to the latest release (1.2.1) on both the Python (pyproject.toml / uv.lock) and Rust (rust_snuba/Cargo.toml / Cargo.lock) sides so the options schema can declare nested object values. sentry-options-validation 1.2.1 adds a `chrono ^0.4.45` dependency, which pulls chrono forward in the Rust lockfile. The validate-sentry-options CI workflow derives the CLI version from uv.lock, so it tracks the bump automatically. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
The shared configurable_component_overrides option can only hold a single value type (the sentry-options meta-schema forbids type-unions), so it can't carry both numeric and object config values. Add a companion option, configurable_component_object_overrides, keyed by the same fully-qualified config key but whose values are nested objects (subkey -> number). This lets an allocation-policy (or any ConfigurableComponent) config declare value_type=dict and hold a structured set of values — e.g. per-organization overrides collapsed under one key instead of many parameterized flat keys. get_config_value routes object-typed configs (value_type is dict) to the new option and numeric configs to the existing one; behavior for all current (numeric) configs is unchanged. The override_component_config test helper routes by the config's declared value_type to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
Provide a single way to specify a ConfigurableComponent setting for a
given org, a given referrer, or both at once, replacing the need for
three separate parameterized configs.
configurable_component_object_overrides now holds two-level nested
objects {organization_id (or "*"): {referrer (or "*"): number}}. The new
resolve_scoped_override helper reads such a value most-specific-first —
(org, referrer) > (org, "*") > ("*", referrer) > code default — so one
object-typed config (value_type=dict) can target an org, a referrer, or
both. Org ids are stringified to match the JSON keys, and a missing org
or referrer simply skips the lookups that would need it.
get_config_value already returns the nested dict as-is for object-typed
configs, so no read-path change was needed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
…nfig get_config_value now reads the sentry-option first and, when a numeric config key is absent, falls back to the legacy Redis runtime config before the code default — so values set directly in Redis / snuba-admin are still honored. This is gated by a new _falls_back_to_runtime_config class flag (True by default for allocation policies and generic components); storage-routing strategies set it False to stay option-then-default, matching their existing fully-migrated behavior. Object-typed configs have no legacy Redis representation, so they never consult Redis and fall straight through to the code default. Schema descriptions and tests updated to cover the fallback and the opt-out. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
…olicy config" This reverts commit d866012. Config resolution stays sentry-options only (option -> code default); no legacy Redis fallback. The global per-class value and the org/referrer override are both expressed through sentry-options. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
… config
Wire resolve_scoped_override into BytesScannedRejectingPolicy so the
scan-limit for an organization_id query is resolved from a single
object-typed config, organization_referrer_scan_limit_overrides, shaped
{organization_id (or '*'): {referrer (or '*'): limit}}:
(org, referrer) > (org, '*') > ('*', referrer) > organization_referrer_scan_limit
The global-per-class default remains the numeric organization_referrer_scan_limit
config. This replaces the three parameterized override configs
(organization_referrer_scan_limit_override, organization_scan_limit_override,
referrer_all_organizations_scan_limit_override) with one nested-object
config read from configurable_component_object_overrides — all
sentry-options, no Redis. The project path (referrer_all_projects_scan_limit_override)
and the max_bytes_to_read caps are unchanged. Gives resolve_scoped_override
a production consumer.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
Resolved dependency-version conflicts (all sentry-options related): - pyproject.toml: keep sentry-options>=1.2.1 (nested-object support), take master's sentry-protos>=0.43.0. - rust_snuba/Cargo.toml: keep sentry-options=1.2.1, take master's sentry-kafka-schemas=2.1.39. - uv.lock: rebased on master's lock, re-locked to sentry-options 1.2.1. - Cargo.lock: auto-merged, consistent (sentry-options / -validation 1.2.1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
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 a3eee8f. Configure here.
sentry-options 1.2.1 deprecated init_with_schemas; the Rust lint runs
clippy with -D warnings, so the deprecation broke the build. Replace all
usages (consumers plus test modules) with the new builder API:
Options::builder().with_schemas(&[("snuba", SNUBA_SCHEMA)]).init().
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
The new Options::builder().init() returns Err(AlreadyInitialized) on a second call, whereas the deprecated init_with_schemas swallowed it. Unit tests across files each call init once against the shared global store, so the second file panicked (and poisoned its Once), failing 14 tests. Add a crate-level init_sentry_options() helper that ignores AlreadyInitialized (Ok), matching the old idempotent behavior, and route every call site (consumers + test modules) through it. Removes the now-unused per-file `use sentry_options::Options;` imports. Verified locally: cargo fmt clean, clippy -D warnings clean, and the 14 previously-failing options tests now pass (the only remaining local failures are ClickHouse/Python integration tests that need services not present in the sandbox and pass in CI). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mg9CJoJ54QdhNbTu8r2g4r
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Final slice of the runtime-config → sentry-options migration. The allocation policies were the last
ConfigurableComponents still reading the legacy Redis runtime config, through the baseConfigurableComponent.get_config_value.get_config_valuenow reads the centrally-managedconfigurable_component_overridessentry-option (values stored as numbers, cast to each config's declaredint/floattype) and then the code default — the legacy Redis runtime config is no longer consulted for anyConfigurableComponent(allocation policies and storage-routing strategies alike). This is exactly the behavior the storage-routing strategies already had via a per-subclassget_config_valueoverride (added in #8115); that override is now redundant and has been removed.No new schema keys are needed: allocation-policy overrides flow through the existing
configurable_component_overridesdict option, keyed by the same fully-qualified config key the configs have always used ({resource}.{ClassName}.{config}[.{param}:{value},...]). The schema description is updated to drop the now-obsolete "falls back to Redis" note.set_config_value/delete_config_valueare retained — snuba-admin still writes them to Redis and surfaces them viaget_current_configs— so live editing moves to sentry-options-automator rather than snuba-admin, consistent with the rest of this migration. Behavior is unchanged until a key is set in sentry-options-automator (defaults match the prior code defaults).Keys migrated here
All allocation-policy configs, which already live under
configurable_component_overrides(is_active,is_enforced,max_threads, and every policy-specific config such asconcurrent_limit,project_referrer_scan_limit, the per-org / per-referrer scan-limit and max-bytes overrides, throttle dividers, etc.).Tests
Tests that toggled config via
set_config_value(...)round-trips now use a sharedoverride_component_config/override_component_configshelper, moved totests/configs/component_config.pyand re-exported from the routing-strategy testcommon.py. The helper sets theconfigurable_component_overridessentry-option for the duration of a context; because that option is a single dict, configs that must be active together are combined into oneoverride_component_configs(...)call rather than nested contexts. Validation-raisingset_config_valuecases and the Redis-backedget_current_configsadmin-display assertions are left as-is.Verification:
ruff check+ruff format --checkclean; the redis-backed allocation-policy, configurable-component and admin config tests pass locally (tests/query/allocation_policies/,tests/test_configurable_component.py). The ClickHouse-dependent API tests (test_snql_api) aren't runnable in the sandbox and are covered by CI.Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
🤖 Generated with Claude Code
Generated by Claude Code