feat(workflow_engine): Add in hook for producing occurrences from the stateful detector#10
Conversation
… stateful detector (#80168) This adds a hook that can be implemented to produce an occurrence specific to the detector that is subclassing the StatefulDetector. Also change the signature of evaluate to return a dict keyed by groupkey instead of a list. This helps avoid the chance of duplicate results for the same group key. <!-- Describe your PR here. -->
There was a problem hiding this comment.
Greptile Summary
This PR implements a significant architectural enhancement to Sentry's workflow engine by adding the ability for stateful detectors to produce issue occurrences. The changes span across four main files:
Core Model Enhancement (detector.py): A new group_type property is added to the Detector model, providing typed access to the GroupType instance associated with each detector. This refactors the existing detector_handler property to eliminate code duplication and improve type safety by centralizing the group type lookup logic.
Handler Migration (grouptype.py): The MetricAlertDetectorHandler is refactored to inherit from StatefulDetectorHandler instead of DetectorHandler, removing its concrete evaluate() method implementation. This aligns with the broader migration towards stateful detection capabilities.
Processor Core Logic (detector.py): The most substantial changes occur in the processor, where the detector evaluation pipeline is enhanced to support occurrence production. Key modifications include:
- Changing return types from lists to dictionaries keyed by
DetectorGroupKeyto eliminate duplicate handling - Adding a new abstract method
build_occurrence_and_event_datathat subclasses must implement - Integrating Kafka occurrence production when detectors transition to non-OK status
- Removing redundant duplicate detection logic in favor of dictionary-based deduplication
Test Infrastructure (test_detector.py): Comprehensive test updates validate the new occurrence production functionality, including mock implementations of the abstract method and verification of Kafka integration.
These changes enable the workflow engine to create and emit proper IssueOccurrence objects to Kafka when detectors identify problems, completing the integration between the detection system and Sentry's issue tracking infrastructure. The architecture now supports both issue creation (via occurrences) and resolution (via status change messages).
Confidence score: 2/5
- This PR introduces significant architectural changes but has several implementation gaps that make it risky to merge
- The score reflects incomplete abstract method implementations and potential runtime failures in the
MetricAlertDetectorHandler - Files needing attention:
src/sentry/incidents/grouptype.py(incomplete implementation),src/sentry/workflow_engine/processors/detector.py(verify abstract method contracts)
4 files reviewed, 3 comments
| class MetricAlertDetectorHandler(StatefulDetectorHandler[QuerySubscriptionUpdate]): | ||
| pass |
There was a problem hiding this comment.
logic: StatefulDetectorHandler is an abstract base class that requires implementing several abstract methods: counter_names, get_dedupe_value, get_group_key_values, and build_occurrence_and_event_data. This class will not be instantiable without these implementations.
| occurrence_2, event_data_2 = build_mock_occurrence_and_event( | ||
| detector.detector_handler, "group_2", 6, PriorityLevel.HIGH |
There was a problem hiding this comment.
logic: The hardcoded value 6 for group_2 seems inconsistent - the test data shows group_2 has value 10, but the expected occurrence is built with value 6
| occurrence_2, event_data_2 = build_mock_occurrence_and_event( | |
| detector.detector_handler, "group_2", 6, PriorityLevel.HIGH | |
| occurrence_2, event_data_2 = build_mock_occurrence_and_event( | |
| detector.detector_handler, "group_2", 10, PriorityLevel.HIGH |
| occurrence, event_data = build_mock_occurrence_and_event( | ||
| handler, "val1", 6, PriorityLevel.HIGH |
There was a problem hiding this comment.
logic: The test builds an expected result using build_mock_occurrence_and_event with 'val1' and value 6, but this doesn't match the test's group_key parameter 'group_key' - potential test logic error
Review Summary🏷️ Draft Comments (1)
|
Test [new PR number opened]