Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 184 additions & 0 deletions src/sentry/explore/translation/am1_metrics_to_transactions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
import logging

import sentry_sdk
from django.db import router

from sentry import features
from sentry.explore.translation.alerts_translation import _get_old_query_info
from sentry.incidents.utils.types import DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION
from sentry.snuba.dataset import Dataset
from sentry.snuba.models import (
QuerySubscription,
SnubaQuery,
SnubaQueryEventType,
)
from sentry.snuba.subscriptions import bulk_update_snuba_subscriptions
from sentry.utils.db import atomic_transaction
from sentry.workflow_engine.models.data_source import DataSource

logger = logging.getLogger(__name__)


def snapshot_snuba_query(snuba_query: SnubaQuery):
if not snuba_query.query_snapshot and snuba_query.dataset in [
Dataset.PerformanceMetrics.value,
]:
query_snapshot = {
"metrics_to_transactions": True,
}
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

snuba_query.query_snapshot = query_snapshot
snuba_query.save()

Comment thread
sentry[bot] marked this conversation as resolved.
return snuba_query


def translate_am1_metrics_detector_and_update_subscription_in_snuba(snuba_query: SnubaQuery):
query_subscription_qs = QuerySubscription.objects.filter(
snuba_query_id=snuba_query.id,
status__in=[QuerySubscription.Status.ACTIVE.value, QuerySubscription.Status.UPDATING.value],
)
query_subscription = query_subscription_qs.first()

if not query_subscription:
logger.info("No active query subscription found for snuba query %s", snuba_query.id)
return

try:
data_source: DataSource = DataSource.objects.get(
source_id=str(query_subscription.id), type=DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION
)
except DataSource.DoesNotExist as e:
logger.info("Data source not found for snuba query %s", snuba_query.id)
sentry_sdk.capture_exception(e)
return
if not features.has(
"organizations:migrate-am1-metrics-alerts-to-transactions", data_source.organization
):
logger.info("Feature flag not enabled")
return

snapshot_snuba_query(snuba_query)

snapshot = snuba_query.query_snapshot
if not snapshot and snuba_query.dataset == Dataset.PerformanceMetrics.value:
logger.info("No snapshot created for snuba query %s", snuba_query.id)
return
Comment thread
cursor[bot] marked this conversation as resolved.

if snapshot and snapshot.get("user_updated"):
logger.info(
"Skipping migration for user-updated query", extra={"snuba_query_id": snuba_query.id}
)
return

old_query_type, old_dataset, old_query, old_aggregate = _get_old_query_info(snuba_query)

snuba_query.dataset = Dataset.Transactions.value

with atomic_transaction(
using=(
router.db_for_write(SnubaQuery),
router.db_for_write(SnubaQueryEventType),
router.db_for_write(QuerySubscription),
)
):
snuba_query.save()

query_subscriptions = list(snuba_query.subscriptions.all())
try:
bulk_update_snuba_subscriptions(
query_subscriptions, old_query_type, old_dataset, old_aggregate, old_query
)
except Exception as e:
logger.info(
"Query not migrated: error updating subscriptions in snuba",
extra={"snuba_query_id": snuba_query.id, "error": e},
)
raise

logger.info(
"Query successfully migrated to transactions", extra={"snuba_query_id": snuba_query.id}
)
return


def rollback_am1_metrics_detector_query_and_update_subscription_in_snuba(snuba_query: SnubaQuery):
# querying for updating as well just in case the subscription gets stuck in updating
query_subscription_qs = QuerySubscription.objects.filter(
snuba_query_id=snuba_query.id,
status__in=[QuerySubscription.Status.ACTIVE.value, QuerySubscription.Status.UPDATING.value],
)
query_subscription = query_subscription_qs.first()

if not query_subscription:
logger.info("No active query subscription found for snuba query %s", snuba_query.id)
return

try:
data_source: DataSource = DataSource.objects.get(
source_id=str(query_subscription.id), type=DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION
)
except DataSource.DoesNotExist as e:
logger.info("Data source not found for snuba query %s", snuba_query.id)
sentry_sdk.capture_exception(e)
return

if not features.has(
"organizations:migrate-am1-metrics-alerts-to-transactions", data_source.organization
):
logger.info("Feature flag not enabled")
return

snapshot = snuba_query.query_snapshot
is_snapshot_am1_metrics = (
snapshot is not None and snapshot.get("metrics_to_transactions", False) is True
)

if not is_snapshot_am1_metrics:
logger.info("No snapshot found for snuba query %s", snuba_query.id)
return
Comment thread
cursor[bot] marked this conversation as resolved.

if snapshot and snapshot.get("user_updated"):
logger.info(
"Skipping rollback for user-updated query", extra={"snuba_query_id": snuba_query.id}
)
return

if snuba_query.dataset == Dataset.PerformanceMetrics.value:
logger.info("Query already migrated to metrics", extra={"snuba_query_id": snuba_query.id})
return
Comment on lines +143 to +148
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 😵‍💫


if snuba_query.dataset != Dataset.Transactions.value:
logger.info(
"Query is not the correct dataset to rollback", extra={"snuba_query_id": snuba_query.id}
)
return

old_query_type, old_dataset, old_query, old_aggregate = _get_old_query_info(snuba_query)

with atomic_transaction(
using=(
router.db_for_write(SnubaQuery),
router.db_for_write(SnubaQueryEventType),
router.db_for_write(QuerySubscription),
)
):
snuba_query.update(
dataset=Dataset.PerformanceMetrics.value,
)
Comment thread
sentry[bot] marked this conversation as resolved.

query_subscriptions = list(snuba_query.subscriptions.all())
try:
bulk_update_snuba_subscriptions(
query_subscriptions, old_query_type, old_dataset, old_aggregate, old_query
)
except Exception as e:
logger.info(
"Query not rolled back: error updating subscriptions in snuba",
extra={"snuba_query_id": snuba_query.id, "error": e},
)
raise

logger.info(
"Query successfully rolled back to legacy", extra={"snuba_query_id": snuba_query.id}
)
return
Loading
Loading