Skip to content

Commit 3129f1a

Browse files
authored
feat(ACI): Handle anomaly detection data condition in validator (#93886)
Update the metric issue validator to handle anomaly detection data condition input - the main difference with these is that `comparison` is a dictionary rather than a number.
1 parent e967020 commit 3129f1a

File tree

3 files changed

+180
-53
lines changed

3 files changed

+180
-53
lines changed

src/sentry/incidents/metric_issue_detector.py

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,17 @@
1111
BaseDetectorTypeValidator,
1212
)
1313
from sentry.workflow_engine.endpoints.validators.base.data_condition import (
14-
AbstractDataConditionValidator,
14+
BaseDataConditionValidator,
1515
)
1616
from sentry.workflow_engine.models import DataSource, Detector
1717
from sentry.workflow_engine.models.data_condition import Condition
1818
from sentry.workflow_engine.types import DetectorPriorityLevel, SnubaQueryDataSourceType
1919

2020

21-
class MetricIssueComparisonConditionValidator(
22-
AbstractDataConditionValidator[float, DetectorPriorityLevel]
23-
):
24-
supported_conditions = frozenset((Condition.GREATER, Condition.LESS))
21+
class MetricIssueComparisonConditionValidator(BaseDataConditionValidator):
22+
supported_conditions = frozenset(
23+
(Condition.GREATER, Condition.LESS, Condition.ANOMALY_DETECTION)
24+
)
2525
supported_condition_results = frozenset(
2626
(DetectorPriorityLevel.HIGH, DetectorPriorityLevel.MEDIUM)
2727
)
@@ -37,13 +37,19 @@ def validate_type(self, value: str) -> Condition:
3737

3838
return type
3939

40-
def validate_comparison(self, value: float | int | str) -> float:
41-
try:
42-
value = float(value)
43-
except ValueError:
44-
raise serializers.ValidationError("A valid number is required.")
40+
def validate_comparison(self, value: dict | float | int | str) -> float | dict:
41+
if isinstance(value, (float, int)):
42+
try:
43+
value = float(value)
44+
except ValueError:
45+
raise serializers.ValidationError("A valid number is required.")
46+
return value
4547

46-
return value
48+
elif isinstance(value, dict):
49+
return super().validate_comparison(value)
50+
51+
else:
52+
raise serializers.ValidationError("A valid number or dict is required.")
4753

4854
def validate_condition_result(self, value: str) -> DetectorPriorityLevel:
4955
try:

src/sentry/workflow_engine/endpoints/validators/base/data_condition.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,27 +40,31 @@ def validate_condition_result(self, value: Any) -> ConditionResult:
4040
class BaseDataConditionValidator(
4141
AbstractDataConditionValidator[Any, Any],
4242
):
43+
44+
@property
45+
def condition_type(self) -> Condition:
46+
if isinstance(self.initial_data, list) and self.initial_data:
47+
return self.initial_data[0].get("type")
48+
49+
return self.initial_data.get("type")
50+
4351
def _get_handler(self) -> type[DataConditionHandler] | None:
4452
if self._is_operator_condition():
4553
return None
4654

47-
condition_type = self.initial_data.get("type")
48-
4955
try:
50-
return condition_handler_registry.get(condition_type)
56+
return condition_handler_registry.get(self.condition_type)
5157
except NoRegistrationExistsError:
52-
raise serializers.ValidationError(f"Invalid condition type: {condition_type}")
58+
raise serializers.ValidationError(f"Invalid condition type: {self.condition_type}")
5359

5460
def _is_operator_condition(self) -> bool:
55-
condition_type = self.initial_data.get("type")
56-
return condition_type in CONDITION_OPS
61+
return self.condition_type in CONDITION_OPS
5762

5863
def validate_comparison(self, value: Any) -> Any:
5964
"""
6065
Validate the comparison field. Get the schema configuration for the type, if
6166
there is no schema configuration, then we assume the comparison field is a primitive value.
6267
"""
63-
6468
handler = self._get_handler()
6569

6670
if not handler:

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

