From 9807ec30ba5c4faa83d783d015e749ce91abf918 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Mon, 20 Oct 2025 16:51:10 -0700 Subject: [PATCH 01/18] implement --- src/sentry/incidents/logic.py | 6 +- src/sentry/incidents/models/alert_rule.py | 4 +- .../seer/anomaly_detection/delete_rule.py | 70 ++++++++++++++++++- .../organization_detector_details.py | 30 +++++++- .../test_organization_detector_details.py | 35 ++++++++++ 5 files changed, 137 insertions(+), 8 deletions(-) diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index 3bf3bbb1a54ef7..04532fcac8146a 100644 --- a/src/sentry/incidents/logic.py +++ b/src/sentry/incidents/logic.py @@ -65,7 +65,7 @@ ) from sentry.search.events.fields import is_function, resolve_field from sentry.search.events.types import SnubaParams -from sentry.seer.anomaly_detection.delete_rule import delete_rule_in_seer +from sentry.seer.anomaly_detection.delete_rule import delete_rule_in_seer_legacy from sentry.seer.anomaly_detection.store_data import send_new_rule_data, update_rule_data_legacy from sentry.sentry_apps.services.app import RpcSentryAppInstallation, app_service from sentry.shared_integrations.exceptions import ( @@ -923,7 +923,7 @@ def update_alert_rule( else: # if this was a dynamic rule, delete the data in Seer if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC: - success = delete_rule_in_seer( + success = delete_rule_in_seer_legacy( alert_rule=alert_rule, ) if not success: @@ -1092,7 +1092,7 @@ def delete_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( + success = delete_rule_in_seer_legacy( alert_rule=alert_rule, ) if not success: diff --git a/src/sentry/incidents/models/alert_rule.py b/src/sentry/incidents/models/alert_rule.py index e47881a8a94e8b..255e9a4f282671 100644 --- a/src/sentry/incidents/models/alert_rule.py +++ b/src/sentry/incidents/models/alert_rule.py @@ -36,7 +36,7 @@ ActionService, ActionTarget, ) -from sentry.seer.anomaly_detection.delete_rule import delete_rule_in_seer +from sentry.seer.anomaly_detection.delete_rule import delete_rule_in_seer_legacy from sentry.snuba.models import QuerySubscription from sentry.types.actor import Actor from sentry.utils import metrics @@ -143,7 +143,7 @@ def clear_alert_rule_subscription_caches(cls, instance: AlertRule, **kwargs: Any @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) + success = delete_rule_in_seer_legacy(alert_rule=instance) if not success: logger.error( "Call to delete rule data in Seer failed", diff --git a/src/sentry/seer/anomaly_detection/delete_rule.py b/src/sentry/seer/anomaly_detection/delete_rule.py index 9655165e39a853..9b1abbec5b4ffe 100644 --- a/src/sentry/seer/anomaly_detection/delete_rule.py +++ b/src/sentry/seer/anomaly_detection/delete_rule.py @@ -5,8 +5,9 @@ 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 @@ -21,7 +22,72 @@ from sentry.incidents.models.alert_rule import AlertRule -def delete_rule_in_seer(alert_rule: "AlertRule") -> bool: +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=organization.id, + alert=AlertInSeer( + id=None, source_id=source_id, source_type=DataSourceType.SNUBA_QUERY_SUBSCRIPTION + ), + ) + extra_data = { + "source_id": source_id, + } + try: + response = make_signed_seer_api_request( + connection_pool=seer_anomaly_detection_connection_pool, + path=SEER_ALERT_DELETION_URL, + body=json.dumps(body).encode("utf-8"), + ) + except (TimeoutError, MaxRetryError): + logger.warning( + "Timeout error when hitting Seer delete rule data endpoint", + extra=extra_data, + ) + return False + + if response.status > 400: + logger.error( + "Error when hitting Seer delete rule data endpoint", + extra={ + "response_data": response.data, + **extra_data, + }, + ) + return False + + try: + decoded_data = response.data.decode("utf-8") + except AttributeError: + logger.exception( + "Failed to parse Seer delete rule data response", + extra=extra_data, + ) + return False + + try: + results = json.loads(decoded_data) + except JSONDecodeError: + logger.exception( + "Failed to parse Seer delete rule data response", + extra=extra_data, + ) + return False + + status = results.get("success") + if status is None or status is not True: + logger.error( + "Request to delete alert rule from Seer was unsuccessful", + extra=extra_data, + ) + return False + + return True + + +def delete_rule_in_seer_legacy(alert_rule: "AlertRule") -> bool: """ Send a request to delete an alert rule from Seer. Returns True if the request was successful. """ diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index 63f4a5e02b1162..1b3bf77ac603ca 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -1,3 +1,5 @@ +import logging + from django.db import router, transaction from drf_spectacular.utils import PolymorphicProxySerializer, extend_schema from rest_framework import status @@ -24,9 +26,11 @@ from sentry.grouping.grouptype import ErrorGroupType from sentry.incidents.grouptype import MetricIssue from sentry.incidents.metric_issue_detector import schedule_update_project_config +from sentry.incidents.models.alert_rule import AlertRuleDetectionType from sentry.issues import grouptype from sentry.models.organization import Organization from sentry.models.project import Project +from sentry.seer.anomaly_detection.delete_rule import delete_rule_in_seer from sentry.utils.audit import create_audit_entry from sentry.workflow_engine.endpoints.serializers.detector_serializer import DetectorSerializer from sentry.workflow_engine.endpoints.validators.detector_workflow import ( @@ -34,7 +38,9 @@ can_edit_detector, ) from sentry.workflow_engine.endpoints.validators.utils import get_unknown_detector_type_error -from sentry.workflow_engine.models import Detector +from sentry.workflow_engine.models import DataSourceDetector, Detector + +logger = logging.getLogger(__name__) def get_detector_validator( @@ -207,6 +213,28 @@ def delete(self, request: Request, organization: Organization, detector: Detecto if detector.type == MetricIssue.slug: schedule_update_project_config(detector) + data_source_detector = DataSourceDetector.objects.filter( + detector_id=detector.id + ).first() + if not data_source_detector: + return Response(status=404) + + if detector.config.get("detection_type") == AlertRuleDetectionType.DYNAMIC: + success = delete_rule_in_seer( + source_id=data_source_detector.data_source_id, organization=organization + ) + # We don't need to surface this to the user, but we should know about it. + if not success: + logger.error( + "Call to delete rule data in Seer failed", + extra={ + "detector_id": detector.id, + }, + ) + + RegionScheduledDeletion.schedule(detector, days=0, actor=request.user) + detector.update(status=ObjectStatus.PENDING_DELETION) + create_audit_entry( request=request, organization=detector.project.organization, diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index fc9d6c740f5364..8eb53bc23883f5 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -10,6 +10,7 @@ from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion from sentry.grouping.grouptype import ErrorGroupType from sentry.incidents.grouptype import MetricIssue +from sentry.incidents.models.alert_rule import AlertRuleDetectionType from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE from sentry.models.auditlogentry import AuditLogEntry from sentry.silo.base import SiloMode @@ -18,6 +19,7 @@ from sentry.snuba.subscriptions import create_snuba_query, create_snuba_subscription from sentry.testutils.asserts import assert_status_code from sentry.testutils.cases import APITestCase +from sentry.testutils.helpers import with_feature from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import assume_test_silo_mode, region_silo_test from sentry.testutils.skips import requires_kafka, requires_snuba @@ -757,3 +759,36 @@ def test_error_group_type(self) -> None: event=audit_log.get_event_id("DETECTOR_REMOVE"), actor=self.user, ).exists() + + @with_feature("organizations:anomaly-detection-alerts") + @mock.patch( + "sentry.workflow_engine.endpoints.organization_detector_details.delete_rule_in_seer" + ) + @mock.patch( + "sentry.workflow_engine.endpoints.organization_detector_details.schedule_update_project_config" + ) + def test_anomaly_detection( + self, mock_schedule_update_project_config, mock_seer_request + ) -> None: + self.detector.config = {"detection_type": AlertRuleDetectionType.DYNAMIC} + self.detector.save() + + mock_seer_request.return_value = True + + with outbox_runner(): + self.get_success_response(self.organization.slug, self.detector.id) + + assert RegionScheduledDeletion.objects.filter( + model_name="Detector", object_id=self.detector.id + ).exists() + with assume_test_silo_mode(SiloMode.CONTROL): + assert AuditLogEntry.objects.filter( + target_object=self.detector.id, + event=audit_log.get_event_id("DETECTOR_REMOVE"), + actor=self.user, + ).exists() + self.detector.refresh_from_db() + assert self.detector.status == ObjectStatus.PENDING_DELETION + mock_seer_request.assert_called_once_with( + source_id=self.data_source.id, organization=self.organization + ) From 4931e0b32271a5ac6c74b116e077b9523bce8956 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Fri, 31 Oct 2025 16:03:41 -0700 Subject: [PATCH 02/18] fix test --- src/sentry/incidents/grouptype.py | 9 +++++- .../seer/anomaly_detection/delete_rule.py | 30 +++++++++++++++++++ .../organization_detector_details.py | 23 +------------- .../test_organization_detector_details.py | 4 +-- 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/sentry/incidents/grouptype.py b/src/sentry/incidents/grouptype.py index 560099f4760afc..5bb56d0e1ffdd6 100644 --- a/src/sentry/incidents/grouptype.py +++ b/src/sentry/incidents/grouptype.py @@ -16,6 +16,7 @@ from sentry.issues.grouptype import GroupCategory, GroupType from sentry.models.organization import Organization from sentry.ratelimits.sliding_windows import Quota +from sentry.seer.anomaly_detection.delete_rule import delete_data_in_seer_for_detector from sentry.snuba.dataset import Dataset from sentry.snuba.metrics import format_mri_field, is_mri_field from sentry.snuba.models import QuerySubscription, SnubaQuery @@ -27,7 +28,12 @@ from sentry.workflow_engine.models.data_condition import Condition, DataCondition from sentry.workflow_engine.models.data_source import DataPacket from sentry.workflow_engine.processors.data_condition_group import ProcessedDataConditionGroup -from sentry.workflow_engine.types import DetectorException, DetectorPriorityLevel, DetectorSettings +from sentry.workflow_engine.types import ( + DetectorException, + DetectorLifeCycleHooks, + DetectorPriorityLevel, + DetectorSettings, +) logger = logging.getLogger(__name__) @@ -353,4 +359,5 @@ class MetricIssue(GroupType): }, }, }, + hooks=DetectorLifeCycleHooks(pending_delete=delete_data_in_seer_for_detector), ) diff --git a/src/sentry/seer/anomaly_detection/delete_rule.py b/src/sentry/seer/anomaly_detection/delete_rule.py index 9b1abbec5b4ffe..20eaa07a100b9c 100644 --- a/src/sentry/seer/anomaly_detection/delete_rule.py +++ b/src/sentry/seer/anomaly_detection/delete_rule.py @@ -20,6 +20,36 @@ 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 + + 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=data_source_detector.data_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: diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index 1b3bf77ac603ca..90e8e51899b44c 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -26,11 +26,9 @@ from sentry.grouping.grouptype import ErrorGroupType from sentry.incidents.grouptype import MetricIssue from sentry.incidents.metric_issue_detector import schedule_update_project_config -from sentry.incidents.models.alert_rule import AlertRuleDetectionType from sentry.issues import grouptype from sentry.models.organization import Organization from sentry.models.project import Project -from sentry.seer.anomaly_detection.delete_rule import delete_rule_in_seer from sentry.utils.audit import create_audit_entry from sentry.workflow_engine.endpoints.serializers.detector_serializer import DetectorSerializer from sentry.workflow_engine.endpoints.validators.detector_workflow import ( @@ -38,7 +36,7 @@ can_edit_detector, ) from sentry.workflow_engine.endpoints.validators.utils import get_unknown_detector_type_error -from sentry.workflow_engine.models import DataSourceDetector, Detector +from sentry.workflow_engine.models import Detector logger = logging.getLogger(__name__) @@ -213,25 +211,6 @@ def delete(self, request: Request, organization: Organization, detector: Detecto if detector.type == MetricIssue.slug: schedule_update_project_config(detector) - data_source_detector = DataSourceDetector.objects.filter( - detector_id=detector.id - ).first() - if not data_source_detector: - return Response(status=404) - - if detector.config.get("detection_type") == AlertRuleDetectionType.DYNAMIC: - success = delete_rule_in_seer( - source_id=data_source_detector.data_source_id, organization=organization - ) - # We don't need to surface this to the user, but we should know about it. - if not success: - logger.error( - "Call to delete rule data in Seer failed", - extra={ - "detector_id": detector.id, - }, - ) - RegionScheduledDeletion.schedule(detector, days=0, actor=request.user) detector.update(status=ObjectStatus.PENDING_DELETION) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 8eb53bc23883f5..8bf972eb1c6f25 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -761,9 +761,7 @@ def test_error_group_type(self) -> None: ).exists() @with_feature("organizations:anomaly-detection-alerts") - @mock.patch( - "sentry.workflow_engine.endpoints.organization_detector_details.delete_rule_in_seer" - ) + @mock.patch("sentry.seer.anomaly_detection.delete_rule.delete_rule_in_seer") @mock.patch( "sentry.workflow_engine.endpoints.organization_detector_details.schedule_update_project_config" ) From 2c8aecdf74a7b4d491a2490b6419adf122539204 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Fri, 31 Oct 2025 16:14:24 -0700 Subject: [PATCH 03/18] wrong ID --- src/sentry/seer/anomaly_detection/delete_rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/seer/anomaly_detection/delete_rule.py b/src/sentry/seer/anomaly_detection/delete_rule.py index 20eaa07a100b9c..8079efee343c74 100644 --- a/src/sentry/seer/anomaly_detection/delete_rule.py +++ b/src/sentry/seer/anomaly_detection/delete_rule.py @@ -41,7 +41,7 @@ def delete_data_in_seer_for_detector(detector: "Detector"): if detector.config.get("detection_type") == AlertRuleDetectionType.DYNAMIC: success = delete_rule_in_seer( - source_id=data_source_detector.data_source_id, organization=organization + source_id=int(data_source_detector.data_source.source_id), organization=organization ) if not success: logger.error( From b19adc45c1b6970037193693b3554f508aac10cd Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Mon, 3 Nov 2025 10:11:23 -0800 Subject: [PATCH 04/18] fixes --- src/sentry/seer/anomaly_detection/delete_rule.py | 6 ++++-- .../endpoints/organization_detector_details.py | 3 --- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/sentry/seer/anomaly_detection/delete_rule.py b/src/sentry/seer/anomaly_detection/delete_rule.py index 8079efee343c74..2a767810b478f9 100644 --- a/src/sentry/seer/anomaly_detection/delete_rule.py +++ b/src/sentry/seer/anomaly_detection/delete_rule.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import logging from typing import TYPE_CHECKING @@ -23,7 +25,7 @@ from sentry.workflow_engine.models.detector import Detector -def delete_data_in_seer_for_detector(detector: "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 @@ -117,7 +119,7 @@ def delete_rule_in_seer(source_id: int, organization: Organization) -> bool: return True -def delete_rule_in_seer_legacy(alert_rule: "AlertRule") -> bool: +def delete_rule_in_seer_legacy(alert_rule: AlertRule) -> bool: """ Send a request to delete an alert rule from Seer. Returns True if the request was successful. """ diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index 90e8e51899b44c..653fefe610a9e1 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -211,9 +211,6 @@ def delete(self, request: Request, organization: Organization, detector: Detecto if detector.type == MetricIssue.slug: schedule_update_project_config(detector) - RegionScheduledDeletion.schedule(detector, days=0, actor=request.user) - detector.update(status=ObjectStatus.PENDING_DELETION) - create_audit_entry( request=request, organization=detector.project.organization, From da765511b741d54b86148f37947d746308e5fc08 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Mon, 3 Nov 2025 11:43:43 -0800 Subject: [PATCH 05/18] correct ID --- .../endpoints/test_organization_detector_details.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 8bf972eb1c6f25..701c71ef9817a2 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -788,5 +788,5 @@ def test_anomaly_detection( self.detector.refresh_from_db() assert self.detector.status == ObjectStatus.PENDING_DELETION mock_seer_request.assert_called_once_with( - source_id=self.data_source.id, organization=self.organization + source_id=int(self.data_source.source_id), organization=self.organization ) From 3e2db73799dd1bd39ad68f9b9bbfc1e3d92819f1 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Mon, 3 Nov 2025 11:44:23 -0800 Subject: [PATCH 06/18] sure --- src/sentry/seer/anomaly_detection/delete_rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/seer/anomaly_detection/delete_rule.py b/src/sentry/seer/anomaly_detection/delete_rule.py index 2a767810b478f9..267dbc954f5720 100644 --- a/src/sentry/seer/anomaly_detection/delete_rule.py +++ b/src/sentry/seer/anomaly_detection/delete_rule.py @@ -80,7 +80,7 @@ def delete_rule_in_seer(source_id: int, organization: Organization) -> bool: ) return False - if response.status > 400: + if response.status >= 400: logger.error( "Error when hitting Seer delete rule data endpoint", extra={ From 8dc00b052025da73c4ac7359576b8b000278d713 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Wed, 5 Nov 2025 18:04:37 -0800 Subject: [PATCH 07/18] remove unneededed legacy code --- src/sentry/incidents/logic.py | 26 ++++---- src/sentry/incidents/models/alert_rule.py | 14 ---- .../seer/anomaly_detection/delete_rule.py | 65 ------------------- .../organization_detector_details.py | 4 -- 4 files changed, 11 insertions(+), 98 deletions(-) diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index 04532fcac8146a..b1f7a2c5b4be8c 100644 --- a/src/sentry/incidents/logic.py +++ b/src/sentry/incidents/logic.py @@ -65,7 +65,7 @@ ) from sentry.search.events.fields import is_function, resolve_field from sentry.search.events.types import SnubaParams -from sentry.seer.anomaly_detection.delete_rule import delete_rule_in_seer_legacy +from sentry.seer.anomaly_detection.delete_rule import delete_rule_in_seer from sentry.seer.anomaly_detection.store_data import send_new_rule_data, update_rule_data_legacy from sentry.sentry_apps.services.app import RpcSentryAppInstallation, app_service from sentry.shared_integrations.exceptions import ( @@ -923,8 +923,16 @@ def update_alert_rule( else: # if this was a dynamic rule, delete the data in Seer if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC: - success = delete_rule_in_seer_legacy( - alert_rule=alert_rule, + try: + source_id = QuerySubscription.objects.get(snuba_query_id=snuba_query.id).id + except QuerySubscription.DoesNotExist: + logger.exception( + "Snuba query missing query subscription", + extra={"snuba_query_id": snuba_query.id}, + ) + success = delete_rule_in_seer( + organization=alert_rule.organization, + source_id=source_id, ) if not success: logger.error( @@ -1090,18 +1098,6 @@ def delete_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_legacy( - 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/models/alert_rule.py b/src/sentry/incidents/models/alert_rule.py index 255e9a4f282671..67c44ab54b236d 100644 --- 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_legacy 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_legacy(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 index 267dbc954f5720..68422b8992a601 100644 --- a/src/sentry/seer/anomaly_detection/delete_rule.py +++ b/src/sentry/seer/anomaly_detection/delete_rule.py @@ -21,7 +21,6 @@ ) if TYPE_CHECKING: - from sentry.incidents.models.alert_rule import AlertRule from sentry.workflow_engine.models.detector import Detector @@ -117,67 +116,3 @@ def delete_rule_in_seer(source_id: int, organization: Organization) -> bool: return False return True - - -def delete_rule_in_seer_legacy(alert_rule: AlertRule) -> 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}, - ) - extra_data = { - "rule_id": alert_rule.id, - } - - try: - response = make_signed_seer_api_request( - connection_pool=seer_anomaly_detection_connection_pool, - path=SEER_ALERT_DELETION_URL, - body=json.dumps(body).encode("utf-8"), - ) - except (TimeoutError, MaxRetryError): - logger.warning( - "Timeout error when hitting Seer delete rule data endpoint", - extra=extra_data, - ) - return False - - if response.status > 400: - logger.error( - "Error when hitting Seer delete rule data endpoint", - extra={ - "response_data": response.data, - **extra_data, - }, - ) - return False - - try: - decoded_data = response.data.decode("utf-8") - except AttributeError: - logger.exception( - "Failed to parse Seer delete rule data response", - extra=extra_data, - ) - return False - - try: - results = json.loads(decoded_data) - except JSONDecodeError: - logger.exception( - "Failed to parse Seer delete rule data response", - extra=extra_data, - ) - return False - - status = results.get("success") - if status is None or status is not True: - logger.error( - "Request to delete alert rule from Seer was unsuccessful", - extra=extra_data, - ) - return False - - return True diff --git a/src/sentry/workflow_engine/endpoints/organization_detector_details.py b/src/sentry/workflow_engine/endpoints/organization_detector_details.py index 653fefe610a9e1..63f4a5e02b1162 100644 --- a/src/sentry/workflow_engine/endpoints/organization_detector_details.py +++ b/src/sentry/workflow_engine/endpoints/organization_detector_details.py @@ -1,5 +1,3 @@ -import logging - from django.db import router, transaction from drf_spectacular.utils import PolymorphicProxySerializer, extend_schema from rest_framework import status @@ -38,8 +36,6 @@ from sentry.workflow_engine.endpoints.validators.utils import get_unknown_detector_type_error from sentry.workflow_engine.models import Detector -logger = logging.getLogger(__name__) - def get_detector_validator( request: Request, project: Project, detector_type_slug: str, instance=None, partial=False From c26a2c267a8160393ffd90b540e885d07a1a0922 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Wed, 5 Nov 2025 18:07:17 -0800 Subject: [PATCH 08/18] bug --- src/sentry/incidents/logic.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index b1f7a2c5b4be8c..7d3663b37bdbb4 100644 --- a/src/sentry/incidents/logic.py +++ b/src/sentry/incidents/logic.py @@ -925,22 +925,22 @@ def update_alert_rule( if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC: 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={ + "rule_id": alert_rule.id, + }, + ) except QuerySubscription.DoesNotExist: logger.exception( "Snuba query missing query subscription", extra={"snuba_query_id": snuba_query.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={ - "rule_id": alert_rule.id, - }, - ) # 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) From 510a4aa01bfa621531e68f17d086c5d9d68bfb35 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Mon, 10 Nov 2025 14:28:57 -0800 Subject: [PATCH 09/18] post lifecyclehook world impl --- src/sentry/incidents/grouptype.py | 9 +------- src/sentry/incidents/logic.py | 22 +++++++++++++++++++ src/sentry/incidents/metric_issue_detector.py | 8 +++++++ 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/sentry/incidents/grouptype.py b/src/sentry/incidents/grouptype.py index 5bb56d0e1ffdd6..560099f4760afc 100644 --- a/src/sentry/incidents/grouptype.py +++ b/src/sentry/incidents/grouptype.py @@ -16,7 +16,6 @@ from sentry.issues.grouptype import GroupCategory, GroupType from sentry.models.organization import Organization from sentry.ratelimits.sliding_windows import Quota -from sentry.seer.anomaly_detection.delete_rule import delete_data_in_seer_for_detector from sentry.snuba.dataset import Dataset from sentry.snuba.metrics import format_mri_field, is_mri_field from sentry.snuba.models import QuerySubscription, SnubaQuery @@ -28,12 +27,7 @@ from sentry.workflow_engine.models.data_condition import Condition, DataCondition from sentry.workflow_engine.models.data_source import DataPacket from sentry.workflow_engine.processors.data_condition_group import ProcessedDataConditionGroup -from sentry.workflow_engine.types import ( - DetectorException, - DetectorLifeCycleHooks, - DetectorPriorityLevel, - DetectorSettings, -) +from sentry.workflow_engine.types import DetectorException, DetectorPriorityLevel, DetectorSettings logger = logging.getLogger(__name__) @@ -359,5 +353,4 @@ class MetricIssue(GroupType): }, }, }, - hooks=DetectorLifeCycleHooks(pending_delete=delete_data_in_seer_for_detector), ) diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index 7d3663b37bdbb4..5eb3bab67bfd47 100644 --- a/src/sentry/incidents/logic.py +++ b/src/sentry/incidents/logic.py @@ -1098,6 +1098,28 @@ def delete_alert_rule( incidents = Incident.objects.filter(alert_rule=alert_rule) if incidents.exists(): + if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC: + # if this was a dynamic rule, delete the data in Seer + try: + source_id = QuerySubscription.objects.get( + snuba_query_id=alert_rule.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={ + "rule_id": alert_rule.id, + }, + ) + except QuerySubscription.DoesNotExist: + logger.exception( + "Snuba query missing query subscription", + extra={"snuba_query_id": alert_rule.snuba_query.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 index 841ec889b8f6e0..ca0d4c0a2f2950 100644 --- 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 @@ -294,3 +295,10 @@ 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 + if self.instance.config.get("detection_type") == AlertRuleDetectionType.DYNAMIC: + delete_data_in_seer_for_detector(self.instance) + + super().delete() From da0082b51ebf19e7c0f53b62f3c7704cafef8400 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Mon, 10 Nov 2025 14:43:49 -0800 Subject: [PATCH 10/18] move imports, don't check for dynamic twice --- src/sentry/incidents/metric_issue_detector.py | 4 +--- src/sentry/seer/anomaly_detection/delete_rule.py | 5 ++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index ca0d4c0a2f2950..0f0ca7ae40de1e 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -298,7 +298,5 @@ def create(self, validated_data: dict[str, Any]): def delete(self): # Let Seer know we're deleting a dynamic detector so the data can be deleted there too - if self.instance.config.get("detection_type") == AlertRuleDetectionType.DYNAMIC: - delete_data_in_seer_for_detector(self.instance) - + delete_data_in_seer_for_detector(self.instance) super().delete() diff --git a/src/sentry/seer/anomaly_detection/delete_rule.py b/src/sentry/seer/anomaly_detection/delete_rule.py index 68422b8992a601..ae3c34a4c3dba4 100644 --- a/src/sentry/seer/anomaly_detection/delete_rule.py +++ b/src/sentry/seer/anomaly_detection/delete_rule.py @@ -7,12 +7,14 @@ from urllib3.exceptions import MaxRetryError, TimeoutError from sentry.conf.server import SEER_ALERT_DELETION_URL +from sentry.incidents.models.alert_rule import AlertRuleDetectionType from sentry.models.organization import Organization from sentry.net.http import connection_from_url 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 +from sentry.workflow_engine.models import DataSourceDetector logger = logging.getLogger(__name__) @@ -25,9 +27,6 @@ def delete_data_in_seer_for_detector(detector: Detector): - from sentry.incidents.models.alert_rule import AlertRuleDetectionType - from sentry.workflow_engine.models import DataSourceDetector - data_source_detector = DataSourceDetector.objects.filter(detector_id=detector.id).first() if not data_source_detector: logger.error( From 8f7c6916e0e114dc91c5a9a587b92b791570bd76 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Mon, 10 Nov 2025 14:53:40 -0800 Subject: [PATCH 11/18] appease typing --- src/sentry/incidents/metric_issue_detector.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 0f0ca7ae40de1e..0383b1ba97df36 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -298,5 +298,8 @@ def create(self, validated_data: dict[str, Any]): def delete(self): # Let Seer know we're deleting a dynamic detector so the data can be deleted there too - delete_data_in_seer_for_detector(self.instance) + assert self.instance is not None + detector: Detector = self.instance + delete_data_in_seer_for_detector(detector) + super().delete() From f78d262beba629789fd6aa016159d50c37e0becb Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Mon, 10 Nov 2025 16:32:11 -0800 Subject: [PATCH 12/18] update tests, put back imports to avoid circular, fix logic.py --- src/sentry/incidents/logic.py | 14 ++--- .../seer/anomaly_detection/delete_rule.py | 5 +- tests/sentry/incidents/test_logic.py | 53 ++++++++----------- 3 files changed, 33 insertions(+), 39 deletions(-) diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index 5eb3bab67bfd47..d6b00b3eebff72 100644 --- a/src/sentry/incidents/logic.py +++ b/src/sentry/incidents/logic.py @@ -1098,6 +1098,12 @@ def delete_alert_rule( incidents = Incident.objects.filter(alert_rule=alert_rule) if incidents.exists(): + AlertRuleActivity.objects.create( + alert_rule=alert_rule, + user_id=user.id if user else None, + type=AlertRuleActivityType.DELETED.value, + ) + else: if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC: # if this was a dynamic rule, delete the data in Seer try: @@ -1112,7 +1118,7 @@ def delete_alert_rule( logger.error( "Call to delete rule data in Seer failed", extra={ - "rule_id": alert_rule.id, + "source_id": source_id, }, ) except QuerySubscription.DoesNotExist: @@ -1120,12 +1126,6 @@ def delete_alert_rule( "Snuba query missing query subscription", extra={"snuba_query_id": alert_rule.snuba_query.id}, ) - AlertRuleActivity.objects.create( - alert_rule=alert_rule, - user_id=user.id if user else None, - type=AlertRuleActivityType.DELETED.value, - ) - else: RegionScheduledDeletion.schedule(instance=alert_rule, days=0, actor=user) bulk_delete_snuba_subscriptions(subscriptions) diff --git a/src/sentry/seer/anomaly_detection/delete_rule.py b/src/sentry/seer/anomaly_detection/delete_rule.py index ae3c34a4c3dba4..68422b8992a601 100644 --- a/src/sentry/seer/anomaly_detection/delete_rule.py +++ b/src/sentry/seer/anomaly_detection/delete_rule.py @@ -7,14 +7,12 @@ from urllib3.exceptions import MaxRetryError, TimeoutError from sentry.conf.server import SEER_ALERT_DELETION_URL -from sentry.incidents.models.alert_rule import AlertRuleDetectionType from sentry.models.organization import Organization from sentry.net.http import connection_from_url 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 -from sentry.workflow_engine.models import DataSourceDetector logger = logging.getLogger(__name__) @@ -27,6 +25,9 @@ def delete_data_in_seer_for_detector(detector: Detector): + from sentry.incidents.models.alert_rule import AlertRuleDetectionType + from sentry.workflow_engine.models import DataSourceDetector + data_source_detector = DataSourceDetector.objects.filter(detector_id=detector.id).first() if not data_source_detector: logger.error( diff --git a/tests/sentry/incidents/test_logic.py b/tests/sentry/incidents/test_logic.py index 5c13a6553b2436..188b36f8023ede 100644 --- a/tests/sentry/incidents/test_logic.py +++ b/tests/sentry/incidents/test_logic.py @@ -2045,13 +2045,13 @@ def test_delete_anomaly_detection_rule(self, mock_seer_request: MagicMock) -> No alert_rule = self.dynamic_alert_rule alert_rule_id = alert_rule.id + seer_return_value: StoreDataResponse = {"success": True} + mock_seer_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200) + with self.tasks(): delete_alert_rule(alert_rule) - assert not AlertRule.objects.filter(id=alert_rule_id).exists() assert AlertRule.objects_with_snapshots.filter(id=alert_rule_id).exists() - seer_return_value: StoreDataResponse = {"success": True} - mock_seer_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200) with self.tasks(): run_scheduled_deletions() @@ -2066,19 +2066,18 @@ def test_delete_anomaly_detection_rule(self, mock_seer_request: MagicMock) -> No "sentry.seer.anomaly_detection.delete_rule.seer_anomaly_detection_connection_pool.urlopen" ) @patch("sentry.seer.anomaly_detection.delete_rule.logger") - @patch("sentry.incidents.models.alert_rule.logger") + @patch("sentry.incidents.logic.logger") def test_delete_anomaly_detection_rule_timeout( self, mock_model_logger, mock_seer_logger, mock_seer_request ): alert_rule = self.dynamic_alert_rule alert_rule_id = alert_rule.id - - with self.tasks(): - delete_alert_rule(alert_rule) + query_sub = QuerySubscription.objects.get(snuba_query_id=alert_rule.snuba_query.id) mock_seer_request.side_effect = TimeoutError with self.tasks(): + delete_alert_rule(alert_rule) run_scheduled_deletions() assert not AlertRule.objects.filter(id=alert_rule_id).exists() @@ -2086,11 +2085,11 @@ def test_delete_anomaly_detection_rule_timeout( mock_seer_logger.warning.assert_called_with( "Timeout error when hitting Seer delete rule data endpoint", - extra={"rule_id": alert_rule_id}, + extra={"source_id": query_sub.id}, ) mock_model_logger.error.assert_called_with( "Call to delete rule data in Seer failed", - extra={"rule_id": alert_rule_id}, + extra={"source_id": query_sub.id}, ) assert mock_seer_request.call_count == 1 @@ -2099,19 +2098,17 @@ def test_delete_anomaly_detection_rule_timeout( "sentry.seer.anomaly_detection.delete_rule.seer_anomaly_detection_connection_pool.urlopen" ) @patch("sentry.seer.anomaly_detection.delete_rule.logger") - @patch("sentry.incidents.models.alert_rule.logger") + @patch("sentry.incidents.logic.logger") def test_delete_anomaly_detection_rule_error( self, mock_model_logger, mock_seer_logger, mock_seer_request ): alert_rule = self.dynamic_alert_rule alert_rule_id = alert_rule.id - - with self.tasks(): - delete_alert_rule(alert_rule) - + query_sub = QuerySubscription.objects.get(snuba_query_id=alert_rule.snuba_query.id) mock_seer_request.return_value = HTTPResponse("Bad request", status=500) with self.tasks(): + delete_alert_rule(alert_rule) run_scheduled_deletions() assert not AlertRule.objects.filter(id=alert_rule_id).exists() @@ -2119,11 +2116,11 @@ def test_delete_anomaly_detection_rule_error( mock_seer_logger.error.assert_called_with( "Error when hitting Seer delete rule data endpoint", - extra={"response_data": "Bad request", "rule_id": alert_rule_id}, + extra={"response_data": "Bad request", "source_id": query_sub.id}, ) mock_model_logger.error.assert_called_with( "Call to delete rule data in Seer failed", - extra={"rule_id": alert_rule_id}, + extra={"source_id": query_sub.id}, ) assert mock_seer_request.call_count == 1 @@ -2132,19 +2129,17 @@ def test_delete_anomaly_detection_rule_error( "sentry.seer.anomaly_detection.delete_rule.seer_anomaly_detection_connection_pool.urlopen" ) @patch("sentry.seer.anomaly_detection.delete_rule.logger") - @patch("sentry.incidents.models.alert_rule.logger") + @patch("sentry.incidents.logic.logger") def test_delete_anomaly_detection_rule_attribute_error( self, mock_model_logger, mock_seer_logger, mock_seer_request ): alert_rule = self.dynamic_alert_rule alert_rule_id = alert_rule.id - - with self.tasks(): - delete_alert_rule(alert_rule) - + query_sub = QuerySubscription.objects.get(snuba_query_id=alert_rule.snuba_query.id) mock_seer_request.return_value = HTTPResponse(None, status=200) # type:ignore[arg-type] with self.tasks(): + delete_alert_rule(alert_rule) run_scheduled_deletions() assert not AlertRule.objects.filter(id=alert_rule_id).exists() @@ -2152,11 +2147,11 @@ def test_delete_anomaly_detection_rule_attribute_error( mock_seer_logger.exception.assert_called_with( "Failed to parse Seer delete rule data response", - extra={"rule_id": alert_rule_id}, + extra={"source_id": query_sub.id}, ) mock_model_logger.error.assert_called_with( "Call to delete rule data in Seer failed", - extra={"rule_id": alert_rule_id}, + extra={"source_id": query_sub.id}, ) assert mock_seer_request.call_count == 1 @@ -2165,20 +2160,18 @@ def test_delete_anomaly_detection_rule_attribute_error( "sentry.seer.anomaly_detection.delete_rule.seer_anomaly_detection_connection_pool.urlopen" ) @patch("sentry.seer.anomaly_detection.delete_rule.logger") - @patch("sentry.incidents.models.alert_rule.logger") + @patch("sentry.incidents.logic.logger") def test_delete_anomaly_detection_rule_failure( self, mock_model_logger, mock_seer_logger, mock_seer_request ): alert_rule = self.dynamic_alert_rule alert_rule_id = alert_rule.id - - with self.tasks(): - delete_alert_rule(alert_rule) - + query_sub = QuerySubscription.objects.get(snuba_query_id=alert_rule.snuba_query.id) seer_return_value: StoreDataResponse = {"success": False} mock_seer_request.return_value = HTTPResponse(orjson.dumps(seer_return_value), status=200) with self.tasks(): + delete_alert_rule(alert_rule) run_scheduled_deletions() assert not AlertRule.objects.filter(id=alert_rule_id).exists() @@ -2186,11 +2179,11 @@ def test_delete_anomaly_detection_rule_failure( mock_seer_logger.error.assert_called_with( "Request to delete alert rule from Seer was unsuccessful", - extra={"rule_id": alert_rule_id}, + extra={"source_id": query_sub.id}, ) mock_model_logger.error.assert_called_with( "Call to delete rule data in Seer failed", - extra={"rule_id": alert_rule_id}, + extra={"source_id": query_sub.id}, ) assert mock_seer_request.call_count == 1 From 1b19f1dd7ac910ddfb86b91d54bf2e915b428dc0 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Mon, 10 Nov 2025 16:45:02 -0800 Subject: [PATCH 13/18] delete regardless of incidents --- src/sentry/incidents/logic.py | 45 ++++++++++++++-------------- tests/sentry/incidents/test_logic.py | 5 ++-- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index d6b00b3eebff72..466c8a496d3149 100644 --- a/src/sentry/incidents/logic.py +++ b/src/sentry/incidents/logic.py @@ -1096,6 +1096,29 @@ def delete_alert_rule( ) subscriptions = _unpack_snuba_query(alert_rule).subscriptions.all() + if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC: + # if this was a dynamic rule, delete the data in Seer + try: + source_id = QuerySubscription.objects.get( + snuba_query_id=alert_rule.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": alert_rule.snuba_query.id}, + ) + incidents = Incident.objects.filter(alert_rule=alert_rule) if incidents.exists(): AlertRuleActivity.objects.create( @@ -1104,28 +1127,6 @@ def delete_alert_rule( type=AlertRuleActivityType.DELETED.value, ) else: - if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC: - # if this was a dynamic rule, delete the data in Seer - try: - source_id = QuerySubscription.objects.get( - snuba_query_id=alert_rule.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": alert_rule.snuba_query.id}, - ) RegionScheduledDeletion.schedule(instance=alert_rule, days=0, actor=user) bulk_delete_snuba_subscriptions(subscriptions) diff --git a/tests/sentry/incidents/test_logic.py b/tests/sentry/incidents/test_logic.py index 188b36f8023ede..cacefd5f4c7adb 100644 --- a/tests/sentry/incidents/test_logic.py +++ b/tests/sentry/incidents/test_logic.py @@ -2008,6 +2008,7 @@ def test_with_incident_anomaly_detection_rule_error( alert_rule_id = alert_rule.id incident = self.create_incident() incident.update(alert_rule=alert_rule) + query_sub = QuerySubscription.objects.get(snuba_query_id=alert_rule.snuba_query.id) mock_seer_request.return_value = HTTPResponse("Bad request", status=500) with self.tasks(): @@ -2020,11 +2021,11 @@ def test_with_incident_anomaly_detection_rule_error( mock_seer_logger.error.assert_called_with( "Error when hitting Seer delete rule data endpoint", - extra={"response_data": "Bad request", "rule_id": alert_rule_id}, + extra={"response_data": "Bad request", "source_id": query_sub.id}, ) mock_model_logger.error.assert_called_with( "Call to delete rule data in Seer failed", - extra={"rule_id": alert_rule_id}, + extra={"source_id": query_sub.id}, ) assert mock_seer_request.call_count == 1 From 0592a425505609d8990f44654a9fe93757fbd64d Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Mon, 10 Nov 2025 17:00:14 -0800 Subject: [PATCH 14/18] dry up --- src/sentry/incidents/logic.py | 66 ++++++++++++++--------------------- 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index 466c8a496d3149..93338dc51ff04a 100644 --- 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,26 +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: - 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={ - "rule_id": alert_rule.id, - }, - ) - except QuerySubscription.DoesNotExist: - logger.exception( - "Snuba query missing query subscription", - extra={"snuba_query_id": snuba_query.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) @@ -1097,27 +1103,7 @@ def delete_alert_rule( subscriptions = _unpack_snuba_query(alert_rule).subscriptions.all() if alert_rule.detection_type == AlertRuleDetectionType.DYNAMIC: - # if this was a dynamic rule, delete the data in Seer - try: - source_id = QuerySubscription.objects.get( - snuba_query_id=alert_rule.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": alert_rule.snuba_query.id}, - ) + delete_anomaly_detection_rule(alert_rule.snuba_query, alert_rule) incidents = Incident.objects.filter(alert_rule=alert_rule) if incidents.exists(): From 1859b62212a28a4b19660675a068be6567831077 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Mon, 10 Nov 2025 17:11:07 -0800 Subject: [PATCH 15/18] typing --- .../endpoints/test_organization_detector_details.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 701c71ef9817a2..245a4034a6ea1c 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -766,7 +766,7 @@ def test_error_group_type(self) -> None: "sentry.workflow_engine.endpoints.organization_detector_details.schedule_update_project_config" ) def test_anomaly_detection( - self, mock_schedule_update_project_config, mock_seer_request + self, mock_schedule_update_project_config: mock.MagicMock, mock_seer_request: mock.MagicMock ) -> None: self.detector.config = {"detection_type": AlertRuleDetectionType.DYNAMIC} self.detector.save() From df56f013f2f7b337770d6c5ddc7d80094a5e5f0e Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Tue, 11 Nov 2025 10:22:15 -0800 Subject: [PATCH 16/18] update test --- tests/sentry/deletions/test_alert_rule.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/sentry/deletions/test_alert_rule.py b/tests/sentry/deletions/test_alert_rule.py index 14f4dfc5758f19..5089b8c5fd1ffe 100644 --- 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 From b52d7e0f90c0509260345aca5a869d6aaee37333 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Wed, 12 Nov 2025 15:09:43 -0800 Subject: [PATCH 17/18] include failure message in logs --- src/sentry/seer/anomaly_detection/delete_rule.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/sentry/seer/anomaly_detection/delete_rule.py b/src/sentry/seer/anomaly_detection/delete_rule.py index 68422b8992a601..1c9d23d794849b 100644 --- a/src/sentry/seer/anomaly_detection/delete_rule.py +++ b/src/sentry/seer/anomaly_detection/delete_rule.py @@ -108,7 +108,14 @@ def delete_rule_in_seer(source_id: int, organization: Organization) -> 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, From 360e1bb70ea54f5332d8c099db1e8ef131735f66 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Wed, 12 Nov 2025 15:37:21 -0800 Subject: [PATCH 18/18] update test --- tests/sentry/incidents/test_logic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/incidents/test_logic.py b/tests/sentry/incidents/test_logic.py index cacefd5f4c7adb..e2ade373fc06ce 100644 --- a/tests/sentry/incidents/test_logic.py +++ b/tests/sentry/incidents/test_logic.py @@ -2180,7 +2180,7 @@ def test_delete_anomaly_detection_rule_failure( mock_seer_logger.error.assert_called_with( "Request to delete alert rule from Seer was unsuccessful", - extra={"source_id": query_sub.id}, + extra={"source_id": query_sub.id, "message": None}, ) mock_model_logger.error.assert_called_with( "Call to delete rule data in Seer failed",