Browse Source

chore(alerts): Remove excluded projects trigger exclusion (#81020)

A follow up to https://github.com/getsentry/sentry/pull/79700 and a step
towards removing the `AlertRuleExcludedProjects` and
`AlertRuleTriggerExclusion` models.

This sets the foreign keys' `db_constraint` to False and removes any
lingering code that references the tables or usage of the columns. I
removed references to the `AlertRule` `excluded_projects` and
`include_all_projects` fields from the serializer as well since they're
unused.

The only place that still references them is in the [backup
tests](https://github.com/getsentry/sentry/blob/master/src/sentry/testutils/helpers/backups.py#L520)
that can't be removed until we're removing the table in the next step.
Colleen O'Rourke 3 months ago
parent
commit
223c73d608

+ 1 - 1
migrations_lockfile.txt

@@ -15,7 +15,7 @@ remote_subscriptions: 0003_drop_remote_subscription
 
 replays: 0004_index_together
 
-sentry: 0792_add_unique_index_apiauthorization
+sentry: 0793_remove_db_constraint_alert_rule_exclusion
 
 social_auth: 0002_default_auto_field
 

+ 0 - 3
src/sentry/incidents/endpoints/organization_alert_rule_details.py

@@ -212,9 +212,6 @@ Metric alert rule trigger actions follow the following structure:
     owner = ActorField(
         required=False, allow_null=True, help_text="The ID of the team or user that owns the rule."
     )
-    excludedProjects = serializers.ListField(
-        child=ProjectField(scope="project:read"), required=False
-    )
     thresholdPeriod = serializers.IntegerField(required=False, default=1, min_value=1, max_value=20)
     monitorType = serializers.IntegerField(
         required=False,

+ 0 - 3
src/sentry/incidents/endpoints/organization_alert_rule_index.py

@@ -409,9 +409,6 @@ Metric alert rule trigger actions follow the following structure:
     owner = ActorField(
         required=False, allow_null=True, help_text="The ID of the team or user that owns the rule."
     )
-    excludedProjects = serializers.ListField(
-        child=ProjectField(scope="project:read"), required=False
-    )
     thresholdPeriod = serializers.IntegerField(required=False, default=1, min_value=1, max_value=20)
     monitorType = serializers.IntegerField(
         required=False,

+ 0 - 14
src/sentry/incidents/endpoints/serializers/alert_rule.py

@@ -17,7 +17,6 @@ from sentry.incidents.models.alert_rule import (
     AlertRule,
     AlertRuleActivity,
     AlertRuleActivityType,
-    AlertRuleExcludedProjects,
     AlertRuleTrigger,
     AlertRuleTriggerAction,
 )
@@ -40,7 +39,6 @@ logger = logging.getLogger(__name__)
 class AlertRuleSerializerResponseOptional(TypedDict, total=False):
     environment: str | None
     projects: list[str] | None
-    excludedProjects: list[dict] | None
     queryType: int | None
     resolveThreshold: float | None
     dataset: str | None
@@ -63,8 +61,6 @@ class AlertRuleSerializerResponseOptional(TypedDict, total=False):
         "status",
         "resolution",
         "thresholdPeriod",
-        "includeAllProjects",
-        "excludedProjects",
         "weeklyAvg",
         "totalThisWeek",
         "latestIncident",
@@ -89,7 +85,6 @@ class AlertRuleSerializerResponse(AlertRuleSerializerResponseOptional):
     resolution: float
     thresholdPeriod: int
     triggers: list[dict]
-    includeAllProjects: bool
     dateModified: datetime
     dateCreated: datetime
     createdBy: dict
@@ -309,7 +304,6 @@ class AlertRuleSerializer(Serializer):
             "thresholdPeriod": obj.threshold_period,
             "triggers": attrs.get("triggers", []),
             "projects": sorted(attrs.get("projects", [])),
-            "includeAllProjects": obj.include_all_projects,
             "owner": attrs.get("owner", None),
             "originalAlertRuleId": attrs.get("originalAlertRuleId", None),
             "comparisonDelta": obj.comparison_delta / 60 if obj.comparison_delta else None,
@@ -343,13 +337,6 @@ class DetailedAlertRuleSerializer(AlertRuleSerializer):
         self, item_list: Sequence[Any], user: User | RpcUser, **kwargs: Any
     ) -> defaultdict[AlertRule, Any]:
         result = super().get_attrs(item_list, user, **kwargs)
-        alert_rules = {item.id: item for item in item_list}
-        for alert_rule_id, project_slug in AlertRuleExcludedProjects.objects.filter(
-            alert_rule__in=item_list
-        ).values_list("alert_rule_id", "project__slug"):
-            exclusions = result[alert_rules[alert_rule_id]].setdefault("excluded_projects", [])
-            exclusions.append(project_slug)
-
         query_to_alert_rule = {ar.snuba_query_id: ar for ar in item_list}
 
         for event_type in SnubaQueryEventType.objects.filter(
@@ -366,7 +353,6 @@ class DetailedAlertRuleSerializer(AlertRuleSerializer):
         self, obj: AlertRule, attrs: Mapping[Any, Any], user: User | RpcUser, **kwargs
     ) -> AlertRuleSerializerResponse:
         data = super().serialize(obj, attrs, user)
-        data["excludedProjects"] = sorted(attrs.get("excluded_projects", []))
         data["eventTypes"] = sorted(attrs.get("event_types", []))
         data["snooze"] = False
         return data

+ 1 - 22
src/sentry/incidents/endpoints/serializers/alert_rule_trigger.py

@@ -5,11 +5,7 @@ from django.db.models import prefetch_related_objects
 
 from sentry.api.serializers import Serializer, register, serialize
 from sentry.incidents.endpoints.utils import translate_threshold
-from sentry.incidents.models.alert_rule import (
-    AlertRuleTrigger,
-    AlertRuleTriggerAction,
-    AlertRuleTriggerExclusion,
-)
+from sentry.incidents.models.alert_rule import AlertRuleTrigger, AlertRuleTriggerAction
 
 
 @register(AlertRuleTrigger)
@@ -45,20 +41,3 @@ class AlertRuleTriggerSerializer(Serializer):
             "dateCreated": obj.date_added,
             "actions": attrs.get("actions", []),
         }
-
-
-class DetailedAlertRuleTriggerSerializer(AlertRuleTriggerSerializer):
-    def get_attrs(self, item_list, user, **kwargs):
-        triggers = {item.id: item for item in item_list}
-        result: dict[str, dict[str, list[str]]] = defaultdict(lambda: defaultdict(list))
-        for trigger_id, project_slug in AlertRuleTriggerExclusion.objects.filter(
-            alert_rule_trigger__in=item_list
-        ).values_list("alert_rule_trigger_id", "query_subscription__project__slug"):
-            if project_slug is not None:
-                result[triggers[trigger_id]]["excludedProjects"].append(project_slug)
-        return result
-
-    def serialize(self, obj, attrs, user, **kwargs):
-        data = super().serialize(obj, attrs, user, **kwargs)
-        data["excludedProjects"] = sorted(attrs.get("excludedProjects", []))
-        return data

+ 5 - 3
src/sentry/incidents/models/alert_rule.py

@@ -251,7 +251,7 @@ class AlertRuleExcludedProjects(Model):
 
     __relocation_scope__ = RelocationScope.Organization
 
-    alert_rule = FlexibleForeignKey("sentry.AlertRule", db_index=False)
+    alert_rule = FlexibleForeignKey("sentry.AlertRule", db_index=False, db_constraint=False)
     project = FlexibleForeignKey("sentry.Project", db_constraint=False)
     date_added = models.DateTimeField(default=timezone.now)
 
@@ -490,8 +490,10 @@ class AlertRuleTriggerExclusion(Model):
 
     __relocation_scope__ = RelocationScope.Organization
 
-    alert_rule_trigger = FlexibleForeignKey("sentry.AlertRuleTrigger", related_name="exclusions")
-    query_subscription = FlexibleForeignKey("sentry.QuerySubscription")
+    alert_rule_trigger = FlexibleForeignKey(
+        "sentry.AlertRuleTrigger", related_name="exclusions", db_constraint=False
+    )
+    query_subscription = FlexibleForeignKey("sentry.QuerySubscription", db_constraint=False)
     date_added = models.DateTimeField(default=timezone.now)
 
     class Meta:

+ 0 - 6
src/sentry/incidents/serializers/alert_rule.py

@@ -76,9 +76,6 @@ class AlertRuleSerializer(CamelSnakeModelSerializer[AlertRule]):
         required=False,
         max_length=1,
     )
-    excluded_projects = serializers.ListField(
-        child=ProjectField(scope="project:read"), required=False
-    )
     triggers = serializers.ListField(required=True)
     query_type = serializers.IntegerField(required=False)
     dataset = serializers.CharField(required=False)
@@ -123,8 +120,6 @@ class AlertRuleSerializer(CamelSnakeModelSerializer[AlertRule]):
             "comparison_delta",
             "aggregate",
             "projects",
-            "include_all_projects",
-            "excluded_projects",
             "triggers",
             "event_types",
             "monitor_type",
@@ -136,7 +131,6 @@ class AlertRuleSerializer(CamelSnakeModelSerializer[AlertRule]):
         ]
         extra_kwargs = {
             "name": {"min_length": 1, "max_length": 256},
-            "include_all_projects": {"default": False},
             "threshold_type": {"required": True},
             "resolve_threshold": {"required": False},
         }