Lines changed: 152 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111
from sentry.incidents.models.alert_rule import AlertRuleDetectionType
1212
from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE
1313
from sentry.models.environment import Environment
14+
from sentry.seer.anomaly_detection.types import (
15+
AnomalyDetectionSeasonality,
16+
AnomalyDetectionSensitivity,
17+
AnomalyDetectionThresholdType,
18+
)
1419
from sentry.snuba.dataset import Dataset
1520
from sentry.snuba.models import (
1621
QuerySubscription,
@@ -26,15 +31,17 @@
2631

2732

2833
class MetricIssueComparisonConditionValidatorTest(BaseValidatorTest):
34+
def setUp(self):
35+
super().setUp()
36+
self.valid_data = {
37+
"type": Condition.GREATER,
38+
"comparison": 100,
39+
"conditionResult": DetectorPriorityLevel.HIGH,
40+
"conditionGroupId": self.data_condition_group.id,
41+
}
42+
2943
def test(self):
30-
validator = MetricIssueComparisonConditionValidator(
31-
data={
32-
"type": Condition.GREATER,
33-
"comparison": 100,
34-
"conditionResult": DetectorPriorityLevel.HIGH,
35-
"conditionGroupId": self.data_condition_group.id,
36-
}
37-
)
44+
validator = MetricIssueComparisonConditionValidator(data=self.valid_data)
3845
assert validator.is_valid()
3946
assert validator.validated_data == {
4047
"comparison": 100.0,
@@ -46,9 +53,8 @@ def test(self):
4653
def test_invalid_condition(self):
4754
unsupported_condition = Condition.EQUAL
4855
data = {
56+
**self.valid_data,
4957
"type": unsupported_condition,
50-
"comparison": 100,
51-
"result": DetectorPriorityLevel.HIGH,
5258
}
5359
validator = MetricIssueComparisonConditionValidator(data=data)
5460
assert not validator.is_valid()
@@ -58,7 +64,7 @@ def test_invalid_condition(self):
5864

5965
def test_unregistered_condition(self):
6066
validator = MetricIssueComparisonConditionValidator(
61-
data={"type": "invalid", "comparison": 100, "result": DetectorPriorityLevel.HIGH}
67+
data={**self.valid_data, "type": "invalid"}
6268
)
6369
assert not validator.is_valid()
6470
assert validator.errors.get("type") == [
@@ -68,23 +74,36 @@ def test_unregistered_condition(self):
6874
def test_invalid_comparison(self):
6975
validator = MetricIssueComparisonConditionValidator(
7076
data={
71-
"type": Condition.GREATER,
77+
**self.valid_data,
7278
"comparison": "not_a_number",
73-
"result": DetectorPriorityLevel.HIGH,
7479
}
7580
)
7681
assert not validator.is_valid()
7782
assert validator.errors.get("comparison") == [
78-
ErrorDetail(string="A valid number is required.", code="invalid")
83+
ErrorDetail(string="A valid number or dict is required.", code="invalid")
84+
]
85+
86+
def test_invalid_comparison_dict(self):
87+
comparison = {"foo": "bar"}
88+
validator = MetricIssueComparisonConditionValidator(
89+
data={
90+
**self.valid_data,
91+
"comparison": comparison,
92+
}
93+
)
94+
assert not validator.is_valid()
95+
assert validator.errors.get("comparison") == [
96+
ErrorDetail(
97+
string=f"Invalid json primitive value: {comparison}. Must be a string, number, or boolean.",
98+
code="invalid",
99+
)
79100
]
80101

81102
def test_invalid_result(self):
82103
validator = MetricIssueComparisonConditionValidator(
83104
data={
84-
"type": Condition.GREATER,
85-
"comparison": 100,
86-
"condition_result": 25,
87-
"condition_group_id": self.data_condition_group.id,
105+
**self.valid_data,
106+
"conditionResult": 25,
88107
}
89108
)
90109
assert not validator.is_valid()
@@ -109,13 +128,13 @@ def setUp(self):
109128
"name": "Test Detector",
110129
"type": MetricIssue.slug,
111130
"dataSource": {
112-
"query_type": SnubaQuery.Type.ERROR.value,
131+
"queryType": SnubaQuery.Type.ERROR.value,
113132
"dataset": Dataset.Events.value,
114133
"query": "test query",
115134
"aggregate": "count()",
116-
"time_window": 3600,
135+
"timeWindow": 3600,
117136
"environment": self.environment.name,
118-
"event_types": [SnubaQueryEventType.EventType.ERROR.name.lower()],
137+
"eventTypes": [SnubaQueryEventType.EventType.ERROR.name.lower()],
119138
},
120139
"conditionGroup": {
121140
"id": self.data_condition_group.id,
@@ -131,23 +150,12 @@ def setUp(self):
131150
],
132151
},
133152
"config": {
134-
"threshold_period": 1,
135-
"detection_type": AlertRuleDetectionType.STATIC.value,
153+
"thresholdPeriod": 1,
154+
"detectionType": AlertRuleDetectionType.STATIC.value,
136155
},
137156
}
138157

139-
@mock.patch("sentry.workflow_engine.endpoints.validators.base.detector.create_audit_entry")
140-
def test_create_with_valid_data(self, mock_audit):
141-
validator = MetricIssueDetectorValidator(
142-
data=self.valid_data,
143-
context=self.context,
144-
)
145-
assert validator.is_valid(), validator.errors
146-
147-
with self.tasks():
148-
detector = validator.save()
149-
150-
# Verify detector in DB
158+
def assert_validated(self, detector):
151159
detector = Detector.objects.get(id=detector.id)
152160
assert detector.name == "Test Detector"
153161
assert detector.type == MetricIssue.slug
@@ -175,6 +183,19 @@ def test_create_with_valid_data(self, mock_audit):
175183
assert snuba_query.environment == self.environment
176184
assert snuba_query.event_types == [SnubaQueryEventType.EventType.ERROR]
177185

186+
@mock.patch("sentry.workflow_engine.endpoints.validators.base.detector.create_audit_entry")
187+
def test_create_with_valid_data(self, mock_audit):
188+
validator = MetricIssueDetectorValidator(
189+
data=self.valid_data,
190+
context=self.context,
191+
)
192+
assert validator.is_valid(), validator.errors
193+
194+
with self.tasks():
195+
detector = validator.save()
196+
197+
# Verify detector in DB
198+
self.assert_validated(detector)
178199
# Verify condition group in DB
179200
condition_group = DataConditionGroup.objects.get(id=detector.workflow_condition_group_id)
180201
assert condition_group.logic_type == DataConditionGroup.Type.ANY
@@ -197,6 +218,102 @@ def test_create_with_valid_data(self, mock_audit):
197218
data=detector.get_audit_log_data(),
198219
)
199220

