Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/sentry/workflow_engine/models/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ def get_handler(self) -> builtins.type[ActionHandler]:
return action_handler_registry.get(action_type)

def trigger(self, event_data: WorkflowEventData, notification_uuid: str) -> None:
from sentry.workflow_engine.processors.detector import get_detector_from_event_data
from sentry.workflow_engine.processors.detector import get_preferred_detector

detector = get_detector_from_event_data(event_data)
detector = get_preferred_detector(event_data)

with metrics.timer(
"workflow_engine.action.trigger.execution_time",
Expand Down
61 changes: 34 additions & 27 deletions src/sentry/workflow_engine/processors/detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,10 @@ def ensure_default_detectors(project: Project) -> tuple[Detector, Detector]:
)


class SpecificDetectorNotFound(Exception):
pass


@dataclass(frozen=True)
class EventDetectors:
issue_stream_detector: Detector | None = None
Expand Down Expand Up @@ -258,17 +262,17 @@ def detectors(self) -> set[Detector]:
return {d for d in [self.issue_stream_detector, self.event_detector] if d is not None}


def get_detectors_for_event(
event_data: WorkflowEventData, detector: Detector | None = None
def get_detectors_for_event_data(
event_data: WorkflowEventData,
detector: Detector | None = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idle thought: this should probably be known_preferred for clarity. But feel free to ignore this thought.

) -> EventDetectors | None:
"""
Returns a list of detectors for the event to process workflows for.

We always return at least the issue stream detector, unless excluded via option.
If the event has an associated detector, we return it too.

If the detector is passed in, use that instead of searching for a detector.
This is used for Activity updates.
We expect a detector to be passed in for Activity updates.
"""
issue_stream_detector: Detector | None = None
exclude_issue_stream = options.get("workflow_engine.exclude_issue_stream_detector")
Expand All @@ -288,10 +292,10 @@ def get_detectors_for_event(
},
)

if detector is None:
if detector is None and isinstance(event_data.event, GroupEvent):
try:
detector = get_detector_by_event(event_data)
except Detector.DoesNotExist:
detector = _get_detector_for_event(event_data.event)
except (Detector.DoesNotExist, SpecificDetectorNotFound):
pass