+ 59 - 0
src/sentry/migrations/0793_remove_db_constraint_alert_rule_exclusion.py

@@ -0,0 +1,59 @@
+# Generated by Django 5.1.1 on 2024-11-22 17:43
+
+import django.db.models.deletion
+from django.db import migrations
+
+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.
+    # 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 post deployment:
+    # - Large data migrations. Typically we want these to be run manually 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
+    #   run this outside deployments so that we don't block them. Note that while adding an index
+    #   is a schema change, it's completely safe to run the operation after the code has deployed.
+    # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
+
+    is_post_deployment = False
+
+    dependencies = [
+        ("sentry", "0792_add_unique_index_apiauthorization"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="alertruleexcludedprojects",
+            name="alert_rule",
+            field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                db_constraint=False,
+                db_index=False,
+                on_delete=django.db.models.deletion.CASCADE,
+                to="sentry.alertrule",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="alertruletriggerexclusion",
+            name="alert_rule_trigger",
+            field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                db_constraint=False,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="exclusions",
+                to="sentry.alertruletrigger",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="alertruletriggerexclusion",
+            name="query_subscription",
+            field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                db_constraint=False,
+                on_delete=django.db.models.deletion.CASCADE,
+                to="sentry.querysubscription",
+            ),
+        ),
+    ]

