fix/feat(workflow): Use action/method in `incident_attachment_info` if available (#23427)

src/sentry/incidents/action_handlers.py

@@ -33,6 +33,18 @@ class ActionHandler(metaclass=abc.ABCMeta):
         pass
 
 
+class DefaultActionHandler(ActionHandler):
+    def fire(self, metric_value):
+        self.send_alert(metric_value, "fire")
+
+    def resolve(self, metric_value):
+        self.send_alert(metric_value, "resolve")
+
+    @abc.abstractmethod
+    def send_alert(self, metric_value):
+        pass
+
+
 @AlertRuleTriggerAction.register_type(
     "email",
     AlertRuleTriggerAction.Type.EMAIL,
@@ -94,18 +106,11 @@ class EmailActionHandler(ActionHandler):
     [AlertRuleTriggerAction.TargetType.SPECIFIC],
     integration_provider="slack",
 )
-class SlackActionHandler(ActionHandler):
-    def fire(self, metric_value):
-        self.send_alert(metric_value)
-
-    def resolve(self, metric_value):
-        self.send_alert(metric_value)
-
-    def send_alert(self, metric_value):
+class SlackActionHandler(DefaultActionHandler):
+    def send_alert(self, metric_value, method):
         from sentry.integrations.slack.utils import send_incident_alert_notification
 
-        # TODO: We should include more information about the trigger/severity etc.
-        send_incident_alert_notification(self.action, self.incident, metric_value)
+        send_incident_alert_notification(self.action, self.incident, metric_value, method)
 
 
 @AlertRuleTriggerAction.register_type(
@@ -114,17 +119,11 @@ class SlackActionHandler(ActionHandler):
     [AlertRuleTriggerAction.TargetType.SPECIFIC],
     integration_provider="msteams",
 )
-class MsTeamsActionHandler(ActionHandler):
-    def fire(self, metric_value):
-        self.send_alert(metric_value)
-
-    def resolve(self, metric_value):
-        self.send_alert(metric_value)
-
-    def send_alert(self, metric_value):
+class MsTeamsActionHandler(DefaultActionHandler):
+    def send_alert(self, metric_value, method):
         from sentry.integrations.msteams.utils import send_incident_alert_notification
 
-        send_incident_alert_notification(self.action, self.incident, metric_value)
+        send_incident_alert_notification(self.action, self.incident, metric_value, method)
 
 
 @AlertRuleTriggerAction.register_type(
@@ -133,17 +132,11 @@ class MsTeamsActionHandler(ActionHandler):
     [AlertRuleTriggerAction.TargetType.SPECIFIC],
     integration_provider="pagerduty",
 )
-class PagerDutyActionHandler(ActionHandler):
-    def fire(self, metric_value):
-        self.send_alert(metric_value)
-
-    def resolve(self, metric_value):
-        self.send_alert(metric_value)
-
-    def send_alert(self, metric_value):
+class PagerDutyActionHandler(DefaultActionHandler):
+    def send_alert(self, metric_value, method):
         from sentry.integrations.pagerduty.utils import send_incident_alert_notification
 
-        send_incident_alert_notification(self.action, self.incident, metric_value)
+        send_incident_alert_notification(self.action, self.incident, metric_value, method)
 
 
 @AlertRuleTriggerAction.register_type(
@@ -151,17 +144,11 @@ class PagerDutyActionHandler(ActionHandler):
     AlertRuleTriggerAction.Type.SENTRY_APP,
     [AlertRuleTriggerAction.TargetType.SENTRY_APP],
 )
-class SentryAppActionHandler(ActionHandler):
-    def fire(self, metric_value):
-        self.send_alert(metric_value)
-
-    def resolve(self, metric_value):
-        self.send_alert(metric_value)
-
-    def send_alert(self, metric_value):
+class SentryAppActionHandler(DefaultActionHandler):
+    def send_alert(self, metric_value, method):
         from sentry.rules.actions.notify_event_service import send_incident_alert_notification
 
-        send_incident_alert_notification(self.action, self.incident, metric_value)
+        send_incident_alert_notification(self.action, self.incident, metric_value, method)
 
 
 def format_duration(minutes):

src/sentry/incidents/models.py

@@ -583,20 +583,20 @@ class AlertRuleTriggerAction(Model):
             # ok to contact this email.
             return self.target_identifier
 
-    def build_handler(self, incident, project):
+    def build_handler(self, action, incident, project):
         type = AlertRuleTriggerAction.Type(self.type)
         if type in self._type_registrations:
-            return self._type_registrations[type].handler(self, incident, project)
+            return self._type_registrations[type].handler(action, incident, project)
         else:
             metrics.incr(f"alert_rule_trigger.unhandled_type.{self.type}")
 
-    def fire(self, incident, project, metric_value):
-        handler = self.build_handler(incident, project)
+    def fire(self, action, incident, project, metric_value):
+        handler = self.build_handler(action, incident, project)
         if handler:
             return handler.fire(metric_value)
 
-    def resolve(self, incident, project, metric_value):
-        handler = self.build_handler(incident, project)
+    def resolve(self, action, incident, project, metric_value):
+        handler = self.build_handler(action, incident, project)
         if handler:
             return handler.resolve(metric_value)
 

src/sentry/incidents/subscription_processor.py

@@ -342,9 +342,6 @@ class SubscriptionProcessor:
             return incident_trigger
 
     def handle_trigger_actions(self, incident_triggers, metric_value):
-        # These will all be for the same incident and status, so just grab the first one
-        incident_trigger = incident_triggers[0]
-        method = "fire" if incident_trigger.status == TriggerStatus.ACTIVE.value else "resolve"
         actions = deduplicate_trigger_actions(
             list(
                 AlertRuleTriggerAction.objects.filter(
@@ -353,6 +350,10 @@ class SubscriptionProcessor:
             )
         )
 
+        # Grab the first trigger to get incident id (they are all the same)
+        # All triggers should either be firing or resolving, so doesn't matter which we grab.
+        incident_trigger = incident_triggers[0]
+        method = "fire" if incident_triggers[0].status == TriggerStatus.ACTIVE.value else "resolve"
         for action in actions:
             transaction.on_commit(
                 handle_trigger_action.s(

src/sentry/incidents/tasks.py

@@ -133,6 +133,7 @@ def handle_trigger_action(action_id, incident_id, project_id, method, metric_val
     except AlertRuleTriggerAction.DoesNotExist:
         metrics.incr("incidents.alert_rules.action.skipping_missing_action")
         return
+
     try:
         incident = Incident.objects.select_related("organization").get(id=incident_id)
     except Incident.DoesNotExist:
@@ -150,7 +151,7 @@ def handle_trigger_action(action_id, incident_id, project_id, method, metric_val
             AlertRuleTriggerAction.Type(action.type).name.lower(), method
         )
     )
-    getattr(action, method)(incident, project, metric_value=metric_value)
+    getattr(action, method)(action, incident, project, metric_value=metric_value)
 
 
 @instrumented_task(

src/sentry/integrations/metric_alerts.py

@@ -1,7 +1,7 @@
 from datetime import timedelta
 from django.core.urlresolvers import reverse
 
-from sentry.incidents.logic import get_incident_aggregates
+from sentry.incidents.logic import get_incident_aggregates, CRITICAL_TRIGGER_LABEL
 from sentry.incidents.models import IncidentStatus, IncidentTrigger, INCIDENT_STATUS
 from sentry.utils.assets import get_asset_url
 from sentry.utils.http import absolute_uri
@@ -12,10 +12,28 @@ QUERY_AGGREGATION_DISPLAY = {
 }
 
 
-def incident_attachment_info(incident, metric_value=None):
+def incident_status_info(incident, metric_value, action, method):
+    if action and method:
+        # Get status from trigger
+        incident_status = (
+            IncidentStatus.CLOSED
+            if method == "resolve"
+            else (
+                IncidentStatus.CRITICAL
+                if action.alert_rule_trigger.label == CRITICAL_TRIGGER_LABEL
+                else IncidentStatus.WARNING
+            )
+        )
+    else:
+        incident_status = incident.status
+    return IncidentStatus(incident_status)
+
+
+def incident_attachment_info(incident, metric_value=None, action=None, method=None):
     logo_url = absolute_uri(get_asset_url("sentry", "images/sentry-email-avatar.png"))
     alert_rule = incident.alert_rule
-    status = INCIDENT_STATUS[IncidentStatus(incident.status)]
+
+    status = INCIDENT_STATUS[incident_status_info(incident, metric_value, action, method)]
 
     agg_text = QUERY_AGGREGATION_DISPLAY.get(
         alert_rule.snuba_query.aggregate, alert_rule.snuba_query.aggregate

src/sentry/integrations/msteams/card_builder.py

@@ -693,8 +693,8 @@ def build_unlinked_card():
     }
 
 
-def build_incident_attachment(incident, metric_value=None):
-    data = incident_attachment_info(incident, metric_value)
+def build_incident_attachment(action, incident, metric_value=None, method=None):
+    data = incident_attachment_info(incident, metric_value, action=action, method=method)
 
     colors = {"Resolved": "good", "Warning": "warning", "Critical": "attention"}
 

src/sentry/integrations/msteams/utils.py

@@ -77,12 +77,12 @@ def get_channel_id(organization, integration_id, name):
     return None
 
 
-def send_incident_alert_notification(action, incident, metric_value):
+def send_incident_alert_notification(action, incident, metric_value, method):
     from .card_builder import build_incident_attachment
 
     channel = action.target_identifier
     integration = action.integration
-    attachment = build_incident_attachment(incident, metric_value)
+    attachment = build_incident_attachment(action, incident, metric_value, method)
     client = MsTeamsClient(integration)
     try:
         client.send_card(channel, attachment)

src/sentry/integrations/pagerduty/utils.py

@@ -3,7 +3,7 @@ import logging
 from django.http import Http404
 
 from sentry.incidents.models import IncidentStatus
-from sentry.integrations.metric_alerts import incident_attachment_info
+from sentry.integrations.metric_alerts import incident_attachment_info, incident_status_info
 from sentry.models import PagerDutyService
 from sentry.shared_integrations.exceptions import ApiError
 
@@ -12,17 +12,18 @@ from .client import PagerDutyClient
 logger = logging.getLogger("sentry.integrations.pagerduty")
 
 
-def build_incident_attachment(incident, integration_key, metric_value=None):
-    data = incident_attachment_info(incident, metric_value)
-    if incident.status == IncidentStatus.CRITICAL.value:
+def build_incident_attachment(action, incident, integration_key, metric_value=None, method=None):
+    data = incident_attachment_info(incident, metric_value, action=action, method=method)
+    incident_status = incident_status_info(incident, metric_value, action=action, method=method)
+    if incident_status == IncidentStatus.CRITICAL:
         severity = "critical"
-    elif incident.status == IncidentStatus.WARNING.value:
+    elif incident_status == IncidentStatus.WARNING:
         severity = "warning"
-    elif incident.status == IncidentStatus.CLOSED.value:
+    elif incident_status == IncidentStatus.CLOSED:
         severity = "info"
 
     event_action = "resolve"
-    if incident.status in [IncidentStatus.WARNING.value, IncidentStatus.CRITICAL.value]:
+    if incident_status in [IncidentStatus.WARNING, IncidentStatus.CRITICAL]:
         event_action = "trigger"
 
     return {
@@ -39,7 +40,7 @@ def build_incident_attachment(incident, integration_key, metric_value=None):
     }
 
 
-def send_incident_alert_notification(action, incident, metric_value):
+def send_incident_alert_notification(action, incident, metric_value, method):
     integration = action.integration
     try:
         service = PagerDutyService.objects.get(id=action.target_identifier)
@@ -56,7 +57,7 @@ def send_incident_alert_notification(action, incident, metric_value):
         raise Http404
     integration_key = service.integration_key
     client = PagerDutyClient(integration_key=integration_key)
-    attachment = build_incident_attachment(incident, integration_key, metric_value)
+    attachment = build_incident_attachment(action, incident, integration_key, metric_value, method)
 
     try:
         client.send_trigger(attachment)

src/sentry/integrations/slack/event_endpoint.py

@@ -77,7 +77,12 @@ def unfurl_incidents(integration, incident_map, event_id_by_url=None):
         return {}
 
     return {
-        v: build_incident_attachment(results[k]) for k, v in incident_map.items() if k in results
+        v: build_incident_attachment(
+            action=None,
+            incident=results[k],
+        )
+        for k, v in incident_map.items()
+        if k in results
     }
 
 

src/sentry/integrations/slack/utils.py

@@ -309,7 +309,7 @@ def build_group_attachment(
     }
 
 
-def build_incident_attachment(incident, metric_value=None):
+def build_incident_attachment(action, incident, metric_value=None, method=None):
     """
     Builds an incident attachment for slack unfurling
     :param incident: The `Incident` to build the attachment for
@@ -318,7 +318,7 @@ def build_incident_attachment(incident, metric_value=None):
     :return:
     """
 
-    data = incident_attachment_info(incident, metric_value)
+    data = incident_attachment_info(incident, metric_value, action=action, method=method)
 
     colors = {
         "Resolved": RESOLVED_COLOR,
@@ -449,8 +449,7 @@ def get_channel_id_with_timeout(integration, name, timeout):
     return (prefix, None, False)
 
 
-def send_incident_alert_notification(action, incident, metric_value):
-
+def send_incident_alert_notification(action, incident, metric_value, method):
     # Make sure organization integration is still active:
     try:
         integration = Integration.objects.get(
@@ -463,7 +462,7 @@ def send_incident_alert_notification(action, incident, metric_value):
         return
 
     channel = action.target_identifier
-    attachment = build_incident_attachment(incident, metric_value)
+    attachment = build_incident_attachment(action, incident, metric_value, method)
     payload = {
         "token": integration.metadata["access_token"],
         "channel": channel,