diff --git a/src/sentry/workflow_engine/migration_helpers/alert_rule.py b/src/sentry/workflow_engine/migration_helpers/alert_rule.py index a665478a0dfba3..56ac82c7234f6e 100644 --- a/src/sentry/workflow_engine/migration_helpers/alert_rule.py +++ b/src/sentry/workflow_engine/migration_helpers/alert_rule.py @@ -431,7 +431,7 @@ def migrate_alert_rule( ) -def update_migrated_alert_rule(alert_rule: AlertRule, updated_fields: dict[str, Any]) -> ( +def dual_update_migrated_alert_rule(alert_rule: AlertRule, updated_fields: dict[str, Any]) -> ( tuple[ DetectorState, Detector, diff --git a/tests/sentry/workflow_engine/migration_helpers/test_migrate_alert_rule.py b/tests/sentry/workflow_engine/migration_helpers/test_migrate_alert_rule.py index d2cd724a4b3c39..c1025c6b9700a5 100644 --- a/tests/sentry/workflow_engine/migration_helpers/test_migrate_alert_rule.py +++ b/tests/sentry/workflow_engine/migration_helpers/test_migrate_alert_rule.py @@ -5,21 +5,27 @@ from sentry.deletions.tasks.scheduled import run_scheduled_deletions from sentry.incidents.grouptype import MetricAlertFire from sentry.incidents.models.alert_rule import ( + AlertRule, AlertRuleDetectionType, AlertRuleThresholdType, + AlertRuleTrigger, AlertRuleTriggerAction, ) +from sentry.incidents.utils.types import DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION from sentry.integrations.models.integration import Integration from sentry.integrations.models.organization_integration import OrganizationIntegration +from sentry.silo.base import SiloMode from sentry.snuba.models import QuerySubscription from sentry.testutils.cases import APITestCase -from sentry.testutils.silo import assume_test_silo_mode_of +from sentry.testutils.silo import assume_test_silo_mode, assume_test_silo_mode_of from sentry.users.services.user.service import user_service from sentry.workflow_engine.migration_helpers.alert_rule import ( + PRIORITY_MAP, MissingDataConditionGroup, dual_delete_migrated_alert_rule, dual_delete_migrated_alert_rule_trigger, dual_delete_migrated_alert_rule_trigger_action, + dual_update_migrated_alert_rule, get_action_filter, get_detector_trigger, get_resolve_threshold, @@ -27,7 +33,6 @@ migrate_metric_action, migrate_metric_data_conditions, migrate_resolve_threshold_data_conditions, - update_migrated_alert_rule, ) from sentry.workflow_engine.models import ( Action, @@ -47,6 +52,7 @@ ) from sentry.workflow_engine.models.data_condition import Condition from sentry.workflow_engine.types import DetectorPriorityLevel +from tests.sentry.workflow_engine.test_base import BaseWorkflowTest def assert_alert_rule_migrated(alert_rule, project_id): @@ -94,11 +100,7 @@ def assert_alert_rule_resolve_trigger_migrated(alert_rule): detector_trigger = DataCondition.objects.get( comparison=alert_rule.resolve_threshold, condition_result=DetectorPriorityLevel.OK, - type=( - Condition.LESS_OR_EQUAL - if alert_rule.threshold_type == AlertRuleThresholdType.ABOVE.value - else Condition.GREATER_OR_EQUAL - ), + type=Condition.LESS_OR_EQUAL, ) detector = AlertRuleDetector.objects.get(alert_rule=alert_rule).detector @@ -152,111 +154,322 @@ def assert_alert_rule_trigger_action_migrated(alert_rule_trigger_action, action_ ).exists() -class AlertRuleMigrationHelpersTest(APITestCase): - def setUp(self): - METADATA = { - "api_key": "1234-ABCD", - "base_url": "https://api.opsgenie.com/", - "domain_name": "test-app.app.opsgenie.com", - } - self.rpc_user = user_service.get_user(user_id=self.user.id) - self.og_team = {"id": "123-id", "team": "cool-team", "integration_key": "1234-5678"} - self.integration = self.create_provider_integration( - provider="opsgenie", name="hello-world", external_id="hello-world", metadata=METADATA +class BaseMetricAlertMigrationTest(APITestCase, BaseWorkflowTest): + """ + Base class with helper methods for the ACI metric alert migration. + """ + + def create_metric_alert_lookup_tables( + self, alert_rule: AlertRule, detector: Detector, workflow: Workflow + ) -> tuple[AlertRuleDetector, AlertRuleWorkflow]: + alert_rule_detector = self.create_alert_rule_detector(alert_rule, detector) + alert_rule_workflow = self.create_alert_rule_workflow(alert_rule, workflow) + return ( + alert_rule_detector, + alert_rule_workflow, ) - self.sentry_app = self.create_sentry_app( - name="foo", + + def create_migrated_metric_alert_objects( + self, metric_alert: AlertRule, name="hojicha" + ) -> tuple[ + DataSource, + DataConditionGroup, + Workflow, + Detector, + DetectorState, + AlertRuleDetector, + AlertRuleWorkflow, + DetectorWorkflow, + DataSourceDetector, + ]: + """ + Set up all the necessary ACI objects for a dual written metric alert. The data is not one to one with + what we'd expect for a dual written alert, but the types of models are. + """ + query_subscription = QuerySubscription.objects.get(snuba_query=metric_alert.snuba_query) + data_source = self.create_data_source( organization=self.organization, - is_alertable=True, - verify_install=False, + query_id=query_subscription.id, + type=DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION, ) - self.create_sentry_app_installation( - slug=self.sentry_app.slug, organization=self.organization, user=self.rpc_user + detector_data_condition_group = self.create_data_condition_group( + organization=self.organization + ) + ( + workflow, + detector, + detector_workflow, + _, # the workflow trigger group for a migrated metric alert rule is None + ) = self.create_detector_and_workflow(workflow_triggers=None, name_prefix=name) + detector.update(workflow_condition_group=detector_data_condition_group) + detector_state = self.create_detector_state( + detector=detector, + active=False, + state=DetectorPriorityLevel.OK, + ) + data_source_detector = self.create_data_source_detector(data_source, detector) + ( + alert_rule_detector, + alert_rule_workflow, + ) = self.create_metric_alert_lookup_tables(metric_alert, detector, workflow) + + return ( + data_source, + detector_data_condition_group, + workflow, + detector, + detector_state, + alert_rule_detector, + alert_rule_workflow, + detector_workflow, + data_source_detector, ) - with assume_test_silo_mode_of(Integration, OrganizationIntegration): - self.integration.add_organization(self.organization, self.user) - self.org_integration = OrganizationIntegration.objects.get( - organization_id=self.organization.id, integration_id=self.integration.id - ) - self.org_integration.config = {"team_table": [self.og_team]} - self.org_integration.save() - self.metric_alert = self.create_alert_rule(resolve_threshold=2) - self.alert_rule_trigger_warning = self.create_alert_rule_trigger( - alert_rule=self.metric_alert, label="warning" + def create_migrated_metric_alert_rule_trigger_objects( + self, + alert_rule_trigger: AlertRuleTrigger, + priority: DetectorPriorityLevel, + detector_trigger_type: Condition, + ) -> tuple[DataCondition, DataCondition]: + """ + Set up all the necessary ACI objects for a dual written metric alert trigger. + """ + # look up the necessary migrated alert rule objects first + alert_rule = alert_rule_trigger.alert_rule + detector = AlertRuleDetector.objects.get(alert_rule=alert_rule).detector + detector_dcg = detector.workflow_condition_group + assert detector_dcg # to appease mypy + workflow = AlertRuleWorkflow.objects.get(alert_rule=alert_rule).workflow + + detector_trigger = self.create_data_condition( + comparison=alert_rule_trigger.alert_threshold, + condition_result=priority, + type=detector_trigger_type, + condition_group=detector_dcg, ) - self.alert_rule_trigger_critical = self.create_alert_rule_trigger( - alert_rule=self.metric_alert, label="critical" + data_condition_group = self.create_data_condition_group(organization=self.organization) + self.create_workflow_data_condition_group( + condition_group=data_condition_group, workflow=workflow ) - self.alert_rule_trigger_action_email = self.create_alert_rule_trigger_action( - alert_rule_trigger=self.alert_rule_trigger_warning + action_filter = self.create_data_condition( + comparison=priority, + condition_result=True, + type=Condition.ISSUE_PRIORITY_EQUALS, + condition_group=data_condition_group, ) - self.alert_rule_trigger_action_integration = self.create_alert_rule_trigger_action( - target_identifier=self.og_team["id"], - type=AlertRuleTriggerAction.Type.OPSGENIE, - target_type=AlertRuleTriggerAction.TargetType.SPECIFIC, - integration=self.integration, - alert_rule_trigger=self.alert_rule_trigger_critical, + + return detector_trigger, action_filter + + def create_migrated_metric_alert_rule_resolve_objects( + self, alert_rule: AlertRule, resolve_threshold, detector_trigger_type: Condition + ) -> tuple[DataCondition, DataCondition]: + """ + Set up all the necessary ACI objects for a dual written metric alert resolution threshold. + """ + detector = AlertRuleDetector.objects.get(alert_rule=alert_rule).detector + detector_dcg = detector.workflow_condition_group + assert detector_dcg # to appease mypy + workflow = AlertRuleWorkflow.objects.get(alert_rule=alert_rule).workflow + + detector_trigger = self.create_data_condition( + comparison=resolve_threshold, + condition_result=DetectorPriorityLevel.OK, + type=detector_trigger_type, + condition_group=detector_dcg, + ) + data_condition_group = self.create_data_condition_group(organization=self.organization) + self.create_workflow_data_condition_group( + condition_group=data_condition_group, workflow=workflow + ) + action_filter = self.create_data_condition( + comparison=DetectorPriorityLevel.OK, + condition_result=True, + type=Condition.ISSUE_PRIORITY_EQUALS, + condition_group=data_condition_group, ) - self.alert_rule_trigger_action_sentry_app = self.create_alert_rule_trigger_action( - target_identifier=self.sentry_app.id, - type=AlertRuleTriggerAction.Type.SENTRY_APP, - target_type=AlertRuleTriggerAction.TargetType.SENTRY_APP, - sentry_app=self.sentry_app, - alert_rule_trigger=self.alert_rule_trigger_critical, + return detector_trigger, action_filter + + def create_migrated_metric_alert_rule_action_objects( + self, alert_rule_trigger_action: AlertRuleTriggerAction + ) -> tuple[Action, DataConditionGroupAction, ActionAlertRuleTriggerAction]: + """ + Set up all the necessary ACI objects for a dual written metric alert action. The data is not one to one with + what we'd expect for a dual written legacy action, but the types of models are. + """ + alert_rule_trigger = alert_rule_trigger_action.alert_rule_trigger + priority = PRIORITY_MAP.get(alert_rule_trigger.label, DetectorPriorityLevel.HIGH) + action_filter = get_action_filter(alert_rule_trigger, priority) + + action = self.create_action() + data_condition_group_action = self.create_data_condition_group_action( + action, action_filter.condition_group + ) + action_alert_rule_trigger_action = ActionAlertRuleTriggerAction.objects.create( + action_id=action.id, + alert_rule_trigger_action_id=alert_rule_trigger_action.id, + ) + return action, data_condition_group_action, action_alert_rule_trigger_action + + @staticmethod + @assume_test_silo_mode(SiloMode.REGION) + def create_alert_rule_detector(alert_rule: AlertRule, detector: Detector) -> AlertRuleDetector: + return AlertRuleDetector.objects.create(alert_rule=alert_rule, detector=detector) + + @staticmethod + @assume_test_silo_mode(SiloMode.REGION) + def create_alert_rule_workflow(alert_rule: AlertRule, workflow: Workflow) -> AlertRuleWorkflow: + return AlertRuleWorkflow.objects.create(alert_rule=alert_rule, workflow=workflow) + + @staticmethod + @assume_test_silo_mode(SiloMode.REGION) + def create_action_alert_rule_trigger_action( + action: Action, alert_rule_trigger_action: AlertRuleTriggerAction + ) -> ActionAlertRuleTriggerAction: + return ActionAlertRuleTriggerAction.objects.create( + action=action, alert_rule_trigger_action=alert_rule_trigger_action ) - def test_create_metric_alert(self): + +class DualWriteAlertRuleTest(APITestCase): + def setUp(self): + self.rpc_user = user_service.get_user(user_id=self.user.id) + self.metric_alert = self.create_alert_rule(resolve_threshold=2) + + def test_dual_write_metric_alert(self): """ Test that when we call the helper methods we create all the ACI models correctly for an alert rule """ migrate_alert_rule(self.metric_alert, self.rpc_user) assert_alert_rule_migrated(self.metric_alert, self.project.id) - def test_delete_metric_alert(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) - alert_rule_workflow = AlertRuleWorkflow.objects.get(alert_rule=self.metric_alert) - alert_rule_detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert) - workflow = Workflow.objects.get(id=alert_rule_workflow.workflow.id) - detector = Detector.objects.get(id=alert_rule_detector.detector.id) - detector_workflow = DetectorWorkflow.objects.get(detector=detector) - data_condition_group = detector.workflow_condition_group - assert data_condition_group is not None - query_subscription = QuerySubscription.objects.get( - snuba_query=self.metric_alert.snuba_query.id - ) - data_source = DataSource.objects.get( - organization_id=self.metric_alert.organization_id, query_id=query_subscription.id - ) - detector_state = DetectorState.objects.get(detector=detector) - data_source_detector = DataSourceDetector.objects.get(data_source=data_source) +class DualDeleteAlertRuleTest(BaseMetricAlertMigrationTest): + def setUp(self): + self.metric_alert = self.create_alert_rule() + ( + self.data_source, + self.detector_data_condition_group, + self.workflow, + self.detector, + self.detector_state, + self.alert_rule_detector, + self.alert_rule_workflow, + self.detector_workflow, + self.data_source_detector, + ) = self.create_migrated_metric_alert_objects(self.metric_alert) + + def test_dual_delete_metric_alert_workflow(self): dual_delete_migrated_alert_rule(self.metric_alert) with self.tasks(): run_scheduled_deletions() # check workflow-related tables - assert not Workflow.objects.filter(id=workflow.id).exists() - assert not AlertRuleWorkflow.objects.filter(id=alert_rule_workflow.id).exists() + assert not Workflow.objects.filter(id=self.workflow.id).exists() + assert not AlertRuleWorkflow.objects.filter(id=self.alert_rule_workflow.id).exists() + + def test_dual_delete_metric_alert_detector(self): + dual_delete_migrated_alert_rule(self.metric_alert) + with self.tasks(): + run_scheduled_deletions() # check detector-related tables - assert not Detector.objects.filter(id=detector.id).exists() - assert not DetectorWorkflow.objects.filter(id=detector_workflow.id).exists() - assert not DetectorState.objects.filter(id=detector_state.id).exists() - assert not DataSourceDetector.objects.filter(id=data_source_detector.id).exists() + assert not Detector.objects.filter(id=self.detector.id).exists() + assert not AlertRuleDetector.objects.filter(id=self.alert_rule_detector.id).exists() + assert not DetectorWorkflow.objects.filter(id=self.detector_workflow.id).exists() + assert not DetectorState.objects.filter(id=self.detector_state.id).exists() + assert not DataSourceDetector.objects.filter(id=self.data_source_detector.id).exists() + assert not DataConditionGroup.objects.filter( + id=self.detector_data_condition_group.id + ).exists() - # check data condition group - assert not DataConditionGroup.objects.filter(id=data_condition_group.id).exists() + def test_dual_delete_metric_alert_data_source(self): + dual_delete_migrated_alert_rule(self.metric_alert) + with self.tasks(): + run_scheduled_deletions() # check data source - assert not DataSource.objects.filter(id=data_source.id).exists() + assert not DataSource.objects.filter(id=self.data_source.id).exists() + + +class DualUpdateAlertRuleTest(BaseMetricAlertMigrationTest): + def setUp(self): + self.metric_alert = self.create_alert_rule() + ( + self.data_source, + self.detector_data_condition_group, + self.workflow, + self.detector, + self.detector_state, + self.alert_rule_detector, + self.alert_rule_workflow, + self.detector_workflow, + self.data_source_detector, + ) = self.create_migrated_metric_alert_objects(self.metric_alert) + + def test_dual_update_metric_alert(self): + detector_state = self.detector_state + detector_state.update(active=True, state=DetectorPriorityLevel.HIGH) + updated_fields = { + "name": "hojicha", + "description": "a Japanese green tea roasted over charcoal", + } + + dual_update_migrated_alert_rule(self.metric_alert, updated_fields) + self.detector.refresh_from_db() + detector_state.refresh_from_db() + + assert self.detector.name == "hojicha" + assert self.detector.description == "a Japanese green tea roasted over charcoal" + + assert detector_state.state == str(DetectorPriorityLevel.OK.value) + assert detector_state.active is False + + def test_dual_update_metric_alert_owner(self): + updated_fields = { + "user_id": self.user.id, + "team_id": None, + } + + dual_update_migrated_alert_rule(self.metric_alert, updated_fields) + self.detector.refresh_from_db() + + assert self.detector.owner_user_id == self.user.id + assert self.detector.owner_team_id is None + + def test_update_metric_alert_config(self): + updated_fields = { + "detection_type": "percent", + "threshold_period": 1, + "sensitivity": None, + "seasonality": None, + "comparison_delta": 3600, + } + + dual_update_migrated_alert_rule(self.metric_alert, updated_fields) + self.detector.refresh_from_db() + + assert self.detector.config == updated_fields + + +class DualWriteAlertRuleTriggerTest(BaseMetricAlertMigrationTest): + def setUp(self): + self.metric_alert = self.create_alert_rule(resolve_threshold=2) + self.metric_alert_no_resolve = self.create_alert_rule() + self.create_migrated_metric_alert_objects(self.metric_alert) + self.create_migrated_metric_alert_objects(self.metric_alert_no_resolve, name="matcha") - def test_create_metric_alert_trigger(self): + self.alert_rule_trigger_warning = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="warning" + ) + self.alert_rule_trigger_critical = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="critical" + ) + + def test_dual_write_metric_alert_trigger(self): """ - Test that when we call the helper methods we create all the ACI models correctly for alert rule triggers + Test that when we call the helper methods we create all the ACI models correctly for an alert rule trigger """ - migrate_alert_rule(self.metric_alert, self.rpc_user) migrate_metric_data_conditions(self.alert_rule_trigger_warning) migrate_metric_data_conditions(self.alert_rule_trigger_critical) migrate_resolve_threshold_data_conditions(self.metric_alert) @@ -265,39 +478,18 @@ def test_create_metric_alert_trigger(self): assert_alert_rule_trigger_migrated(self.alert_rule_trigger_critical) assert_alert_rule_resolve_trigger_migrated(self.metric_alert) - def test_calculate_resolve_threshold_critical_only(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) - migrate_metric_data_conditions(self.alert_rule_trigger_critical) - - detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert).detector - detector_dcg = detector.workflow_condition_group - assert detector_dcg - resolve_threshold = get_resolve_threshold(detector_dcg) - assert resolve_threshold == self.alert_rule_trigger_critical.alert_threshold - - def test_calculate_resolve_threshold_with_warning(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) - migrate_metric_data_conditions(self.alert_rule_trigger_warning) - migrate_metric_data_conditions(self.alert_rule_trigger_critical) - - detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert).detector - detector_dcg = detector.workflow_condition_group - assert detector_dcg - resolve_threshold = get_resolve_threshold(detector_dcg) - assert resolve_threshold == self.alert_rule_trigger_warning.alert_threshold - - def test_create_metric_alert_trigger_auto_resolve(self): + def test_dual_write_metric_alert_trigger_auto_resolve(self): """ Test that we create the correct resolution DataConditions when an AlertRule has no explicit resolve threshold """ - metric_alert = self.create_alert_rule() - critical_trigger = self.create_alert_rule_trigger(alert_rule=metric_alert, label="critical") + critical_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert_no_resolve, label="critical", alert_threshold=500 + ) - migrate_alert_rule(metric_alert, self.rpc_user) migrate_metric_data_conditions(critical_trigger) - migrate_resolve_threshold_data_conditions(metric_alert) + migrate_resolve_threshold_data_conditions(self.metric_alert_no_resolve) - detector = AlertRuleDetector.objects.get(alert_rule=metric_alert).detector + detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert_no_resolve).detector resolve_detector_trigger = DataCondition.objects.get( condition_result=DetectorPriorityLevel.OK @@ -321,14 +513,15 @@ def test_create_metric_alert_trigger_auto_resolve_less_than(self): """ Test that we assign the resolve detector trigger the correct type if the threshold type is BELOW """ - metric_alert = self.create_alert_rule(threshold_type=AlertRuleThresholdType.BELOW) - critical_trigger = self.create_alert_rule_trigger(alert_rule=metric_alert, label="critical") + self.metric_alert_no_resolve.update(threshold_type=AlertRuleThresholdType.BELOW.value) + critical_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert_no_resolve, label="critical", alert_threshold=500 + ) - migrate_alert_rule(metric_alert, self.rpc_user) migrate_metric_data_conditions(critical_trigger) - migrate_resolve_threshold_data_conditions(metric_alert) + migrate_resolve_threshold_data_conditions(self.metric_alert_no_resolve) - detector = AlertRuleDetector.objects.get(alert_rule=metric_alert).detector + detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert_no_resolve).detector resolve_detector_trigger = DataCondition.objects.get( condition_result=DetectorPriorityLevel.OK @@ -339,15 +532,145 @@ def test_create_metric_alert_trigger_auto_resolve_less_than(self): assert resolve_detector_trigger.condition_result == DetectorPriorityLevel.OK assert resolve_detector_trigger.condition_group == detector.workflow_condition_group - def test_create_metric_alert_trigger_action(self): + +class DualDeleteAlertRuleTriggerTest(BaseMetricAlertMigrationTest): + def setUp(self): + self.metric_alert = self.create_alert_rule() + self.alert_rule_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="critical" + ) + self.create_migrated_metric_alert_objects(self.metric_alert) + self.detector_trigger, self.action_filter = ( + self.create_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) + ) + + def test_dual_delete_migrated_alert_rule_trigger(self): + dual_delete_migrated_alert_rule_trigger(self.alert_rule_trigger) + assert not DataCondition.objects.filter(id=self.detector_trigger.id).exists() + assert not DataCondition.objects.filter(id=self.action_filter.id).exists() + + @mock.patch("sentry.workflow_engine.migration_helpers.alert_rule.logger") + def test_dual_delete_unmigrated_alert_rule_trigger(self, mock_logger): """ - Test that when we call the helper methods we create all the ACI models correctly for alert rule trigger actions + Test that nothing weird happens if we try to dual delete a trigger whose alert rule was + never dual written. """ - migrate_alert_rule(self.metric_alert, self.rpc_user) + metric_alert = self.create_alert_rule() + unmigrated_trigger = self.create_alert_rule_trigger(alert_rule=metric_alert) + assert not AlertRuleDetector.objects.filter(alert_rule_id=metric_alert.id).exists() + dual_delete_migrated_alert_rule_trigger(unmigrated_trigger) + mock_logger.info.assert_called_with( + "alert rule was not dual written, returning early", + extra={"alert_rule": metric_alert}, + ) - migrate_metric_data_conditions(self.alert_rule_trigger_warning) - migrate_metric_data_conditions(self.alert_rule_trigger_critical) +class DualUpdateAlertRuleTriggerTest(BaseMetricAlertMigrationTest): + def setUp(self): + self.metric_alert = self.create_alert_rule() + self.alert_rule_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="critical", alert_threshold=200 + ) + self.create_migrated_metric_alert_objects(self.metric_alert) + self.critical_detector_trigger, self.critical_action_filter = ( + self.create_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) + ) + self.resolve_detector_trigger, self.resolve_action_filter = ( + self.create_migrated_metric_alert_rule_resolve_objects( + self.metric_alert, 200, Condition.LESS_OR_EQUAL + ) + ) + + def test_dual_update_metric_alert_threshold_type(self): + # This field affects the data conditions, but it lives on the alert rule. + updated_fields = {"threshold_type": AlertRuleThresholdType.BELOW} + dual_update_migrated_alert_rule(self.metric_alert, updated_fields) + + self.critical_detector_trigger.refresh_from_db() + self.resolve_detector_trigger.refresh_from_db() + + assert self.critical_detector_trigger.type == Condition.LESS + assert self.resolve_detector_trigger.type == Condition.GREATER_OR_EQUAL + + def test_dual_update_metric_alert_resolve_threshold(self): + # This field affects the data conditions, but it lives on the alert rule. + updated_fields = {"resolve_threshold": 10} + dual_update_migrated_alert_rule(self.metric_alert, updated_fields) + self.resolve_detector_trigger.refresh_from_db() + + assert self.resolve_detector_trigger.comparison == 10 + + # TODO (mifu67): remaining trigger update tests once those changes land + + +class DualWriteAlertRuleTriggerActionTest(BaseMetricAlertMigrationTest): + def setUp(self): + # set up legacy objects + METADATA = { + "api_key": "1234-ABCD", + "base_url": "https://api.opsgenie.com/", + "domain_name": "test-app.app.opsgenie.com", + } + self.rpc_user = user_service.get_user(user_id=self.user.id) + self.og_team = {"id": "123-id", "team": "cool-team", "integration_key": "1234-5678"} + self.integration = self.create_provider_integration( + provider="opsgenie", name="hello-world", external_id="hello-world", metadata=METADATA + ) + self.sentry_app = self.create_sentry_app( + name="foo", + organization=self.organization, + is_alertable=True, + verify_install=False, + ) + self.create_sentry_app_installation( + slug=self.sentry_app.slug, organization=self.organization, user=self.rpc_user + ) + with assume_test_silo_mode_of(Integration, OrganizationIntegration): + self.integration.add_organization(self.organization, self.user) + self.org_integration = OrganizationIntegration.objects.get( + organization_id=self.organization.id, integration_id=self.integration.id + ) + self.org_integration.config = {"team_table": [self.og_team]} + self.org_integration.save() + self.metric_alert = self.create_alert_rule() + self.critical_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="critical", alert_threshold=200 + ) + self.warning_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="warning", alert_threshold=100 + ) + self.alert_rule_trigger_action_email = self.create_alert_rule_trigger_action( + alert_rule_trigger=self.warning_trigger + ) + self.alert_rule_trigger_action_integration = self.create_alert_rule_trigger_action( + target_identifier=self.og_team["id"], + type=AlertRuleTriggerAction.Type.OPSGENIE, + target_type=AlertRuleTriggerAction.TargetType.SPECIFIC, + integration=self.integration, + alert_rule_trigger=self.critical_trigger, + ) + + self.alert_rule_trigger_action_sentry_app = self.create_alert_rule_trigger_action( + target_identifier=self.sentry_app.id, + type=AlertRuleTriggerAction.Type.SENTRY_APP, + target_type=AlertRuleTriggerAction.TargetType.SENTRY_APP, + sentry_app=self.sentry_app, + alert_rule_trigger=self.critical_trigger, + ) + # set up ACI objects + self.create_migrated_metric_alert_objects(self.metric_alert) + self.create_migrated_metric_alert_rule_trigger_objects( + self.critical_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) + self.create_migrated_metric_alert_rule_trigger_objects( + self.warning_trigger, DetectorPriorityLevel.MEDIUM, Condition.GREATER + ) + + def test_dual_write_metric_alert_trigger_action(self): migrate_metric_action(self.alert_rule_trigger_action_email) migrate_metric_action(self.alert_rule_trigger_action_integration) migrate_metric_action(self.alert_rule_trigger_action_sentry_app) @@ -363,16 +686,13 @@ def test_create_metric_alert_trigger_action(self): ) @mock.patch("sentry.workflow_engine.migration_helpers.alert_rule.logger") - def test_create_metric_alert_trigger_action_no_type(self, mock_logger): + def test_dual_write_metric_alert_trigger_action_no_type(self, mock_logger): """ Test that if for some reason we don't find a match for Action.Type for the integration provider we return None and log. """ with assume_test_silo_mode_of(Integration, OrganizationIntegration): self.integration.update(provider="hellboy") self.integration.save() - - migrate_alert_rule(self.metric_alert, self.rpc_user) - migrate_metric_data_conditions(self.alert_rule_trigger_critical) migrated = migrate_metric_action(self.alert_rule_trigger_action_integration) assert migrated is None mock_logger.warning.assert_called_with( @@ -382,236 +702,178 @@ def test_create_metric_alert_trigger_action_no_type(self, mock_logger): }, ) - def test_update_metric_alert(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) - updated_fields = { - "name": "hojicha", - "description": "a Japanese green tea roasted over charcoal", - } - - alert_rule_detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert) - detector = alert_rule_detector.detector - assert detector.name == self.metric_alert.name - assert detector.description == self.metric_alert.description - detector_state = DetectorState.objects.get(detector=detector) - detector_state.update( - active=True, state=DetectorPriorityLevel.HIGH - ) # so we can confirm that our update actually changes things - assert detector_state.active - - update_migrated_alert_rule(self.metric_alert, updated_fields) - detector.refresh_from_db() - detector_state.refresh_from_db() - - assert detector.name == "hojicha" - assert detector.description == "a Japanese green tea roasted over charcoal" - - assert detector_state.state == str(DetectorPriorityLevel.OK.value) - assert detector_state.active is False - - def test_update_metric_alert_owner(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) - updated_fields = { - "user_id": self.user.id, - "team_id": None, - } - alert_rule_detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert) - detector = alert_rule_detector.detector - assert detector.owner_user_id == self.metric_alert.user_id - assert detector.owner_team_id == self.metric_alert.team_id - update_migrated_alert_rule(self.metric_alert, updated_fields) - detector.refresh_from_db() +class DualDeleteAlertRuleTriggerActionTest(BaseMetricAlertMigrationTest): + def setUp(self): + self.metric_alert = self.create_alert_rule() + self.alert_rule_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="critical", alert_threshold=200 + ) + self.alert_rule_trigger_action = self.create_alert_rule_trigger_action( + alert_rule_trigger=self.alert_rule_trigger + ) - assert detector.owner_user_id == self.user.id - assert detector.owner_team_id is None + self.create_migrated_metric_alert_objects(self.metric_alert) + self.create_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) + self.action, self.data_condition_group_action, self.aarta = ( + self.create_migrated_metric_alert_rule_action_objects(self.alert_rule_trigger_action) + ) - def test_update_metric_alert_config(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) - updated_fields = { - "detection_type": "percent", - "threshold_period": 1, - "sensitivity": None, - "seasonality": None, - "comparison_delta": 3600, - } - alert_rule_detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert) - detector = alert_rule_detector.detector - config = detector.config - assert config == { - "detection_type": "static", - "threshold_period": 1, - "sensitivity": None, - "seasonality": None, - "comparison_delta": None, - } + def test_dual_delete_migrated_alert_rule_trigger_action(self): + dual_delete_migrated_alert_rule_trigger_action(self.alert_rule_trigger_action) + assert not Action.objects.filter(id=self.action.id).exists() + assert not ActionAlertRuleTriggerAction.objects.filter(id=self.aarta.id).exists() + assert not DataConditionGroupAction.objects.filter( + id=self.data_condition_group_action.id + ).exists() - update_migrated_alert_rule(self.metric_alert, updated_fields) - detector.refresh_from_db() + @mock.patch("sentry.workflow_engine.migration_helpers.alert_rule.logger") + def test_dual_delete_unmigrated_alert_rule_trigger_action(self, mock_logger): + """ + Test that nothing weird happens if we try to dual delete a trigger action whose alert + rule was never dual written. + """ + unmigrated_trigger_action = self.create_alert_rule_trigger_action() + metric_alert = unmigrated_trigger_action.alert_rule_trigger.alert_rule + dual_delete_migrated_alert_rule_trigger_action(unmigrated_trigger_action) + mock_logger.info.assert_called_with( + "alert rule was not dual written, returning early", + extra={"alert_rule": metric_alert}, + ) - assert detector.config == updated_fields + def test_dual_delete_action_missing_aarta(self): + """ + Test that we raise an exception if the aarta entry for a migrated trigger action is missing + """ + self.aarta.delete() + with pytest.raises(ActionAlertRuleTriggerAction.DoesNotExist): + dual_delete_migrated_alert_rule_trigger_action(self.alert_rule_trigger_action) - def test_update_metric_alert_threshold_type(self): - metric_alert = self.create_alert_rule() - critical_trigger = self.create_alert_rule_trigger(alert_rule=metric_alert, label="critical") - migrate_alert_rule(metric_alert, self.rpc_user) - migrate_metric_data_conditions(critical_trigger) - migrate_resolve_threshold_data_conditions(metric_alert) - # because there are only two objects in the DB - critical_detector_trigger = DataCondition.objects.get( - condition_result=DetectorPriorityLevel.HIGH +class DualUpdateAlertRuleTriggerActionTest(BaseMetricAlertMigrationTest): + def setUp(self): + self.metric_alert = self.create_alert_rule() + self.alert_rule_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="critical", alert_threshold=200 ) - resolve_detector_trigger = DataCondition.objects.get( - condition_result=DetectorPriorityLevel.OK + self.alert_rule_trigger_action = self.create_alert_rule_trigger_action( + alert_rule_trigger=self.alert_rule_trigger ) - assert critical_detector_trigger.type == Condition.GREATER - assert resolve_detector_trigger.type == Condition.LESS_OR_EQUAL - - updated_fields = {"threshold_type": AlertRuleThresholdType.BELOW} - update_migrated_alert_rule(metric_alert, updated_fields) + self.create_migrated_metric_alert_objects(self.metric_alert) + self.create_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) + self.action, self.data_condition_group_action, self.aarta = ( + self.create_migrated_metric_alert_rule_action_objects(self.alert_rule_trigger_action) + ) - critical_detector_trigger.refresh_from_db() - resolve_detector_trigger.refresh_from_db() + # TODO (mifu67): add these tests once the update changes land - assert critical_detector_trigger.type == Condition.LESS - assert resolve_detector_trigger.type == Condition.GREATER_OR_EQUAL - def test_update_metric_alert_resolve_threshold(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) - migrate_metric_data_conditions(self.alert_rule_trigger_critical) - migrate_resolve_threshold_data_conditions(self.metric_alert) +class CalculateResolveThresholdHelperTest(BaseMetricAlertMigrationTest): + """ + Tests for get_resolve_threshold(), which calculates the resolution threshold for an alert rule + if none is explicitly specified. + """ - resolve_detector_trigger = DataCondition.objects.get( - condition_result=DetectorPriorityLevel.OK + def setUp(self): + self.metric_alert = self.create_alert_rule() + self.alert_rule_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="critical" + ) + self.create_migrated_metric_alert_objects(self.metric_alert) + self.create_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER ) - assert resolve_detector_trigger.comparison == 2 - updated_fields = {"resolve_threshold": 10} - update_migrated_alert_rule(self.metric_alert, updated_fields) - resolve_detector_trigger.refresh_from_db() + def test_calculate_resolve_threshold_critical_only(self): + detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert).detector + detector_dcg = detector.workflow_condition_group + assert detector_dcg # to appease mypy + resolve_threshold = get_resolve_threshold(detector_dcg) + assert resolve_threshold == self.alert_rule_trigger.alert_threshold - assert resolve_detector_trigger.comparison == 10 + def test_calculate_resolve_threshold_with_warning(self): + warning_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="warning", alert_threshold=50 + ) + self.create_migrated_metric_alert_rule_trigger_objects( + warning_trigger, DetectorPriorityLevel.MEDIUM, Condition.GREATER + ) - def test_dual_delete_migrated_alert_rule_trigger(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) - data_conditions = migrate_metric_data_conditions(self.alert_rule_trigger_critical) - assert data_conditions is not None - detector_trigger, action_filter = data_conditions + detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert).detector + detector_dcg = detector.workflow_condition_group + assert detector_dcg # to appease mypy + resolve_threshold = get_resolve_threshold(detector_dcg) + assert resolve_threshold == warning_trigger.alert_threshold - detector_trigger_id = detector_trigger.id - action_filter_id = action_filter.id - dual_delete_migrated_alert_rule_trigger(self.alert_rule_trigger_critical) - assert not DataCondition.objects.filter(id=detector_trigger_id).exists() - assert not DataCondition.objects.filter(id=action_filter_id).exists() - def test_dual_delete_migrated_alert_rule_trigger_action(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) - migrate_metric_data_conditions(self.alert_rule_trigger_critical) - migrate_metric_action(self.alert_rule_trigger_action_integration) +class DataConditionLookupHelpersTest(BaseMetricAlertMigrationTest): + """ + Tests for get_detector_trigger() and get_action_filter(), which are used to fetch the ACI + objects corresponding to an AlertRuleTrigger. + """ - aarta = ActionAlertRuleTriggerAction.objects.get( - alert_rule_trigger_action=self.alert_rule_trigger_action_integration + def setUp(self): + self.metric_alert = self.create_alert_rule() + self.alert_rule_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert, label="critical" ) - aarta_id = aarta.id - action_id = aarta.action.id - dual_delete_migrated_alert_rule_trigger_action(self.alert_rule_trigger_action_integration) - assert not Action.objects.filter(id=action_id).exists() - assert not ActionAlertRuleTriggerAction.objects.filter(id=aarta_id).exists() - - @mock.patch("sentry.workflow_engine.migration_helpers.alert_rule.logger") - def test_dual_delete_unmigrated_alert_rule_trigger(self, mock_logger): - """ - Test that nothing weird happens if we try to dual delete a trigger whose alert rule was - never dual written. - """ - assert not AlertRuleDetector.objects.filter(alert_rule_id=self.metric_alert.id).exists() - dual_delete_migrated_alert_rule_trigger(self.alert_rule_trigger_critical) - mock_logger.info.assert_called_with( - "alert rule was not dual written, returning early", - extra={"alert_rule": self.metric_alert}, + self.create_migrated_metric_alert_objects(self.metric_alert) + self.detector_trigger, self.action_filter = ( + self.create_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) ) - @mock.patch("sentry.workflow_engine.migration_helpers.alert_rule.logger") - def test_dual_delete_unmigrated_alert_rule_trigger_action(self, mock_logger): - """ - Test that nothing weird happens if we try to dual delete a trigger action whose alert - rule was never dual written. - """ - assert not AlertRuleDetector.objects.filter(alert_rule_id=self.metric_alert.id).exists() - dual_delete_migrated_alert_rule_trigger_action(self.alert_rule_trigger_action_integration) - mock_logger.info.assert_called_with( - "alert rule was not dual written, returning early", - extra={"alert_rule": self.metric_alert}, - ) + def test_get_detector_trigger(self): + detector_trigger = get_detector_trigger(self.alert_rule_trigger, DetectorPriorityLevel.HIGH) + assert detector_trigger == self.detector_trigger + + def test_get_action_filter(self): + action_filter = get_action_filter(self.alert_rule_trigger, DetectorPriorityLevel.HIGH) + assert action_filter == self.action_filter def test_get_detector_trigger_no_detector_condition_group(self): """ Test that we raise an exception if the corresponding detector for an alert rule trigger is missing its workflow condition group. """ - migrate_alert_rule(self.metric_alert, self.rpc_user) detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert).detector detector.update(workflow_condition_group=None) with pytest.raises(MissingDataConditionGroup): - get_detector_trigger(self.alert_rule_trigger_critical, DetectorPriorityLevel.HIGH) + get_detector_trigger(self.alert_rule_trigger, DetectorPriorityLevel.HIGH) def test_get_detector_trigger_no_detector_trigger(self): """ Test that we raise an exception if the corresponding detector trigger for an alert rule trigger is missing. """ - migrate_alert_rule(self.metric_alert, self.rpc_user) - data_conditions = migrate_metric_data_conditions(self.alert_rule_trigger_critical) - assert data_conditions is not None - detector_trigger, _ = data_conditions - - detector_trigger.delete() + self.detector_trigger.delete() with pytest.raises(DataCondition.DoesNotExist): - get_detector_trigger(self.alert_rule_trigger_critical, DetectorPriorityLevel.HIGH) + get_detector_trigger(self.alert_rule_trigger, DetectorPriorityLevel.HIGH) def test_get_action_filter_no_workflow(self): """ Test that we raise an exception if the corresponding workflow for an alert rule trigger action does not exist. """ - migrate_alert_rule(self.metric_alert, self.rpc_user) workflow = AlertRuleWorkflow.objects.get(alert_rule=self.metric_alert).workflow workflow.delete() with pytest.raises(AlertRuleWorkflow.DoesNotExist): - get_action_filter(self.alert_rule_trigger_critical, DetectorPriorityLevel.HIGH) + get_action_filter(self.alert_rule_trigger, DetectorPriorityLevel.HIGH) def test_get_action_filter_no_action_filter(self): """ Test that we raise an exception if the corresponding action filter for an alert rule trigger action does not exist. """ - migrate_alert_rule(self.metric_alert, self.rpc_user) - data_conditions = migrate_metric_data_conditions(self.alert_rule_trigger_critical) - assert data_conditions is not None - _, action_filter = data_conditions - action_filter.delete() + self.action_filter.delete() with pytest.raises(DataCondition.DoesNotExist): - get_action_filter(self.alert_rule_trigger_critical, DetectorPriorityLevel.HIGH) - - def test_dual_delete_action_missing_aarta(self): - """ - Test that we raise an exception if the aarta entry for a migrated trigger action is missing - """ - migrate_alert_rule(self.metric_alert, self.rpc_user) - migrate_metric_data_conditions(self.alert_rule_trigger_critical) - migrate_metric_action(self.alert_rule_trigger_action_integration) - - aarta = ActionAlertRuleTriggerAction.objects.get( - alert_rule_trigger_action=self.alert_rule_trigger_action_integration - ) - aarta.delete() - - with pytest.raises(ActionAlertRuleTriggerAction.DoesNotExist): - dual_delete_migrated_alert_rule_trigger_action( - self.alert_rule_trigger_action_integration - ) + get_action_filter(self.alert_rule_trigger, DetectorPriorityLevel.HIGH)