diff --git a/src/sentry/workflow_engine/endpoints/utils/test_fire_action.py b/src/sentry/workflow_engine/endpoints/utils/test_fire_action.py index f52f76ac2bdb..2078495159e8 100644 --- a/src/sentry/workflow_engine/endpoints/utils/test_fire_action.py +++ b/src/sentry/workflow_engine/endpoints/utils/test_fire_action.py @@ -5,6 +5,7 @@ from sentry.shared_integrations.exceptions import IntegrationFormError from sentry.workflow_engine.models import Action +from sentry.workflow_engine.processors.detector import get_preferred_detector from sentry.workflow_engine.types import WorkflowEventData, WorkflowId logger = logging.getLogger(__name__) @@ -22,6 +23,7 @@ def test_fire_action( event_data=event_data, notification_uuid=str(uuid.uuid4()), workflow_id=workflow_id, + detector=get_preferred_detector(event_data), ) except Exception as exc: if isinstance(exc, IntegrationFormError): diff --git a/src/sentry/workflow_engine/models/action.py b/src/sentry/workflow_engine/models/action.py index a9d5a13bf7be..5526e25ef200 100644 --- a/src/sentry/workflow_engine/models/action.py +++ b/src/sentry/workflow_engine/models/action.py @@ -3,7 +3,7 @@ import builtins import logging from enum import StrEnum -from typing import ClassVar, TypedDict +from typing import TYPE_CHECKING, ClassVar, TypedDict from django.db import models from django.db.models import Q @@ -27,6 +27,9 @@ logger = logging.getLogger(__name__) +if TYPE_CHECKING: + from sentry.workflow_engine.models.detector import Detector + class ActionSnapshot(TypedDict): id: int @@ -131,11 +134,9 @@ def trigger( event_data: WorkflowEventData, notification_uuid: str, workflow_id: WorkflowId, + *, + detector: Detector, ) -> None: - from sentry.workflow_engine.processors.detector import get_preferred_detector - - detector = get_preferred_detector(event_data) - with metrics.timer( "workflow_engine.action.trigger.execution_time", tags={"action_type": self.type, "detector_type": detector.type}, diff --git a/src/sentry/workflow_engine/tasks/actions.py b/src/sentry/workflow_engine/tasks/actions.py index a6eed4d120bf..3848145398ab 100644 --- a/src/sentry/workflow_engine/tasks/actions.py +++ b/src/sentry/workflow_engine/tasks/actions.py @@ -172,7 +172,12 @@ def trigger_action( # in this scope is grouped properly. with timeout_grouping_context(action.type), action_context_scope(ActionSource.SYSTEM): try: - action.trigger(event_data, notification_uuid=notification_uuid, workflow_id=workflow_id) + action.trigger( + event_data, + notification_uuid=notification_uuid, + workflow_id=workflow_id, + detector=detector, + ) except TRIGGER_ACTION_RETRY_IGNORED_EXCEPTIONS: return except (Exception, ProcessingDeadlineExceeded) as error: diff --git a/tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py b/tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py index cdbec33df168..3cbbc2fa7cab 100644 --- a/tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py +++ b/tests/sentry/workflow_engine/endpoints/validators/actions/test_sentry_app.py @@ -151,4 +151,5 @@ def test_validate_settings_action_trigger( self.event_data, notification_uuid=str(uuid.uuid4()), workflow_id=self.workflow.id, + detector=self.detector, ) # action should be triggerable diff --git a/tests/sentry/workflow_engine/handlers/action/test_action_handlers.py b/tests/sentry/workflow_engine/handlers/action/test_action_handlers.py index d5bfc48e516a..8c24a9064163 100644 --- a/tests/sentry/workflow_engine/handlers/action/test_action_handlers.py +++ b/tests/sentry/workflow_engine/handlers/action/test_action_handlers.py @@ -34,7 +34,10 @@ def test_execute_error_group_type(self, mock_registry_get: mock.MagicMock) -> No notification_uuid = str(uuid.uuid4()) self.action.trigger( - self.event_data, notification_uuid=notification_uuid, workflow_id=self.workflow.id + self.event_data, + notification_uuid=notification_uuid, + workflow_id=self.workflow.id, + detector=self.detector, ) mock_registry_get.assert_called_once_with(ErrorGroupType.slug) @@ -73,7 +76,10 @@ def test_execute_metric_alert_type(self, mock_registry_get: mock.MagicMock) -> N mock_registry_get.return_value = mock_handler self.action.trigger( - self.event_data, notification_uuid=str(uuid.uuid4()), workflow_id=self.workflow.id + self.event_data, + notification_uuid=str(uuid.uuid4()), + workflow_id=self.workflow.id, + detector=self.detector, ) mock_registry_get.assert_called_once_with(MetricIssue.slug) @@ -99,7 +105,10 @@ def test_execute_unknown_detector( """Test that execute does nothing when we can't find the detector""" self.action.trigger( - self.event_data, notification_uuid=str(uuid.uuid4()), workflow_id=self.workflow.id + self.event_data, + notification_uuid=str(uuid.uuid4()), + workflow_id=self.workflow.id, + detector=self.detector, ) mock_logger.warning.assert_called_once_with( diff --git a/tests/sentry/workflow_engine/models/test_action.py b/tests/sentry/workflow_engine/models/test_action.py index e56263a0f4e1..3d8a85bfb73b 100644 --- a/tests/sentry/workflow_engine/models/test_action.py +++ b/tests/sentry/workflow_engine/models/test_action.py @@ -93,7 +93,10 @@ def test_trigger_calls_handler_execute(self, mock_get_detector: MagicMock) -> No notification_uuid = str(uuid.uuid4()) with patch.object(self.action, "get_handler", return_value=mock_handler): self.action.trigger( - self.mock_event, notification_uuid=notification_uuid, workflow_id=self.workflow.id + self.mock_event, + notification_uuid=notification_uuid, + workflow_id=self.workflow.id, + detector=mock_detector, ) assert mock_handler.execute.call_count == 1 @@ -102,12 +105,12 @@ def test_trigger_calls_handler_execute(self, mock_get_detector: MagicMock) -> No assert invocation.event_data == self.mock_event assert invocation.action == self.action assert invocation.detector == mock_detector + mock_get_detector.assert_not_called() - @patch("sentry.workflow_engine.processors.detector.get_preferred_detector") - def test_trigger_with_failing_handler(self, mock_get_detector: MagicMock) -> None: + def test_trigger_with_failing_handler(self) -> None: mock_handler = Mock(spec=ActionHandler) mock_handler.execute.side_effect = Exception("Handler failed") - mock_get_detector.return_value = Mock(spec=Detector, type="error") + detector = Mock(spec=Detector, type="error") with patch.object(self.action, "get_handler", return_value=mock_handler): with pytest.raises(Exception, match="Handler failed"): @@ -115,17 +118,20 @@ def test_trigger_with_failing_handler(self, mock_get_detector: MagicMock) -> Non self.mock_event, notification_uuid=str(uuid.uuid4()), workflow_id=self.workflow.id, + detector=detector, ) @patch("sentry.utils.metrics.incr") - @patch("sentry.workflow_engine.processors.detector.get_preferred_detector") - def test_trigger_metrics(self, mock_get_detector: MagicMock, mock_incr: MagicMock) -> None: + def test_trigger_metrics(self, mock_incr: MagicMock) -> None: mock_handler = Mock(spec=ActionHandler) - mock_get_detector.return_value = Mock(spec=Detector, type="error") + detector = Mock(spec=Detector, type="error") with patch.object(self.action, "get_handler", return_value=mock_handler): self.action.trigger( - self.mock_event, notification_uuid=str(uuid.uuid4()), workflow_id=self.workflow.id + self.mock_event, + notification_uuid=str(uuid.uuid4()), + workflow_id=self.workflow.id, + detector=detector, ) mock_handler.execute.assert_called_once() diff --git a/tests/sentry/workflow_engine/tasks/test_actions.py b/tests/sentry/workflow_engine/tasks/test_actions.py index 125f6f9a78c5..6026694862bf 100644 --- a/tests/sentry/workflow_engine/tasks/test_actions.py +++ b/tests/sentry/workflow_engine/tasks/test_actions.py @@ -102,7 +102,7 @@ def call_trigger_action(self, action: Mock, retry_state: RetryState) -> None: patch( "sentry.workflow_engine.processors.detector.get_preferred_detector", return_value=detector, - ), + ) as get_detector, ): trigger_action( action_id=action.id, @@ -120,6 +120,9 @@ def call_trigger_action(self, action: Mock, retry_state: RetryState) -> None: has_escalated=False, ) + get_detector.assert_called_once_with(event_data) + assert action.trigger.call_args.kwargs["detector"] is detector + @patch("sentry.workflow_engine.tasks.actions.sentry_sdk.capture_exception") def test_reports_and_suppresses_trigger_exception_on_final_attempt( self, mock_capture_exception: Mock