Browse Source

rename issue_alert creators (#65207)

`create_alert_rule` is too easily conflated with the existing
`create_alert_rule` method associated with the AlertRule models.

Renaming this to be more clear
Nathan Hsieh 1 year ago
parent
commit
9fc06d207f

+ 14 - 12
src/sentry/monitors/endpoints/organization_monitor_details.py

@@ -36,10 +36,10 @@ from sentry.monitors.models import (
 )
 from sentry.monitors.serializers import MonitorSerializer
 from sentry.monitors.utils import (
-    create_alert_rule,
+    create_issue_alert_rule,
     get_checkin_margin,
     get_max_runtime,
-    update_alert_rule,
+    update_issue_alert_rule,
 )
 from sentry.monitors.validators import MonitorValidator
 from sentry.utils.outcomes import Outcome
@@ -186,22 +186,24 @@ class OrganizationMonitorDetailsEndpoint(MonitorEndpoint):
         # Update alert rule after in case slug or name changed
         if "alert_rule" in result:
             # Check to see if rule exists
-            alert_rule = monitor.get_alert_rule()
+            issue_alert_rule = monitor.get_issue_alert_rule()
             # If rule exists, update as necessary
-            if alert_rule:
-                alert_rule_id = update_alert_rule(
-                    request, project, monitor, alert_rule, result["alert_rule"]
+            if issue_alert_rule:
+                issue_alert_rule_id = update_issue_alert_rule(
+                    request, project, monitor, issue_alert_rule, result["alert_rule"]
                 )
             # If rule does not exist, create
             else:
-                alert_rule_id = create_alert_rule(request, project, monitor, result["alert_rule"])
+                issue_alert_rule_id = create_issue_alert_rule(
+                    request, project, monitor, result["alert_rule"]
+                )
 
-            if alert_rule_id:
+            if issue_alert_rule_id:
                 # If config is not sent, use existing config to update alert_rule_id
                 if "config" not in params:
                     params["config"] = monitor.config
 
-                params["config"]["alert_rule_id"] = alert_rule_id
+                params["config"]["alert_rule_id"] = issue_alert_rule_id
                 monitor.update(**params)
 
         return self.respond(serialize(monitor, request.user))
@@ -257,12 +259,12 @@ class OrganizationMonitorDetailsEndpoint(MonitorEndpoint):
 
                 # Mark rule for deletion if present and monitor is being deleted
                 monitor = monitor_objects.first()
-                alert_rule_id = monitor.config.get("alert_rule_id") if monitor else None
-                if alert_rule_id:
+                issue_alert_rule_id = monitor.config.get("alert_rule_id") if monitor else None
+                if issue_alert_rule_id:
                     rule = (
                         Rule.objects.filter(
                             project_id=monitor.project_id,
-                            id=alert_rule_id,
+                            id=issue_alert_rule_id,
                         )
                         .exclude(
                             status__in=[

+ 8 - 6
src/sentry/monitors/endpoints/organization_monitor_index.py

@@ -44,7 +44,7 @@ from sentry.monitors.serializers import (
     MonitorSerializer,
     MonitorSerializerResponse,
 )
-from sentry.monitors.utils import create_alert_rule, signal_monitor_created
+from sentry.monitors.utils import create_issue_alert_rule, signal_monitor_created
 from sentry.monitors.validators import MonitorBulkEditValidator, MonitorValidator
 from sentry.search.utils import tokenize_query
 from sentry.utils.outcomes import Outcome
@@ -288,13 +288,15 @@ class OrganizationMonitorIndexEndpoint(OrganizationEndpoint):
         project = result["project"]
         signal_monitor_created(project, request.user, False)
 
-        validated_alert_rule = result.get("alert_rule")
-        if validated_alert_rule:
-            alert_rule_id = create_alert_rule(request, project, monitor, validated_alert_rule)
+        validated_issue_alert_rule = result.get("alert_rule")
+        if validated_issue_alert_rule:
+            issue_alert_rule_id = create_issue_alert_rule(
+                request, project, monitor, validated_issue_alert_rule
+            )
 
-            if alert_rule_id:
+            if issue_alert_rule_id:
                 config = monitor.config
-                config["alert_rule_id"] = alert_rule_id
+                config["alert_rule_id"] = issue_alert_rule_id
                 monitor.update(config=config)
 
         return self.respond(serialize(monitor, request.user), status=201)

+ 19 - 19
src/sentry/monitors/models.py

@@ -316,30 +316,30 @@ class Monitor(Model):
         except jsonschema.ValidationError:
             logging.exception("Monitor: %s invalid config: %s", self.id, self.config)
 
-    def get_alert_rule(self):
-        alert_rule_id = self.config.get("alert_rule_id")
-        if alert_rule_id:
-            alert_rule = Rule.objects.filter(
+    def get_issue_alert_rule(self):
+        issue_alert_rule_id = self.config.get("alert_rule_id")
+        if issue_alert_rule_id:
+            issue_alert_rule = Rule.objects.filter(
                 project_id=self.project_id,
-                id=alert_rule_id,
+                id=issue_alert_rule_id,
                 source=RuleSource.CRON_MONITOR,
                 status=ObjectStatus.ACTIVE,
             ).first()
-            if alert_rule:
-                return alert_rule
+            if issue_alert_rule:
+                return issue_alert_rule
 
-            # If alert_rule_id is stale, clear it from the config
+            # If issue_alert_rule_id is stale, clear it from the config
             clean_config = self.config.copy()
             clean_config.pop("alert_rule_id", None)
             self.update(config=clean_config)
 
         return None
 
-    def get_alert_rule_data(self):
-        alert_rule = self.get_alert_rule()
-        if alert_rule:
-            data = alert_rule.data
-            alert_rule_data: dict[str, Any | None] = dict()
+    def get_issue_alert_rule_data(self):
+        issue_alert_rule = self.get_issue_alert_rule()
+        if issue_alert_rule:
+            data = issue_alert_rule.data
+            issue_alert_rule_data: dict[str, Any | None] = dict()
 
             # Build up alert target data
             targets = []
@@ -352,18 +352,18 @@ class Monitor(Model):
                             "targetType": action.get("targetType"),
                         }
                     )
-            alert_rule_data["targets"] = targets
+            issue_alert_rule_data["targets"] = targets
 
-            environment, alert_rule_environment_id = None, alert_rule.environment_id
-            if alert_rule_environment_id:
+            environment, issue_alert_rule_environment_id = None, issue_alert_rule.environment_id
+            if issue_alert_rule_environment_id:
                 try:
-                    environment = Environment.objects.get(id=alert_rule_environment_id).name
+                    environment = Environment.objects.get(id=issue_alert_rule_environment_id).name
                 except Environment.DoesNotExist:
                     pass
 
-            alert_rule_data["environment"] = environment
+            issue_alert_rule_data["environment"] = environment
 
-            return alert_rule_data
+            return issue_alert_rule_data
 
         return None
 

+ 1 - 1
src/sentry/monitors/serializers.py

@@ -129,7 +129,7 @@ class MonitorSerializer(Serializer):
 
         if self._expand("alertRule"):
             for item in item_list:
-                attrs[item]["alertRule"] = item.get_alert_rule_data()
+                attrs[item]["alertRule"] = item.get_issue_alert_rule_data()
 
         return attrs
 

+ 28 - 20
src/sentry/monitors/utils.py

@@ -199,21 +199,23 @@ def fetch_associated_groups(
     return trace_groups
 
 
-def create_alert_rule(
-    request: Request, project: Project, monitor: Monitor, validated_alert_rule: dict
+def create_issue_alert_rule(
+    request: Request, project: Project, monitor: Monitor, validated_issue_alert_rule: dict
 ):
     """
-    Create an alert rule from a request with the given data
+    Creates an Issue Alert `Rule` instance from a request with the given data
     :param request: Request object
     :param project: Project object
     :param monitor: Monitor object being created
-    :param alert_rule: Dictionary of configurations for an associated Rule
+    :param validated_issue_alert_rule: Dictionary of configurations for an associated Rule
     :return: dict
     """
-    alert_rule_data = create_alert_rule_data(project, request.user, monitor, validated_alert_rule)
+    issue_alert_rule_data = create_issue_alert_rule_data(
+        project, request.user, monitor, validated_issue_alert_rule
+    )
     serializer = RuleSerializer(
         context={"project": project, "organization": project.organization},
-        data=alert_rule_data,
+        data=issue_alert_rule_data,
     )
 
     if not serializer.is_valid():
@@ -245,16 +247,18 @@ def create_alert_rule(
     return rule.id
 
 
-def create_alert_rule_data(project: Project, user: User, monitor: Monitor, alert_rule: dict):
+def create_issue_alert_rule_data(
+    project: Project, user: User, monitor: Monitor, issue_alert_rule: dict
+):
     """
     Gets a dict formatted alert rule to create alongside the monitor
     :param project: Project object
     :param user: User object that made the request
     :param monitor: Monitor object being created
-    :param alert_rule: Dictionary of configurations for an associated Rule
+    :param issue_alert_rule: Dictionary of configurations for an associated Rule
     :return: dict
     """
-    alert_rule_data = {
+    issue_alert_rule_data = {
         "actionMatch": "any",
         "actions": [],
         "conditions": [
@@ -271,7 +275,7 @@ def create_alert_rule_data(project: Project, user: User, monitor: Monitor, alert
             "name": user.email,
         },
         "dateCreated": timezone.now().strftime("%Y-%m-%dT%H:%M:%S.%fZ"),
-        "environment": alert_rule.get("environment", None),
+        "environment": issue_alert_rule.get("environment", None),
         "filterMatch": "all",
         "filters": [
             {
@@ -288,7 +292,7 @@ def create_alert_rule_data(project: Project, user: User, monitor: Monitor, alert
         "snooze": False,
     }
 
-    for target in alert_rule.get("targets", []):
+    for target in issue_alert_rule.get("targets", []):
         target_identifier = target["target_identifier"]
         target_type = target["target_type"]
 
@@ -297,16 +301,20 @@ def create_alert_rule_data(project: Project, user: User, monitor: Monitor, alert
             "targetIdentifier": target_identifier,
             "targetType": target_type,
         }
-        alert_rule_data["actions"].append(action)
+        issue_alert_rule_data["actions"].append(action)
 
-    return alert_rule_data
+    return issue_alert_rule_data
 
 
-def update_alert_rule(
-    request: Request, project: Project, monitor: Monitor, alert_rule: Rule, alert_rule_data: dict
+def update_issue_alert_rule(
+    request: Request,
+    project: Project,
+    monitor: Monitor,
+    issue_alert_rule: Rule,
+    issue_alert_rule_data: dict,
 ):
     actions = []
-    for target in alert_rule_data.get("targets", []):
+    for target in issue_alert_rule_data.get("targets", []):
         target_identifier = target["target_identifier"]
         target_type = target["target_type"]
 
@@ -321,7 +329,7 @@ def update_alert_rule(
         context={"project": project, "organization": project.organization},
         data={
             "actions": actions,
-            "environment": alert_rule_data.get("environment", None),
+            "environment": issue_alert_rule_data.get("environment", None),
         },
         partial=True,
     )
@@ -330,7 +338,7 @@ def update_alert_rule(
         data = serializer.validated_data
 
         # update only slug conditions
-        conditions = alert_rule.data.get("conditions", [])
+        conditions = issue_alert_rule.data.get("conditions", [])
         updated = False
         for condition in conditions:
             if condition.get("key") == "monitor.slug":
@@ -356,10 +364,10 @@ def update_alert_rule(
             "conditions": conditions,
         }
 
-        updated_rule = Updater.run(rule=alert_rule, request=request, **kwargs)
+        updated_rule = Updater.run(rule=issue_alert_rule, request=request, **kwargs)
 
         RuleActivity.objects.create(
             rule=updated_rule, user_id=request.user.id, type=RuleActivityType.UPDATED.value
         )
 
-    return alert_rule.id
+    return issue_alert_rule.id

+ 1 - 1
src/sentry/testutils/cases.py

@@ -3062,7 +3062,7 @@ class MonitorTestCase(APITestCase):
             monitor=monitor, environment=environment, **monitorenvironment_defaults
         )
 
-    def _create_alert_rule(self, monitor):
+    def _create_issue_alert_rule(self, monitor):
         conditions = [
             {
                 "id": "sentry.rules.conditions.first_seen_event.FirstSeenEventCondition",

+ 14 - 14
tests/sentry/monitors/endpoints/test_organization_monitor_details.py

@@ -65,17 +65,17 @@ class OrganizationMonitorDetailsTest(MonitorTestCase):
         )
         assert len(response.data["environments"]) == 1
 
-    def test_expand_alert_rule(self):
+    def test_expand_issue_alert_rule(self):
         monitor = self._create_monitor()
 
         resp = self.get_success_response(self.organization.slug, monitor.slug, expand=["alertRule"])
         assert resp.data["alertRule"] is None
 
-        self._create_alert_rule(monitor)
+        self._create_issue_alert_rule(monitor)
         resp = self.get_success_response(self.organization.slug, monitor.slug, expand=["alertRule"])
-        alert_rule = resp.data["alertRule"]
-        assert alert_rule is not None
-        assert alert_rule["environment"] is not None
+        issue_alert_rule = resp.data["alertRule"]
+        assert issue_alert_rule is not None
+        assert issue_alert_rule["environment"] is not None
 
 
 @region_silo_test
@@ -302,9 +302,9 @@ class UpdateMonitorTest(MonitorTestCase):
             second=0, microsecond=0
         ) + timedelta(minutes=TIMEOUT)
 
-    def test_existing_alert_rule(self):
+    def test_existing_issue_alert_rule(self):
         monitor = self._create_monitor()
-        rule = self._create_alert_rule(monitor)
+        rule = self._create_issue_alert_rule(monitor)
         new_environment = self.create_environment(name="jungle")
         new_user = self.create_user()
         self.create_team_membership(user=new_user, team=self.team)
@@ -325,7 +325,7 @@ class UpdateMonitorTest(MonitorTestCase):
         assert resp.data["slug"] == "new-slug"
 
         monitor = Monitor.objects.get(id=monitor.id)
-        monitor_rule = monitor.get_alert_rule()
+        monitor_rule = monitor.get_issue_alert_rule()
         assert monitor_rule.id == rule.id
         assert monitor_rule.label == "Monitor Alert: new-name"
 
@@ -354,7 +354,7 @@ class UpdateMonitorTest(MonitorTestCase):
         rule_environment = Environment.objects.get(id=monitor_rule.environment_id)
         assert rule_environment.name == new_environment.name
 
-    def test_without_existing_alert_rule(self):
+    def test_without_existing_issue_alert_rule(self):
         monitor = self._create_monitor()
         resp = self.get_success_response(
             self.organization.slug,
@@ -369,7 +369,7 @@ class UpdateMonitorTest(MonitorTestCase):
         assert resp.data["slug"] == monitor.slug
 
         monitor = Monitor.objects.get(id=monitor.id)
-        rule = monitor.get_alert_rule()
+        rule = monitor.get_issue_alert_rule()
         assert rule is not None
 
     def test_invalid_config_param(self):
@@ -691,9 +691,9 @@ class DeleteMonitorTest(MonitorTestCase):
             qs_params={"environment": "jungle"},
         )
 
-    def test_simple_with_alert_rule(self):
+    def test_simple_with_issue_alert_rule(self):
         monitor = self._create_monitor()
-        self._create_alert_rule(monitor)
+        self._create_issue_alert_rule(monitor)
 
         self.get_success_response(
             self.organization.slug, monitor.slug, method="DELETE", status_code=202
@@ -703,9 +703,9 @@ class DeleteMonitorTest(MonitorTestCase):
         assert rule.status == ObjectStatus.PENDING_DELETION
         assert RuleActivity.objects.filter(rule=rule, type=RuleActivityType.DELETED.value).exists()
 
-    def test_simple_with_alert_rule_deleted(self):
+    def test_simple_with_issue_alert_rule_deleted(self):
         monitor = self._create_monitor()
-        rule = self._create_alert_rule(monitor)
+        rule = self._create_issue_alert_rule(monitor)
         rule.delete()
 
         self.get_success_response(