Browse Source

ActivatedAlertRule activations - add query subscription id column (#67483)

Activations will always be explicitly tied to the creation of a query
subscription, however the subscription will be cleaned up after expiry,
so we want to simply keep reference to the subscription id which will
allow us to reference which subscription updates should determine when
we should update the activation

This is the model changes for
https://github.com/getsentry/sentry/pull/67383
Nathan Hsieh 11 months ago
parent
commit
dc3119522e

+ 5 - 0
fixtures/backup/model_dependencies/detailed.json

@@ -409,6 +409,11 @@
         "kind": "FlexibleForeignKey",
         "model": "sentry.alertrule",
         "nullable": false
+      },
+      "query_subscription": {
+        "kind": "FlexibleForeignKey",
+        "model": "sentry.querysubscription",
+        "nullable": true
       }
     },
     "model": "sentry.alertruleactivations",

+ 2 - 1
fixtures/backup/model_dependencies/flat.json

@@ -58,7 +58,8 @@
     "sentry.alertrule"
   ],
   "sentry.alertruleactivations": [
-    "sentry.alertrule"
+    "sentry.alertrule",
+    "sentry.querysubscription"
   ],
   "sentry.alertruleactivity": [
     "sentry.alertrule",

+ 1 - 1
migrations_lockfile.txt

@@ -9,5 +9,5 @@ feedback: 0004_index_together
 hybridcloud: 0015_apitokenreplica_hashed_token_index
 nodestore: 0002_nodestore_no_dictfield
 replays: 0004_index_together
-sentry: 0678_add_is_hidden_dashboard_widget_query
+sentry: 0679_add_query_sub_fk_to_aar_activations
 social_auth: 0002_default_auto_field

+ 4 - 1
src/sentry/incidents/logic.py

@@ -86,6 +86,7 @@ from sentry.utils.snuba import is_measurement
 
 if TYPE_CHECKING:
     from sentry.incidents.utils.types import AlertRuleActivationConditionType
+    from sentry.snuba.models import QuerySubscription
 
 
 # We can return an incident as "windowed" which returns a range of points around the start of the incident
@@ -941,14 +942,16 @@ class ProjectsNotAssociatedWithAlertRuleError(Exception):
 
 def create_alert_rule_activation(
     alert_rule: AlertRule,
+    query_subscription: QuerySubscription,
     metric_value: int | None = None,
     finished_at: datetime | None = None,
 ):
     with transaction.atomic(router.db_for_write(AlertRuleActivations)):
         activation = AlertRuleActivations.objects.create(
             alert_rule=alert_rule,
-            metric_value=metric_value,
             finished_at=finished_at,
+            metric_value=metric_value,
+            query_subscription=query_subscription,
         )
 
     return activation

+ 10 - 3
src/sentry/incidents/models/alert_rule_activations.py

@@ -2,7 +2,7 @@ from __future__ import annotations
 
 import logging
 from datetime import datetime
-from typing import ClassVar
+from typing import TYPE_CHECKING, ClassVar
 
 from django.db import models
 from django.utils import timezone
@@ -10,7 +10,9 @@ from django.utils import timezone
 from sentry.backup.scopes import RelocationScope
 from sentry.db.models import FlexibleForeignKey, Model, region_silo_only_model
 from sentry.db.models.manager import BaseManager
-from sentry.incidents.models.alert_rule import AlertRule
+
+if TYPE_CHECKING:
+    from sentry.incidents.models.alert_rule import AlertRule
 
 logger = logging.getLogger(__name__)
 
@@ -63,7 +65,12 @@ class AlertRuleActivations(Model):
     # metric value represents the query results at the end of the activated time window.
     # since activated alerts are not run on a continuously shifting time window, the value
     # of this metric value will only continually tick upwards until the monitor window has expired.
-    metric_value = models.FloatField()
+    metric_value = models.FloatField(null=True)
+    # query_subscriptions are cleaned up after every run
+    # if a query_subscription is null, finished_at must be NOT be null
+    query_subscription = FlexibleForeignKey(
+        "sentry.QuerySubscription", null=True, on_delete=models.SET_NULL
+    )
 
     class Meta:
         app_label = "sentry"

+ 42 - 0
src/sentry/migrations/0679_add_query_sub_fk_to_aar_activations.py

@@ -0,0 +1,42 @@
+# Generated by Django 5.0.3 on 2024-03-22 18:18
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+import sentry.db.models.fields.foreignkey
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, this should only be used for operations where it's safe to run the migration
+    # after your code has deployed. So this should not be used for most operations that alter the
+    # schema of a table.
+    # Here are some things that make sense to mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that they can
+    #   be monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = False
+
+    dependencies = [
+        ("sentry", "0678_add_is_hidden_dashboard_widget_query"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="alertruleactivations",
+            name="query_subscription",
+            field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                to="sentry.querysubscription",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="alertruleactivations",
+            name="metric_value",
+            field=models.FloatField(null=True),
+        ),
+    ]

+ 4 - 1
src/sentry/testutils/factories.py

@@ -1528,10 +1528,13 @@ class Factories:
     @assume_test_silo_mode(SiloMode.REGION)
     def create_alert_rule_activation(
         alert_rule,
+        query_subscription,
         **kwargs,
     ):
 
-        return create_alert_rule_activation(alert_rule, **kwargs)
+        return create_alert_rule_activation(
+            alert_rule=alert_rule, query_subscription=query_subscription, **kwargs
+        )
 
     @staticmethod
     @assume_test_silo_mode(SiloMode.REGION)

+ 18 - 2
src/sentry/testutils/fixtures.py

@@ -391,12 +391,28 @@ class Fixtures:
             projects = [self.project]
         return Factories.create_alert_rule(organization, projects, *args, **kwargs)
 
-    def create_alert_rule_activation(self, alert_rule=None, *args, **kwargs):
+    def create_alert_rule_activation(
+        self, alert_rule=None, query_subscriptions=None, project=None, *args, **kwargs
+    ):
         if not alert_rule:
             alert_rule = self.create_alert_rule(
                 monitor_type=AlertRuleMonitorType.ACTIVATED,
             )
-        return Factories.create_alert_rule_activation(alert_rule=alert_rule, *args, **kwargs)
+        if not query_subscriptions:
+            projects = [project] if project else [self.project]
+            query_subscriptions = alert_rule.subscribe_projects(
+                projects=projects,
+                monitor_type=AlertRuleMonitorType.ACTIVATED,
+            )
+
+        created_activations = []
+        for sub in query_subscriptions:
+            created_activations.append(
+                Factories.create_alert_rule_activation(
+                    alert_rule=alert_rule, query_subscription=sub, *args, **kwargs
+                )
+            )
+        return created_activations
 
     def create_alert_rule_trigger(self, alert_rule=None, *args, **kwargs):
         if not alert_rule:

+ 3 - 1
src/sentry/testutils/helpers/backups.py

@@ -466,7 +466,9 @@ class BackupTestCase(TransactionTestCase):
             monitor_type=AlertRuleMonitorType.ACTIVATED,
             activation_condition=AlertRuleActivationConditionType.RELEASE_CREATION,
         )
-        self.create_alert_rule_activation(alert_rule=activated_alert, metric_value=100)
+        self.create_alert_rule_activation(
+            alert_rule=activated_alert, project=project, metric_value=100
+        )
         activated_trigger = self.create_alert_rule_trigger(alert_rule=activated_alert)
         self.create_alert_rule_trigger_action(alert_rule_trigger=activated_trigger)