fix the processing and some of the tests to be dynamic to the configurations.

Author: saponifi3d

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

@@ -278,14 +278,33 @@ class StatefulDetectorHandler(
     def __init__(self, detector: Detector, thresholds: DetectorThresholds | None = None):
         super().__init__(detector)
 
+        # Default to 1 for all the possible levels on a given detector
+        default_thresholds = {level: 1 for level in self._get_configured_detector_levels()}
+
         self._thresholds: DetectorThresholds = {
-            **{level: 1 for level in DetectorPriorityLevel},  # Default to 0 for all levels
+            DetectorPriorityLevel.OK: 1,  # Make sure the OK level is always set
+            **default_thresholds,
             **(self.thresholds or {}),  # Allow each handler to override
             **(thresholds or {}),  # Allow each instance to override
         }
 
         self.state_manager = DetectorStateManager(detector, list(self._thresholds.keys()))
 
+    def _get_configured_detector_levels(self) -> list[DetectorPriorityLevel]:
+        # TODO - Is this something that should be provided by the detector itself rather
+        # than having to query the db for each level?
+        priority_levels: list[DetectorPriorityLevel] = [level for level in DetectorPriorityLevel]
+
+        if self.detector.workflow_condition_group is None:
+            # TODO - Should this default to _all_ levels or no levels?
+            return priority_levels
+
+        condition_result_levels = self.detector.workflow_condition_group.conditions.filter(
+            condition_result__in=priority_levels
+        ).values_list("condition_result", flat=True)
+
+        return list(DetectorPriorityLevel(level) for level in condition_result_levels)
+
     def _create_decorated_issue_occurrence(
         self,
         detector_occurrence: DetectorOccurrence,
@@ -379,11 +398,15 @@ def _has_breached_threshold(
         self,
         updated_threshold_counts: DetectorCounters,
     ) -> DetectorPriorityLevel | None:
-        # TODO @saponifi3d - Should this only be for thresholds configured on the detector?
-        # If so, gather the self.detector.when_condition_group.conditions()
-        # and see which ones are configured.
+        """
+        Get the list of configured thresholds, then sort them to find the highest
+        breached threshold.
+
+        If the threshold is breached, return the highest breached threshold level.
+        """
         threshold_keys: list[DetectorPriorityLevel] = list(self._thresholds.keys())
         threshold_keys.sort(reverse=True)
+
         for level in threshold_keys:
             level_count = updated_threshold_counts.get(level)
             if level_count is not None and level_count >= self._thresholds[level]:
@@ -524,7 +547,7 @@ def evaluate_group_key_value(
         )
 
         breached_threshold = self._has_breached_threshold(updated_threshold_count)
-        if not breached_threshold:
+        if breached_threshold is None:
             # We haven't met the threshold yet, so don't trigger
             return None
 

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

@@ -47,7 +47,6 @@ def setUp(self):
             detector=self.detector,
             thresholds={
                 DetectorPriorityLevel.HIGH: 2,
-                DetectorPriorityLevel.MEDIUM: 3,
             },
         )
 
@@ -59,9 +58,9 @@ def test_increment_detector_thresholds(self):
         self.handler.state_manager.commit_state_updates()
         updated_state = self.handler.state_manager.get_state_data([self.group_key])[self.group_key]
 
-        # All states should be incremented by 1, except for OK
+        # Default to all states since the detector is not configured with any.
         assert updated_state.counter_updates == {
-            **{level: 1 for level in DetectorPriorityLevel},
+            **{level: 1 for level in self.handler._thresholds},
             DetectorPriorityLevel.OK: None,
         }
 
@@ -144,13 +143,7 @@ def setUp(self):
 
     def test_evaualte__override_threshold(self):
         result = self.handler.evaluate(self.data_packet)
-        evaluation_result = result[self.group_key]
-
-        # Check that the detector is triggered in a medium priority.
-        assert evaluation_result
-        assert evaluation_result.priority == DetectorPriorityLevel.MEDIUM
-        assert evaluation_result.is_triggered is True
-        assert isinstance(evaluation_result.result, IssueOccurrence)
+        assert result == {}
 
     def test_evaluate__override_threshold__triggered(self):
         self.handler.evaluate(self.data_packet)
@@ -173,10 +166,39 @@ def test_evaluate__detector_state(self):
         assert state_data.is_triggered is True
         assert state_data.status == DetectorPriorityLevel.HIGH
 
+        # Only has configured states
         assert state_data.counter_updates == {
             DetectorPriorityLevel.HIGH: 2,
-            DetectorPriorityLevel.MEDIUM: 2,
-            DetectorPriorityLevel.LOW: 2,
+            DetectorPriorityLevel.OK: None,
+        }
+
+    def test_evaluate__detector_state__all_levels(self):
+        # Add additional levels to the detector
+        self.create_data_condition(
+            type="gte",
+            comparison=0,
+            condition_group=self.detector.workflow_condition_group,
+            condition_result=DetectorPriorityLevel.LOW,
+        )
+
+        self.create_data_condition(
+            type="gte",
+            comparison=0,
+            condition_group=self.detector.workflow_condition_group,
+            condition_result=DetectorPriorityLevel.MEDIUM,
+        )
+
+        # Reinitialize the handler to include the new levels
+        self.handler = MockDetectorStateHandler(
+            detector=self.detector, thresholds={DetectorPriorityLevel.HIGH: 2}
+        )
+
+        self.handler.evaluate(self.data_packet)
+        state_data = self.handler.state_manager.get_state_data([self.group_key])[self.group_key]
+
+        # Verify all the levels are present now
+        assert state_data.counter_updates == {
+            **{level: 1 for level in DetectorPriorityLevel},
             DetectorPriorityLevel.OK: None,
         }
 
@@ -200,7 +222,10 @@ def test_evaluate__resolve__detector_state(self):
         # Check that the state is reset
         assert state_data.is_triggered is False
         assert state_data.status == DetectorPriorityLevel.OK
-        assert state_data.counter_updates == {level: None for level in DetectorPriorityLevel}
+        # Only has configured states
+        assert state_data.counter_updates == {
+            **{level: None for level in self.handler._thresholds},
+        }
 
     def test_evaluate__trigger_after_resolve(self):
         self.handler.evaluate(self.data_packet)