Skip to content

Commit d569cc2

Browse files
ref(workflow_engine): Refactor the StatefulGroupingDetector Handler (#91246)
## Description This PR pulls the logic for evaluating a stateful detector that doesn't include grouping. --------- Co-authored-by: Colleen O'Rourke <colleen@sentry.io>
1 parent 1c865c9 commit d569cc2

File tree

12 files changed

+318
-157
lines changed

12 files changed

+318
-157
lines changed

src/sentry/grouping/grouptype.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,13 @@
55
from sentry.models.group import DEFAULT_TYPE_ID
66
from sentry.types.group import PriorityLevel
77
from sentry.workflow_engine.endpoints.validators.error_detector import ErrorDetectorValidator
8-
from sentry.workflow_engine.handlers.detector.base import DetectorEvaluationResult, DetectorHandler
8+
from sentry.workflow_engine.handlers.detector.base import DetectorHandler
99
from sentry.workflow_engine.models.data_source import DataPacket
10-
from sentry.workflow_engine.types import DetectorGroupKey, DetectorSettings
10+
from sentry.workflow_engine.types import (
11+
DetectorEvaluationResult,
12+
DetectorGroupKey,
13+
DetectorSettings,
14+
)
1115

1216
T = TypeVar("T")
1317

src/sentry/incidents/grouptype.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
)
1919
from sentry.workflow_engine.handlers.detector.base import EvidenceData
2020
from sentry.workflow_engine.models.data_source import DataPacket
21+
from sentry.workflow_engine.processors.data_condition_group import ProcessedDataConditionGroup
2122
from sentry.workflow_engine.types import DetectorGroupKey, DetectorPriorityLevel, DetectorSettings
2223

2324
COMPARISON_DELTA_CHOICES: list[None | int] = [choice.value for choice in ComparisonDeltaChoices]
@@ -32,7 +33,8 @@ class MetricIssueEvidenceData(EvidenceData):
3233
class MetricAlertDetectorHandler(StatefulGroupingDetectorHandler[QuerySubscriptionUpdate, int]):
3334
def create_occurrence(
3435
self,
35-
value: int,
36+
evaluation_result: ProcessedDataConditionGroup,
37+
data_packet: DataPacket[QuerySubscriptionUpdate],
3638
priority: DetectorPriorityLevel,
3739
) -> tuple[DetectorOccurrence, dict[str, Any]]:
3840
# Returning a placeholder for now, this may require us passing more info
Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,11 @@
11
__all__ = [
22
"DataPacketEvaluationType",
33
"DataPacketType",
4-
"DetectorEvaluationResult",
54
"DetectorHandler",
65
"DetectorOccurrence",
76
"DetectorStateData",
87
"StatefulGroupingDetectorHandler",
98
]
109

11-
from .base import (
12-
DataPacketEvaluationType,
13-
DataPacketType,
14-
DetectorEvaluationResult,
15-
DetectorHandler,
16-
DetectorOccurrence,
17-
)
10+
from .base import DataPacketEvaluationType, DataPacketType, DetectorHandler, DetectorOccurrence
1811
from .stateful import DetectorStateData, StatefulGroupingDetectorHandler

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

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,21 @@
88

99
from sentry.issues.grouptype import GroupType
1010
from sentry.issues.issue_occurrence import IssueEvidence, IssueOccurrence
11-
from sentry.issues.status_change_message import StatusChangeMessage
1211
from sentry.types.actor import Actor
1312
from sentry.workflow_engine.models import Condition, DataConditionGroup, DataPacket, Detector
14-
from sentry.workflow_engine.types import DetectorGroupKey, DetectorPriorityLevel
13+
from sentry.workflow_engine.processors.data_condition_group import ProcessedDataConditionGroup
14+
from sentry.workflow_engine.types import (
15+
DetectorEvaluationResult,
16+
DetectorGroupKey,
17+
DetectorPriorityLevel,
18+
)
1519

1620
logger = logging.getLogger(__name__)
1721

1822
DataPacketType = TypeVar("DataPacketType")
1923
DataPacketEvaluationType = TypeVar("DataPacketEvaluationType")
24+
25+
# TODO - get more info about how this is used in issue platform
2026
EventData = dict[str, Any]
2127

2228

@@ -35,14 +41,14 @@ class EvidenceData(Generic[DataPacketEvaluationType]):
3541
class DetectorOccurrence:
3642
issue_title: str
3743
subtitle: str
38-
resource_id: str | None = None
3944
evidence_data: Mapping[str, Any] = dataclasses.field(default_factory=dict)
4045
evidence_display: Sequence[IssueEvidence] = dataclasses.field(default_factory=list)
4146
type: type[GroupType]
4247
level: str
4348
culprit: str
44-
priority: int | None = None
49+
resource_id: str | None = None
4550
assignee: Actor | None = None
51+
priority: DetectorPriorityLevel | None = None
4652

4753
def to_issue_occurrence(
4854
self,
@@ -73,20 +79,6 @@ def to_issue_occurrence(
7379
)
7480

7581

76-
@dataclasses.dataclass(frozen=True)
77-
class DetectorEvaluationResult:
78-
# TODO - Should group key live at this level?
79-
group_key: DetectorGroupKey
80-
# TODO: Are these actually necessary? We're going to produce the occurrence in the detector, so we probably don't
81-
# need to know the other results externally
82-
is_triggered: bool
83-
priority: DetectorPriorityLevel
84-
# TODO: This is only temporarily optional. We should always have a value here if returning a result
85-
result: IssueOccurrence | StatusChangeMessage | None = None
86-
# Event data to supplement the `IssueOccurrence`, if passed.
87-
event_data: dict[str, Any] | None = None
88-
89-
9082
# TODO - DetectorHandler -> AbstractDetectorHandler? (then DetectorHandler is the base implementation)
9183
class DetectorHandler(abc.ABC, Generic[DataPacketType, DataPacketEvaluationType]):
9284
def __init__(self, detector: Detector):
@@ -109,7 +101,7 @@ def __init__(self, detector: Detector):
109101
@abc.abstractmethod
110102
def evaluate(
111103
self, data_packet: DataPacket[DataPacketType]
112-
) -> dict[DetectorGroupKey, DetectorEvaluationResult]:
104+
) -> dict[DetectorGroupKey, DetectorEvaluationResult] | None:
113105
"""
114106
This method is used to evaluate the data packet's value against the conditions on the detector.
115107
"""
@@ -118,10 +110,9 @@ def evaluate(
118110
@abc.abstractmethod
119111
def create_occurrence(
120112
self,
121-
value: DataPacketEvaluationType,
113+
evaluation_result: ProcessedDataConditionGroup,
114+
data_packet: DataPacket[DataPacketType],
122115
priority: DetectorPriorityLevel,
123-
# data_packet: DataPacketType, # TODO - having access to all the data being evaluated seems good
124-
# data_conditions: list[DataCondition], # TODO - list of the failing conditions might be nice
125116
) -> tuple[DetectorOccurrence, EventData]:
126117
"""
127118
This method provides the value that was evaluated against, the data packet that was

0 commit comments

Comments
 (0)