Skip to content

fix(spans-migration): make script to migrate the AM1 metrics queries to transactions#112621

Merged
nikkikapadia merged 4 commits intomasterfrom
nikki/fix/am1-metrics-to-transaction-alerts-script
Apr 14, 2026
Merged

fix(spans-migration): make script to migrate the AM1 metrics queries to transactions#112621
nikkikapadia merged 4 commits intomasterfrom
nikki/fix/am1-metrics-to-transaction-alerts-script

Conversation

@nikkikapadia
Copy link
Copy Markdown
Member

welp another migration 😞

turns out we have a bunch of generic metrics alerts on am1 💀 but they really shouldn't be on generic metrics cause am1 doesn't have dynamic sampling (oh and we're killing metrics backend so lovely) so these should all be transactions alerts. Gonna migrate these because lowkey they haven't even been working for a while and who knows maybe this will help fix some other issues we have.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 9, 2026
@nikkikapadia nikkikapadia marked this pull request as ready for review April 9, 2026 20:48
@nikkikapadia nikkikapadia requested a review from a team as a code owner April 9, 2026 20:48
Comment thread src/sentry/explore/translation/am1_metrics_to_transactions.py
"dataset": snuba_query.dataset,
"query": snuba_query.query,
"aggregate": snuba_query.aggregate,
}
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.

Snapshot format missing keys breaks alerts_translation rollback

Medium Severity

The am1 snapshot_snuba_query writes a snapshot to the shared SnubaQuery.query_snapshot field that omits type and time_window keys. The existing alerts_translation.py rollback directly accesses snapshot["type"] and snapshot["time_window"], which would raise a KeyError if it encounters an am1-format snapshot. Since the am1 migration moves queries to Transactions dataset — a valid source for the alerts_translation migration — and both modules skip snapshot creation when one already exists, the alerts_translation could inherit an incompatible am1 snapshot if both migrations are applied sequentially to the same SnubaQuery.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9b74711. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we shouldn't need to run the alerts translation script again, if we do we will make the appropriate edits to it but this won't affect what's going to happen for these runs

Comment thread src/sentry/explore/translation/am1_metrics_to_transactions.py
Comment thread src/sentry/explore/translation/am1_metrics_to_transactions.py
Comment thread src/sentry/explore/translation/am1_metrics_to_transactions.py Outdated
Comment thread src/sentry/explore/translation/am1_metrics_to_transactions.py
Comment thread src/sentry/explore/translation/am1_metrics_to_transactions.py Outdated
Comment thread src/sentry/explore/translation/am1_metrics_to_transactions.py Outdated
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.

There are 2 total unresolved issues (including 1 from previous review).

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 8ad8c86. Configure here.

Comment thread src/sentry/explore/translation/am1_metrics_to_transactions.py Outdated
Comment on lines +143 to +148
)
return

if snuba_query.dataset == Dataset.PerformanceMetrics.value:
logger.info("Query already migrated to metrics", extra={"snuba_query_id": snuba_query.id})
return
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 rollback function rollback_am1_metrics_detector_query_and_update_subscription_in_snuba will silently fail if the organizations:migrate-am1-metrics-alerts-to-transactions feature flag is disabled, preventing rollbacks.
Severity: HIGH

Suggested Fix

Introduce a bypass_flag_check: bool = False parameter to the rollback_am1_metrics_detector_query_and_update_subscription_in_snuba function. The feature flag check should be skipped if bypass_flag_check is True, allowing rollbacks to proceed regardless of the flag's state. This aligns with the pattern used in similar migration rollback functions.

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/explore/translation/am1_metrics_to_transactions.py#L143-L148

Potential issue: The
`rollback_am1_metrics_detector_query_and_update_subscription_in_snuba` function
unconditionally checks for the
`organizations:migrate-am1-metrics-alerts-to-transactions` feature flag before
executing. If a migration is performed and the flag is later disabled for any reason,
any attempt to roll back the migration will silently fail, logging only an INFO message.
This leaves the system in an inconsistent state, as the alert queries remain migrated to
the Transactions dataset with no way to revert them without re-enabling the feature
flag. This behavior is inconsistent with other migration patterns that include a bypass
mechanism for rollbacks.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

that's kind of like... the whole point 😵‍💫

@nikkikapadia nikkikapadia merged commit 9e98c19 into master Apr 14, 2026
56 checks passed
@nikkikapadia nikkikapadia deleted the nikki/fix/am1-metrics-to-transaction-alerts-script branch April 14, 2026 19:10
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