feat(dynamic-sampling): Add per-org sample rates summary log - #120650
Conversation
The per-org pipeline logs the EAP-vs-generic-metrics comparison as one line per project and one line per transaction, so reconstructing what an org looked like in a single cycle means joining many log entries. Add a single summary log at the end of the per-org task carrying the org sample rate plus, per project, the project sample rate and the transaction sample rates (named and implicit) for both the EAP and the generic metrics pipeline. The existing per-project and per-transaction comparison logs are unchanged. Gate it on a new deterministic rollout-rate option keyed on organization id, defaulting to off. The payload scales with projects x transactions, so the gate also covers the extra Redis reads the log needs: with it off, the transaction cache is read only for EAP-rebalanced projects and the project cache stays inside the balancing branch, matching the previous read volume. Pipeline the transaction cache reads into a single round trip, since serving the log widens that fetch to every project in the org. Co-authored-by: Claude <noreply@anthropic.com>
github-actions
Bot
added
the
Scope: Backend
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
…ple-rates-summary-log # Conflicts: # src/sentry/dynamic_sampling/per_org/gate.py
![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 95f2fe4. Configure here.
| cached_project_sample_rates=cached_sample_rates, | ||
| rebalanced_transactions=rebalanced_transactions, | ||
| cached_transaction_sample_rates=cached_transaction_sample_rates, | ||
| ) |
There was a problem hiding this comment.
Summary log skipped on early returns
Medium Severity
log_sample_rates_summary only runs after transaction balancing finishes, so orgs that exit at ALL_PROJECTS_AT_FULL_SAMPLE_RATE or NO_TRANSACTION_VOLUMES never emit the per-cycle summary. Project cache is already fetched when log_summary is on for those paths, but the log is never written, so rolled-out orgs under quota or without transaction volume are missing from the comparison.
Reviewed by Cursor Bugbot for commit 95f2fe4. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| def get_cached_organization_sample_rate(org_id: int) -> float | None: | ||
| """ | ||
| The organization sample rate the legacy (generic metrics) pipeline would serve: the | ||
| cached sliding-window rate, or the target sample rate option for custom sampling orgs. | ||
| Returns None on a cache miss instead of falling back to the blended rate, so the | ||
| comparison logging can distinguish "no cached value" from "cached value equals blended". | ||
| """ | ||
| sample_rate, _ = get_org_sample_rate(org_id=org_id, default_sample_rate=None) | ||
| return sample_rate |
There was a problem hiding this comment.
Bug: The function get_cached_organization_sample_rate incorrectly returns 1.0 instead of None on a cache miss for organizations with custom dynamic sampling but no explicit rate set.
Severity: LOW
Suggested Fix
Modify the get_org_sample_rate function. When it is called with default_sample_rate=None and an organization has dynamic-sampling-custom enabled without a specific sentry:target_sample_rate, it should return None instead of falling back to TARGET_SAMPLE_RATE_DEFAULT. This will align the function's behavior with the documented contract of its caller, get_cached_organization_sample_rate.
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: src/sentry/dynamic_sampling/per_org/calculations.py#L207-L215
Potential issue: The function `get_cached_organization_sample_rate` is documented to
return `None` on a cache miss to distinguish it from a cached value. However, for
organizations with the `dynamic-sampling-custom` feature enabled but no specific
`sentry:target_sample_rate` option set, it incorrectly returns a default value of `1.0`.
This is caused by an underlying helper, `get_org_sample_rate`, which falls back to
`TARGET_SAMPLE_RATE_DEFAULT` instead of propagating `None`. As a result, logging that
relies on this function will be misleading, making it impossible to differentiate
between a genuine cached value of `1.0` and a cache miss.
Did we get this right? 👍 / 👎 to inform future reviews.
2 participants
dynamic_sampling.per_org.sample_rates_summarylog per org per cycle with the org, project, and transaction sample rates — EAP and generic metrics side by side.dynamic-sampling.per_org.sample-rates-summary-log-rollout-rate, default off.