fix(metric_alerts): Fix bug when updating a performance alert from metrics -> transactions. (#44274)

This fixes a bug that occurs when a user updates an existing performance
alert that was on the metrics dataset that moves to the transactions
dataset. We end up throwing an error when attempting to delete the
previous subscription, since we fail to resolve the entity correctly.

This actually works out fine in the end. Even though the update fails,
we have a checker tasks that catches these kinds of problems and
successfully creates the new subscription later. And when we receive an
update from a snuba subscription that has no corresponding subscription
in sentry, we delete that. So the state works out fine even without this
fix. The fix just avoids this bug and any delay in getting the state to
a good place.

Fixes SENTRY-YS5

src/sentry/snuba/subscriptions.py

@@ -90,6 +90,8 @@ def update_snuba_query(
     removed_event_types = current_event_types - set(event_types)
     old_query_type = SnubaQuery.Type(snuba_query.type)
     old_dataset = Dataset(snuba_query.dataset)
+    old_query = snuba_query.query
+    old_aggregate = snuba_query.aggregate
     with transaction.atomic():
         query_subscriptions = list(snuba_query.subscriptions.all())
         snuba_query.update(
@@ -113,7 +115,9 @@ def update_snuba_query(
                 snuba_query=snuba_query, type__in=[et.value for et in removed_event_types]
             ).delete()
 
-        bulk_update_snuba_subscriptions(query_subscriptions, old_query_type, old_dataset)
+        bulk_update_snuba_subscriptions(
+            query_subscriptions, old_query_type, old_dataset, old_aggregate, old_query
+        )
 
 
 def bulk_create_snuba_subscriptions(projects, subscription_type, snuba_query):
@@ -156,7 +160,9 @@ def create_snuba_subscription(project, subscription_type, snuba_query):
     return subscription
 
 
-def bulk_update_snuba_subscriptions(subscriptions, old_query_type, old_dataset):
+def bulk_update_snuba_subscriptions(
+    subscriptions, old_query_type, old_dataset, old_aggregate, old_query
+):
     """
     Updates a list of query subscriptions.
 
@@ -168,12 +174,14 @@ def bulk_update_snuba_subscriptions(subscriptions, old_query_type, old_dataset):
     # TODO: Batch this up properly once we care about multi-project rules.
     for subscription in subscriptions:
         updated_subscriptions.append(
-            update_snuba_subscription(subscription, old_query_type, old_dataset)
+            update_snuba_subscription(
+                subscription, old_query_type, old_dataset, old_aggregate, old_query
+            )
         )
     return subscriptions
 
 
-def update_snuba_subscription(subscription, old_query_type, old_dataset):
+def update_snuba_subscription(subscription, old_query_type, old_dataset, old_aggregate, old_query):
     """
     Updates a subscription to a snuba query.
 
@@ -191,6 +199,8 @@ def update_snuba_subscription(subscription, old_query_type, old_dataset):
                 "query_subscription_id": subscription.id,
                 "old_query_type": old_query_type.value,
                 "old_dataset": old_dataset.value,
+                "old_aggregate": old_aggregate,
+                "old_query": old_query,
             },
             countdown=5,
         )

src/sentry/snuba/tasks.py

@@ -82,7 +82,12 @@ def create_subscription_in_snuba(query_subscription_id, **kwargs):
     max_retries=5,
 )
 def update_subscription_in_snuba(
-    query_subscription_id, old_query_type=None, old_dataset=None, **kwargs
+    query_subscription_id,
+    old_query_type=None,
+    old_dataset=None,
+    old_aggregate=None,
+    old_query=None,
+    **kwargs,
 ):
     """
     Task to update a corresponding subscription in Snuba from a `QuerySubscription` in
@@ -106,10 +111,14 @@ def update_subscription_in_snuba(
         query_type = SnubaQuery.Type(
             old_query_type if old_query_type is not None else subscription.snuba_query.type
         )
+        query = old_query if old_query is not None else subscription.snuba_query.query
+        aggregate = (
+            old_aggregate if old_aggregate is not None else subscription.snuba_query.aggregate
+        )
         old_entity_subscription = get_entity_subscription(
             query_type,
             dataset,
-            subscription.snuba_query.aggregate,
+            aggregate,
             subscription.snuba_query.time_window,
             extra_fields={
                 "org_id": subscription.project.organization_id,
@@ -118,7 +127,7 @@ def update_subscription_in_snuba(
         )
         old_entity_key = get_entity_key_from_query_builder(
             old_entity_subscription.build_query_builder(
-                subscription.snuba_query.query,
+                query,
                 [subscription.project_id],
                 None,
                 {"organization_id": subscription.project.organization_id},

tests/sentry/snuba/test_subscriptions.py

@@ -1,5 +1,7 @@
 from datetime import timedelta
 
+import pytest
+
 from sentry.snuba.dataset import Dataset
 from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
 from sentry.snuba.subscriptions import (
@@ -12,6 +14,8 @@ from sentry.snuba.subscriptions import (
 )
 from sentry.testutils import TestCase
 
+pytestmark = pytest.mark.sentry_metrics
+
 
 class CreateSnubaQueryTest(TestCase):
     def test(self):
@@ -290,12 +294,14 @@ class UpdateSnubaQueryTest(TestCase):
 class UpdateSnubaSubscriptionTest(TestCase):
     def test(self):
         old_dataset = Dataset.Events
+        old_query = "level:error"
+        old_aggregate = "count()"
         with self.tasks():
             snuba_query = create_snuba_query(
                 SnubaQuery.Type.ERROR,
                 old_dataset,
-                "level:error",
-                "count()",
+                old_query,
+                old_aggregate,
                 timedelta(minutes=10),
                 timedelta(minutes=1),
                 None,
@@ -320,7 +326,7 @@ class UpdateSnubaSubscriptionTest(TestCase):
             aggregate=aggregate,
         )
         assert subscription_id is not None
-        update_snuba_subscription(subscription, old_type, old_dataset)
+        update_snuba_subscription(subscription, old_type, old_dataset, old_aggregate, old_query)
         assert subscription.status == QuerySubscription.Status.UPDATING.value
         assert subscription.subscription_id == subscription_id
         assert subscription.snuba_query.dataset == dataset.value
@@ -332,11 +338,13 @@ class UpdateSnubaSubscriptionTest(TestCase):
     def test_with_task(self):
         with self.tasks():
             old_dataset = Dataset.Events
+            old_query = "level:error"
+            old_aggregate = "count()"
             snuba_query = create_snuba_query(
                 SnubaQuery.Type.ERROR,
                 old_dataset,
-                "level:error",
-                "count()",
+                old_query,
+                old_aggregate,
                 timedelta(minutes=10),
                 timedelta(minutes=1),
                 None,
@@ -361,7 +369,47 @@ class UpdateSnubaSubscriptionTest(TestCase):
                 environment=self.environment,
                 aggregate=aggregate,
             )
-            update_snuba_subscription(subscription, old_type, old_dataset)
+            update_snuba_subscription(subscription, old_type, old_dataset, old_aggregate, old_query)
+            subscription = QuerySubscription.objects.get(id=subscription.id)
+            assert subscription.status == QuerySubscription.Status.ACTIVE.value
+            assert subscription.subscription_id is not None
+            assert subscription.subscription_id != subscription_id
+
+    def test_perf_metric_to_transaction(self):
+        with self.tasks():
+            old_dataset = Dataset.PerformanceMetrics
+            old_query = ""
+            old_aggregate = "count()"
+            snuba_query = create_snuba_query(
+                SnubaQuery.Type.PERFORMANCE,
+                old_dataset,
+                old_query,
+                old_aggregate,
+                timedelta(minutes=10),
+                timedelta(minutes=1),
+                None,
+            )
+            subscription = create_snuba_subscription(self.project, "something", snuba_query)
+            old_type = SnubaQuery.Type(snuba_query.type)
+
+            dataset = Dataset.Transactions
+            query = "level:warning"
+            aggregate = "count()"
+            time_window = timedelta(minutes=20)
+            resolution = timedelta(minutes=2)
+            subscription = QuerySubscription.objects.get(id=subscription.id)
+            subscription_id = subscription.subscription_id
+            assert subscription_id is not None
+            snuba_query.update(
+                type=SnubaQuery.Type.PERFORMANCE.value,
+                dataset=dataset.value,
+                query=query,
+                time_window=int(time_window.total_seconds()),
+                resolution=int(resolution.total_seconds()),
+                environment=self.environment,
+                aggregate=aggregate,
+            )
+            update_snuba_subscription(subscription, old_type, old_dataset, old_aggregate, old_query)
             subscription = QuerySubscription.objects.get(id=subscription.id)
             assert subscription.status == QuerySubscription.Status.ACTIVE.value
             assert subscription.subscription_id is not None