feat(slack): Show culprit in notifications (#72980)

Just wrapping @olagjo's PR
https://github.com/getsentry/sentry/pull/72587 around a FF so we can
quickly turn it on/off.

---------

Co-authored-by: Ola Gjønnes <ola.gjonnes@gmail.com>
Co-authored-by: Yash Kamothi <yash.kamothi@gmail.com>
Co-authored-by: Cathy Teng <70817427+cathteng@users.noreply.github.com>

src/sentry/features/temporary.py

@@ -384,6 +384,8 @@ def register_temporary_features(manager: FeatureManager):
     manager.add("organizations:session-replay-web-vitals", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, default=False)
     # Lets organizations manage grouping configs
     manager.add("organizations:set-grouping-config", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
+    # Enable Culprit Blocks in Slack Notifications
+    manager.add("organizations:slack-culprit-blocks", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE)
     # Enable description field in Slack metric alerts
     manager.add("organizations:slack-metric-alert-description", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
     # Enable improvements to Slack notifications

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

@@ -9,7 +9,7 @@ import orjson
 from django.core.exceptions import ObjectDoesNotExist
 from sentry_relay.processing import parse_release
 
-from sentry import tagstore
+from sentry import features, tagstore
 from sentry.api.endpoints.group_details import get_group_global_count
 from sentry.constants import LOG_LEVELS_MAP
 from sentry.eventstore.models import GroupEvent
@@ -518,6 +518,11 @@ class SlackIssuesMessageBuilder(BlockSlackMessageBuilder):
 
         return self.get_markdown_block(title_text)
 
+    def get_culprit_block(self, event_or_group: GroupEvent | Group) -> SlackBlock | None:
+        if event_or_group.culprit and isinstance(event_or_group.culprit, str):
+            return self.get_context_block(event_or_group.culprit)
+        return None
+
     def get_text_block(self, text) -> SlackBlock:
         if self.group.issue_category == GroupCategory.FEEDBACK:
             max_block_text_length = USER_FEEDBACK_MAX_BLOCK_TEXT_LENGTH
@@ -602,6 +607,11 @@ class SlackIssuesMessageBuilder(BlockSlackMessageBuilder):
 
         blocks = [self.get_title_block(rule_id, notification_uuid, obj, has_action)]
 
+        if features.has("organizations:slack-culprit-blocks", project.organization) and (
+            culprit_block := self.get_culprit_block(obj)
+        ):
+            blocks.append(culprit_block)
+
         # build up text block
         text = text.lstrip(" ")
         # XXX(CEO): sometimes text is " " and slack will error if we pass an empty string (now "")

src/sentry/testutils/cases.py

@@ -2970,6 +2970,38 @@ class SlackActivityNotificationTest(ActivityTestCase):
             == f"{project_slug} | production | <http://testserver/settings/account/notifications/{alert_type}/?referrer={referrer}-user&notification_uuid={notification_uuid}{optional_org_id}|Notification Settings>"
         )
 
+    def assert_performance_issue_blocks_with_culprit_blocks(
+        self,
+        blocks,
+        org: Organization,
+        project_slug: str,
+        group,
+        referrer,
+        alert_type: FineTuningAPIKey = FineTuningAPIKey.WORKFLOW,
+        issue_link_extra_params=None,
+    ):
+        notification_uuid = self.get_notification_uuid(blocks[1]["text"]["text"])
+        issue_link = f"http://testserver/organizations/{org.slug}/issues/{group.id}/?referrer={referrer}&notification_uuid={notification_uuid}"
+        if issue_link_extra_params is not None:
+            issue_link += issue_link_extra_params
+        assert (
+            blocks[1]["text"]["text"]
+            == f":large_blue_circle: :chart_with_upwards_trend: <{issue_link}|*N+1 Query*>"
+        )
+        assert blocks[2]["elements"][0]["text"] == "/books/"
+        assert (
+            blocks[3]["text"]["text"]
+            == "```db - SELECT `books_author`.`id`, `books_author`.`name` FROM `books_author` WHERE `books_author`.`id` = %s LIMIT 21```"
+        )
+        assert (
+            blocks[4]["elements"][0]["text"] == "State: *Ongoing*   First Seen: *10\xa0minutes ago*"
+        )
+        optional_org_id = f"&organizationId={org.id}" if alert_page_needs_org_id(alert_type) else ""
+        assert (
+            blocks[5]["elements"][0]["text"]
+            == f"{project_slug} | production | <http://testserver/settings/account/notifications/{alert_type}/?referrer={referrer}-user&notification_uuid={notification_uuid}{optional_org_id}|Notification Settings>"
+        )
+
     def assert_generic_issue_attachments(
         self, attachment, project_slug, referrer, alert_type="workflow"
     ):
@@ -2990,6 +3022,7 @@ class SlackActivityNotificationTest(ActivityTestCase):
         referrer,
         alert_type="workflow",
         issue_link_extra_params=None,
+        with_culprit=False,
     ):
         notification_uuid = self.get_notification_uuid(blocks[1]["text"]["text"])
         issue_link = f"http://testserver/organizations/{org.slug}/issues/{group.id}/?referrer={referrer}&notification_uuid={notification_uuid}"
@@ -2999,8 +3032,14 @@ class SlackActivityNotificationTest(ActivityTestCase):
             blocks[1]["text"]["text"]
             == f":red_circle: <{issue_link}|*{TEST_ISSUE_OCCURRENCE.issue_title}*>"
         )
