feat(aci milestone 3): delete data in Seer when AD detector is deleted (#101843)

mifu67 · ceorourke · web-flow · commit 62b1f7316217 · 2025-11-13T09:48:22.000-08:00
When a detector is deleted, we need to delete the rule data in Seer.
Similar implementation to the legacy alert rule code, but uses detector
ID instead of alert rule ID.

Note that because we are no longer able to use the post delete signal
(the `QuerySubscription` model is deleted by the time we get to post
delete so we cannot access the `source_id` needed) we cannot ensure Seer
knows to delete data on their end during a cascade deletion - however
Seer will delete data that doesn't receive an update for 90 days so it
will eventually be cleaned up.

---------

Co-authored-by: Colleen O'Rourke &lt;colleen@sentry.io&gt;
diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py
@@ -755,6 +755,30 @@ def nullify_id(model: Model) -> None:
         )
 
 
+def delete_anomaly_detection_rule(snuba_query: SnubaQuery, alert_rule: AlertRule) -> None:
+    """
+    Delete accompanying data in Seer for anomaly detection rules
+    """
+    try:
+        source_id = QuerySubscription.objects.get(snuba_query_id=snuba_query.id).id
+        success = delete_rule_in_seer(
+            organization=alert_rule.organization,
+            source_id=source_id,
+        )
+        if not success:
+            logger.error(
+                "Call to delete rule data in Seer failed",
+                extra={
+                    "source_id": source_id,
+                },
+            )
+    except QuerySubscription.DoesNotExist:
+        logger.exception(
+            "Snuba query missing query subscription",
+            extra={"snuba_query_id": snuba_query.id},
+        )
+
+
 def update_alert_rule(
     alert_rule: AlertRule,
     query_type: SnubaQuery.Type | None = None,
@@ -921,18 +945,8 @@ def update_alert_rule(
                 alert_rule, project, snuba_query, updated_fields, updated_query_fields
             )
         else:
-            # if this was a dynamic rule, delete the data in Seer
             if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC:
-                success = delete_rule_in_seer(
-                    alert_rule=alert_rule,
-                )
-                if not success:
-                    logger.error(
-                        "Call to delete rule data in Seer failed",
-                        extra={
-                            "rule_id": alert_rule.id,
-                        },
-                    )
+                delete_anomaly_detection_rule(snuba_query, alert_rule)
             # if this alert was previously a dynamic alert, then we should update the rule to be ready
             if alert_rule.status == AlertRuleStatus.NOT_ENOUGH_DATA.value:
                 alert_rule.update(status=AlertRuleStatus.PENDING.value)
@@ -1088,20 +1102,11 @@ def delete_alert_rule(
             )
         subscriptions = _unpack_snuba_query(alert_rule).subscriptions.all()
 
+        if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC:
+            delete_anomaly_detection_rule(alert_rule.snuba_query, alert_rule)
+
         incidents = Incident.objects.filter(alert_rule=alert_rule)
         if incidents.exists():
-            # if this was a dynamic rule, delete the data in Seer
-            if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC:
-                success = delete_rule_in_seer(
-                    alert_rule=alert_rule,
-                )
-                if not success:
-                    logger.error(
-                        "Call to delete rule data in Seer failed",
-                        extra={
-                            "rule_id": alert_rule.id,
-                        },
-                    )
             AlertRuleActivity.objects.create(
                 alert_rule=alert_rule,
                 user_id=user.id if user else None,
diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py
@@ -8,6 +8,7 @@
 from sentry.incidents.logic import enable_disable_subscriptions
 from sentry.incidents.models.alert_rule import AlertRuleDetectionType
 from sentry.relay.config.metric_extraction import on_demand_metrics_feature_flags
+from sentry.seer.anomaly_detection.delete_rule import delete_data_in_seer_for_detector
 from sentry.seer.anomaly_detection.store_data_workflow_engine import send_new_detector_data
 from sentry.snuba.dataset import Dataset
 from sentry.snuba.metrics.extraction import should_use_on_demand_metrics
@@ -332,3 +333,11 @@ def create(self, validated_data: dict[str, Any]):
 
         schedule_update_project_config(detector)
         return detector
+
+    def delete(self):
+        # Let Seer know we're deleting a dynamic detector so the data can be deleted there too
+        assert self.instance is not None
+        detector: Detector = self.instance
+        delete_data_in_seer_for_detector(detector)
+
+        super().delete()
diff --git a/src/sentry/incidents/models/alert_rule.py b/src/sentry/incidents/models/alert_rule.py
@@ -36,7 +36,6 @@
     ActionService,
     ActionTarget,
 )
-from sentry.seer.anomaly_detection.delete_rule import delete_rule_in_seer
 from sentry.snuba.models import QuerySubscription
 from sentry.types.actor import Actor
 from sentry.utils import metrics
@@ -140,18 +139,6 @@ def clear_alert_rule_subscription_caches(cls, instance: AlertRule, **kwargs: Any
                 for sub_id in subscription_ids
             )
 
-    @classmethod
-    def delete_data_in_seer(cls, instance: AlertRule, **kwargs: Any) -> None:
-        if instance.detection_type == AlertRuleDetectionType.DYNAMIC:
-            success = delete_rule_in_seer(alert_rule=instance)
-            if not success:
-                logger.error(
-                    "Call to delete rule data in Seer failed",
-                    extra={
-                        "rule_id": instance.id,
-                    },
-                )
-
 
 @region_silo_model
 class AlertRuleProjects(Model):
@@ -624,7 +611,6 @@ class Meta:
 
 
 post_delete.connect(AlertRuleManager.clear_subscription_cache, sender=QuerySubscription)
-post_delete.connect(AlertRuleManager.delete_data_in_seer, sender=AlertRule)
 post_save.connect(AlertRuleManager.clear_subscription_cache, sender=QuerySubscription)
 post_save.connect(AlertRuleManager.clear_alert_rule_subscription_caches, sender=AlertRule)
 post_delete.connect(AlertRuleManager.clear_alert_rule_subscription_caches, sender=AlertRule)
diff --git a/src/sentry/seer/anomaly_detection/delete_rule.py b/src/sentry/seer/anomaly_detection/delete_rule.py
@@ -1,12 +1,15 @@
+from __future__ import annotations
+
 import logging
 from typing import TYPE_CHECKING
 
 from django.conf import settings
 from urllib3.exceptions import MaxRetryError, TimeoutError
 
 from sentry.conf.server import SEER_ALERT_DELETION_URL
+from sentry.models.organization import Organization
 from sentry.net.http import connection_from_url
-from sentry.seer.anomaly_detection.types import DeleteAlertDataRequest
+from sentry.seer.anomaly_detection.types import AlertInSeer, DataSourceType, DeleteAlertDataRequest
 from sentry.seer.signed_seer_api import make_signed_seer_api_request
 from sentry.utils import json
 from sentry.utils.json import JSONDecodeError
@@ -18,21 +21,51 @@
 )
 
 if TYPE_CHECKING:
