Skip to content

Commit d5ca711

Browse files
authored
fix(detectors): Make AlertRuleDetector.objects filter out pending deletion Detectors by default (#112550)
If a Detector is being deleted, the AlertRuleDetector row should typically be filtered out.
1 parent b61fe04 commit d5ca711

File tree

8 files changed

+150
-8
lines changed

8 files changed

+150
-8
lines changed
Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
from sentry.deletions.base import BaseRelation, ModelDeletionTask, ModelRelation
22
from sentry.incidents.models.alert_rule import AlertRule
3+
from sentry.workflow_engine.models.alertrule_detector import AlertRuleDetector
4+
5+
6+
class AlertRuleDetectorDeletionTask(ModelDeletionTask[AlertRuleDetector]):
7+
manager_name = "objects_for_deletion"
38

49

510
class AlertRuleDeletionTask(ModelDeletionTask[AlertRule]):
@@ -10,7 +15,15 @@ class AlertRuleDeletionTask(ModelDeletionTask[AlertRule]):
1015
def get_child_relations(self, instance: AlertRule) -> list[BaseRelation]:
1116
from sentry.incidents.models.alert_rule import AlertRuleTrigger
1217
from sentry.incidents.models.incident import Incident
13-
from sentry.workflow_engine.models import AlertRuleDetector, AlertRuleWorkflow
18+
from sentry.workflow_engine.models import AlertRuleWorkflow
1419

15-
model_list = (AlertRuleTrigger, Incident, AlertRuleDetector, AlertRuleWorkflow)
16-
return [ModelRelation(m, {"alert_rule_id": instance.id}) for m in model_list]
20+
return [
21+
ModelRelation(AlertRuleTrigger, {"alert_rule_id": instance.id}),
22+
ModelRelation(Incident, {"alert_rule_id": instance.id}),
23+
ModelRelation(
24+
AlertRuleDetector,
25+
{"alert_rule_id": instance.id},
26+
task=AlertRuleDetectorDeletionTask,
27+
),
28+
ModelRelation(AlertRuleWorkflow, {"alert_rule_id": instance.id}),
29+
]

src/sentry/deletions/defaults/detector.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ def get_child_relations(self, instance: Detector) -> list[BaseRelation]:
2626
# QuerySubscriptionDeletionTask sees no remaining AlertRule referencing
2727
# the SnubaQuery and can safely delete it.
2828
alert_rule_ids = list(
29-
AlertRuleDetector.objects.filter(detector_id=instance.id).values_list(
29+
AlertRuleDetector.objects_for_deletion.filter(detector_id=instance.id).values_list(
3030
"alert_rule_id", flat=True
3131
)
3232
)

src/sentry/deletions/defaults/rule.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from collections.abc import Sequence
33

44
from sentry.deletions.base import BaseRelation, ModelDeletionTask, ModelRelation
5+
from sentry.deletions.defaults.alertrule import AlertRuleDetectorDeletionTask
56
from sentry.models.rule import Rule
67
from sentry.workflow_engine.models import Workflow
78

@@ -19,7 +20,11 @@ def get_child_relations(self, instance: Rule) -> list[BaseRelation]:
1920
ModelRelation(GroupRuleStatus, {"rule_id": instance.id}),
2021
ModelRelation(RuleFireHistory, {"rule_id": instance.id}),
2122
ModelRelation(RuleActivity, {"rule_id": instance.id}),
22-
ModelRelation(AlertRuleDetector, {"rule_id": instance.id}),
23+
ModelRelation(
24+
AlertRuleDetector,
25+
{"rule_id": instance.id},
26+
task=AlertRuleDetectorDeletionTask,
27+
),
2328
]
2429

2530
alert_rule_workflow = AlertRuleWorkflow.objects.filter(rule_id=instance.id).first()

src/sentry/incidents/endpoints/organization_alert_rule_details.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ def remove_alert_rule(
155155
try:
156156
remove_detector(request, organization, target)
157157
try:
158-
ard = AlertRuleDetector.objects.get(detector_id=target.id)
158+
ard = AlertRuleDetector.objects_for_deletion.get(detector_id=target.id)
159159
target = AlertRule.objects.get(id=ard.alert_rule_id, organization=organization)
160160
delete_alert_rule(
161161
target,

src/sentry/workflow_engine/models/alertrule_detector.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,36 @@
1+
from typing import ClassVar
2+
13
from django.db.models import CheckConstraint, Q, UniqueConstraint
24

35
from sentry.backup.scopes import RelocationScope
6+
from sentry.constants import ObjectStatus
47
from sentry.db.models import (
58
BoundedBigIntegerField,
69
DefaultFieldsModel,
710
FlexibleForeignKey,
811
cell_silo_model,
912
)
13+
from sentry.db.models.manager.base import BaseManager
14+
from sentry.db.models.manager.base_query_set import BaseQuerySet
15+
16+
17+
class AlertRuleDetectorManager(BaseManager["AlertRuleDetector"]):
18+
"""
19+
Default manager that excludes rows whose related Detector is
20+
pending deletion or being deleted.
21+
"""
22+
23+
def get_queryset(self) -> BaseQuerySet["AlertRuleDetector"]:
24+
return (
25+
super()
26+
.get_queryset()
27+
.exclude(
28+
detector__status__in=(
29+
ObjectStatus.PENDING_DELETION,
30+
ObjectStatus.DELETION_IN_PROGRESS,
31+
)
32+
)
33+
)
1034

1135

1236
@cell_silo_model
@@ -17,6 +41,9 @@ class AlertRuleDetector(DefaultFieldsModel):
1741

1842
__relocation_scope__ = RelocationScope.Organization
1943

44+
objects: ClassVar[AlertRuleDetectorManager] = AlertRuleDetectorManager()
45+
objects_for_deletion: ClassVar[BaseManager["AlertRuleDetector"]] = BaseManager()
46+
2047
alert_rule_id = BoundedBigIntegerField(null=True, db_index=True)
2148
rule_id = BoundedBigIntegerField(null=True, db_index=True)
2249
detector = FlexibleForeignKey("workflow_engine.Detector")

tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from sentry.api.serializers import serialize
2424
from sentry.auth.access import OrganizationGlobalAccess
2525
from sentry.conf.server import SEER_ANOMALY_DETECTION_STORE_DATA_URL
26+
from sentry.constants import ObjectStatus
2627
from sentry.deletions.tasks.scheduled import run_scheduled_deletions
2728
from sentry.incidents.endpoints.serializers.alert_rule import DetailedAlertRuleSerializer
2829
from sentry.incidents.endpoints.serializers.utils import get_fake_id_from_object_id
@@ -321,6 +322,33 @@ def test_workflow_engine_serializer(self) -> None:
321322
assert resp.data["id"] == str(self.alert_rule.id)
322323
assert resp.data["name"] == self.detector.name
323324

325+
@with_feature("organizations:workflow-engine-rule-serializers")
326+
@with_feature("organizations:incidents")
327+
def test_pending_deletion_detector_returns_404(self) -> None:
328+
self.create_team(organization=self.organization, members=[self.user])
329+
self.login_as(self.user)
330+
331+
# Dual-written detector (has an AlertRuleDetector mapping) — look up by alert_rule_id
332+
dual_written_detector = self.create_detector(
333+
project=self.project,
334+
type=MetricIssue.slug,
335+
status=ObjectStatus.PENDING_DELETION,
336+
)
337+
alert_rule = self.create_alert_rule(projects=[self.project])
338+
self.create_alert_rule_detector(detector=dual_written_detector, alert_rule_id=alert_rule.id)
339+
340+
self.get_error_response(self.organization.slug, alert_rule.id, status_code=404)
341+
342+
# Single-written detector (no ARD) — look up by fake detector ID
343+
single_written_detector = self.create_detector(
344+
project=self.project,
345+
type=MetricIssue.slug,
346+
status=ObjectStatus.PENDING_DELETION,
347+
)
348+
fake_detector_id = get_fake_id_from_object_id(single_written_detector.id)
349+
350+
self.get_error_response(self.organization.slug, fake_detector_id, status_code=404)
351+
324352
@with_feature("organizations:incidents")
325353
@freeze_time("2024-12-11 03:21:34")
326354
def test_workflow_engine_serializer_matches_old_serializer(self) -> None:

tests/sentry/incidents/endpoints/test_organization_incident_index.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from django.utils import timezone
55

66
from sentry.api.serializers import serialize
7+
from sentry.constants import ObjectStatus
78
from sentry.incidents.endpoints.serializers.utils import get_fake_id_from_object_id
89
from sentry.incidents.grouptype import MetricIssue
910
from sentry.incidents.logic import update_incident_status
@@ -27,7 +28,7 @@
2728
migrate_metric_data_conditions,
2829
migrate_resolve_threshold_data_condition,
2930
)
30-
from sentry.workflow_engine.models import DetectorGroup, IncidentGroupOpenPeriod
31+
from sentry.workflow_engine.models import Detector, DetectorGroup, IncidentGroupOpenPeriod
3132
from sentry.workflow_engine.models.data_condition import Condition
3233
from sentry.workflow_engine.types import DetectorPriorityLevel
3334

@@ -444,3 +445,39 @@ def test_filter_by_fake_alert_rule_id(self) -> None:
444445
)
445446

446447
assert len(resp_empty.data) == 0
448+
449+
@with_feature(
450+
[
451+
"organizations:incidents",
452+
"organizations:performance-view",
453+
"organizations:workflow-engine-rule-serializers",
454+
]
455+
)
456+
def test_filter_by_alert_rule_excludes_pending_deletion_detector(self) -> None:
457+
self.create_team(organization=self.organization, members=[self.user])
458+
self.login_as(self.user)
459+
460+
alert_rule = self.create_alert_rule()
461+
trigger = self.create_alert_rule_trigger(alert_rule=alert_rule)
462+
_, _, _, detector, _, _, _, _ = migrate_alert_rule(alert_rule)
463+
migrate_metric_data_conditions(trigger)
464+
migrate_resolve_threshold_data_condition(alert_rule)
465+
466+
with assume_test_silo_mode(SiloMode.CELL):
467+
group = self.create_group(type=MetricIssue.type_id, project=self.project)
468+
group.priority = PriorityLevel.HIGH.value
469+
group.save()
470+
DetectorGroup.objects.create(detector=detector, group=group)
471+
472+
# Verify the incident shows up before deletion
473+
resp = self.get_success_response(self.organization.slug, alertRule=str(alert_rule.id))
474+
assert len(resp.data) == 1
475+
476+
# Mark the detector as pending deletion (bypass custom manager)
477+
Detector.objects_for_deletion.filter(id=detector.id).update(
478+
status=ObjectStatus.PENDING_DELETION
479+
)
480+
481+
# The alert rule filter should no longer find this detector
482+
resp = self.get_success_response(self.organization.slug, alertRule=str(alert_rule.id))
483+
assert len(resp.data) == 0

tests/sentry/incidents/test_charts.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from django.utils.dateparse import parse_datetime
66

77
from sentry.api.serializers import serialize
8+
from sentry.constants import ObjectStatus
89
from sentry.incidents.charts import (
910
build_metric_alert_chart,
1011
fetch_metric_issue_open_periods,
@@ -30,7 +31,7 @@
3031
from sentry.testutils.helpers.datetime import freeze_time
3132
from sentry.testutils.helpers.features import with_feature
3233
from sentry.types.group import PriorityLevel
33-
from sentry.workflow_engine.models import DetectorGroup
34+
from sentry.workflow_engine.models import Detector, DetectorGroup
3435
from tests.sentry.incidents.utils.test_metric_issue_base import BaseMetricIssueTest
3536

3637
now = "2022-05-16T20:00:00"
@@ -272,6 +273,37 @@ def test_get_incidents_from_detector(self) -> None:
272273
assert created_activity_resp["type"] == IncidentActivityType.CREATED.value
273274
assert created_activity_resp["dateCreated"] == created_activity.date_added
274275

276+
@freeze_time(frozen_time)
277+
@with_feature("organizations:incidents")
278+
def test_pending_deletion_detector_not_resolved_to_alert_rule(self) -> None:
279+
self.create_detector() # dummy so detector ID != alert rule ID
280+
detector = self.create_detector(project=self.project)
281+
alert_rule = self.create_alert_rule(organization=self.organization, projects=[self.project])
282+
self.create_alert_rule_detector(detector=detector, alert_rule_id=alert_rule.id)
283+
incident = self.create_incident(
284+
date_started=must_parse_datetime("2022-05-16T18:55:00Z"),
285+
status=IncidentStatus.CRITICAL.value,
286+
alert_rule=alert_rule,
287+
)
288+
self.create_incident_activity(
289+
incident,
290+
IncidentActivityType.DETECTED.value,
291+
date_added=incident.date_started,
292+
)
293+
294+
time_period = incident_date_range(60, incident.date_started, incident.date_closed)
295+
296+
# Mark the detector as pending deletion (bypass custom manager)
297+
Detector.objects_for_deletion.filter(id=detector.id).update(
298+
status=ObjectStatus.PENDING_DELETION
299+
)
300+
301+
# The AlertRuleDetector lookup should skip the deleted detector,
302+
# so the detector ID won't be mapped to the alert rule ID and
303+
# no incidents will be found
304+
chart_data = fetch_metric_issue_open_periods(self.organization, detector.id, time_period)
305+
assert len(chart_data) == 0
306+
275307
@freeze_time(frozen_time)
276308
@with_feature("organizations:incidents")
277309
@with_feature("organizations:workflow-engine-ui")

0 commit comments

Comments
 (0)