221+
@mock.patch("sentry.workflow_engine.endpoints.validators.base.detector.create_audit_entry")
222+
def test_anomaly_detection(self, mock_audit):
223+
data = {
224+
**self.valid_data,
225+
"conditionGroup": {
226+
"id": self.data_condition_group.id,
227+
"organizationId": self.organization.id,
228+
"logicType": self.data_condition_group.logic_type,
229+
"conditions": [
230+
{
231+
"type": Condition.ANOMALY_DETECTION,
232+
"comparison": {
233+
"sensitivity": AnomalyDetectionSensitivity.HIGH,
234+
"seasonality": AnomalyDetectionSeasonality.AUTO,
235+
"threshold_type": AnomalyDetectionThresholdType.ABOVE_AND_BELOW,
236+
},
237+
"conditionResult": DetectorPriorityLevel.HIGH,
238+
"conditionGroupId": self.data_condition_group.id,
239+
},
240+
],
241+
},
242+
"config": {
243+
"threshold_period": 1,
244+
"detection_type": AlertRuleDetectionType.DYNAMIC.value,
245+
},
246+
}
247+
validator = MetricIssueDetectorValidator(
248+
data=data,
249+
context=self.context,
250+
)
251+
assert validator.is_valid(), validator.errors
252+
253+
with self.tasks():
254+
detector = validator.save()
255+
256+
# Verify detector in DB
257+
self.assert_validated(detector)
258+
259+
# Verify condition group in DB
260+
condition_group = DataConditionGroup.objects.get(id=detector.workflow_condition_group_id)
261+
assert condition_group.logic_type == DataConditionGroup.Type.ANY
262+
assert condition_group.organization_id == self.project.organization_id
263+
264+
# Verify conditions in DB
265+
conditions = list(DataCondition.objects.filter(condition_group=condition_group))
266+
assert len(conditions) == 1
267+
268+
condition = conditions[0]
269+
assert condition.type == Condition.ANOMALY_DETECTION
270+
assert condition.comparison == {
271+
"sensitivity": AnomalyDetectionSensitivity.HIGH,
272+
"seasonality": AnomalyDetectionSeasonality.AUTO,
273+
"threshold_type": AnomalyDetectionThresholdType.ABOVE_AND_BELOW,
274+
}
275+
assert condition.condition_result == DetectorPriorityLevel.HIGH
276+
277+
# Verify audit log
278+
mock_audit.assert_called_once_with(
279+
request=self.context["request"],
280+
organization=self.project.organization,
281+
target_object=detector.id,
282+
event=audit_log.get_event_id("DETECTOR_ADD"),
283+
data=detector.get_audit_log_data(),
284+
)
285+
286+
def test_anomaly_detection__invalid_comparison(self):
287+
data = {
288+
**self.valid_data,
289+
"conditionGroup": {
290+
"id": self.data_condition_group.id,
291+
"organizationId": self.organization.id,
292+
"logicType": self.data_condition_group.logic_type,
293+
"conditions": [
294+
{
295+
"type": Condition.ANOMALY_DETECTION,
296+
"comparison": {
297+
"sensitivity": "super sensitive",
298+
"seasonality": AnomalyDetectionSeasonality.AUTO,
299+
"threshold_type": AnomalyDetectionThresholdType.ABOVE_AND_BELOW,
300+
},
301+
"conditionResult": DetectorPriorityLevel.HIGH,
302+
"conditionGroupId": self.data_condition_group.id,
303+
},
304+
],
305+
},
306+
"config": {
307+
"threshold_period": 1,
308+
"detection_type": AlertRuleDetectionType.DYNAMIC.value,
309+
},
310+
}
311+
validator = MetricIssueDetectorValidator(
312+
data=data,
313+
context=self.context,
314+
)
315+
assert not validator.is_valid()
316+
200317
def test_invalid_detector_type(self):
201318
data = {**self.valid_data, "type": "invalid_type"}
202319
validator = MetricIssueDetectorValidator(data=data, context=self.context)

0 commit comments

Comments
 (0)