feat(workflow): Add graph to slack incident message (#34937)

Adds a metric alert graph to incident slack alerts. Changes the slack messages over to the slack "blocks" format. I've added what was previously part of the footer as part of the message since footers aren't supported via blocks.

src/sentry/integrations/metric_alerts.py

@@ -76,7 +76,7 @@ def get_incident_status_text(alert_rule: AlertRule, metric_value: str) -> str:
     return text
 
 
-def incident_attachment_info(incident, new_status: IncidentStatus, metric_value=None, unfurl=False):
+def incident_attachment_info(incident, new_status: IncidentStatus, metric_value=None):
     alert_rule = incident.alert_rule
 
     status = INCIDENT_STATUS[new_status]
@@ -87,22 +87,17 @@ def incident_attachment_info(incident, new_status: IncidentStatus, metric_value=
     text = get_incident_status_text(alert_rule, metric_value)
     title = f"{status}: {alert_rule.name}"
 
-    if unfurl:
-        # this URL is needed for the Slack unfurl, but nothing else
-        title_link = absolute_uri(
-            f"organizations/{incident.organization.slug}/alerts/rules/details/{incident.identifier}"
-        )
-
-    else:
-        title_link = absolute_uri(
-            reverse(
-                "sentry-metric-alert",
-                kwargs={
-                    "organization_slug": incident.organization.slug,
-                    "incident_id": incident.identifier,
-                },
-            )
+    title_link = absolute_uri(
+        reverse(
+            "sentry-metric-alert-details",
+            kwargs={
+                "organization_slug": alert_rule.organization.slug,
+                "alert_rule_id": alert_rule.id,
+            },
         )
+    )
+    params = parse.urlencode({"alert": str(incident.identifier)})
+    title_link += f"?{params}"
 
     return {
         "title": title,

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

@@ -3,23 +3,28 @@ from typing import Optional
 
 from sentry.incidents.models import Incident, IncidentStatus
 from sentry.integrations.metric_alerts import incident_attachment_info
-from sentry.integrations.slack.message_builder import INCIDENT_COLOR_MAPPING, SlackBody
-from sentry.integrations.slack.message_builder.base.base import SlackMessageBuilder
+from sentry.integrations.slack.message_builder import (
+    INCIDENT_COLOR_MAPPING,
+    LEVEL_TO_COLOR,
+    SlackBody,
+)
+from sentry.integrations.slack.message_builder.base.block import BlockSlackMessageBuilder
 from sentry.utils.dates import to_timestamp
 
 
-def get_footer(timestamp: datetime) -> str:
-    return "<!date^{:.0f}^Sentry Incident - Started {} at {} | Sentry Incident>".format(
+def get_started_at(timestamp: datetime) -> str:
+    return "<!date^{:.0f}^Started {} at {} | Sentry Incident>".format(
         to_timestamp(timestamp), "{date_pretty}", "{time}"
     )
 
 
-class SlackIncidentsMessageBuilder(SlackMessageBuilder):
+class SlackIncidentsMessageBuilder(BlockSlackMessageBuilder):
     def __init__(
         self,
         incident: Incident,
         new_status: IncidentStatus,
         metric_value: Optional[int] = None,
+        chart_url: Optional[str] = None,
     ) -> None:
         """
         Builds an incident attachment for slack unfurling.
@@ -33,30 +38,22 @@ class SlackIncidentsMessageBuilder(SlackMessageBuilder):
         self.incident = incident
         self.metric_value = metric_value
         self.new_status = new_status
+        self.chart_url = chart_url
 
-    def build(self, unfurl: bool = False) -> SlackBody:
-        data = incident_attachment_info(
-            self.incident, self.new_status, self.metric_value, unfurl=unfurl
-        )
+    def build(self) -> SlackBody:
+        data = incident_attachment_info(self.incident, self.new_status, self.metric_value)
 
-        return self._build(
-            actions=[],
-            color=INCIDENT_COLOR_MAPPING.get(data["status"]),
-            fallback=data["title"],
-            fields=[],
-            footer=get_footer(data["ts"]),
-            text=data["text"],
-            title=data["title"],
-            title_link=data["title_link"],
-        )
+        blocks = [
+            self.get_markdown_block(
+                text=f"<{data['title_link']}|*{data['title']}*>  \n{data['text']}\n{get_started_at(data['ts'])}"
+            )
+        ]
 
+        if self.chart_url:
+            blocks.append(self.get_image_block(self.chart_url, alt="Metric Alert Chart"))
 
-def build_incident_attachment(
-    incident: Incident,
-    metric_value: Optional[int] = None,
-    unfurl: bool = False,
-) -> SlackBody:
-    """@deprecated"""
-    return SlackIncidentsMessageBuilder(
-        incident, IncidentStatus(incident.status), metric_value
-    ).build(unfurl=unfurl)
+        color = LEVEL_TO_COLOR.get(INCIDENT_COLOR_MAPPING.get(data["status"], ""))
+        return self._build_blocks(
+            *blocks,
+            color=color,
+        )

src/sentry/integrations/slack/utils/notifications.py

@@ -2,7 +2,11 @@ from __future__ import annotations
 
 from typing import Mapping
 
+import sentry_sdk
+
+from sentry import features
 from sentry.constants import ObjectStatus
+from sentry.incidents.charts import build_metric_alert_chart
 from sentry.incidents.models import AlertRuleTriggerAction, Incident, IncidentStatus
 from sentry.integrations.slack.client import SlackClient
 from sentry.integrations.slack.message_builder.incidents import SlackIncidentsMessageBuilder
@@ -30,8 +34,19 @@ def send_incident_alert_notification(
         # Integration removed, but rule is still active.
         return
 
+    chart_url = None
+    if features.has("organizations:metric-alert-chartcuterie", incident.organization):
+        try:
+            chart_url = build_metric_alert_chart(
+                organization=incident.organization,
+                alert_rule=incident.alert_rule,
+                selected_incident=incident,
+            )
+        except Exception as e:
+            sentry_sdk.capture_exception(e)
+
     channel = action.target_identifier
-    attachment = SlackIncidentsMessageBuilder(incident, new_status, metric_value).build()
+    attachment = SlackIncidentsMessageBuilder(incident, new_status, metric_value, chart_url).build()
     payload = {
         "token": integration.metadata["access_token"],
         "channel": channel,

tests/sentry/incidents/action_handlers/__init__.py

@@ -9,7 +9,7 @@ class FireTest(abc.ABC):
     def run_test(self, incident: Incident, method: str):
         pass
 
-    def run_fire_test(self, method="fire"):
+    def run_fire_test(self, method="fire", chart_url=None):
         self.alert_rule = self.create_alert_rule()
         incident = self.create_incident(
             alert_rule=self.alert_rule, status=IncidentStatus.CLOSED.value

tests/sentry/incidents/action_handlers/test_pagerduty.py

@@ -66,7 +66,10 @@ class PagerDutyActionHandlerTest(FireTest, TestCase):
             "details": "1000 events in the last 10 minutes"
         }
         assert data["links"][0]["text"] == f"Critical: {alert_rule.name}"
-        assert data["links"][0]["href"] == "http://testserver/organizations/baz/alerts/1/"
+        assert (
+            data["links"][0]["href"]
+            == f"http://testserver/organizations/baz/alerts/rules/details/{alert_rule.id}/?alert={incident.identifier}"
+        )
 
     @responses.activate
     def run_test(self, incident, method):

tests/sentry/incidents/action_handlers/test_slack.py

@@ -15,8 +15,8 @@ from . import FireTest
 @freeze_time()
 class SlackActionHandlerTest(FireTest, TestCase):
     @responses.activate
-    def run_test(self, incident, method):
-        from sentry.integrations.slack.message_builder.incidents import build_incident_attachment
+    def run_test(self, incident, method, chart_url=None):
+        from sentry.integrations.slack.message_builder.incidents import SlackIncidentsMessageBuilder
 
         token = "xoxp-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx"
         integration = Integration.objects.create(
@@ -57,8 +57,11 @@ class SlackActionHandlerTest(FireTest, TestCase):
         data = parse_qs(responses.calls[1].request.body)
         assert data["channel"] == [channel_id]
         assert data["token"] == [token]
-        assert json.loads(data["attachments"][0])[0] == build_incident_attachment(
-            incident, metric_value
+        assert (
+            json.loads(data["attachments"][0])[0]
+            == SlackIncidentsMessageBuilder(
+                incident, IncidentStatus(incident.status), metric_value, chart_url
+            ).build()
         )
 
     def test_fire_metric_alert(self):
@@ -67,12 +70,15 @@ class SlackActionHandlerTest(FireTest, TestCase):
     def test_resolve_metric_alert(self):
         self.run_fire_test("resolve")
 
+    def test_fire_metric_alert_with_chart(self):
+        self.run_fire_test(chart_url="chart-url")
+
 
 @freeze_time()
 class SlackWorkspaceActionHandlerTest(FireTest, TestCase):
     @responses.activate
     def run_test(self, incident, method):
-        from sentry.integrations.slack.message_builder.incidents import build_incident_attachment
+        from sentry.integrations.slack.message_builder.incidents import SlackIncidentsMessageBuilder
 
         token = "xoxb-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx"
         integration = Integration.objects.create(
@@ -113,8 +119,11 @@ class SlackWorkspaceActionHandlerTest(FireTest, TestCase):
         data = parse_qs(responses.calls[1].request.body)
         assert data["channel"] == [channel_id]
         assert data["token"] == [token]
-        assert json.loads(data["attachments"][0])[0] == build_incident_attachment(
-            incident, metric_value
+        assert (
+            json.loads(data["attachments"][0])[0]
+            == SlackIncidentsMessageBuilder(
+                incident, IncidentStatus(incident.status), metric_value
+            ).build()
         )
 
     def test_fire_metric_alert(self):

tests/sentry/incidents/test_subscription_processor.py

@@ -260,12 +260,15 @@ class ProcessUpdateTest(ProcessUpdateBaseClass):
         return processor
 
     def assert_slack_calls(self, trigger_labels):
-        expected = [f"{label}: some rule 2" for label in trigger_labels]
+        expected_result = [f"{label}: some rule 2" for label in trigger_labels]
         actual = [
-            json.loads(call_kwargs["data"]["attachments"])[0]["title"]
+            json.loads(call_kwargs["data"]["attachments"])[0]["blocks"][0]["text"]["text"]
             for (_, call_kwargs) in self.slack_client.call_args_list
         ]
-        assert expected == actual
+
+        assert len(expected_result) == len(actual)
+        for expected, result in zip(expected_result, actual):
+            assert expected in result
         self.slack_client.reset_mock()
 
     def test_removed_alert_rule(self):
@@ -1075,6 +1078,80 @@ class ProcessUpdateTest(ProcessUpdateBaseClass):
         self.assert_slack_calls(["Warning"])
         self.assert_active_incident(rule)
 
+    @patch("sentry.incidents.charts.generate_chart", return_value="chart-url")
+    def test_slack_metric_alert_chart(self, mock_generate_chart):
+        from sentry.incidents.action_handlers import SlackActionHandler
+
+        slack_handler = SlackActionHandler
+
+        # Create Slack Integration
+        integration = Integration.objects.create(
+            provider="slack",
+            name="Team A",
+            external_id="TXXXXXXX1",
+            metadata={
+                "access_token": "xoxp-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx",
+                "installation_type": "born_as_bot",
+            },
+        )
+        integration.add_organization(self.project.organization, self.user)
+
+        # Register Slack Handler
+        AlertRuleTriggerAction.register_type(
+            "slack",
+            AlertRuleTriggerAction.Type.SLACK,
+            [AlertRuleTriggerAction.TargetType.SPECIFIC],
+            integration_provider="slack",
+        )(slack_handler)
+
+        rule = self.create_alert_rule(
+            projects=[self.project, self.other_project],
+            name="some rule 2",
+            query="",
+            aggregate="count()",
+            time_window=1,
+            threshold_type=AlertRuleThresholdType.ABOVE,
+            resolve_threshold=10,
+            threshold_period=1,
+        )
+
+        trigger = create_alert_rule_trigger(rule, "critical", 100)
+        channel_name = "#workflow"
+        create_alert_rule_trigger_action(
+            trigger,
+            AlertRuleTriggerAction.Type.SLACK,
+            AlertRuleTriggerAction.TargetType.SPECIFIC,
+            integration=integration,
+            input_channel_id=channel_name,
+            target_identifier=channel_name,
+        )
+
+        # Send Critical Update
+        with self.feature(
+            [
+                "organizations:incidents",
+                "organizations:discover",
+                "organizations:discover-basic",
+                "organizations:metric-alert-chartcuterie",
+            ]
+        ):
+            self.send_update(
+                rule,
+                trigger.alert_threshold + 5,
+                timedelta(minutes=-10),
+                subscription=rule.snuba_query.subscriptions.filter(project=self.project).get(),
+            )
+
+        self.assert_slack_calls(["Critical"])
+        incident = self.assert_active_incident(rule)
+
+        assert len(mock_generate_chart.mock_calls) == 1
+        chart_data = mock_generate_chart.call_args[0][1]
+        assert chart_data["rule"]["id"] == str(rule.id)
+        assert chart_data["selectedIncident"]["identifier"] == str(incident.identifier)
+        series_data = chart_data["timeseriesData"][0]["data"]
+        assert len(series_data) > 0
+
     def test_slack_multiple_triggers_critical_fired_twice_before_warning(self):
         """
         Test that ensures that when we get a critical update is sent followed by a warning update,

tests/sentry/integrations/slack/test_message_builder.py

@@ -14,7 +14,6 @@ from sentry.integrations.slack.message_builder.issues import SlackIssuesMessageB
 from sentry.integrations.slack.message_builder.metric_alerts import SlackMetricAlertMessageBuilder
 from sentry.models import Group, Team, User
 from sentry.testutils import TestCase
-from sentry.utils.assets import get_asset_url
 from sentry.utils.dates import to_timestamp
 from sentry.utils.http import absolute_uri
 
@@ -140,7 +139,6 @@ class BuildGroupAttachmentTest(TestCase):
 
 class BuildIncidentAttachmentTest(TestCase):
     def test_simple(self):
-        logo_url = absolute_uri(get_asset_url("sentry", "images/sentry-email-avatar.png"))
         alert_rule = self.create_alert_rule()
         incident = self.create_incident(alert_rule=alert_rule, status=2)
         trigger = self.create_alert_rule_trigger(alert_rule, CRITICAL_TRIGGER_LABEL, 100)
@@ -148,34 +146,35 @@ class BuildIncidentAttachmentTest(TestCase):
             alert_rule_trigger=trigger, triggered_for_incident=incident
         )
         title = f"Resolved: {alert_rule.name}"
-        incident_footer_ts = (
-            "<!date^{:.0f}^Sentry Incident - Started {} at {} | Sentry Incident>".format(
-                to_timestamp(incident.date_started), "{date_pretty}", "{time}"
-            )
+        timestamp = "<!date^{:.0f}^Started {} at {} | Sentry Incident>".format(
+            to_timestamp(incident.date_started), "{date_pretty}", "{time}"
         )
-        assert SlackIncidentsMessageBuilder(incident, IncidentStatus.CLOSED).build() == {
-            "fallback": title,
-            "title": title,
-            "title_link": absolute_uri(
+        link = (
+            absolute_uri(
                 reverse(
-                    "sentry-metric-alert",
+                    "sentry-metric-alert-details",
                     kwargs={
-                        "organization_slug": incident.organization.slug,
-                        "incident_id": incident.identifier,
+                        "organization_slug": alert_rule.organization.slug,
+                        "alert_rule_id": alert_rule.id,
                     },
                 )
-            ),
-            "text": "0 events in the last 10 minutes",
-            "fields": [],
-            "mrkdwn_in": ["text"],
-            "footer_icon": logo_url,
-            "footer": incident_footer_ts,
+            )
+            + f"?alert={incident.identifier}"
+        )
+        assert SlackIncidentsMessageBuilder(incident, IncidentStatus.CLOSED).build() == {
+            "blocks": [
+                {
+                    "type": "section",
+                    "text": {
+                        "type": "mrkdwn",
+                        "text": f"<{link}|*{title}*>  \n0 events in the last 10 minutes\n{timestamp}",
+                    },
+                }
+            ],
             "color": LEVEL_TO_COLOR["_incident_resolved"],
-            "actions": [],
         }
 
     def test_metric_value(self):
-        logo_url = absolute_uri(get_asset_url("sentry", "images/sentry-email-avatar.png"))
         alert_rule = self.create_alert_rule()
         incident = self.create_incident(alert_rule=alert_rule, status=2)
 
@@ -186,34 +185,72 @@ class BuildIncidentAttachmentTest(TestCase):
         self.create_alert_rule_trigger_action(
             alert_rule_trigger=trigger, triggered_for_incident=incident
         )
-        incident_footer_ts = (
-            "<!date^{:.0f}^Sentry Incident - Started {} at {} | Sentry Incident>".format(
-                to_timestamp(incident.date_started), "{date_pretty}", "{time}"
+        timestamp = "<!date^{:.0f}^Started {} at {} | Sentry Incident>".format(
+            to_timestamp(incident.date_started), "{date_pretty}", "{time}"
+        )
+        link = (
+            absolute_uri(
+                reverse(
+                    "sentry-metric-alert-details",
+                    kwargs={
+                        "organization_slug": alert_rule.organization.slug,
+                        "alert_rule_id": alert_rule.id,
+                    },
+                )
             )
+            + f"?alert={incident.identifier}"
         )
         # This should fail because it pulls status from `action` instead of `incident`
         assert SlackIncidentsMessageBuilder(
             incident, IncidentStatus.CRITICAL, metric_value=metric_value
         ).build() == {
-            "fallback": title,
-            "title": title,
-            "title_link": absolute_uri(
+            "blocks": [
+                {
+                    "type": "section",
+                    "text": {
+                        "type": "mrkdwn",
+                        "text": f"<{link}|*{title}*>  \n5000 events in the last 10 minutes\n{timestamp}",
+                    },
+                }
+            ],
+            "color": LEVEL_TO_COLOR["fatal"],
+        }
+
+    def test_chart(self):
+        alert_rule = self.create_alert_rule()
+        incident = self.create_incident(alert_rule=alert_rule, status=2)
+        trigger = self.create_alert_rule_trigger(alert_rule, CRITICAL_TRIGGER_LABEL, 100)
+        self.create_alert_rule_trigger_action(
+            alert_rule_trigger=trigger, triggered_for_incident=incident
+        )
+        title = f"Resolved: {alert_rule.name}"
+        timestamp = "<!date^{:.0f}^Started {} at {} | Sentry Incident>".format(
+            to_timestamp(incident.date_started), "{date_pretty}", "{time}"
+        )
+        link = (
+            absolute_uri(
                 reverse(
-                    "sentry-metric-alert",
+                    "sentry-metric-alert-details",
                     kwargs={
-                        "organization_slug": incident.organization.slug,
-                        "incident_id": incident.identifier,
+                        "organization_slug": alert_rule.organization.slug,
+                        "alert_rule_id": alert_rule.id,
                     },
                 )
-            ),
-            "text": f"{metric_value} events in the last 10 minutes",
-            "fields": [],
-            "mrkdwn_in": ["text"],
-            "footer_icon": logo_url,
-            "footer": incident_footer_ts,
-            "color": LEVEL_TO_COLOR["fatal"],
-            "actions": [],
-        }
+            )
+            + f"?alert={incident.identifier}"
+        )
+        assert SlackIncidentsMessageBuilder(
+            incident, IncidentStatus.CLOSED, chart_url="chart-url"
+        ).build()["blocks"] == [
+            {
+                "type": "section",
+                "text": {
+                    "type": "mrkdwn",
+                    "text": f"<{link}|*{title}*>  \n0 events in the last 10 minutes\n{timestamp}",
+                },
+            },
+            {"alt_text": "Metric Alert Chart", "image_url": "chart-url", "type": "image"},
+        ]
 
 
 class BuildMetricAlertAttachmentTest(TestCase):

tests/sentry/integrations/test_metric_alerts.py

@@ -35,7 +35,10 @@ class IncidentAttachmentInfoTest(TestCase, BaseIncidentsTest):
         assert data["status"] == "Resolved"
         assert data["text"] == "123 events in the last 10 minutes"
         assert data["ts"] == date_started
-        assert data["title_link"] == "http://testserver/organizations/baz/alerts/1/"
+        assert (
+            data["title_link"]
+            == f"http://testserver/organizations/baz/alerts/rules/details/{alert_rule.id}/?alert={incident.identifier}"
+        )
         assert (
             data["logo_url"]
             == "http://testserver/_static/{version}/sentry/images/sentry-email-avatar.png"
@@ -79,7 +82,10 @@ class IncidentAttachmentInfoTest(TestCase, BaseIncidentsTest):
         assert data["status"] == "Critical"  # Should pull from the action/trigger.
         assert data["text"] == "4 events in the last 10 minutes"
         assert data["ts"] == date_started
-        assert data["title_link"] == "http://testserver/organizations/baz/alerts/1/"
+        assert (
+            data["title_link"]
+            == f"http://testserver/organizations/baz/alerts/rules/details/{alert_rule.id}/?alert={incident.identifier}"
+        )
         assert (
             data["logo_url"]
             == "http://testserver/_static/{version}/sentry/images/sentry-email-avatar.png"
@@ -91,7 +97,10 @@ class IncidentAttachmentInfoTest(TestCase, BaseIncidentsTest):
         assert data["status"] == "Resolved"
         assert data["text"] == "4 events in the last 10 minutes"
         assert data["ts"] == date_started
-        assert data["title_link"] == "http://testserver/organizations/baz/alerts/1/"
+        assert (
+            data["title_link"]
+            == f"http://testserver/organizations/baz/alerts/rules/details/{alert_rule.id}/?alert={incident.identifier}"
+        )
         assert (
             data["logo_url"]
             == "http://testserver/_static/{version}/sentry/images/sentry-email-avatar.png"
@@ -103,7 +112,10 @@ class IncidentAttachmentInfoTest(TestCase, BaseIncidentsTest):
         assert data["status"] == "Resolved"
         assert data["text"] == "4 events in the last 10 minutes"
         assert data["ts"] == date_started
-        assert data["title_link"] == "http://testserver/organizations/baz/alerts/1/"
+        assert (
+            data["title_link"]
+            == f"http://testserver/organizations/baz/alerts/rules/details/{alert_rule.id}/?alert={incident.identifier}"
+        )
         assert (
             data["logo_url"]
             == "http://testserver/_static/{version}/sentry/images/sentry-email-avatar.png"