+ 0 - 2
tests/sentry/incidents/endpoints/serializers/test_alert_rule.py

@@ -54,7 +54,6 @@ class BaseAlertRuleSerializerTest:
         assert result["resolution"] == alert_rule.snuba_query.resolution / 60
         assert result["thresholdPeriod"] == alert_rule.threshold_period
         assert result["projects"] == alert_rule_projects
-        assert result["includeAllProjects"] == alert_rule.include_all_projects
         if alert_rule.created_by_id:
             created_by = user_service.get_user(user_id=alert_rule.created_by_id)
             assert created_by is not None
@@ -264,7 +263,6 @@ class DetailedAlertRuleSerializerTest(BaseAlertRuleSerializerTest, TestCase):
         result = serialize(alert_rule, serializer=DetailedAlertRuleSerializer())
         self.assert_alert_rule_serialized(alert_rule, result)
         assert sorted(result["projects"]) == sorted(p.slug for p in projects)
-        assert result["excludedProjects"] == []
         assert result["eventTypes"] == [SnubaQueryEventType.EventType.ERROR.name.lower()]
 
     def test_triggers(self):

+ 0 - 12
tests/sentry/incidents/endpoints/serializers/test_alert_rule_trigger.py

@@ -1,7 +1,4 @@
 from sentry.api.serializers import serialize
-from sentry.incidents.endpoints.serializers.alert_rule_trigger import (
-    DetailedAlertRuleTriggerSerializer,
-)
 from sentry.incidents.logic import create_alert_rule_trigger
 from sentry.incidents.models.alert_rule import AlertRuleDetectionType, AlertRuleThresholdType
 from sentry.testutils.cases import TestCase
@@ -52,12 +49,3 @@ class AlertRuleTriggerSerializerTest(BaseAlertRuleTriggerSerializerTest, TestCas
         trigger = create_alert_rule_trigger(alert_rule, "hi", 80)
         result = serialize(trigger)
         self.assert_alert_rule_trigger_serialized(trigger, result, 20)
-
-
-class DetailedAlertRuleTriggerSerializerTest(BaseAlertRuleTriggerSerializerTest, TestCase):
-    def test_simple(self):
-        alert_rule = self.create_alert_rule(resolve_threshold=200)
-        trigger = create_alert_rule_trigger(alert_rule, "hi", 1000)
-        result = serialize(trigger, serializer=DetailedAlertRuleTriggerSerializer())
-        self.assert_alert_rule_trigger_serialized(trigger, result)
-        assert result["excludedProjects"] == []

Some files were not shown because too many files changed in this diff