Skip to content

Commit ce0da12

Browse files
authored
fix(detectors): Ensure metric Detector deletion cleans up the AlertRule (#112425)
We don't really dual-write both ways, but we don't want to leave behind the AlertRule when we delete a metric Detector. Fixes ISWF-2389.
1 parent 78c74d1 commit ce0da12

File tree

3 files changed

+90
-2
lines changed

3 files changed

+90
-2
lines changed

src/sentry/deletions/defaults/detector.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,29 @@ class DetectorDeletionTask(ModelDeletionTask[Detector]):
1010
manager_name = "objects_for_deletion"
1111

1212
def get_child_relations(self, instance: Detector) -> list[BaseRelation]:
13-
from sentry.workflow_engine.models import DataConditionGroup, DataSource
13+
from sentry.incidents.models.alert_rule import AlertRule
14+
from sentry.workflow_engine.models import (
15+
AlertRuleDetector,
16+
DataConditionGroup,
17+
DataSource,
18+
)
1419

1520
model_relations: list[BaseRelation] = []
1621

22+
# If this detector was dual-written from an AlertRule, cascade deletion
23+
# to the AlertRule. AlertRuleDeletionTask will handle cleaning up
24+
# AlertRuleDetector, AlertRuleWorkflow, triggers, and incidents.
25+
# NOTE: AlertRule must be deleted before DataSource so that
26+
# QuerySubscriptionDeletionTask sees no remaining AlertRule referencing
27+
# the SnubaQuery and can safely delete it.
28+
alert_rule_ids = list(
29+
AlertRuleDetector.objects.filter(detector_id=instance.id).values_list(
30+
"alert_rule_id", flat=True
31+
)
32+
)
33+
if alert_rule_ids:
34+
model_relations.append(ModelRelation(AlertRule, {"id__in": alert_rule_ids}))
35+
1736
# check that no other rows are related to the data source
1837
data_source_ids = DataSource.objects.filter(detector=instance.id).values_list(
1938
"id", flat=True

tests/sentry/deletions/test_detector.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,14 @@
55
from sentry.constants import ObjectStatus
66
from sentry.deletions.tasks.scheduled import reattempt_deletions, run_scheduled_deletions
77
from sentry.incidents.grouptype import MetricIssue
8+
from sentry.incidents.models.alert_rule import AlertRule, AlertRuleTrigger
89
from sentry.snuba.models import QuerySubscription, SnubaQuery
910
from sentry.testutils.hybrid_cloud import HybridCloudTestMixin
1011
from sentry.uptime.models import UptimeSubscription, get_uptime_subscription
12+
from sentry.workflow_engine.migration_helpers.alert_rule import dual_write_alert_rule
1113
from sentry.workflow_engine.models import (
14+
AlertRuleDetector,
15+
AlertRuleWorkflow,
1216
DataCondition,
1317
DataConditionGroup,
1418
DataSource,
@@ -262,3 +266,34 @@ def test_delete_uptime_subscription_without_detector(self) -> None:
262266
run_scheduled_deletions()
263267

264268
assert not UptimeSubscription.objects.filter(id=uptime_sub_id).exists()
269+
270+
271+
class DeleteDualWrittenDetectorTest(BaseWorkflowTest, HybridCloudTestMixin):
272+
def test_deleting_detector_deletes_associated_alert_rule(self) -> None:
273+
alert_rule = self.create_alert_rule(
274+
organization=self.organization,
275+
projects=[self.project],
276+
)
277+
trigger = self.create_alert_rule_trigger(alert_rule=alert_rule)
278+
dual_write_alert_rule(alert_rule)
279+
280+
snuba_query = alert_rule.snuba_query
281+
subscription = QuerySubscription.objects.get(snuba_query=snuba_query)
282+
detector = AlertRuleDetector.objects.get(alert_rule_id=alert_rule.id).detector
283+
data_source = DataSource.objects.get(source_id=str(subscription.id))
284+
285+
detector.status = ObjectStatus.PENDING_DELETION
286+
detector.save()
287+
self.ScheduledDeletion.schedule(instance=detector, days=0)
288+
289+
with self.tasks():
290+
run_scheduled_deletions()
291+
292+
assert not Detector.objects.filter(id=detector.id).exists()
293+
assert not AlertRuleDetector.objects.filter(alert_rule_id=alert_rule.id).exists()
294+
assert not AlertRuleWorkflow.objects.filter(alert_rule_id=alert_rule.id).exists()
295+
assert not AlertRule.objects.filter(id=alert_rule.id).exists()
296+
assert not AlertRuleTrigger.objects.filter(id=trigger.id).exists()
297+
assert not DataSource.objects.filter(id=data_source.id).exists()
298+
assert not QuerySubscription.objects.filter(id=subscription.id).exists()
299+
assert not SnubaQuery.objects.filter(id=snuba_query.id).exists()

tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
from sentry.deletions.tasks.scheduled import run_scheduled_deletions
1010
from sentry.grouping.grouptype import ErrorGroupType
1111
from sentry.incidents.grouptype import MetricIssue
12+
from sentry.incidents.models.alert_rule import AlertRule
1213
from sentry.models.environment import Environment
1314
from sentry.monitors.grouptype import MonitorIncidentType
1415
from sentry.search.utils import _HACKY_INVALID_USER
16+
from sentry.snuba.models import QuerySubscription, SnubaQuery
1517
from sentry.testutils.asserts import assert_org_audit_log_exists
1618
from sentry.testutils.cases import APITestCase
1719
from sentry.testutils.helpers.datetime import before_now
@@ -23,7 +25,12 @@
2325
DATA_SOURCE_UPTIME_SUBSCRIPTION,
2426
)
2527
from sentry.workflow_engine.endpoints.organization_detector_index import convert_assignee_values
26-
from sentry.workflow_engine.models import DataConditionGroup, Detector
28+
from sentry.workflow_engine.migration_helpers.alert_rule import dual_write_alert_rule
29+
from sentry.workflow_engine.models import (
30+
AlertRuleDetector,
31+
DataConditionGroup,
32+
Detector,
33+
)
2734
from sentry.workflow_engine.models.detector_group import DetectorGroup
2835
from sentry.workflow_engine.typings.grouptype import IssueStreamGroupType
2936

@@ -1369,3 +1376,30 @@ def test_delete_system_and_user_created_with_query_filters(self) -> None:
13691376
)
13701377

13711378
self.assert_unaffected_detectors([self.detector, error_detector])
1379+
1380+
def test_delete_dual_written_detector_cleans_up_alert_rule(self) -> None:
1381+
alert_rule = self.create_alert_rule(
1382+
organization=self.organization,
1383+
projects=[self.project],
1384+
)
1385+
self.create_alert_rule_trigger(alert_rule=alert_rule)
1386+
dual_write_alert_rule(alert_rule)
1387+
1388+
detector = AlertRuleDetector.objects.get(alert_rule_id=alert_rule.id).detector
1389+
snuba_query = alert_rule.snuba_query
1390+
subscription = QuerySubscription.objects.get(snuba_query=snuba_query)
1391+
1392+
with outbox_runner():
1393+
self.get_success_response(
1394+
self.organization.slug,
1395+
qs_params={"id": str(detector.id)},
1396+
status_code=204,
1397+
)
1398+
1399+
with self.tasks():
1400+
run_scheduled_deletions()
1401+
1402+
assert not Detector.objects.filter(id=detector.id).exists()
1403+
assert not AlertRule.objects.filter(id=alert_rule.id).exists()
1404+
assert not QuerySubscription.objects.filter(id=subscription.id).exists()
1405+
assert not SnubaQuery.objects.filter(id=snuba_query.id).exists()

0 commit comments

Comments
 (0)