-    from sentry.incidents.models.alert_rule import AlertRule
+    from sentry.workflow_engine.models.detector import Detector
+
 
+def delete_data_in_seer_for_detector(detector: Detector):
+    from sentry.incidents.models.alert_rule import AlertRuleDetectionType
+    from sentry.workflow_engine.models import DataSourceDetector
 
-def delete_rule_in_seer(alert_rule: "AlertRule") -> bool:
+    data_source_detector = DataSourceDetector.objects.filter(detector_id=detector.id).first()
+    if not data_source_detector:
+        logger.error(
+            "No data source found for detector",
+            extra={
+                "detector_id": detector.id,
+            },
+        )
+        return
+
+    organization = detector.project.organization
+
+    if detector.config.get("detection_type") == AlertRuleDetectionType.DYNAMIC:
+        success = delete_rule_in_seer(
+            source_id=int(data_source_detector.data_source.source_id), organization=organization
+        )
+        if not success:
+            logger.error(
+                "Call to delete rule data in Seer failed",
+                extra={
+                    "detector_id": detector.id,
+                },
+            )
+
+
+def delete_rule_in_seer(source_id: int, organization: Organization) -> bool:
     """
     Send a request to delete an alert rule from Seer. Returns True if the request was successful.
     """
     body = DeleteAlertDataRequest(
-        organization_id=alert_rule.organization.id,
-        alert={"id": alert_rule.id},
+        organization_id=organization.id,
+        alert=AlertInSeer(
+            id=None, source_id=source_id, source_type=DataSourceType.SNUBA_QUERY_SUBSCRIPTION
+        ),
     )
     extra_data = {
-        "rule_id": alert_rule.id,
+        "source_id": source_id,
     }
-
     try:
         response = make_signed_seer_api_request(
             connection_pool=seer_anomaly_detection_connection_pool,
@@ -46,7 +79,7 @@ def delete_rule_in_seer(alert_rule: "AlertRule") -> bool:
         )
         return False
 
-    if response.status > 400:
+    if response.status >= 400:
         logger.error(
             "Error when hitting Seer delete rule data endpoint",
             extra={
@@ -75,7 +108,14 @@ def delete_rule_in_seer(alert_rule: "AlertRule") -> bool:
         return False
 
     status = results.get("success")
-    if status is None or status is not True:
+    if status is None:
+        logger.error(
+            "Request to delete alert rule from Seer was unsuccessful",
+            extra=extra_data,
+        )
+        return False
+    elif status is not True:
+        extra_data["message"] = results.get("message")
         logger.error(
             "Request to delete alert rule from Seer was unsuccessful",
             extra=extra_data,
diff --git a/tests/sentry/deletions/test_alert_rule.py b/tests/sentry/deletions/test_alert_rule.py
@@ -77,20 +77,14 @@ def test_simple(self) -> None:
         assert not AlertRuleWorkflow.objects.filter(alert_rule_id=alert_rule.id).exists()
 
     @with_feature("organizations:anomaly-detection-alerts")
-    @patch(
-        "sentry.seer.anomaly_detection.delete_rule.seer_anomaly_detection_connection_pool.urlopen"
-    )
     @patch(
         "sentry.seer.anomaly_detection.store_data.seer_anomaly_detection_connection_pool.urlopen"
     )
-    def test_dynamic_alert_rule(
-        self, mock_store_request: MagicMock, mock_delete_request: MagicMock
-    ) -> None:
+    def test_dynamic_alert_rule(self, mock_store_request: MagicMock) -> None:
         organization = self.create_organization()
 
         seer_return_value = {"success": True}
         mock_store_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200)
-        mock_delete_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200)
 
         alert_rule = self.create_alert_rule(
             sensitivity=AlertRuleSensitivity.HIGH,
@@ -120,5 +114,3 @@ def test_dynamic_alert_rule(
         assert not AlertRule.objects.filter(id=alert_rule.id).exists()
         assert not AlertRuleTrigger.objects.filter(id=alert_rule_trigger.id).exists()
         assert not Incident.objects.filter(id=incident.id).exists()
-
-        assert mock_delete_request.call_count == 1
diff --git a/tests/sentry/incidents/test_logic.py b/tests/sentry/incidents/test_logic.py
diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py
