From 989caf23fe4efbbcac3603d24cc268663d207bb7 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Thu, 12 Dec 2024 18:10:06 -0800 Subject: [PATCH] handle above and below condition for anomaly detection --- src/sentry/workflow_engine/metric_helpers.py | 13 +- .../workflow_engine/models/data_condition.py | 1 + .../workflow_engine/test_metric_helpers.py | 128 +++++++++++++++++- 3 files changed, 134 insertions(+), 8 deletions(-) diff --git a/src/sentry/workflow_engine/metric_helpers.py b/src/sentry/workflow_engine/metric_helpers.py index db31c73f37e1f4..022017e3cf1279 100644 --- a/src/sentry/workflow_engine/metric_helpers.py +++ b/src/sentry/workflow_engine/metric_helpers.py @@ -62,6 +62,12 @@ def create_metric_data_condition(alert_rule_trigger: AlertRuleTrigger) -> None: except AlertRuleDetector.DoesNotExist: return None + threshold_to_condition = { + AlertRuleThresholdType.ABOVE.value: Condition.GREATER, + AlertRuleThresholdType.BELOW.value: Condition.LESS, + AlertRuleThresholdType.ABOVE_AND_BELOW.value: Condition.ABOVE_AND_BELOW, + } + data_condition_group = alert_rule_detector.detector.workflow_condition_group if not data_condition_group: return None @@ -72,12 +78,11 @@ def create_metric_data_condition(alert_rule_trigger: AlertRuleTrigger) -> None: else DetectorPriorityLevel.HIGH ) threshold_type = alert_rule_trigger.alert_rule.threshold_type - condition = ( - Condition.GREATER if threshold_type == AlertRuleThresholdType.ABOVE else Condition.LESS - ) + # XXX: we read the threshold type off of the alert_rule and NOT the alert_rule_trigger + # alert_rule_trigger.threshold_type is a deprecated feature we are not moving over data_condition = DataCondition.objects.create( - condition=condition, + condition=threshold_to_condition[threshold_type], comparison=alert_rule_trigger.alert_threshold, condition_result=condition_result, type=ConditionType.METRIC_CONDITION, diff --git a/src/sentry/workflow_engine/models/data_condition.py b/src/sentry/workflow_engine/models/data_condition.py index 13fdbdb462b145..8c1f24add81e47 100644 --- a/src/sentry/workflow_engine/models/data_condition.py +++ b/src/sentry/workflow_engine/models/data_condition.py @@ -31,6 +31,7 @@ class Condition(StrEnum): LESS = "lt" NOT_EQUAL = "ne" GROUP_EVENT_ATTR_COMPARISON = "group_event_attr_comparison" + ABOVE_AND_BELOW = "above_and_below" condition_ops = { diff --git a/tests/sentry/workflow_engine/test_metric_helpers.py b/tests/sentry/workflow_engine/test_metric_helpers.py index 9382b60f860452..3130bb1168276f 100644 --- a/tests/sentry/workflow_engine/test_metric_helpers.py +++ b/tests/sentry/workflow_engine/test_metric_helpers.py @@ -1,6 +1,19 @@ +from unittest.mock import patch + +import orjson +from urllib3.response import HTTPResponse + from sentry.incidents.grouptype import MetricAlertFire +from sentry.incidents.models.alert_rule import ( + AlertRuleDetectionType, + AlertRuleSeasonality, + AlertRuleSensitivity, + AlertRuleThresholdType, +) +from sentry.seer.anomaly_detection.types import StoreDataResponse from sentry.snuba.models import QuerySubscription from sentry.testutils.cases import APITestCase +from sentry.testutils.helpers.features import with_feature from sentry.users.services.user.service import user_service from sentry.workflow_engine.metric_helpers import ( create_metric_action, @@ -58,9 +71,9 @@ def test_create_metric_alert(self): assert detector.type == MetricAlertFire.slug assert detector.config == { "threshold_period": self.metric_alert.threshold_period, - "sensitivity": self.metric_alert.sensitivity, - "seasonality": self.metric_alert.seasonality, - "comparison_delta": self.metric_alert.comparison_delta, + "sensitivity": None, + "seasonality": None, + "comparison_delta": None, } detector_workflow = DetectorWorkflow.objects.get(detector=detector) @@ -84,6 +97,91 @@ def test_create_metric_alert(self): data_source_detector = DataSourceDetector.objects.get(data_source=data_source) assert data_source_detector.detector == detector + @with_feature("organizations:anomaly-detection-alerts") + @with_feature("organizations:anomaly-detection-rollout") + @patch( + "sentry.seer.anomaly_detection.store_data.seer_anomaly_detection_connection_pool.urlopen" + ) + def test_create_dynamic_metric_alert(self, mock_seer_request): + """ + Test that when we call the helper methods we create all the ACI models correctly for a dynamic alert rule + """ + seer_return_value: StoreDataResponse = {"success": True} + mock_seer_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200) + + dynamic_metric_alert = self.create_alert_rule( + seasonality=AlertRuleSeasonality.AUTO, + sensitivity=AlertRuleSensitivity.HIGH, + threshold_type=AlertRuleThresholdType.ABOVE_AND_BELOW, + detection_type=AlertRuleDetectionType.DYNAMIC, + time_window=30, + ) + create_metric_detector_and_workflow(dynamic_metric_alert, self.rpc_user) + + alert_rule_workflow = AlertRuleWorkflow.objects.get(alert_rule=dynamic_metric_alert) + alert_rule_detector = AlertRuleDetector.objects.get(alert_rule=dynamic_metric_alert) + + workflow = Workflow.objects.get(id=alert_rule_workflow.workflow.id) + assert workflow.name == dynamic_metric_alert.name + assert dynamic_metric_alert.organization + assert workflow.organization_id == dynamic_metric_alert.organization.id + detector = Detector.objects.get(id=alert_rule_detector.detector.id) + assert detector.name == dynamic_metric_alert.name + assert detector.project_id == self.project.id + assert detector.enabled is True + assert detector.description == dynamic_metric_alert.description + assert detector.owner_user_id == dynamic_metric_alert.user_id + assert detector.owner_team == dynamic_metric_alert.team + assert detector.type == MetricAlertFire.slug + assert detector.config == { + "threshold_period": dynamic_metric_alert.threshold_period, + "sensitivity": dynamic_metric_alert.sensitivity, + "seasonality": dynamic_metric_alert.seasonality, + "comparison_delta": None, + } + + detector_workflow = DetectorWorkflow.objects.get(detector=detector) + assert detector_workflow.workflow == workflow + + workflow_data_condition_group = WorkflowDataConditionGroup.objects.get(workflow=workflow) + assert workflow_data_condition_group.condition_group == workflow.when_condition_group + + assert dynamic_metric_alert.snuba_query + query_subscription = QuerySubscription.objects.get( + snuba_query=dynamic_metric_alert.snuba_query.id + ) + data_source = DataSource.objects.get( + organization_id=dynamic_metric_alert.organization_id, query_id=query_subscription.id + ) + assert data_source.type == DataSourceType.SNUBA_QUERY_SUBSCRIPTION + detector_state = DetectorState.objects.get(detector=detector) + assert detector_state.active is False + assert detector_state.state == str(DetectorPriorityLevel.OK.value) + + data_source_detector = DataSourceDetector.objects.get(data_source=data_source) + assert data_source_detector.detector == detector + + # test the trigger + dynamic_alert_rule_trigger = self.create_alert_rule_trigger( + alert_rule=dynamic_metric_alert, alert_threshold=0 + ) + create_metric_data_condition(dynamic_alert_rule_trigger) + alert_rule_trigger_data_condition = AlertRuleTriggerDataCondition.objects.get( + alert_rule_trigger=dynamic_alert_rule_trigger + ) + data_condition_group_id = ( + alert_rule_trigger_data_condition.data_condition.condition_group.id + ) + data_condition = DataCondition.objects.get(condition_group=data_condition_group_id) + alert_rule_workflow = AlertRuleWorkflow.objects.get(alert_rule=dynamic_metric_alert) + workflow = Workflow.objects.get(id=alert_rule_workflow.workflow.id) + data_condition_group = workflow.when_condition_group + + assert data_condition.condition == Condition.ABOVE_AND_BELOW + assert data_condition.comparison == dynamic_alert_rule_trigger.alert_threshold + assert data_condition.condition_result == DetectorPriorityLevel.HIGH + assert data_condition.condition_group == data_condition_group + def test_create_metric_alert_trigger(self): """ Test that when we call the helper methods we create all the ACI models correctly for an alert rule trigger @@ -101,7 +199,29 @@ def test_create_metric_alert_trigger(self): workflow = Workflow.objects.get(id=alert_rule_workflow.workflow.id) data_condition_group = workflow.when_condition_group - assert data_condition.condition == Condition.LESS + assert data_condition.condition == Condition.GREATER + assert data_condition.comparison == self.alert_rule_trigger.alert_threshold + assert data_condition.condition_result == DetectorPriorityLevel.HIGH + assert data_condition.condition_group == data_condition_group + + def test_create_dynamic_metric_alert_trigger(self): + """ + Test that when we call the helper methods we create all the ACI models correctly for a dynamic alert rule trigger + """ + create_metric_detector_and_workflow(self.metric_alert, self.rpc_user) + create_metric_data_condition(self.alert_rule_trigger) + alert_rule_trigger_data_condition = AlertRuleTriggerDataCondition.objects.get( + alert_rule_trigger=self.alert_rule_trigger + ) + data_condition_group_id = ( + alert_rule_trigger_data_condition.data_condition.condition_group.id + ) + data_condition = DataCondition.objects.get(condition_group=data_condition_group_id) + alert_rule_workflow = AlertRuleWorkflow.objects.get(alert_rule=self.metric_alert) + workflow = Workflow.objects.get(id=alert_rule_workflow.workflow.id) + data_condition_group = workflow.when_condition_group + + assert data_condition.condition == Condition.GREATER assert data_condition.comparison == self.alert_rule_trigger.alert_threshold assert data_condition.condition_result == DetectorPriorityLevel.HIGH assert data_condition.condition_group == data_condition_group