Skip to content

feat(workflow_engine): Add in hook for producing occurrences from the stateful detector#11

Open
ShashankFC wants to merge 1 commit into
workflow-engine-stateful-detector-beforefrom
workflow-engine-stateful-detector-after
Open

feat(workflow_engine): Add in hook for producing occurrences from the stateful detector#11
ShashankFC wants to merge 1 commit into
workflow-engine-stateful-detector-beforefrom
workflow-engine-stateful-detector-after

Conversation

@ShashankFC

Copy link
Copy Markdown
Contributor

Test 10

Summary by CodeRabbit

  • Refactor
    • Restructured metric alert detection system to use group-based result tracking instead of list-based aggregation.
    • Enhanced detector evaluation framework with improved state management and occurrence data handling.

✏️ Tip: You can customize this high-level summary in your review settings.


Replicated from ai-code-review-evaluation/sentry-coderabbit#10

… 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. -->
@ShashankFC ShashankFC requested a review from Copilot January 30, 2026 10:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the metric alert detection system to use group-based result tracking with dictionaries instead of list-based aggregation, and adds support for producing issue occurrences from the stateful detector.

Changes:

  • Modified DetectorEvaluationResult handling to use dictionaries keyed by DetectorGroupKey instead of lists
  • Implemented build_occurrence_and_event_data abstract method in stateful detector handlers to generate issue occurrences
  • Added test coverage for occurrence production with mocked Kafka calls

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/sentry/workflow_engine/processors/test_detector.py Updated tests to work with dict-based results and added occurrence/event data validation
src/sentry/workflow_engine/processors/detector.py Changed return type from list to dict, removed duplicate key checking, added occurrence building logic
src/sentry/workflow_engine/models/detector.py Added group_type property to Detector model
src/sentry/incidents/grouptype.py Changed MetricAlertDetectorHandler to extend StatefulDetectorHandler

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

event_data,
)
occurrence_2, event_data_2 = build_mock_occurrence_and_event(
detector.detector_handler, "group_2", 6, PriorityLevel.HIGH

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The value passed to build_mock_occurrence_and_event should be 10, not 6, to match the group_vals for 'group_2' which is set to 10 on line 177.

Suggested change
detector.detector_handler, "group_2", 6, PriorityLevel.HIGH
detector.detector_handler, "group_2", 10, PriorityLevel.HIGH

Copilot uses AI. Check for mistakes.
self, data_packet: DataPacket[T]
) -> dict[DetectorGroupKey, DetectorEvaluationResult]:
"""
Evaluates a given data packet and returns a list of `DetectorEvaluationResult`.

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The docstring refers to returning 'a list' but the method now returns a dict[DetectorGroupKey, DetectorEvaluationResult]. Update the documentation to reflect the correct return type.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants