Browse Source

Use Activated Alerts API in Release Thresholds (#67411)

## Description
- API fixes to enable the UI
  - Added filtering by monitor_type
  - fixes for serializer
- fixes for the project not existing in snuba for activated alerts (API
always filtered activated alerts)
Josh Callender 11 months ago
parent
commit
4571bc98c2

+ 13 - 3
src/sentry/api/serializers/models/alert_rule.py

@@ -16,7 +16,6 @@ from sentry.incidents.models.alert_rule import (
     AlertRuleActivity,
     AlertRuleActivityType,
     AlertRuleExcludedProjects,
-    AlertRuleMonitorType,
     AlertRuleTrigger,
     AlertRuleTriggerAction,
 )
@@ -128,9 +127,20 @@ class AlertRuleSerializer(Serializer):
                     ).get("uuid")
             alert_rule_triggers.append(serialized)
 
-        alert_rule_projects = AlertRule.objects.filter(
+        alert_rule_projects = set()
+        for alert_rule in alert_rules.values():
+            try:
+                alert_rule_projects.add([alert_rule.id, alert_rule.projects.get().slug])
+            except Exception:
+                pass
+
+        # TODO - Cleanup Subscription Project Mapping
+        snuba_alert_rule_projects = AlertRule.objects.filter(
             id__in=[item.id for item in item_list]
         ).values_list("id", "snuba_query__subscriptions__project__slug")
+
+        alert_rule_projects.update(snuba_alert_rule_projects)
+
         for alert_rule_id, project_slug in alert_rule_projects:
             rule_result = result[alert_rules[alert_rule_id]].setdefault("projects", [])
             rule_result.append(project_slug)
@@ -244,7 +254,7 @@ class AlertRuleSerializer(Serializer):
             "dateModified": obj.date_modified,
             "dateCreated": obj.date_added,
             "createdBy": attrs.get("created_by", None),
-            "monitorType": attrs.get("monitor_type", AlertRuleMonitorType.CONTINUOUS.value),
+            "monitorType": obj.monitor_type,
         }
         rule_snooze = RuleSnooze.objects.filter(
             Q(user_id=user.id) | Q(user_id=None), alert_rule=obj

+ 12 - 2
src/sentry/incidents/endpoints/organization_alert_rule_details.py

@@ -300,8 +300,18 @@ class OrganizationAlertRuleDetailsEndpoint(OrganizationAlertRuleEndpoint):
 
     def check_project_access(func):
         def wrapper(self, request: Request, organization, alert_rule):
-            # a metric alert is only associated with one project at a time
-            project = alert_rule.snuba_query.subscriptions.get().project
+            project = None
+
+            try:
+                # check to see if there's a project associated with the alert rule
+                project = alert_rule.projects.get()
+            except Exception:
+                pass
+
+            # TODO - Cleanup Subscription Project Mapping
+            # if not, check to see if there's a project associated with the snuba query
+            if project is None:
+                project = alert_rule.snuba_query.subscriptions.get().project
 
             if not request.access.has_project_access(project):
                 return Response(status=status.HTTP_403_FORBIDDEN)

+ 10 - 1
src/sentry/incidents/endpoints/organization_alert_rule_index.py

@@ -60,12 +60,16 @@ class AlertRuleIndexMixin(Endpoint):
         if not project:
             projects = self.get_projects(request, organization)
             alert_rules = AlertRule.objects.fetch_for_organization(organization, projects)
+
         else:
             alert_rules = AlertRule.objects.fetch_for_project(project)
         if not features.has("organizations:performance-view", organization):
-            # Filter to only error alert rules
             alert_rules = alert_rules.filter(snuba_query__dataset=Dataset.Events.value)
 
+        monitor_type = request.GET.get("monitor_type", None)
+        if monitor_type is not None:
+            alert_rules = alert_rules.filter(monitor_type=monitor_type)
+
         return self.paginate(
             request,
             queryset=alert_rules,
@@ -178,6 +182,11 @@ class OrganizationCombinedRuleIndexEndpoint(OrganizationEndpoint):
                 team_filter_query = team_filter_query | Q(owner_id=None)
 
         alert_rules = AlertRule.objects.fetch_for_organization(organization, projects)
+
+        monitor_type = request.GET.get("monitor_type", None)
+        if monitor_type is not None:
+            alert_rules = alert_rules.filter(monitor_type=monitor_type)
+
         issue_rules = Rule.objects.filter(
             status__in=[ObjectStatus.ACTIVE, ObjectStatus.DISABLED],
             source__in=[RuleSource.ISSUE],

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

@@ -10,7 +10,7 @@ from typing import Any, ClassVar, Self
 from django.conf import settings
 from django.core.cache import cache
 from django.db import models
-from django.db.models import QuerySet
+from django.db.models import Q, QuerySet
 from django.db.models.signals import post_delete, post_save
 from django.utils import timezone
 
@@ -87,11 +87,18 @@ class AlertRuleManager(BaseManager["AlertRule"]):
     def fetch_for_organization(self, organization, projects=None):
         queryset = self.filter(organization=organization)
         if projects is not None:
-            queryset = queryset.filter(snuba_query__subscriptions__project__in=projects).distinct()
+            # TODO - Cleanup Subscription Project Mapping
+            queryset = queryset.filter(
+                Q(snuba_query__subscriptions__project__in=projects) | Q(projects__in=projects)
+            ).distinct()
+
         return queryset
 
     def fetch_for_project(self, project):
-        return self.filter(snuba_query__subscriptions__project=project)
+        # TODO - Cleanup Subscription Project Mapping
+        return self.filter(
+            Q(snuba_query__subscriptions__project=project) | Q(projects=project)
+        ).distinct()
 
     @classmethod
     def __build_subscription_cache_key(cls, subscription_id):

+ 14 - 0
tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py

@@ -12,6 +12,7 @@ from sentry import audit_log
 from sentry.api.serializers import serialize
 from sentry.incidents.models.alert_rule import (
     AlertRule,
+    AlertRuleMonitorType,
     AlertRuleThresholdType,
     AlertRuleTrigger,
     AlertRuleTriggerAction,
@@ -106,6 +107,19 @@ class AlertRuleListEndpointTest(AlertRuleIndexBase):
         resp = self.get_response(self.organization.slug)
         assert resp.status_code == 404
 
+    def test_filter_by_monitor_type(self):
+        self.create_team(organization=self.organization, members=[self.user])
+        alert_rule1 = self.create_alert_rule(monitor_type=AlertRuleMonitorType.ACTIVATED)
+        alert_rule2 = self.create_alert_rule(monitor_type=AlertRuleMonitorType.CONTINUOUS)
+        self.login_as(self.user)
+
+        params = {"monitor_type": 1}
+        with self.feature("organizations:incidents"):
+            resp = self.get_response(self.organization.slug, **params)
+
+        assert serialize([alert_rule2]) not in resp.data
+        assert resp.data == serialize([alert_rule1])
+
 
 @region_silo_test
 @freeze_time()

+ 42 - 0
tests/sentry/incidents/models/test_alert_rule.py

@@ -215,6 +215,30 @@ class AlertRuleFetchForOrganizationTest(TestCase):
             AlertRule.objects.fetch_for_organization(self.organization, [project])
         )
 
+    def test_project_on_alert(self):
+        project = self.create_project()
+        alert_rule = self.create_alert_rule()
+        alert_rule.projects.add(project)
+
+        assert [alert_rule] == list(AlertRule.objects.fetch_for_organization(self.organization))
+
+    def test_project_on_alert_and_snuba(self):
+        project1 = self.create_project()
+        alert_rule1 = self.create_alert_rule(projects=[project1])
+        alert_rule1.projects.add(project1)
+
+        # will fetch if there's 1 project in snuba
+        assert [alert_rule1] == list(AlertRule.objects.fetch_for_organization(self.organization))
+
+        project2 = self.create_project()
+        alert_rule2 = self.create_alert_rule(projects=[project2, self.project])
+        alert_rule2.projects.add(project1)
+
+        # Will fetch if there's 1 project in snuba and 1 in alert rule
+        assert {alert_rule1, alert_rule2} == set(
+            AlertRule.objects.fetch_for_organization(self.organization, [project1])
+        )
+
 
 class AlertRuleTriggerActionTargetTest(TestCase):
     def test_user(self):
@@ -392,3 +416,21 @@ class CleanExpiredAlertsTest(TestCase):
         callback = alert_subscription_callback_registry[AlertRuleMonitorType.CONTINUOUS]
         result = callback(subscription)
         assert result is True
+
+
+class AlertRuleFetchForProjectTest(TestCase):
+    def test_simple(self):
+        project = self.create_project()
+        alert_rule = self.create_alert_rule(projects=[project])
+
+        assert [alert_rule] == list(AlertRule.objects.fetch_for_project(project))
+
+    def test_projects_on_snuba_and_alert(self):
+        project1 = self.create_project()
+        alert_rule1 = self.create_alert_rule(projects=[project1, self.project])
+
+        project2 = self.create_project()
+        alert_rule2 = self.create_alert_rule(projects=[project2, self.project])
+        alert_rule2.projects.add(project2)
+
+        assert {alert_rule1, alert_rule2} == set(AlertRule.objects.fetch_for_project(self.project))