feat(workflow_engine): Add in hook for producing occurrences from the stateful detector#11
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. -->
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| with mock.patch("sentry.workflow_engine.processors.detector.metrics") as mock_metrics: | ||
| occurrence, event_data = build_mock_occurrence_and_event( | ||
| handler, "val1", 6, PriorityLevel.HIGH | ||
| ) |
There was a problem hiding this comment.
Test uses wrong group_key for expected occurrence
Medium Severity
In TestEvaluateGroupKeyValue.test_dedupe, build_mock_occurrence_and_event is called with group_key="val1", but the actual evaluation uses group_key="group_key". The expected occurrence gets a fingerprint derived from "val1", while the real result gets one from "group_key". This mismatch is silently hidden because IssueOccurrence.__eq__ only compares the hardcoded id field, so the test passes without actually validating that the correct group_key is used when building the occurrence.
| event_data, | ||
| ) | ||
| 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.
Test uses wrong value for group_2 occurrence
Low Severity
In test_state_results_multi_group, the expected occurrence for "group_2" is built with value=6, but the data packet specifies "group_2": 10. The test only passes because build_mock_occurrence_and_event ignores the value parameter entirely, meaning this test doesn't actually verify the correct value is forwarded to build_occurrence_and_event_data.
2 participants
Test 10
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
Replicated from ai-code-review-evaluation/sentry-coderabbit#10
Note
Medium Risk
Changes the detector evaluation contract (list→dict) and introduces Kafka production on state transitions, which could affect downstream processing and event volume if handlers return unexpected results.
Overview
Adds an occurrence-production hook to the workflow engine detector pipeline:
process_detectorsnow expects handlers to return results keyed bygroup_keyand will emit either anIssueOccurrenceorStatusChangeMessageto Kafka for each result.Refactors
StatefulDetectorHandlerto generate occurrences on non-OK status transitions via a newbuild_occurrence_and_event_data(...)abstract method, and adds aDetector.group_typehelper to reuse grouptype lookup. Tests are updated to cover occurrence emission (including multi-group) and the dict-based result shape; the metric alert handler is switched to subclassStatefulDetectorHandler(stubbed).Written by Cursor Bugbot for commit de60b7f. Configure here.