Skip to content

feat(dynamic-sampling): per-org transaction rebalancing#116475

Open
constantinius wants to merge 8 commits into
masterfrom
constantinius/feat/ds/-per-org-transaction-rebalancing
Open

feat(dynamic-sampling): per-org transaction rebalancing#116475
constantinius wants to merge 8 commits into
masterfrom
constantinius/feat/ds/-per-org-transaction-rebalancing

Conversation

@constantinius
Copy link
Copy Markdown
Contributor

  • Add calculations for transaction rebalancing to the per-org pipeline
  • Compare calculation outputs with the legacy pipeline and log results

Closes TET-2403

@constantinius constantinius requested a review from a team as a code owner May 29, 2026 13:47
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 29, 2026

TET-2403

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 29, 2026
@constantinius constantinius requested a review from shellmayr May 29, 2026 13:48
Comment thread src/sentry/dynamic_sampling/per_org/calculations.py
Comment thread src/sentry/dynamic_sampling/per_org/calculations.py Outdated
Comment thread src/sentry/dynamic_sampling/per_org/calculations.py Outdated
Comment thread src/sentry/dynamic_sampling/per_org/calculations.py Outdated
Comment thread src/sentry/dynamic_sampling/per_org/calculations.py Outdated
compare_rebalanced_projects_with_cache(config, rebalanced_projects, cached_sample_rates)

if not get_eap_transaction_volumes(config):
transaction_volumes = get_eap_transaction_volumes(config)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The default interval on this query is 5 minutes, the one in the legacy pipeline is 1 hour (here vs here) - if we are never going to use the 5 minutes, we should probably change the default.

Comment thread tests/sentry/dynamic_sampling/per_org/test_calculations.py Outdated
Comment thread src/sentry/dynamic_sampling/per_org/calculations.py Outdated
) -> dict[int, tuple[list[RebalancedItem], float]]:
intensity = options.get("dynamic-sampling.prioritise_transactions.rebalance_intensity")
sample_rates = _project_sample_rates(config)
return {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we not clamping the sample rates yet? If no, should we record information about it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not yet

constantinius and others added 2 commits May 29, 2026 17:01
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 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 c30e3ed. Configure here.

Comment thread src/sentry/dynamic_sampling/per_org/calculations.py Outdated
Comment on lines +132 to +142
TransactionsRebalancingInput(
classes=[
RebalancedItem(id=transaction_name, count=count)
for transaction_name, count in project_data["transaction_counts"]
],
sample_rate=sample_rate,
total_num_classes=project_data.get("total_num_classes"),
total=project_data.get("total_num_transactions"),
intensity=intensity,
)
)
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.

Bug: The call to TransactionsRebalancingModel().run() lacks error handling. An empty transaction_counts list will raise an unhandled InvalidModelInputError, crashing the background task.
Severity: MEDIUM

Suggested Fix

Before calling the model, add a guard to check if transaction_counts is empty and return early if it is. This mimics the safe handling present in the legacy code. Alternatively, wrap the call to TransactionsRebalancingModel().run() in a try...except InvalidModelInputError block to handle the validation failure gracefully without crashing the task.

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#L131-L142

Potential issue: In `run_transaction_balancing`, the call to
`TransactionsRebalancingModel().run()` is not wrapped in any error handling. The model's
`run` method will raise an `InvalidModelInputError` if its input validation fails, which
occurs if the `transaction_counts` list is empty. This is a realistic edge case for
projects that have no transactions passing the volume filter. The unhandled exception
propagates up, causing the `run_calculations_per_org_task` to fail and preventing
dynamic sampling calculations from completing for that organization.

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.

2 participants