Skip to content

Commit 8224bdc

Browse files
saponifi3dandrewshie-sentry
authored andcommitted
feat(workflow_engine): Add registry hook for Activity creation (#93522)
## Description Starting implementation for: https://www.notion.so/sentry/Workflow-Status-Changes-1fa8b10e4b5d80a48acddb95d160da1f?source=copy_link#1fa8b10e4b5d80e6bb1aef39cab2c6dc This will add a registry of handlers when an activity is created, we'll use this in the [workflow engine to kick-off a task](#93553) to execute `process_workflows`. This also adds a way to pass the detector_id through the StatusMessageData all the way to a hook that we can invoke and read directly from the StatusMessageData later. We are also passed the `activity` so `workflow_engine` can trigger `.send_notification` (if evaluated to do so).
1 parent dac33dc commit 8224bdc

File tree

7 files changed

+152
-4
lines changed

7 files changed

+152
-4
lines changed

src/sentry/issues/status_change_consumer.py

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@
22

33
import logging
44
from collections import defaultdict
5-
from collections.abc import Iterable, Mapping, Sequence
5+
from collections.abc import Callable, Iterable, Mapping, Sequence
66
from typing import Any
77

88
from sentry_sdk.tracing import NoOpSpan, Span, Transaction
99

1010
from sentry.integrations.tasks.kick_off_status_syncs import kick_off_status_syncs
1111
from sentry.issues.escalating.escalating import manage_issue_states
1212
from sentry.issues.status_change_message import StatusChangeMessageData
13+
from sentry.models.activity import Activity
1314
from sentry.models.group import Group, GroupStatus
1415
from sentry.models.grouphash import GroupHash
1516
from sentry.models.groupinbox import (
@@ -23,11 +24,13 @@
2324
from sentry.types.activity import ActivityType
2425
from sentry.types.group import IGNORED_SUBSTATUS_CHOICES, GroupSubStatus
2526
from sentry.utils import metrics
27+
from sentry.utils.registry import Registry
2628

2729
logger = logging.getLogger(__name__)
2830

2931

3032
def update_status(group: Group, status_change: StatusChangeMessageData) -> None:
33+
activity_type: ActivityType | None = None
3134
new_status = status_change["new_status"]
3235
new_substatus = status_change["new_substatus"]
3336

@@ -58,11 +61,12 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None:
5861
return
5962

6063
if new_status == GroupStatus.RESOLVED:
64+
activity_type = ActivityType.SET_RESOLVED
6165
Group.objects.update_group_status(
6266
groups=[group],
6367
status=new_status,
6468
substatus=new_substatus,
65-
activity_type=ActivityType.SET_RESOLVED,
69+
activity_type=activity_type,
6670
)
6771
remove_group_from_inbox(group, action=GroupInboxRemoveAction.RESOLVED)
6872
kick_off_status_syncs.apply_async(
@@ -80,11 +84,12 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None:
8084
)
8185
return
8286

87+
activity_type = ActivityType.SET_IGNORED
8388
Group.objects.update_group_status(
8489
groups=[group],
8590
status=new_status,
8691
substatus=new_substatus,
87-
activity_type=ActivityType.SET_IGNORED,
92+
activity_type=activity_type,
8893
)
8994
remove_group_from_inbox(group, action=GroupInboxRemoveAction.IGNORED)
9095
kick_off_status_syncs.apply_async(
@@ -136,6 +141,19 @@ def update_status(group: Group, status_change: StatusChangeMessageData) -> None:
136141
f"Unsupported status: {status_change['new_status']} {status_change['new_substatus']}"
137142
)
138143

144+
if activity_type is not None:
145+
"""
146+
If we have set created an activity, then we'll also notify any registered handlers
147+
that the group status has changed.
148+
149+
This is used to trigger the `workflow_engine` processing status changes.
150+
"""
151+
latest_activity = Activity.objects.filter(
152+
group_id=group.id, type=activity_type.value
153+
).order_by("-datetime")
154+
for handler in group_status_update_registry.registrations.values():
155+
handler(group, status_change, latest_activity[0])
156+
139157

140158
def get_group_from_fingerprint(project_id: int, fingerprint: Sequence[str]) -> Group | None:
141159
results = bulk_get_groups_from_fingerprints([(project_id, fingerprint)])
@@ -255,3 +273,7 @@ def process_status_change_message(
255273
update_status(group, status_change_data)
256274

257275
return group
276+
277+
278+
GroupUpdateHandler = Callable[[Group, StatusChangeMessageData, Activity], None]
279+
group_status_update_registry = Registry[GroupUpdateHandler](enable_reverse_lookup=False)

src/sentry/issues/status_change_message.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ class StatusChangeMessageData(TypedDict):
1212
new_status: int
1313
new_substatus: int | None
1414
id: str
15+
detector_id: int | None
1516

1617

1718
@dataclass(frozen=True)
@@ -20,6 +21,7 @@ class StatusChangeMessage:
2021
project_id: int
2122
new_status: int
2223
new_substatus: int | None
24+
detector_id: int | None = None
2325
id: str = field(default_factory=lambda: uuid4().hex)
2426

2527
def to_dict(
@@ -30,5 +32,6 @@ def to_dict(
3032
"project_id": self.project_id,
3133
"new_status": self.new_status,
3234
"new_substatus": self.new_substatus,
35+
"detector_id": self.detector_id,
3336
"id": self.id,
3437
}

src/sentry/models/activity.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ def create_group_activity(
9898
if user_id is not None:
9999
activity_args["user_id"] = user_id
100100
activity = self.create(**activity_args)
101+
101102
if send_notification:
102103
activity.send_notification()
103104

src/sentry/workflow_engine/handlers/detector/stateful.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,7 @@ def _create_resolve_message(self, group_key: DetectorGroupKey = None) -> StatusC
365365
project_id=self.detector.project_id,
366366
new_status=GroupStatus.RESOLVED,
367367
new_substatus=None,
368+
detector_id=self.detector.id,
368369
)
369370

370371
def _extract_value_from_packet(

tests/sentry/issues/test_status_change_consumer.py

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
from unittest.mock import MagicMock, patch
55

66
from sentry.issues.occurrence_consumer import _process_message
7-
from sentry.issues.status_change_consumer import bulk_get_groups_from_fingerprints
7+
from sentry.issues.status_change_consumer import bulk_get_groups_from_fingerprints, update_status
8+
from sentry.issues.status_change_message import StatusChangeMessageData
89
from sentry.models.activity import Activity
910
from sentry.models.group import Group, GroupStatus
1011
from sentry.models.grouphistory import GroupHistory, GroupHistoryStatus
@@ -20,13 +21,15 @@ def get_test_message_status_change(
2021
fingerprint: list[str] | None = None,
2122
new_status: int = GroupStatus.RESOLVED,
2223
new_substatus: int | None = None,
24+
detector_id: int | None = None,
2325
) -> dict[str, Any]:
2426
payload = {
2527
"project_id": project_id,
2628
"fingerprint": fingerprint or ["group-1"],
2729
"new_status": new_status,
2830
"new_substatus": new_substatus,
2931
"payload_type": "status_change",
32+
"detector_id": detector_id,
3033
}
3134

3235
return payload
@@ -345,3 +348,119 @@ def test_bulk_get_single_project_multiple_hash(self) -> None:
345348
tuple([*other_occurrence.fingerprint, *self.occurrence.fingerprint]),
346349
): other_group
347350
}
351+
352+
353+
class TestStatusChangeRegistry(IssueOccurrenceTestBase):
354+
def setUp(self) -> None:
355+
super().setUp()
356+
self.detector = self.create_detector()
357+
self.group = self.create_group(
358+
project=self.project,
359+
status=GroupStatus.UNRESOLVED,
360+
substatus=GroupSubStatus.ESCALATING,
361+
)
362+
363+
status_change = get_test_message_status_change(
364+
self.project.id,
365+
new_status=GroupStatus.RESOLVED,
366+
detector_id=self.detector.id,
367+
)
368+
369+
self.message = StatusChangeMessageData(
370+
id="test-id",
371+
project_id=status_change["project_id"],
372+
fingerprint=status_change["fingerprint"],
373+
new_status=status_change["new_status"],
374+
new_substatus=status_change.get("new_substatus"),
375+
detector_id=status_change.get("detector_id"),
376+
)
377+
378+
def get_latest_activity(self, activity_type: ActivityType) -> Activity:
379+
latest_activity = (
380+
Activity.objects.filter(group_id=self.group.id, type=activity_type.value)
381+
.order_by("-datetime")
382+
.first()
383+
)
384+
385+
if latest_activity is None:
386+
raise AssertionError(f"No activity found for type {activity_type}")
387+
388+
return latest_activity
389+
390+
def test_handler_is_called__resolved(self) -> None:
391+
with patch(
392+
"sentry.issues.status_change_consumer.group_status_update_registry",
393+
) as mock_registry:
394+
mock_handler = MagicMock()
395+
mock_registry.registrations = {
396+
"test_status_change": mock_handler,
397+
}
398+
399+
update_status(self.group, self.message)
400+
latest_activity = self.get_latest_activity(ActivityType.SET_RESOLVED)
401+
402+
mock_handler.assert_called_once_with(self.group, self.message, latest_activity)
403+
404+
def test_handler_is_not_called__unresolved_escalating(self) -> None:
405+
# There will be an issue occurrence that triggers this instead
406+
407+
self.message["new_status"] = GroupStatus.UNRESOLVED
408+
self.message["new_substatus"] = GroupSubStatus.ESCALATING
409+
with patch(
410+
"sentry.issues.status_change_consumer.group_status_update_registry",
411+
) as mock_registry:
412+
mock_handler = MagicMock()
413+
mock_registry.registrations = {
414+
"test_status_change": mock_handler,
415+
}
416+
417+
update_status(self.group, self.message)
418+
assert mock_handler.call_count == 0
419+
420+
def test_handler_is_called_unresolved_ongoing(self) -> None:
421+
self.message["new_status"] = GroupStatus.UNRESOLVED
422+
self.message["new_substatus"] = GroupSubStatus.ONGOING
423+
424+
with patch(
425+
"sentry.issues.status_change_consumer.group_status_update_registry",
426+
) as mock_registry:
427+
mock_handler = MagicMock()
428+
mock_registry.registrations = {
429+
"test_status_change": mock_handler,
430+
}
431+
432+
update_status(self.group, self.message)
433+
latest_activity = self.get_latest_activity(ActivityType.AUTO_SET_ONGOING)
434+
mock_handler.assert_called_once_with(self.group, self.message, latest_activity)
435+
436+
def test_handler_is_called__unresolved_regressed(self) -> None:
437+
self.message["new_status"] = GroupStatus.UNRESOLVED
438+
self.message["new_substatus"] = GroupSubStatus.REGRESSED
439+
440+
with patch(
441+
"sentry.issues.status_change_consumer.group_status_update_registry",
442+
) as mock_registry:
443+
mock_handler = MagicMock()
444+
mock_registry.registrations = {
445+
"test_status_change": mock_handler,
446+
}
447+
448+
update_status(self.group, self.message)
449+
latest_activity = self.get_latest_activity(ActivityType.SET_REGRESSION)
450+
mock_handler.assert_called_once_with(self.group, self.message, latest_activity)
451+
452+
def test_handler_is_called__ignored(self) -> None:
453+
self.message["new_status"] = GroupStatus.IGNORED
454+
self.message["new_substatus"] = GroupSubStatus.FOREVER
455+
456+
with patch(
457+
"sentry.issues.status_change_consumer.group_status_update_registry",
458+
) as mock_registry:
459+
mock_handler = MagicMock()
460+
mock_registry.registrations = {
461+
"test_status_change": mock_handler,
462+
}
463+
464+
update_status(self.group, self.message)
465+
latest_activity = self.get_latest_activity(ActivityType.SET_IGNORED)
466+
mock_handler.assert_called_once_with(self.group, self.message, latest_activity)

tests/sentry/issues/test_utils.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ def build_statuschange_data(self, **overrides: Any) -> StatusChangeMessageData:
9494
"fingerprint": ["some-fingerprint"],
9595
"new_status": 1,
9696
"new_substatus": 1,
97+
"detector_id": None,
9798
}
9899
kwargs.update(overrides) # type: ignore[typeddict-item]
99100

tests/sentry/workflow_engine/handlers/detector/test_stateful.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ def test_evaluate__resolve(self):
218218
assert evaluation_result
219219
assert evaluation_result.priority == DetectorPriorityLevel.OK
220220
assert isinstance(evaluation_result.result, StatusChangeMessage)
221+
assert evaluation_result.result.detector_id == self.detector.id
221222

222223
def test_evaluate__resolve__detector_state(self):
223224
self.handler.evaluate(self.data_packet)

0 commit comments

Comments
 (0)