From bf0b59306557b65d8a40c70c0ca5e7e50d209fd4 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Wed, 29 Jan 2025 16:06:35 -0800 Subject: [PATCH 1/6] wip --- src/sentry/testutils/factories.py | 29 + src/sentry/testutils/fixtures.py | 6 + .../migration_helpers/alert_rule.py | 2 +- .../test_migrate_alert_rule.py | 538 +++++------------- tests/sentry/workflow_engine/test_base.py | 3 + 5 files changed, 170 insertions(+), 408 deletions(-) diff --git a/src/sentry/testutils/factories.py b/src/sentry/testutils/factories.py index ce00dcd2fa5ca4..00e15d2a623ba4 100644 --- a/src/sentry/testutils/factories.py +++ b/src/sentry/testutils/factories.py @@ -42,6 +42,7 @@ query_datasets_to_type, ) from sentry.incidents.models.alert_rule import ( + AlertRule, AlertRuleDetectionType, AlertRuleThresholdType, AlertRuleTriggerAction, @@ -170,6 +171,8 @@ from sentry.utils.performance_issues.performance_problem import PerformanceProblem from sentry.workflow_engine.models import ( Action, + AlertRuleDetector, + AlertRuleWorkflow, DataCondition, DataConditionGroup, DataConditionGroupAction, @@ -2215,3 +2218,29 @@ def create_data_condition_group_action( return DataConditionGroupAction.objects.create( action=action, condition_group=condition_group, **kwargs ) + + @staticmethod + @assume_test_silo_mode(SiloMode.REGION) + def create_alert_rule_detector( + alert_rule: AlertRule | None = None, + detector: Detector | None = None, + **kwargs, + ) -> AlertRuleDetector: + if alert_rule is None: + alert_rule = Factories.create_alert_rule() + if detector is None: + detector = Factories.create_detector() + 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 | None = None, + workflow: Workflow | None = None, + **kwargs, + ) -> AlertRuleWorkflow: + if alert_rule is None: + alert_rule = Factories.create_alert_rule() + if workflow is None: + workflow = Factories.create_workflow() + return AlertRuleWorkflow.objects.create(alert_rule=alert_rule, workflow=workflow) diff --git a/src/sentry/testutils/fixtures.py b/src/sentry/testutils/fixtures.py index 90fef1158fd2e5..154a73c00621cc 100644 --- a/src/sentry/testutils/fixtures.py +++ b/src/sentry/testutils/fixtures.py @@ -660,6 +660,12 @@ def create_detector_workflow(self, *args, **kwargs): def create_workflow_data_condition_group(self, *args, **kwargs): return Factories.create_workflow_data_condition_group(*args, **kwargs) + def create_alert_rule_detector(self, *args, **kwargs): + return Factories.create_alert_rule_detector(*args, **kwargs) + + def create_alert_rule_workflow(self, *args, **kwargs): + return Factories.create_alert_rule_workflow(*args, **kwargs) + # workflow_engine.models.action def create_action(self, *args, **kwargs): return Factories.create_action(*args, **kwargs) 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..affcf2a0a71b20 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,10 +5,12 @@ 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, 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.snuba.models import QuerySubscription @@ -20,6 +22,7 @@ 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 +30,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 +49,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 +97,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,254 +151,154 @@ def assert_alert_rule_trigger_action_migrated(alert_rule_trigger_action, action_ ).exists() -class AlertRuleMigrationHelpersTest(APITestCase): +class DualWriteAlertRuleTest(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 - ) - 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(resolve_threshold=2) - 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" - ) - self.alert_rule_trigger_action_email = self.create_alert_rule_trigger_action( - alert_rule_trigger=self.alert_rule_trigger_warning - ) - 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, - ) - - 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, - ) - def test_create_metric_alert(self): + 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) - - 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() - - # 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() - - # check data condition group - assert not DataConditionGroup.objects.filter(id=data_condition_group.id).exists() - - # check data source - assert not DataSource.objects.filter(id=data_source.id).exists() - - def test_create_metric_alert_trigger(self): - """ - Test that when we call the helper methods we create all the ACI models correctly for alert rule triggers - """ - 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) - - assert_alert_rule_trigger_migrated(self.alert_rule_trigger_warning) - 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 +class BaseMetricAlertMigrationTest(APITestCase, BaseWorkflowTest): + """ + Base class with helper methods for the ACI metric alert migration. + """ - def test_create_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") - - migrate_alert_rule(metric_alert, self.rpc_user) - migrate_metric_data_conditions(critical_trigger) - migrate_resolve_threshold_data_conditions(metric_alert) - - detector = AlertRuleDetector.objects.get(alert_rule=metric_alert).detector - - resolve_detector_trigger = DataCondition.objects.get( - condition_result=DetectorPriorityLevel.OK + 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, ) - assert resolve_detector_trigger.type == Condition.LESS_OR_EQUAL - assert resolve_detector_trigger.comparison == critical_trigger.alert_threshold - assert resolve_detector_trigger.condition_result == DetectorPriorityLevel.OK - assert resolve_detector_trigger.condition_group == detector.workflow_condition_group - - resolve_data_condition = DataCondition.objects.get(comparison=DetectorPriorityLevel.OK) - - assert resolve_data_condition.type == Condition.ISSUE_PRIORITY_EQUALS - assert resolve_data_condition.condition_result is True - assert resolve_data_condition.condition_group == resolve_data_condition.condition_group - assert WorkflowDataConditionGroup.objects.filter( - condition_group=resolve_data_condition.condition_group - ).exists() - - def test_create_metric_alert_trigger_auto_resolve_less_than(self): + def set_up_migrated_metric_alert_objects(self, metric_alert: AlertRule) -> tuple[ + DataSource, + DataConditionGroup, + Workflow, + Detector, + DetectorState, + AlertRuleDetector, + AlertRuleWorkflow, + DetectorWorkflow, + DataSourceDetector, + ]: """ - Test that we assign the resolve detector trigger the correct type if the threshold type is BELOW + 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. """ - metric_alert = self.create_alert_rule(threshold_type=AlertRuleThresholdType.BELOW) - 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) - - detector = AlertRuleDetector.objects.get(alert_rule=metric_alert).detector - - resolve_detector_trigger = DataCondition.objects.get( - condition_result=DetectorPriorityLevel.OK + query_subscription = QuerySubscription.objects.get(snuba_query=metric_alert.snuba_query) + data_source = self.create_data_source( + organization=self.organization, + query_id=query_subscription.id, + type=DATA_SOURCE_SNUBA_QUERY_SUBSCRIPTION, + ) + 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) + 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, ) - assert resolve_detector_trigger.type == Condition.GREATER_OR_EQUAL - assert resolve_detector_trigger.comparison == critical_trigger.alert_threshold - 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): - """ - Test that when we call the helper methods we create all the ACI models correctly for alert rule trigger actions - """ - 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) +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.set_up_migrated_metric_alert_objects(self.metric_alert) + + def test_dual_delete_metric_alert(self): + dual_delete_migrated_alert_rule(self.metric_alert) + with self.tasks(): + run_scheduled_deletions() - 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) + # check workflow-related tables + assert not Workflow.objects.filter(id=self.workflow.id).exists() + assert not AlertRuleWorkflow.objects.filter(id=self.alert_rule_workflow.id).exists() - assert_alert_rule_trigger_action_migrated( - self.alert_rule_trigger_action_email, Action.Type.EMAIL - ) - assert_alert_rule_trigger_action_migrated( - self.alert_rule_trigger_action_integration, Action.Type.OPSGENIE - ) - assert_alert_rule_trigger_action_migrated( - self.alert_rule_trigger_action_sentry_app, Action.Type.SENTRY_APP - ) + # check detector-related tables + 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() + + # check data condition groups + assert not DataConditionGroup.objects.filter( + id=self.detector_data_condition_group.id + ).exists() - @mock.patch("sentry.workflow_engine.migration_helpers.alert_rule.logger") - def test_create_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() + # check data source + assert not DataSource.objects.filter(id=self.data_source.id).exists() - 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( - "Could not find a matching Action.Type for the trigger action", - extra={ - "alert_rule_trigger_action_id": self.alert_rule_trigger_action_integration.id, - }, - ) - def test_update_metric_alert(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) +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.set_up_migrated_metric_alert_objects(self.metric_alert) + + def test_dual_update_metric_alert(self): + detector = self.detector + 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", } - 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) + dual_update_migrated_alert_rule(self.metric_alert, updated_fields) detector.refresh_from_db() detector_state.refresh_from_db() @@ -409,25 +308,21 @@ def test_update_metric_alert(self): 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) + def test_dual_update_metric_alert_owner(self): + detector = self.detector 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) + dual_update_migrated_alert_rule(self.metric_alert, updated_fields) detector.refresh_from_db() assert detector.owner_user_id == self.user.id assert detector.owner_team_id is None def test_update_metric_alert_config(self): - migrate_alert_rule(self.metric_alert, self.rpc_user) + detector = self.detector updated_fields = { "detection_type": "percent", "threshold_period": 1, @@ -435,183 +330,12 @@ def test_update_metric_alert_config(self): "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, - } - update_migrated_alert_rule(self.metric_alert, updated_fields) + dual_update_migrated_alert_rule(self.metric_alert, updated_fields) detector.refresh_from_db() assert detector.config == updated_fields - 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 - ) - resolve_detector_trigger = DataCondition.objects.get( - condition_result=DetectorPriorityLevel.OK - ) - - 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) - - critical_detector_trigger.refresh_from_db() - resolve_detector_trigger.refresh_from_db() - - 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) - - resolve_detector_trigger = DataCondition.objects.get( - condition_result=DetectorPriorityLevel.OK - ) - 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() - - assert resolve_detector_trigger.comparison == 10 - - 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_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) - - aarta = ActionAlertRuleTriggerAction.objects.get( - alert_rule_trigger_action=self.alert_rule_trigger_action_integration - ) - 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}, - ) - - @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_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) - - 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() - with pytest.raises(DataCondition.DoesNotExist): - get_detector_trigger(self.alert_rule_trigger_critical, 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) - - 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() - - 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 - ) +class DualWriteAlertRuleTriggerTest(APITestCase, BaseWorkflowTest): + pass diff --git a/tests/sentry/workflow_engine/test_base.py b/tests/sentry/workflow_engine/test_base.py index ff4545fb8ab9d9..559264a7623920 100644 --- a/tests/sentry/workflow_engine/test_base.py +++ b/tests/sentry/workflow_engine/test_base.py @@ -4,6 +4,7 @@ from sentry.eventstore.models import Event, GroupEvent from sentry.incidents.grouptype import MetricAlertFire +from sentry.incidents.models.alert_rule import AlertRule from sentry.incidents.utils.types import QuerySubscriptionUpdate from sentry.issues.issue_occurrence import IssueOccurrence from sentry.models.group import Group @@ -14,6 +15,8 @@ from sentry.utils.registry import AlreadyRegisteredError from sentry.workflow_engine.models import ( Action, + AlertRuleDetector, + AlertRuleWorkflow, DataConditionGroup, DataPacket, DataSource, From ee3153f1427c3805679c227955071119dc41a304 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Thu, 30 Jan 2025 16:19:14 -0800 Subject: [PATCH 2/6] add trigger tests, move temp lookup table creation to migration base test class --- src/sentry/testutils/factories.py | 26 -- src/sentry/testutils/fixtures.py | 6 - .../test_migrate_alert_rule.py | 236 +++++++++++++++--- 3 files changed, 206 insertions(+), 62 deletions(-) diff --git a/src/sentry/testutils/factories.py b/src/sentry/testutils/factories.py index 00e15d2a623ba4..ba3c9ca80c0b97 100644 --- a/src/sentry/testutils/factories.py +++ b/src/sentry/testutils/factories.py @@ -2218,29 +2218,3 @@ def create_data_condition_group_action( return DataConditionGroupAction.objects.create( action=action, condition_group=condition_group, **kwargs ) - - @staticmethod - @assume_test_silo_mode(SiloMode.REGION) - def create_alert_rule_detector( - alert_rule: AlertRule | None = None, - detector: Detector | None = None, - **kwargs, - ) -> AlertRuleDetector: - if alert_rule is None: - alert_rule = Factories.create_alert_rule() - if detector is None: - detector = Factories.create_detector() - 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 | None = None, - workflow: Workflow | None = None, - **kwargs, - ) -> AlertRuleWorkflow: - if alert_rule is None: - alert_rule = Factories.create_alert_rule() - if workflow is None: - workflow = Factories.create_workflow() - return AlertRuleWorkflow.objects.create(alert_rule=alert_rule, workflow=workflow) diff --git a/src/sentry/testutils/fixtures.py b/src/sentry/testutils/fixtures.py index 154a73c00621cc..90fef1158fd2e5 100644 --- a/src/sentry/testutils/fixtures.py +++ b/src/sentry/testutils/fixtures.py @@ -660,12 +660,6 @@ def create_detector_workflow(self, *args, **kwargs): def create_workflow_data_condition_group(self, *args, **kwargs): return Factories.create_workflow_data_condition_group(*args, **kwargs) - def create_alert_rule_detector(self, *args, **kwargs): - return Factories.create_alert_rule_detector(*args, **kwargs) - - def create_alert_rule_workflow(self, *args, **kwargs): - return Factories.create_alert_rule_workflow(*args, **kwargs) - # workflow_engine.models.action def create_action(self, *args, **kwargs): return Factories.create_action(*args, **kwargs) 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 affcf2a0a71b20..a869c2e86353be 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 @@ -8,14 +8,17 @@ 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.factories import Factories +from sentry.testutils.silo import assume_test_silo_mode from sentry.users.services.user.service import user_service from sentry.workflow_engine.migration_helpers.alert_rule import ( MissingDataConditionGroup, @@ -46,6 +49,7 @@ DetectorWorkflow, Workflow, WorkflowDataConditionGroup, + data_condition_group, ) from sentry.workflow_engine.models.data_condition import Condition from sentry.workflow_engine.types import DetectorPriorityLevel @@ -151,19 +155,6 @@ def assert_alert_rule_trigger_action_migrated(alert_rule_trigger_action, action_ ).exists() -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) - - class BaseMetricAlertMigrationTest(APITestCase, BaseWorkflowTest): """ Base class with helper methods for the ACI metric alert migration. @@ -233,6 +224,96 @@ def set_up_migrated_metric_alert_objects(self, metric_alert: AlertRule) -> tuple data_source_detector, ) + def set_up_migrated_metric_alert_rule_trigger_objects( + self, alert_rule_trigger: AlertRuleTrigger + ) -> 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 + + # just hardcode values (we can change them later if necessary) + detector_trigger = self.create_data_condition( + comparison=200, + condition_result=DetectorPriorityLevel.HIGH, + type=Condition.GREATER, + 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.HIGH, + condition_result=True, + type=Condition.ISSUE_PRIORITY_EQUALS, + condition_group=data_condition_group, + ) + + return detector_trigger, action_filter + + def set_up_migrated_metric_alert_rule_resolve_objects( + self, alert_rule: AlertRule + ) -> 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 + resolve_threshold = ( + alert_rule.resolve_threshold if alert_rule.resolve_threshold is not None else 200 + ) # same as the critical trigger + + # just hardcode values (we can change them later if necessary) + detector_trigger = self.create_data_condition( + comparison=resolve_threshold, + condition_result=DetectorPriorityLevel.OK, + type=Condition.LESS_OR_EQUAL, + 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, + ) + + return detector_trigger, action_filter + + @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) + + +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) + class DualDeleteAlertRuleTest(BaseMetricAlertMigrationTest): def setUp(self): @@ -249,7 +330,7 @@ def setUp(self): self.data_source_detector, ) = self.set_up_migrated_metric_alert_objects(self.metric_alert) - def test_dual_delete_metric_alert(self): + def test_dual_delete_metric_alert_workflow(self): dual_delete_migrated_alert_rule(self.metric_alert) with self.tasks(): run_scheduled_deletions() @@ -258,18 +339,26 @@ def test_dual_delete_metric_alert(self): 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=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() - - # check data condition groups assert not DataConditionGroup.objects.filter( id=self.detector_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=self.data_source.id).exists() @@ -290,7 +379,6 @@ def setUp(self): ) = self.set_up_migrated_metric_alert_objects(self.metric_alert) def test_dual_update_metric_alert(self): - detector = self.detector detector_state = self.detector_state detector_state.update(active=True, state=DetectorPriorityLevel.HIGH) updated_fields = { @@ -299,30 +387,28 @@ def test_dual_update_metric_alert(self): } dual_update_migrated_alert_rule(self.metric_alert, updated_fields) - detector.refresh_from_db() + self.detector.refresh_from_db() detector_state.refresh_from_db() - assert detector.name == "hojicha" - assert detector.description == "a Japanese green tea roasted over charcoal" + 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): - detector = self.detector updated_fields = { "user_id": self.user.id, "team_id": None, } dual_update_migrated_alert_rule(self.metric_alert, updated_fields) - detector.refresh_from_db() + self.detector.refresh_from_db() - assert detector.owner_user_id == self.user.id - assert detector.owner_team_id is None + assert self.detector.owner_user_id == self.user.id + assert self.detector.owner_team_id is None def test_update_metric_alert_config(self): - detector = self.detector updated_fields = { "detection_type": "percent", "threshold_period": 1, @@ -332,10 +418,100 @@ def test_update_metric_alert_config(self): } dual_update_migrated_alert_rule(self.metric_alert, updated_fields) - detector.refresh_from_db() + 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.set_up_migrated_metric_alert_objects(self.metric_alert) + + 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" + ) - assert detector.config == updated_fields + def test_dual_write_metric_alert_trigger(self): + """ + Test that when we call the helper methods we create all the ACI models correctly for an alert rule trigger + """ + 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) + + assert_alert_rule_trigger_migrated(self.alert_rule_trigger_warning) + assert_alert_rule_trigger_migrated(self.alert_rule_trigger_critical) + assert_alert_rule_resolve_trigger_migrated(self.metric_alert) + + +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.set_up_migrated_metric_alert_objects(self.metric_alert) + self.detector_trigger, self.action_filter = ( + self.set_up_migrated_metric_alert_rule_trigger_objects(self.alert_rule_trigger) + ) + + 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 nothing weird happens if we try to dual delete a trigger whose alert rule was + never dual written. + """ + 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}, + ) + + +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.set_up_migrated_metric_alert_objects(self.metric_alert) + self.critical_detector_trigger, self.critical_action_filter = ( + self.set_up_migrated_metric_alert_rule_trigger_objects(self.alert_rule_trigger) + ) + self.resolve_detector_trigger, self.resolve_action_filter = ( + self.set_up_migrated_metric_alert_rule_resolve_objects(self.metric_alert) + ) + + 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 -class DualWriteAlertRuleTriggerTest(APITestCase, BaseWorkflowTest): - pass + # TODO (mifu67): remaining trigger update tests once those changes land From 569cae21cb145e0435d9b6cb3f494e8aa517f7fd Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Thu, 30 Jan 2025 16:53:48 -0800 Subject: [PATCH 3/6] small changes to the setup methods + dual write action tests --- .../test_migrate_alert_rule.py | 132 +++++++++++++++--- 1 file changed, 116 insertions(+), 16 deletions(-) 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 a869c2e86353be..2a76e52ef1f89c 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 @@ -18,7 +18,7 @@ from sentry.snuba.models import QuerySubscription from sentry.testutils.cases import APITestCase from sentry.testutils.factories import Factories -from sentry.testutils.silo import assume_test_silo_mode +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 ( MissingDataConditionGroup, @@ -225,7 +225,10 @@ def set_up_migrated_metric_alert_objects(self, metric_alert: AlertRule) -> tuple ) def set_up_migrated_metric_alert_rule_trigger_objects( - self, alert_rule_trigger: AlertRuleTrigger + 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. @@ -237,11 +240,10 @@ def set_up_migrated_metric_alert_rule_trigger_objects( assert detector_dcg # to appease mypy workflow = AlertRuleWorkflow.objects.get(alert_rule=alert_rule).workflow - # just hardcode values (we can change them later if necessary) detector_trigger = self.create_data_condition( - comparison=200, - condition_result=DetectorPriorityLevel.HIGH, - type=Condition.GREATER, + comparison=alert_rule_trigger.alert_threshold, + condition_result=priority, + type=detector_trigger_type, condition_group=detector_dcg, ) data_condition_group = self.create_data_condition_group(organization=self.organization) @@ -249,7 +251,7 @@ def set_up_migrated_metric_alert_rule_trigger_objects( condition_group=data_condition_group, workflow=workflow ) action_filter = self.create_data_condition( - comparison=DetectorPriorityLevel.HIGH, + comparison=priority, condition_result=True, type=Condition.ISSUE_PRIORITY_EQUALS, condition_group=data_condition_group, @@ -258,7 +260,7 @@ def set_up_migrated_metric_alert_rule_trigger_objects( return detector_trigger, action_filter def set_up_migrated_metric_alert_rule_resolve_objects( - self, alert_rule: AlertRule + 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. @@ -267,15 +269,11 @@ def set_up_migrated_metric_alert_rule_resolve_objects( detector_dcg = detector.workflow_condition_group assert detector_dcg # to appease mypy workflow = AlertRuleWorkflow.objects.get(alert_rule=alert_rule).workflow - resolve_threshold = ( - alert_rule.resolve_threshold if alert_rule.resolve_threshold is not None else 200 - ) # same as the critical trigger - # just hardcode values (we can change them later if necessary) detector_trigger = self.create_data_condition( comparison=resolve_threshold, condition_result=DetectorPriorityLevel.OK, - type=Condition.LESS_OR_EQUAL, + type=detector_trigger_type, condition_group=detector_dcg, ) data_condition_group = self.create_data_condition_group(organization=self.organization) @@ -457,7 +455,9 @@ def setUp(self): ) self.set_up_migrated_metric_alert_objects(self.metric_alert) self.detector_trigger, self.action_filter = ( - self.set_up_migrated_metric_alert_rule_trigger_objects(self.alert_rule_trigger) + self.set_up_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) ) def test_dual_delete_migrated_alert_rule_trigger(self): @@ -489,10 +489,14 @@ def setUp(self): ) self.set_up_migrated_metric_alert_objects(self.metric_alert) self.critical_detector_trigger, self.critical_action_filter = ( - self.set_up_migrated_metric_alert_rule_trigger_objects(self.alert_rule_trigger) + self.set_up_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) ) self.resolve_detector_trigger, self.resolve_action_filter = ( - self.set_up_migrated_metric_alert_rule_resolve_objects(self.metric_alert) + self.set_up_migrated_metric_alert_rule_resolve_objects( + self.metric_alert, 200, Condition.LESS_OR_EQUAL + ) ) def test_dual_update_metric_alert_threshold_type(self): @@ -515,3 +519,99 @@ def test_dual_update_metric_alert_resolve_threshold(self): 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.set_up_migrated_metric_alert_objects(self.metric_alert) + self.set_up_migrated_metric_alert_rule_trigger_objects( + self.critical_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) + self.set_up_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) + + assert_alert_rule_trigger_action_migrated( + self.alert_rule_trigger_action_email, Action.Type.EMAIL + ) + assert_alert_rule_trigger_action_migrated( + self.alert_rule_trigger_action_integration, Action.Type.OPSGENIE + ) + assert_alert_rule_trigger_action_migrated( + self.alert_rule_trigger_action_sentry_app, Action.Type.SENTRY_APP + ) + + @mock.patch("sentry.workflow_engine.migration_helpers.alert_rule.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() + migrated = migrate_metric_action(self.alert_rule_trigger_action_integration) + assert migrated is None + mock_logger.warning.assert_called_with( + "Could not find a matching Action.Type for the trigger action", + extra={ + "alert_rule_trigger_action_id": self.alert_rule_trigger_action_integration.id, + }, + ) From 1f4d265d3c26263c83fe69d5c98fa643d8dc9398 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Fri, 31 Jan 2025 11:51:20 -0800 Subject: [PATCH 4/6] completed refactor --- .../test_migrate_alert_rule.py | 270 +++++++++++++++++- 1 file changed, 266 insertions(+), 4 deletions(-) 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 2a76e52ef1f89c..9de4ace3b8a058 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 @@ -17,10 +17,10 @@ from sentry.silo.base import SiloMode from sentry.snuba.models import QuerySubscription from sentry.testutils.cases import APITestCase -from sentry.testutils.factories import Factories 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, @@ -49,7 +49,6 @@ DetectorWorkflow, Workflow, WorkflowDataConditionGroup, - data_condition_group, ) from sentry.workflow_engine.models.data_condition import Condition from sentry.workflow_engine.types import DetectorPriorityLevel @@ -170,7 +169,9 @@ def create_metric_alert_lookup_tables( alert_rule_workflow, ) - def set_up_migrated_metric_alert_objects(self, metric_alert: AlertRule) -> tuple[ + def set_up_migrated_metric_alert_objects( + self, metric_alert: AlertRule, name="hojicha" + ) -> tuple[ DataSource, DataConditionGroup, Workflow, @@ -199,7 +200,7 @@ def set_up_migrated_metric_alert_objects(self, metric_alert: AlertRule) -> tuple detector, detector_workflow, _, # the workflow trigger group for a migrated metric alert rule is None - ) = self.create_detector_and_workflow(workflow_triggers=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, @@ -289,6 +290,27 @@ def set_up_migrated_metric_alert_rule_resolve_objects( return detector_trigger, action_filter + def set_up_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: @@ -299,6 +321,15 @@ def create_alert_rule_detector(alert_rule: AlertRule, detector: Detector) -> Ale 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 + ) + class DualWriteAlertRuleTest(APITestCase): def setUp(self): @@ -426,6 +457,7 @@ def setUp(self): self.metric_alert = self.create_alert_rule(resolve_threshold=2) self.metric_alert_no_resolve = self.create_alert_rule() self.set_up_migrated_metric_alert_objects(self.metric_alert) + self.set_up_migrated_metric_alert_objects(self.metric_alert_no_resolve, name="matcha") self.alert_rule_trigger_warning = self.create_alert_rule_trigger( alert_rule=self.metric_alert, label="warning" @@ -446,6 +478,60 @@ def test_dual_write_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_dual_write_metric_alert_trigger_auto_resolve(self): + """ + Test that we create the correct resolution DataConditions when an AlertRule has no explicit resolve threshold + """ + critical_trigger = self.create_alert_rule_trigger( + alert_rule=self.metric_alert_no_resolve, label="critical", alert_threshold=500 + ) + + migrate_metric_data_conditions(critical_trigger) + migrate_resolve_threshold_data_conditions(self.metric_alert_no_resolve) + + detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert_no_resolve).detector + + resolve_detector_trigger = DataCondition.objects.get( + condition_result=DetectorPriorityLevel.OK + ) + + assert resolve_detector_trigger.type == Condition.LESS_OR_EQUAL + assert resolve_detector_trigger.comparison == critical_trigger.alert_threshold + assert resolve_detector_trigger.condition_result == DetectorPriorityLevel.OK + assert resolve_detector_trigger.condition_group == detector.workflow_condition_group + + resolve_data_condition = DataCondition.objects.get(comparison=DetectorPriorityLevel.OK) + + assert resolve_data_condition.type == Condition.ISSUE_PRIORITY_EQUALS + assert resolve_data_condition.condition_result is True + assert resolve_data_condition.condition_group == resolve_data_condition.condition_group + assert WorkflowDataConditionGroup.objects.filter( + condition_group=resolve_data_condition.condition_group + ).exists() + + 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 + """ + 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_metric_data_conditions(critical_trigger) + migrate_resolve_threshold_data_conditions(self.metric_alert_no_resolve) + + detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert_no_resolve).detector + + resolve_detector_trigger = DataCondition.objects.get( + condition_result=DetectorPriorityLevel.OK + ) + + assert resolve_detector_trigger.type == Condition.GREATER_OR_EQUAL + assert resolve_detector_trigger.comparison == critical_trigger.alert_threshold + assert resolve_detector_trigger.condition_result == DetectorPriorityLevel.OK + assert resolve_detector_trigger.condition_group == detector.workflow_condition_group + class DualDeleteAlertRuleTriggerTest(BaseMetricAlertMigrationTest): def setUp(self): @@ -615,3 +701,179 @@ def test_dual_write_metric_alert_trigger_action_no_type(self, mock_logger): "alert_rule_trigger_action_id": self.alert_rule_trigger_action_integration.id, }, ) + + +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 + ) + + self.set_up_migrated_metric_alert_objects(self.metric_alert) + self.set_up_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) + self.action, self.data_condition_group_action, self.aarta = ( + self.set_up_migrated_metric_alert_rule_action_objects(self.alert_rule_trigger_action) + ) + + 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() + + @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}, + ) + + 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) + + +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 + ) + self.alert_rule_trigger_action = self.create_alert_rule_trigger_action( + alert_rule_trigger=self.alert_rule_trigger + ) + + self.set_up_migrated_metric_alert_objects(self.metric_alert) + self.set_up_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) + self.action, self.data_condition_group_action, self.aarta = ( + self.set_up_migrated_metric_alert_rule_action_objects(self.alert_rule_trigger_action) + ) + + # TODO (mifu67): add these tests once the update changes land + + +class CalculateResolveThresholdHelperTest(BaseMetricAlertMigrationTest): + """ + Tests for get_resolve_threshold(), which calculates the resolution threshold for an alert rule + if none is explicitly specified. + """ + + 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.set_up_migrated_metric_alert_objects(self.metric_alert) + self.set_up_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) + + 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 + + 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.set_up_migrated_metric_alert_rule_trigger_objects( + warning_trigger, DetectorPriorityLevel.MEDIUM, Condition.GREATER + ) + + 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 + + +class DataConditionLookupHelpersTest(BaseMetricAlertMigrationTest): + """ + Tests for get_detector_trigger() and get_action_filter(), which are used to fetch the ACI + objects corresponding to an AlertRuleTrigger. + """ + + 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.set_up_migrated_metric_alert_objects(self.metric_alert) + self.detector_trigger, self.action_filter = ( + self.set_up_migrated_metric_alert_rule_trigger_objects( + self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER + ) + ) + + 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. + """ + 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, 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. + """ + self.detector_trigger.delete() + with pytest.raises(DataCondition.DoesNotExist): + 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. + """ + workflow = AlertRuleWorkflow.objects.get(alert_rule=self.metric_alert).workflow + workflow.delete() + + with pytest.raises(AlertRuleWorkflow.DoesNotExist): + 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. + """ + self.action_filter.delete() + + with pytest.raises(DataCondition.DoesNotExist): + get_action_filter(self.alert_rule_trigger, DetectorPriorityLevel.HIGH) From 6ca9e8160c49e36c3e72bba3f8537952d369c00a Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Fri, 31 Jan 2025 11:58:56 -0800 Subject: [PATCH 5/6] remove unused imports --- src/sentry/testutils/factories.py | 3 --- tests/sentry/workflow_engine/test_base.py | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/sentry/testutils/factories.py b/src/sentry/testutils/factories.py index ba3c9ca80c0b97..ce00dcd2fa5ca4 100644 --- a/src/sentry/testutils/factories.py +++ b/src/sentry/testutils/factories.py @@ -42,7 +42,6 @@ query_datasets_to_type, ) from sentry.incidents.models.alert_rule import ( - AlertRule, AlertRuleDetectionType, AlertRuleThresholdType, AlertRuleTriggerAction, @@ -171,8 +170,6 @@ from sentry.utils.performance_issues.performance_problem import PerformanceProblem from sentry.workflow_engine.models import ( Action, - AlertRuleDetector, - AlertRuleWorkflow, DataCondition, DataConditionGroup, DataConditionGroupAction, diff --git a/tests/sentry/workflow_engine/test_base.py b/tests/sentry/workflow_engine/test_base.py index 559264a7623920..ff4545fb8ab9d9 100644 --- a/tests/sentry/workflow_engine/test_base.py +++ b/tests/sentry/workflow_engine/test_base.py @@ -4,7 +4,6 @@ from sentry.eventstore.models import Event, GroupEvent from sentry.incidents.grouptype import MetricAlertFire -from sentry.incidents.models.alert_rule import AlertRule from sentry.incidents.utils.types import QuerySubscriptionUpdate from sentry.issues.issue_occurrence import IssueOccurrence from sentry.models.group import Group @@ -15,8 +14,6 @@ from sentry.utils.registry import AlreadyRegisteredError from sentry.workflow_engine.models import ( Action, - AlertRuleDetector, - AlertRuleWorkflow, DataConditionGroup, DataPacket, DataSource, From 8ded431ecdd840c66681e95f28af4ad5c26d7791 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Mon, 3 Feb 2025 15:18:34 -0800 Subject: [PATCH 6/6] name nits --- .../test_migrate_alert_rule.py | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) 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 9de4ace3b8a058..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 @@ -169,7 +169,7 @@ def create_metric_alert_lookup_tables( alert_rule_workflow, ) - def set_up_migrated_metric_alert_objects( + def create_migrated_metric_alert_objects( self, metric_alert: AlertRule, name="hojicha" ) -> tuple[ DataSource, @@ -225,7 +225,7 @@ def set_up_migrated_metric_alert_objects( data_source_detector, ) - def set_up_migrated_metric_alert_rule_trigger_objects( + def create_migrated_metric_alert_rule_trigger_objects( self, alert_rule_trigger: AlertRuleTrigger, priority: DetectorPriorityLevel, @@ -260,7 +260,7 @@ def set_up_migrated_metric_alert_rule_trigger_objects( return detector_trigger, action_filter - def set_up_migrated_metric_alert_rule_resolve_objects( + def create_migrated_metric_alert_rule_resolve_objects( self, alert_rule: AlertRule, resolve_threshold, detector_trigger_type: Condition ) -> tuple[DataCondition, DataCondition]: """ @@ -290,7 +290,7 @@ def set_up_migrated_metric_alert_rule_resolve_objects( return detector_trigger, action_filter - def set_up_migrated_metric_alert_rule_action_objects( + def create_migrated_metric_alert_rule_action_objects( self, alert_rule_trigger_action: AlertRuleTriggerAction ) -> tuple[Action, DataConditionGroupAction, ActionAlertRuleTriggerAction]: """ @@ -357,7 +357,7 @@ def setUp(self): self.alert_rule_workflow, self.detector_workflow, self.data_source_detector, - ) = self.set_up_migrated_metric_alert_objects(self.metric_alert) + ) = 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) @@ -405,7 +405,7 @@ def setUp(self): self.alert_rule_workflow, self.detector_workflow, self.data_source_detector, - ) = self.set_up_migrated_metric_alert_objects(self.metric_alert) + ) = self.create_migrated_metric_alert_objects(self.metric_alert) def test_dual_update_metric_alert(self): detector_state = self.detector_state @@ -456,8 +456,8 @@ 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.set_up_migrated_metric_alert_objects(self.metric_alert) - self.set_up_migrated_metric_alert_objects(self.metric_alert_no_resolve, name="matcha") + self.create_migrated_metric_alert_objects(self.metric_alert) + self.create_migrated_metric_alert_objects(self.metric_alert_no_resolve, name="matcha") self.alert_rule_trigger_warning = self.create_alert_rule_trigger( alert_rule=self.metric_alert, label="warning" @@ -539,9 +539,9 @@ def setUp(self): self.alert_rule_trigger = self.create_alert_rule_trigger( alert_rule=self.metric_alert, label="critical" ) - self.set_up_migrated_metric_alert_objects(self.metric_alert) + self.create_migrated_metric_alert_objects(self.metric_alert) self.detector_trigger, self.action_filter = ( - self.set_up_migrated_metric_alert_rule_trigger_objects( + self.create_migrated_metric_alert_rule_trigger_objects( self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER ) ) @@ -573,14 +573,14 @@ def setUp(self): self.alert_rule_trigger = self.create_alert_rule_trigger( alert_rule=self.metric_alert, label="critical", alert_threshold=200 ) - self.set_up_migrated_metric_alert_objects(self.metric_alert) + self.create_migrated_metric_alert_objects(self.metric_alert) self.critical_detector_trigger, self.critical_action_filter = ( - self.set_up_migrated_metric_alert_rule_trigger_objects( + 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.set_up_migrated_metric_alert_rule_resolve_objects( + self.create_migrated_metric_alert_rule_resolve_objects( self.metric_alert, 200, Condition.LESS_OR_EQUAL ) ) @@ -662,11 +662,11 @@ def setUp(self): alert_rule_trigger=self.critical_trigger, ) # set up ACI objects - self.set_up_migrated_metric_alert_objects(self.metric_alert) - self.set_up_migrated_metric_alert_rule_trigger_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.set_up_migrated_metric_alert_rule_trigger_objects( + self.create_migrated_metric_alert_rule_trigger_objects( self.warning_trigger, DetectorPriorityLevel.MEDIUM, Condition.GREATER ) @@ -713,12 +713,12 @@ def setUp(self): alert_rule_trigger=self.alert_rule_trigger ) - self.set_up_migrated_metric_alert_objects(self.metric_alert) - self.set_up_migrated_metric_alert_rule_trigger_objects( + 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.set_up_migrated_metric_alert_rule_action_objects(self.alert_rule_trigger_action) + self.create_migrated_metric_alert_rule_action_objects(self.alert_rule_trigger_action) ) def test_dual_delete_migrated_alert_rule_trigger_action(self): @@ -762,12 +762,12 @@ def setUp(self): alert_rule_trigger=self.alert_rule_trigger ) - self.set_up_migrated_metric_alert_objects(self.metric_alert) - self.set_up_migrated_metric_alert_rule_trigger_objects( + 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.set_up_migrated_metric_alert_rule_action_objects(self.alert_rule_trigger_action) + self.create_migrated_metric_alert_rule_action_objects(self.alert_rule_trigger_action) ) # TODO (mifu67): add these tests once the update changes land @@ -784,8 +784,8 @@ def setUp(self): self.alert_rule_trigger = self.create_alert_rule_trigger( alert_rule=self.metric_alert, label="critical" ) - self.set_up_migrated_metric_alert_objects(self.metric_alert) - self.set_up_migrated_metric_alert_rule_trigger_objects( + 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 ) @@ -800,7 +800,7 @@ 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.set_up_migrated_metric_alert_rule_trigger_objects( + self.create_migrated_metric_alert_rule_trigger_objects( warning_trigger, DetectorPriorityLevel.MEDIUM, Condition.GREATER ) @@ -822,9 +822,9 @@ def setUp(self): self.alert_rule_trigger = self.create_alert_rule_trigger( alert_rule=self.metric_alert, label="critical" ) - self.set_up_migrated_metric_alert_objects(self.metric_alert) + self.create_migrated_metric_alert_objects(self.metric_alert) self.detector_trigger, self.action_filter = ( - self.set_up_migrated_metric_alert_rule_trigger_objects( + self.create_migrated_metric_alert_rule_trigger_objects( self.alert_rule_trigger, DetectorPriorityLevel.HIGH, Condition.GREATER ) )