🐛 fix(aci): update legacy metric alert action firing logic & fix ff check for activity notifs (#97355)

Author: iamrajjoshi

src/sentry/incidents/tasks.py

@@ -65,6 +65,9 @@ def handle_trigger_action(
     metric_value: int | None = None,
     **kwargs: Any,
 ) -> None:
+    from sentry.incidents.grouptype import MetricIssue
+    from sentry.notifications.notification_action.utils import should_fire_workflow_actions
+
     try:
         action = AlertRuleTriggerAction.objects.select_related(
             "alert_rule_trigger", "alert_rule_trigger__alert_rule"
@@ -92,20 +95,22 @@ def handle_trigger_action(
     if notification_uuid is None:
         metrics.incr("incidents.alert_rules.action.incident_activity_missing")
 
-    metrics.incr(
-        "incidents.alert_rules.action.{}.{}".format(
-            AlertRuleTriggerAction.Type(action.type).name.lower(), method
+    # We should only fire using the legacy registry if we are not using the workflow engine
+    if not should_fire_workflow_actions(incident.organization, MetricIssue.type_id):
+        metrics.incr(
+            "incidents.alert_rules.action.{}.{}".format(
+                AlertRuleTriggerAction.Type(action.type).name.lower(), method
+            )
         )
-    )
 
-    getattr(action, method)(
-        action,
-        incident,
-        project,
-        metric_value=metric_value,
-        new_status=IncidentStatus(new_status),
-        notification_uuid=notification_uuid,
-    )
+        getattr(action, method)(
+            action,
+            incident,
+            project,
+            metric_value=metric_value,
+            new_status=IncidentStatus(new_status),
+            notification_uuid=notification_uuid,
+        )
 
 
 @instrumented_task(

src/sentry/workflow_engine/handlers/workflow/workflow_status_update_handler.py

@@ -1,4 +1,3 @@
-from sentry import features
 from sentry.issues.status_change_consumer import group_status_update_registry
 from sentry.issues.status_change_message import StatusChangeMessageData
 from sentry.models.activity import Activity
@@ -20,6 +19,9 @@ def workflow_status_update_handler(
     Since this handler is called in process for the activity, we want
     to queue a task to process workflows asynchronously.
     """
+
+    from sentry.notifications.notification_action.utils import should_fire_workflow_actions
+
     metrics.incr(
         "workflow_engine.tasks.process_workflows.activity_update",
         tags={"activity_type": activity.type},
@@ -36,11 +38,8 @@ def workflow_status_update_handler(
         metrics.incr("workflow_engine.tasks.error.no_detector_id")
         return
 
-    if features.has(
-        "organizations:workflow-engine-single-process-metric-issues", group.organization
-    ) or features.has(
-        "organizations:workflow-engine-process-metric-issue-workflows", group.organization
-    ):
+    # We should only fire actions for activity updates if we should be firing actions
+    if should_fire_workflow_actions(group.organization, group.type):
         process_workflow_activity.delay(
             activity_id=activity.id,
             group_id=group.id,

tests/sentry/incidents/test_tasks.py

@@ -15,6 +15,7 @@
 from sentry.incidents.tasks import handle_trigger_action
 from sentry.testutils.cases import TestCase
 from sentry.testutils.helpers.alert_rule import TemporaryAlertRuleTriggerActionRegistry
+from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.skips import requires_kafka, requires_snuba
 
 pytestmark = [pytest.mark.sentry_metrics, requires_snuba, requires_kafka]
@@ -99,3 +100,24 @@ def test(self) -> None:
                 metric_value=metric_value,
                 notification_uuid=str(activity.notification_uuid),
             )
+
+    @with_feature("organizations:workflow-engine-single-process-metric-issues")
+    def test_when_workflow_engine_is_enabled(self) -> None:
+        with TemporaryAlertRuleTriggerActionRegistry.registry_patched():
+            mock_handler = Mock()
+            AlertRuleTriggerAction.register_type("email", AlertRuleTriggerAction.Type.EMAIL, [])(
+                mock_handler
+            )
+            incident = self.create_incident()
+            metric_value = 1234
+            with self.tasks():
+                handle_trigger_action.delay(
+                    self.action.id,
+                    incident.id,
+                    self.project.id,
+                    "fire",
+                    IncidentStatus.CRITICAL.value,
+                    metric_value=metric_value,
+                )
+            # We should not fire the action if the workflow engine is enabled
+            mock_handler.assert_not_called()

tests/sentry/workflow_engine/test_task.py

@@ -3,6 +3,7 @@
 import sentry_sdk
 from google.api_core.exceptions import RetryError
 
+from sentry.incidents.grouptype import MetricIssue
 from sentry.issues.status_change_consumer import process_status_change_message, update_status
 from sentry.issues.status_change_message import StatusChangeMessageData
 from sentry.models.activity import Activity
@@ -91,10 +92,10 @@ def test__feature_flag(self) -> None:
             workflow_status_update_handler(group, message, activity)
             mock_delay.assert_not_called()
 
-    @with_feature("organizations:workflow-engine-process-metric-issue-workflows")
+    @with_feature("organizations:workflow-engine-single-process-metric-issues")
     def test(self) -> None:
         detector = self.create_detector(project=self.project)
-        group = self.create_group(project=self.project)
+        group = self.create_group(project=self.project, type=MetricIssue.type_id)
         activity = Activity(
             project=self.project,
             group=group,
@@ -124,15 +125,15 @@ def test(self) -> None:
 
 class TestProcessWorkflowActivity(TestCase):
     def setUp(self) -> None:
-        self.group = self.create_group(project=self.project)
+        self.group = self.create_group(project=self.project, type=MetricIssue.type_id)
         self.activity = Activity(
             project=self.project,
             group=self.group,
             type=ActivityType.SET_RESOLVED.value,
             data={"fingerprint": ["test_fingerprint"]},
         )
         self.activity.save()
-        self.detector = self.create_detector()
+        self.detector = self.create_detector(type=MetricIssue.slug)
 
     def test_process_workflow_activity__no_workflows(self) -> None:
         with mock.patch(
@@ -208,7 +209,7 @@ def test_process_workflow_activity(self, mock_filter_actions: mock.MagicMock) ->
 
         mock_filter_actions.assert_called_once_with({self.action_group}, expected_event_data)
 
-    @with_feature("organizations:workflow-engine-process-metric-issue-workflows")
+    @with_feature("organizations:workflow-engine-single-process-metric-issues")
     @mock.patch("sentry.workflow_engine.tasks.workflows.metrics.incr")
     def test__e2e__issue_plat_to_processed(self, mock_incr: mock.MagicMock) -> None:
         self.message = StatusChangeMessageData(
@@ -248,6 +249,7 @@ def test__e2e__issue_plat_to_processed(self, mock_incr: mock.MagicMock) -> None:
                 sample_rate=1.0,
             )
 
+    @with_feature("organizations:workflow-engine-single-process-metric-issues")
     @with_feature("organizations:workflow-engine-process-metric-issue-workflows")
     @mock.patch("sentry.issues.status_change_consumer.get_group_from_fingerprint")
     @mock.patch("sentry.workflow_engine.tasks.workflows.metrics.incr")