♻️ ref: refactor slack metric alert logic

Author: iamrajjoshi

src/sentry/incidents/typings/metric_detector.py

@@ -14,7 +14,7 @@
 from sentry.incidents.models.incident import Incident, IncidentStatus
 from sentry.issues.issue_occurrence import IssueOccurrence
 from sentry.models.group import Group, GroupStatus
-from sentry.snuba.models import SnubaQuery
+from sentry.snuba.models import QuerySubscription, SnubaQuery
 from sentry.types.group import PriorityLevel
 from sentry.workflow_engine.models import Action, Detector
 
@@ -60,6 +60,7 @@ class NotificationContext:
     NotificationContext is a dataclass that represents the context required send a notification.
     """
 
+    id: int
     integration_id: int | None = None
     target_identifier: str | None = None
     target_display: str | None = None
@@ -69,6 +70,7 @@ class NotificationContext:
     @classmethod
     def from_alert_rule_trigger_action(cls, action: AlertRuleTriggerAction) -> NotificationContext:
         return cls(
+            id=action.id,
             integration_id=action.integration_id,
             target_identifier=action.target_identifier,
             target_display=action.target_display,
@@ -79,6 +81,7 @@ def from_alert_rule_trigger_action(cls, action: AlertRuleTriggerAction) -> Notif
     def from_action_model(cls, action: Action) -> NotificationContext:
         if action.type == Action.Type.SENTRY_APP:
             return cls(
+                id=action.id,
                 integration_id=action.integration_id,
                 target_display=action.config.get("target_display"),
                 sentry_app_config=action.data.get("settings"),
@@ -87,6 +90,7 @@ def from_action_model(cls, action: Action) -> NotificationContext:
             )
         elif action.type == Action.Type.OPSGENIE or action.type == Action.Type.PAGERDUTY:
             return cls(
+                id=action.id,
                 integration_id=action.integration_id,
                 target_identifier=action.config.get("target_identifier"),
                 target_display=action.config.get("target_display"),
@@ -95,6 +99,7 @@ def from_action_model(cls, action: Action) -> NotificationContext:
         # TODO(iamrajjoshi): Add support for email here
 
         return cls(
+            id=action.id,
             integration_id=action.integration_id,
             target_identifier=action.config.get("target_identifier"),
             target_display=action.config.get("target_display"),
@@ -103,8 +108,10 @@ def from_action_model(cls, action: Action) -> NotificationContext:
 
 @dataclass
 class MetricIssueContext:
-    open_period_identifier: int
+    id: int
+    open_period_identifier: int  # Used for link building
     snuba_query: SnubaQuery
+    subscription: QuerySubscription
     new_status: IncidentStatus
     metric_value: float | None
 
@@ -128,6 +135,17 @@ def _get_snuba_query(cls, occurrence: IssueOccurrence) -> SnubaQuery:
             raise ValueError("Snuba query does not exist") from e
         return query
 
+    @classmethod
+    def _get_subscription(cls, occurrence: IssueOccurrence) -> QuerySubscription:
+        subscription_id = occurrence.evidence_data.get("subscription_id")
+        if not subscription_id:
+            raise ValueError("Subscription ID is required for alert context")
+        try:
+            subscription = QuerySubscription.objects.get(id=subscription_id)
+        except QuerySubscription.DoesNotExist as e:
+            raise ValueError("Subscription does not exist") from e
+        return subscription
+
     @classmethod
     def _get_metric_value(cls, occurrence: IssueOccurrence) -> float:
         if (metric_value := occurrence.evidence_data.get("metric_value")) is None:
@@ -144,8 +162,11 @@ def from_group_event(cls, group_event: GroupEvent) -> MetricIssueContext:
             # TODO(iamrajjoshi): Replace with something once we know how we want to build the link
             # If we store open periods in the database, we can use the id from that
             # Otherwise, we can use the issue id
+            id=group.id,
+            # TODO(iamrajjoshi): This should probably be the id of the latest open period
             open_period_identifier=group.id,
             snuba_query=cls._get_snuba_query(occurrence),
+            subscription=cls._get_subscription(occurrence),
             new_status=cls._get_new_status(group, occurrence),
             metric_value=cls._get_metric_value(occurrence),
         )
@@ -158,8 +179,10 @@ def from_legacy_models(
         metric_value: float | None = None,
     ) -> MetricIssueContext:
         return cls(
+            id=incident.id,
             open_period_identifier=incident.identifier,
             snuba_query=incident.alert_rule.snuba_query,
+            subscription=incident.subscription,
             new_status=new_status,
             metric_value=metric_value,
         )

src/sentry/integrations/slack/message_builder/incidents.py

@@ -1,7 +1,6 @@
 from datetime import datetime
 
-from sentry.incidents.models.incident import IncidentStatus
-from sentry.incidents.typings.metric_detector import AlertContext
+from sentry.incidents.typings.metric_detector import AlertContext, MetricIssueContext
 from sentry.integrations.metric_alerts import incident_attachment_info
 from sentry.integrations.slack.message_builder.base.block import BlockSlackMessageBuilder
 from sentry.integrations.slack.message_builder.types import (
@@ -11,7 +10,6 @@
 )
 from sentry.integrations.slack.utils.escape import escape_slack_text
 from sentry.models.organization import Organization
-from sentry.snuba.models import SnubaQuery
 
 
 def get_started_at(timestamp: datetime | None) -> str:
@@ -26,12 +24,9 @@ class SlackIncidentsMessageBuilder(BlockSlackMessageBuilder):
     def __init__(
         self,
         alert_context: AlertContext,
-        open_period_identifier: int,
+        metric_issue_context: MetricIssueContext,
         organization: Organization,
-        snuba_query: SnubaQuery,
-        new_status: IncidentStatus,
         date_started: datetime,
-        metric_value: float | None = None,
         chart_url: str | None = None,
         notification_uuid: str | None = None,
     ) -> None:
@@ -45,11 +40,11 @@ def __init__(
         """
         super().__init__()
         self.alert_context = alert_context
-        self.open_period_identifier = open_period_identifier
+        self.open_period_identifier = metric_issue_context.open_period_identifier
         self.organization = organization
-        self.snuba_query = snuba_query
-        self.metric_value = metric_value
-        self.new_status = new_status
+        self.snuba_query = metric_issue_context.snuba_query
+        self.metric_value = metric_issue_context.metric_value
+        self.new_status = metric_issue_context.new_status
         self.date_started = date_started
         self.chart_url = chart_url
         self.notification_uuid = notification_uuid