+        if with_culprit:
+            assert blocks[2]["elements"][0]["text"] == "raven.tasks.run_a_test"
+            evidence_index = 3
+        else:
+            evidence_index = 2
+
         assert (
-            blocks[2]["text"]["text"]
+            blocks[evidence_index]["text"]["text"]
             == "```" + TEST_ISSUE_OCCURRENCE.evidence_display[0].value + "```"
         )
 

tests/sentry/integrations/slack/notifications/test_assigned.py

@@ -7,6 +7,7 @@ from sentry.integrations.types import ExternalProviders
 from sentry.models.activity import Activity
 from sentry.notifications.notifications.activity.assigned import AssignedActivityNotification
 from sentry.testutils.cases import PerformanceIssueTestCase, SlackActivityNotificationTest
+from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE, TEST_PERF_ISSUE_OCCURRENCE
 from sentry.testutils.helpers.slack import get_blocks_and_fallback_text
 from sentry.testutils.skips import requires_snuba
@@ -186,6 +187,33 @@ class SlackAssignedNotificationTest(SlackActivityNotificationTest, PerformanceIs
             "assigned_activity-slack",
         )
 
+    @responses.activate
+    @mock.patch(
+        "sentry.eventstore.models.GroupEvent.occurrence",
+        return_value=TEST_PERF_ISSUE_OCCURRENCE,
+        new_callable=mock.PropertyMock,
+    )
+    @with_feature("organizations:slack-culprit-blocks")
+    def test_assignment_performance_issue_block_with_culprit_blocks(self, occurrence):
+        """
+        Test that a Slack message is sent with the expected payload when a performance issue is assigned
+        and block kit is enabled.
+        """
+        event = self.create_performance_issue()
+        with self.tasks():
+            self.create_notification(event.group, AssignedActivityNotification).send()
+
+        blocks, fallback_text = get_blocks_and_fallback_text()
+        assert fallback_text == f"Issue assigned to {self.name} by themselves"
+        assert blocks[0]["text"]["text"] == fallback_text
+        self.assert_performance_issue_blocks_with_culprit_blocks(
+            blocks,
+            event.organization,
+            event.project.slug,
+            event.group,
+            "assigned_activity-slack",
+        )
+
     def test_automatic_assignment(self):
         notification = AssignedActivityNotification(
             Activity(

tests/sentry/integrations/slack/notifications/test_issue_alert.py

@@ -31,6 +31,7 @@ from sentry.plugins.base import Notification
 from sentry.silo.base import SiloMode
 from sentry.tasks.digests import deliver_digest
 from sentry.testutils.cases import PerformanceIssueTestCase, SlackActivityNotificationTest
+from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE, TEST_PERF_ISSUE_OCCURRENCE
 from sentry.testutils.helpers.slack import get_blocks_and_fallback_text
 from sentry.testutils.silo import assume_test_silo_mode
@@ -136,6 +137,44 @@ class SlackIssueAlertNotificationTest(SlackActivityNotificationTest, Performance
             issue_link_extra_params=f"&alert_rule_id={self.rule.id}&alert_type=issue",
         )
 
+    @responses.activate
+    @mock.patch("sentry.integrations.slack.message_builder.issues.get_tags", new=fake_get_tags)
+    @mock.patch(
+        "sentry.eventstore.models.GroupEvent.occurrence",
+        return_value=TEST_PERF_ISSUE_OCCURRENCE,
+        new_callable=mock.PropertyMock,
+    )
+    @with_feature("organizations:slack-culprit-blocks")
+    def test_performance_issue_alert_user_block_with_culprit_blocks(self, occurrence):
+        """
+        Test that performance issue alerts are sent to a Slack user with the proper payload when
+        block kit is enabled.
+        """
+
+        event = self.create_performance_issue()
+        # this is a PerformanceNPlusOneGroupType event
+        notification = AlertRuleNotification(
+            Notification(event=event, rule=self.rule), ActionTargetType.MEMBER, self.user.id
+        )
+        with self.tasks():
+            notification.send()
+
+        blocks, fallback_text = get_blocks_and_fallback_text()
+        assert (
+            fallback_text
+            == f"Alert triggered <http://testserver/organizations/{event.organization.slug}/alerts/rules/{event.project.slug}/{self.rule.id}/details/|ja rule>"
+        )
+        assert blocks[0]["text"]["text"] == fallback_text
+        self.assert_performance_issue_blocks_with_culprit_blocks(
+            blocks,
+            event.organization,
+            event.project.slug,
+            event.group,
+            "issue_alert-slack",
+            alert_type=FineTuningAPIKey.ALERTS,
+            issue_link_extra_params=f"&alert_rule_id={self.rule.id}&alert_type=issue",
+        )
+
     @mock.patch("sentry.integrations.slack.message_builder.issues.get_tags", new=fake_get_tags)
     @responses.activate
     def test_crons_issue_alert_user_block(self):
@@ -452,6 +491,109 @@ class SlackIssueAlertNotificationTest(SlackActivityNotificationTest, Performance
             == f"{event.project.slug} | <http://testserver/settings/{event.organization.slug}/teams/{self.team.slug}/notifications/?referrer=issue_alert-slack-team&notification_uuid={notification_uuid}|Notification Settings>"
         )
 
+    @responses.activate
+    @with_feature("organizations:slack-culprit-blocks")
+    def test_issue_alert_team_issue_owners_block_with_culprit_blocks(self):
+        """
+        Test that issue alerts are sent to a team in Slack via an Issue Owners rule action with the
+        proper payload when block kit is enabled.
+        """
+
+        # add a second user to the team so we can be sure it's only
+        # sent once (to the team, and not to each individual user)
+        user2 = self.create_user(is_superuser=False)
+        self.create_member(teams=[self.team], user=user2, organization=self.organization)
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            self.idp = self.create_identity_provider(type="slack", external_id="TXXXXXXX2")
+            self.identity = Identity.objects.create(
+                external_id="UXXXXXXX2",
+                idp=self.idp,
+                user=user2,
+                status=IdentityStatus.VALID,
+                scopes=[],
+            )
+        # update the team's notification settings
+        ExternalActor.objects.create(
+            team_id=self.team.id,
+            organization=self.organization,
+            integration_id=self.integration.id,
+            provider=ExternalProviders.SLACK.value,
+            external_name="goma",
+            external_id="CXXXXXXX2",
+        )
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            # provider is disabled by default
+            NotificationSettingProvider.objects.create(
+                team_id=self.team.id,
+                scope_type="team",
+                scope_identifier=self.team.id,
+                provider="slack",
+                type="alerts",
+                value="always",
+            )
+
+        g_rule = GrammarRule(Matcher("path", "*"), [Owner("team", self.team.slug)])
+        ProjectOwnership.objects.create(project_id=self.project.id, schema=dump_schema([g_rule]))
+
+        event = self.store_event(
+            data={
+                "message": "Hello world",
+                "level": "error",
+                "stacktrace": {"frames": [{"filename": "foo.py"}]},
+            },
+            project_id=self.project.id,
+        )
+
+        action_data = {
+            "id": "sentry.mail.actions.NotifyEmailAction",
+            "targetType": "IssueOwners",
+            "targetIdentifier": "",
+        }
+        rule = Rule.objects.create(
+            project=self.project,
+            label="ja rule",
+            data={
+                "match": "all",
+                "actions": [action_data],
+            },
+        )
+
+        notification = AlertRuleNotification(
+            Notification(event=event, rule=rule), ActionTargetType.ISSUE_OWNERS, self.team.id
+        )
+
+        with self.tasks():
+            notification.send()
+
+        # check that only one was sent out - more would mean each user is being notified
+        # rather than the team
+        assert len(responses.calls) == 1
+
+        # check that the team got a notification
+        data = parse_qs(responses.calls[0].request.body)
+        assert data["channel"] == ["CXXXXXXX2"]
+        assert "blocks" in data
+        assert "text" in data
+        blocks = orjson.loads(data["blocks"][0])
+        fallback_text = data["text"][0]
+        notification_uuid = notification.notification_uuid
+
+        assert (
+            fallback_text
+            == f"Alert triggered <http://testserver/organizations/{event.organization.slug}/alerts/rules/{event.project.slug}/{rule.id}/details/|ja rule>"
+        )
+        assert blocks[0]["text"]["text"] == fallback_text
+        assert event.group
+        assert (
+            blocks[1]["text"]["text"]
+            == f":red_circle: <http://testserver/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&alert_rule_id={rule.id}&alert_type=issue|*Hello world*>"
+        )
+        assert blocks[6]["elements"][0]["text"] == f"Suggested Assignees: #{self.team.slug}"
+        assert (
+            blocks[7]["elements"][0]["text"]
+            == f"{event.project.slug} | <http://testserver/settings/{event.organization.slug}/teams/{self.team.slug}/notifications/?referrer=issue_alert-slack-team&notification_uuid={notification_uuid}|Notification Settings>"
+        )
+
     @responses.activate
     def test_disabled_org_integration_for_team(self):
         # update the team's notification settings

tests/sentry/integrations/slack/notifications/test_regression.py

@@ -5,6 +5,7 @@ import responses
 from sentry.models.activity import Activity
 from sentry.notifications.notifications.activity.regression import RegressionActivityNotification
 from sentry.testutils.cases import PerformanceIssueTestCase, SlackActivityNotificationTest
+from sentry.testutils.helpers import with_feature
 from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE, TEST_PERF_ISSUE_OCCURRENCE
 from sentry.testutils.helpers.slack import get_blocks_and_fallback_text
 from sentry.testutils.skips import requires_snuba
@@ -94,6 +95,33 @@ class SlackRegressionNotificationTest(SlackActivityNotificationTest, Performance
             "regression_activity-slack",
         )
 
+    @responses.activate
+    @mock.patch(
+        "sentry.eventstore.models.GroupEvent.occurrence",
+        return_value=TEST_PERF_ISSUE_OCCURRENCE,
+        new_callable=mock.PropertyMock,
+    )
+    @with_feature("organizations:slack-culprit-blocks")
+    def test_regression_performance_issue_block_with_culprit_blocks(self, occurrence):
+        """
+        Test that a Slack message is sent with the expected payload when a performance issue regresses
+        and block kit is enabled.
+        """
+        event = self.create_performance_issue()
+        with self.tasks():
+            self.create_notification(event.group).send()
+
+        blocks, fallback_text = get_blocks_and_fallback_text()
+        assert fallback_text == "Issue marked as regression"
+        assert blocks[0]["text"]["text"] == fallback_text
+        self.assert_performance_issue_blocks_with_culprit_blocks(
+            blocks,
+            event.organization,
+            event.project.slug,
+            event.group,
+            "regression_activity-slack",
+        )
+
     @responses.activate
     @mock.patch(
         "sentry.eventstore.models.GroupEvent.occurrence",
@@ -106,7 +134,49 @@ class SlackRegressionNotificationTest(SlackActivityNotificationTest, Performance
         and block kit is enabled.
         """
         event = self.store_event(
-            data={"message": "Hellboy's world", "level": "error"}, project_id=self.project.id
+            data={
+                "message": "Hellboy's world",
+                "level": "error",
+                "culprit": "raven.tasks.run_a_test",
+            },
+            project_id=self.project.id,
+        )
+        group_event = event.for_group(event.groups[0])
+
+        with self.tasks():
+            self.create_notification(group_event.group).send()
+
+        blocks, fallback_text = get_blocks_and_fallback_text()
+        assert fallback_text == "Issue marked as regression"
+        assert blocks[0]["text"]["text"] == fallback_text
+        self.assert_generic_issue_blocks(
+            blocks,
+            group_event.organization,
+            group_event.project.slug,
+            group_event.group,
+            "regression_activity-slack",
+            with_culprit=False,
+        )
+
+    @responses.activate
+    @mock.patch(
+        "sentry.eventstore.models.GroupEvent.occurrence",
+        return_value=TEST_ISSUE_OCCURRENCE,
+        new_callable=mock.PropertyMock,
+    )
+    @with_feature("organizations:slack-culprit-blocks")
+    def test_regression_generic_issue_block_with_culprit_blocks(self, occurrence):
+        """
+        Test that a Slack message is sent with the expected payload when a generic issue type regresses
+        and block kit is enabled.
+        """
+        event = self.store_event(
+            data={
+                "message": "Hellboy's world",
+                "level": "error",
+                "culprit": "raven.tasks.run_a_test",
+            },
+            project_id=self.project.id,
         )
         group_event = event.for_group(event.groups[0])
 
@@ -122,4 +192,5 @@ class SlackRegressionNotificationTest(SlackActivityNotificationTest, Performance
             group_event.project.slug,
             group_event.group,
             "regression_activity-slack",
+            with_culprit=True,
         )

tests/sentry/integrations/slack/notifications/test_resolved.py

@@ -5,6 +5,7 @@ import responses
 from sentry.models.activity import Activity
 from sentry.notifications.notifications.activity.resolved import ResolvedActivityNotification
 from sentry.testutils.cases import PerformanceIssueTestCase, SlackActivityNotificationTest
+from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE, TEST_PERF_ISSUE_OCCURRENCE
 from sentry.testutils.helpers.slack import get_blocks_and_fallback_text
 from sentry.testutils.skips import requires_snuba
@@ -83,6 +84,37 @@ class SlackResolvedNotificationTest(SlackActivityNotificationTest, PerformanceIs
             "resolved_activity-slack",
         )
 
+    @responses.activate
+    @mock.patch(
+        "sentry.eventstore.models.GroupEvent.occurrence",
+        return_value=TEST_PERF_ISSUE_OCCURRENCE,
+        new_callable=mock.PropertyMock,
+    )
+    @with_feature("organizations:slack-culprit-blocks")
+    def test_resolved_performance_issue_block_with_culprit_blocks(self, occurrence):
+        """
+        Test that a Slack message is sent with the expected payload when a performance issue is resolved
+        and block kit is enabled.
+        """
+        event = self.create_performance_issue()
+        with self.tasks():
+            self.create_notification(event.group).send()
+
+        blocks, fallback_text = get_blocks_and_fallback_text()
+        notification_uuid = self.get_notification_uuid(blocks[0]["text"]["text"])
+        assert (
+            fallback_text
+            == f"{self.name} marked <http://testserver/organizations/{self.organization.slug}/issues/{event.group.id}/?referrer=activity_notification&notification_uuid={notification_uuid}|{self.project.slug.upper()}-{event.group.short_id}> as resolved"
+        )
+        assert blocks[0]["text"]["text"] == fallback_text
+        self.assert_performance_issue_blocks_with_culprit_blocks(
+            blocks,
+            event.organization,
+            event.project.slug,
+            event.group,
+            "resolved_activity-slack",
+        )
+
     @responses.activate
     @mock.patch(
         "sentry.eventstore.models.GroupEvent.occurrence",

tests/sentry/integrations/slack/notifications/test_resolved_in_release.py

@@ -7,6 +7,7 @@ from sentry.notifications.notifications.activity.resolved_in_release import (
     ResolvedInReleaseActivityNotification,
 )
 from sentry.testutils.cases import PerformanceIssueTestCase, SlackActivityNotificationTest
+from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE, TEST_PERF_ISSUE_OCCURRENCE
 from sentry.testutils.helpers.slack import get_blocks_and_fallback_text
 from sentry.testutils.skips import requires_snuba
@@ -77,6 +78,35 @@ class SlackResolvedInReleaseNotificationTest(
             "resolved_in_release_activity-slack",
         )
 
+    @responses.activate
+    @mock.patch(
+        "sentry.eventstore.models.GroupEvent.occurrence",
+        return_value=TEST_PERF_ISSUE_OCCURRENCE,
+        new_callable=mock.PropertyMock,
+    )
+    @with_feature("organizations:slack-culprit-blocks")
+    def test_resolved_in_release_performance_issue_block_with_culprit_blocks(self, occurrence):
+        """
+        Test that a Slack message is sent with the expected payload when a performance issue is resolved in a release
+        and block kit is enabled.
+        """
+        event = self.create_performance_issue()
+        notification = self.create_notification(event.group)
+        with self.tasks():
+            notification.send()
+
+        blocks, fallback_text = get_blocks_and_fallback_text()
+        release_name = notification.activity.data["version"]
+        assert fallback_text == f"Issue marked as resolved in {release_name} by {self.name}"
+        assert blocks[0]["text"]["text"] == fallback_text
+        self.assert_performance_issue_blocks_with_culprit_blocks(
+            blocks,
+            event.organization,
+            event.project.slug,
+            event.group,
+            "resolved_in_release_activity-slack",
+        )
+
     @responses.activate
     @mock.patch(
         "sentry.eventstore.models.GroupEvent.occurrence",

tests/sentry/integrations/slack/notifications/test_unassigned.py

@@ -5,6 +5,7 @@ import responses
 from sentry.models.activity import Activity
 from sentry.notifications.notifications.activity.unassigned import UnassignedActivityNotification
 from sentry.testutils.cases import PerformanceIssueTestCase, SlackActivityNotificationTest
+from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE, TEST_PERF_ISSUE_OCCURRENCE
 from sentry.testutils.helpers.slack import get_blocks_and_fallback_text
 from sentry.testutils.skips import requires_snuba
@@ -72,6 +73,33 @@ class SlackUnassignedNotificationTest(SlackActivityNotificationTest, Performance
             "unassigned_activity-slack",
         )
 
+    @responses.activate
+    @mock.patch(
+        "sentry.eventstore.models.GroupEvent.occurrence",
+        return_value=TEST_PERF_ISSUE_OCCURRENCE,
+        new_callable=mock.PropertyMock,
+    )
+    @with_feature("organizations:slack-culprit-blocks")
+    def test_unassignment_performance_issue_block_with_culprit_blocks(self, occurrence):
+        """
+        Test that a Slack message is sent with the expected payload when a performance issue is unassigned
+        and block kit is enabled.
+        """
+        event = self.create_performance_issue()
+        with self.tasks():
+            self.create_notification(event.group).send()
+
+        blocks, fallback_text = get_blocks_and_fallback_text()
+        assert fallback_text == f"Issue unassigned by {self.name}"
+        assert blocks[0]["text"]["text"] == fallback_text
+        self.assert_performance_issue_blocks_with_culprit_blocks(
+            blocks,
+            event.organization,
+            event.project.slug,
+            event.group,
+            "unassigned_activity-slack",
+        )
+
     @responses.activate
     @mock.patch(
         "sentry.eventstore.models.GroupEvent.occurrence",