fix(aci milestone 3): check for resolution condition in metric detector validator (#102922)

Verify on the backend that metric detectors have a resolution condition.

Author: mifu67

src/sentry/incidents/metric_issue_detector.py

@@ -82,7 +82,13 @@ def schedule_update_project_config(detector: Detector) -> None:
 
 class MetricIssueComparisonConditionValidator(BaseDataConditionValidator):
     supported_conditions = frozenset(
-        (Condition.GREATER, Condition.LESS, Condition.ANOMALY_DETECTION)
+        (
+            Condition.GREATER,
+            Condition.LESS,
+            Condition.GREATER_OR_EQUAL,
+            Condition.LESS_OR_EQUAL,
+            Condition.ANOMALY_DETECTION,
+        )
     )
     supported_condition_results = frozenset(
         (DetectorPriorityLevel.HIGH, DetectorPriorityLevel.MEDIUM, DetectorPriorityLevel.OK)
@@ -132,6 +138,12 @@ def validate_conditions(self, value):
         MetricIssueComparisonConditionValidator(data=value, many=True).is_valid(
             raise_exception=True
         )
+        if not any(
+            condition["condition_result"] == DetectorPriorityLevel.OK for condition in value
+        ) and not any(condition["type"] == Condition.ANOMALY_DETECTION for condition in value):
+            raise serializers.ValidationError(
+                "Resolution condition required for metric issue detector."
+            )
         return value
 
 
@@ -146,7 +158,7 @@ def validate(self, attrs):
 
         if "condition_group" in attrs:
             conditions = attrs.get("condition_group", {}).get("conditions")
-            if len(conditions) > 2:
+            if len(conditions) > 3:
                 raise serializers.ValidationError("Too many conditions")
 
         return attrs

tests/sentry/incidents/endpoints/validators/test_validators.py

@@ -161,6 +161,12 @@ def setUp(self) -> None:
                         "conditionResult": DetectorPriorityLevel.HIGH,
                         "conditionGroupId": self.data_condition_group.id,
                     },
+                    {
+                        "type": Condition.LESS_OR_EQUAL,
+                        "comparison": 100,
+                        "conditionResult": DetectorPriorityLevel.OK,
+                        "conditionGroupId": self.data_condition_group.id,
+                    },
                 ],
             },
             "config": {
@@ -244,7 +250,7 @@ def test_create_with_valid_data(
 
         # Verify conditions in DB
         conditions = list(DataCondition.objects.filter(condition_group=condition_group))
-        assert len(conditions) == 1
+        assert len(conditions) == 2
         condition = conditions[0]
         assert condition.type == Condition.GREATER
         assert condition.comparison == 100
@@ -399,6 +405,31 @@ def test_invalid_detector_type(self) -> None:
             )
         ]
 
+    def test_no_resolution_condition(self) -> None:
+        data = {
+            **self.valid_data,
+            "conditionGroup": {
+                "id": self.data_condition_group.id,
+                "organizationId": self.organization.id,
+                "logicType": self.data_condition_group.logic_type,
+                "conditions": [
+                    {
+                        "type": Condition.GREATER,
+                        "comparison": 100,
+                        "conditionResult": DetectorPriorityLevel.HIGH,
+                        "conditionGroupId": self.data_condition_group.id,
+                    },
+                ],
+            },
+        }
+        validator = MetricIssueDetectorValidator(data=data, context=self.context)
+        assert not validator.is_valid()
+        assert validator.errors.get("conditionGroup", {}).get("conditions") == [
+            ErrorDetail(
+                string="Resolution condition required for metric issue detector.", code="invalid"
+            )
+        ]
+
     def test_too_many_conditions(self) -> None:
         data = {
             **self.valid_data,
@@ -425,6 +456,12 @@ def test_too_many_conditions(self) -> None:
                         "conditionResult": DetectorPriorityLevel.HIGH,
                         "conditionGroupId": self.data_condition_group.id,
                     },
+                    {
+                        "type": Condition.LESS_OR_EQUAL,
+                        "comparison": 100,
+                        "conditionResult": DetectorPriorityLevel.OK,
+                        "conditionGroupId": self.data_condition_group.id,
+                    },
                 ],
             },
         }

tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py

@@ -79,6 +79,12 @@ def setUp(self) -> None:
             comparison=50,
             condition_result=DetectorPriorityLevel.LOW,
         )
+        self.resolve_condition = self.create_data_condition(
+            condition_group=self.data_condition_group,
+            type=Condition.GREATER_OR_EQUAL,
+            comparison=50,
+            condition_result=DetectorPriorityLevel.OK,
+        )
         self.detector = self.create_detector(
             project_id=self.project.id,
             name="Test Detector",
@@ -180,6 +186,13 @@ def setUp(self) -> None:
                         "conditionResult": DetectorPriorityLevel.HIGH,
                         "conditionGroupId": self.condition.condition_group.id,
                     },
+                    {
+                        "id": self.resolve_condition.id,
+                        "comparison": 100,
+                        "type": Condition.LESS_OR_EQUAL,
+                        "conditionResult": DetectorPriorityLevel.OK,
+                        "conditionGroupId": self.condition.condition_group.id,
+                    },
                 ],
             },
             "config": self.detector.config,
@@ -223,7 +236,7 @@ def test_update(self, mock_schedule_update_project_config: mock.MagicMock) -> No
         self.assert_condition_group_updated(condition_group)
 
         conditions = list(DataCondition.objects.filter(condition_group=condition_group))
-        assert len(conditions) == 1
+        assert len(conditions) == 2
         condition = conditions[0]
         self.assert_data_condition_updated(condition)
 
@@ -276,7 +289,7 @@ def test_update_add_data_condition(self) -> None:
         condition_group = detector.workflow_condition_group
         assert condition_group
         conditions = list(DataCondition.objects.filter(condition_group=condition_group))
-        assert len(conditions) == 2
+        assert len(conditions) == 3
 
     def test_update_bad_schema(self) -> None:
         """

tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py

@@ -727,7 +727,13 @@ def setUp(self) -> None:
                         "comparison": 100,
                         "conditionResult": DetectorPriorityLevel.HIGH,
                         "conditionGroupId": self.data_condition_group.id,
-                    }
+                    },
+                    {
+                        "type": Condition.LESS_OR_EQUAL,
+                        "comparison": 100,
+                        "conditionResult": DetectorPriorityLevel.OK,
+                        "conditionGroupId": self.data_condition_group.id,
+                    },
                 ],
             },
             "config": {
@@ -868,7 +874,7 @@ def test_valid_creation(
         assert condition_group.organization_id == self.organization.id
 
         conditions = list(DataCondition.objects.filter(condition_group=condition_group))
-        assert len(conditions) == 1
+        assert len(conditions) == 2
         condition = conditions[0]
         assert condition.type == Condition.GREATER
         assert condition.comparison == 100