try:
Expand All @@ -300,24 +304,21 @@ def get_detectors_for_event(
return None


def get_detector_by_event(event_data: WorkflowEventData) -> Detector:
evt = event_data.event

if not isinstance(evt, GroupEvent):
raise TypeError(
"Can only use `get_detector_by_event` for a new event, Activity updates are not supported"
)
def _get_detector_for_event(event: GroupEvent) -> Detector:
"""
Returns the detector from the GroupEvent in event_data.
"""

issue_occurrence = evt.occurrence
issue_occurrence = event.occurrence

try:
if issue_occurrence is None or evt.group.issue_type.detector_settings is None:
# if there are no detector settings, default to the error detector
detector = Detector.get_error_detector_for_project(evt.project_id)
if issue_occurrence is not None:
detector_id = issue_occurrence.evidence_data.get("detector_id")
if detector_id is None:
raise SpecificDetectorNotFound
detector = Detector.objects.get(id=detector_id)
else:
detector = Detector.objects.get(
id=issue_occurrence.evidence_data.get("detector_id", None)
)
detector = Detector.get_error_detector_for_project(event.group.project_id)
except Detector.DoesNotExist:
metrics.incr("workflow_engine.detectors.error")
detector_id = (
Expand All @@ -327,8 +328,8 @@ def get_detector_by_event(event_data: WorkflowEventData) -> Detector:
logger.exception(
"Detector not found for event",
extra={
"event_id": evt.event_id,
"group_id": evt.group_id,
"event_id": event.event_id,
"group_id": event.group_id,
"detector_id": detector_id,
},
)
Expand All @@ -337,7 +338,7 @@ def get_detector_by_event(event_data: WorkflowEventData) -> Detector:
return detector


def get_detector_by_group(group: Group) -> Detector:
def _get_detector_for_group(group: Group) -> Detector:
"""
Returns Detector associated with this group, either based on DetectorGroup,
(project, type), or if those fail, returns the Issue Stream detector.
Expand All @@ -360,12 +361,18 @@ def get_detector_by_group(group: Group) -> Detector:
return Detector.objects.get(project_id=group.project_id, type=IssueStreamGroupType.slug)


def get_detector_from_event_data(event_data: WorkflowEventData) -> Detector:
def get_preferred_detector(event_data: WorkflowEventData) -> Detector:
"""
Attempts to fetch the specific detector based on the GroupEvent or Activity in event_data
"""
try:
if isinstance(event_data.event, GroupEvent):
return get_detector_by_event(event_data)
event_detectors = get_detectors_for_event_data(event_data)
if event_detectors is None:
raise Detector.DoesNotExist("No detectors found for event")
return event_detectors.preferred_detector
elif isinstance(event_data.event, Activity):
return get_detector_by_group(event_data.group)
return _get_detector_for_group(event_data.group)
else:
raise TypeError(f"Cannot determine the detector from {type(event_data.event)}.")
except Detector.DoesNotExist:
Expand Down
16 changes: 9 additions & 7 deletions src/sentry/workflow_engine/processors/workflow.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from collections.abc import Collection, Sequence
from collections.abc import Sequence
from dataclasses import asdict, replace
from datetime import datetime
from enum import StrEnum
Expand Down Expand Up @@ -30,7 +30,7 @@
get_data_conditions_for_group,
process_data_condition_group,
)
from sentry.workflow_engine.processors.detector import get_detectors_for_event
from sentry.workflow_engine.processors.detector import get_detectors_for_event_data
from sentry.workflow_engine.processors.workflow_fire_history import create_workflow_fire_histories
from sentry.workflow_engine.types import (
WorkflowEvaluation,
Expand Down Expand Up @@ -380,7 +380,7 @@ def get_environment_by_event(event_data: WorkflowEventData) -> Environment | Non

@scopedstats.timer()
def _get_associated_workflows(
detectors: Collection[Detector], environment: Environment | None, event_data: WorkflowEventData
detector: Detector, environment: Environment | None, event_data: WorkflowEventData
) -> set[Workflow]:
"""
This is a wrapper method to get the workflows associated with a detector and environment.
Expand All @@ -394,7 +394,7 @@ def _get_associated_workflows(
workflows = set(
Workflow.objects.filter(
environment_filter,
detectorworkflow__detector_id__in=[detector.id for detector in detectors],
detectorworkflow__detector_id=detector.id,
enabled=True,
)
.select_related("environment")
Expand All @@ -421,7 +421,7 @@ def _get_associated_workflows(
"event_data": asdict(event_data),
"event_environment_id": environment.id if environment else None,
"workflows": [workflow.id for workflow in workflows],
"detector_types": [detector.type for detector in detectors],
"detector_type": detector.type,
},
)

Expand Down Expand Up @@ -454,7 +454,7 @@ def process_workflows(
)

try:
event_detectors = get_detectors_for_event(event_data, detector)
event_detectors = get_detectors_for_event_data(event_data, detector)

if not event_detectors:
raise Detector.DoesNotExist("No Detectors associated with the issue were found")
Expand Down Expand Up @@ -500,7 +500,9 @@ def process_workflows(
if features.has("organizations:workflow-engine-process-workflows-logs", organization):
log_context.set_verbose(True)

workflows = _get_associated_workflows(event_detectors.detectors, environment, event_data)
workflows = _get_associated_workflows(
event_detectors.preferred_detector, environment, event_data
)
workflow_evaluation_data.workflows = workflows

if not workflows:
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/workflow_engine/tasks/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def trigger_action(
import uuid

from sentry.notifications.notification_action.utils import should_fire_workflow_actions
from sentry.workflow_engine.processors.detector import get_detector_from_event_data
from sentry.workflow_engine.processors.detector import get_preferred_detector

# Generate UUID if not provided (handles version skew at task boundary)
if notification_uuid is None:
Expand Down Expand Up @@ -129,7 +129,7 @@ def trigger_action(
)
raise ValueError("Exactly one of event_id or activity_id must be provided")

detector = get_detector_from_event_data(event_data)
detector = get_preferred_detector(event_data)

metrics.incr(
"workflow_engine.tasks.trigger_action_task_started",
Expand Down
6 changes: 3 additions & 3 deletions tests/sentry/workflow_engine/models/test_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def test_get_handler_unregistered_type(self) -> None:
# Verify the registry was queried with the correct action type
mock_get.assert_called_once_with(Action.Type.SLACK)

@patch("sentry.workflow_engine.processors.detector.get_detector_from_event_data")
@patch("sentry.workflow_engine.processors.detector.get_preferred_detector")
def test_trigger_calls_handler_execute(self, mock_get_detector: MagicMock) -> None:
mock_handler = Mock(spec=ActionHandler)
mock_detector = Mock(spec=Detector, type="error")
Expand All @@ -88,7 +88,7 @@ def test_trigger_calls_handler_execute(self, mock_get_detector: MagicMock) -> No
assert invocation.action == self.action
assert invocation.detector == mock_detector

@patch("sentry.workflow_engine.processors.detector.get_detector_from_event_data")
@patch("sentry.workflow_engine.processors.detector.get_preferred_detector")
def test_trigger_with_failing_handler(self, mock_get_detector: MagicMock) -> None:
mock_handler = Mock(spec=ActionHandler)
mock_handler.execute.side_effect = Exception("Handler failed")
Expand All @@ -99,7 +99,7 @@ def test_trigger_with_failing_handler(self, mock_get_detector: MagicMock) -> Non
self.action.trigger(self.mock_event, notification_uuid=str(uuid.uuid4()))

@patch("sentry.utils.metrics.incr")
@patch("sentry.workflow_engine.processors.detector.get_detector_from_event_data")
@patch("sentry.workflow_engine.processors.detector.get_preferred_detector")
def test_trigger_metrics(self, mock_get_detector: MagicMock, mock_incr: MagicMock) -> None:
mock_handler = Mock(spec=ActionHandler)
mock_get_detector.return_value = Mock(spec=Detector, type="error")
Expand Down
Loading
Loading