From ff56003bf104426841223f2b21658d87b65941f1 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Tue, 19 May 2026 14:31:43 -0700 Subject: [PATCH] chore(alerts): Remove unused metric alert handlers --- pyproject.toml | 1 - src/sentry/incidents/models/alert_rule.py | 57 +---------- src/sentry/integrations/discord/spec.py | 35 ------- src/sentry/integrations/messaging/spec.py | 98 ------------------- src/sentry/integrations/msteams/spec.py | 33 ------- src/sentry/integrations/slack/spec.py | 33 ------- src/sentry/testutils/helpers/alert_rule.py | 28 ------ .../incidents/models/test_alert_rule.py | 33 ------- 8 files changed, 5 insertions(+), 313 deletions(-) delete mode 100644 src/sentry/testutils/helpers/alert_rule.py diff --git a/pyproject.toml b/pyproject.toml index 7aee684d2753..3205f9543c1b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1505,7 +1505,6 @@ module = [ "sentry.testutils.factories", "sentry.testutils.fixtures", "sentry.testutils.helpers", - "sentry.testutils.helpers.alert_rule", "sentry.testutils.helpers.analytics", "sentry.testutils.helpers.apigateway", "sentry.testutils.helpers.auth_header", diff --git a/src/sentry/incidents/models/alert_rule.py b/src/sentry/incidents/models/alert_rule.py index 6e497ed1a32d..8731713204b6 100644 --- a/src/sentry/incidents/models/alert_rule.py +++ b/src/sentry/incidents/models/alert_rule.py @@ -1,6 +1,5 @@ from __future__ import annotations -import abc import logging from collections.abc import Callable, Collection, Iterable from enum import Enum, IntEnum, StrEnum @@ -37,7 +36,6 @@ ) from sentry.snuba.models import QuerySubscription from sentry.types.actor import Actor -from sentry.utils import metrics if TYPE_CHECKING: from sentry.incidents.action_handlers import ActionHandler @@ -328,12 +326,8 @@ def get_queryset(self) -> BaseQuerySet[AlertRuleTriggerAction]: return super().get_queryset().exclude(status=ObjectStatus.PENDING_DELETION) -class ActionHandlerFactory(abc.ABC): - """A factory for action handlers tied to a specific incident service. - - The factory's builder method is augmented with metadata about which service it is - for and which target types that service supports. - """ +class ActionHandlerFactory: + """Metadata for an incident action service: slug, supported target types, and integration provider.""" def __init__( self, @@ -347,32 +341,6 @@ def __init__( self.supported_target_types = frozenset(supported_target_types) self.integration_provider = integration_provider - @abc.abstractmethod - def build_handler(self) -> ActionHandler: - raise NotImplementedError - - -class _AlertRuleActionHandlerClassFactory(ActionHandlerFactory): - """A factory derived from a concrete ActionHandler class. - - The factory builds a handler simply by instantiating the provided class. The - `AlertRuleTriggerAction.register_type` decorator provides the rest of the metadata. - """ - - def __init__( - self, - slug: str, - service_type: ActionService, - supported_target_types: Iterable[ActionTarget], - integration_provider: str | None, - trigger_action_class: type[ActionHandler], - ) -> None: - super().__init__(slug, service_type, supported_target_types, integration_provider) - self.trigger_action_class = trigger_action_class - - def build_handler(self) -> ActionHandler: - return self.trigger_action_class() - class _FactoryRegistry: def __init__(self) -> None: @@ -403,8 +371,6 @@ class AlertRuleTriggerAction(AbstractNotificationAction): Type = ActionService TargetType = ActionTarget - # As a test utility, TemporaryAlertRuleTriggerActionRegistry has privileged - # access to this otherwise private class variable _factory_registrations = _FactoryRegistry() INTEGRATION_TYPES = frozenset( @@ -467,15 +433,6 @@ def target(self) -> OrganizationMember | Team | str | None: return self.target_identifier return None - @staticmethod - def build_handler(type: ActionService) -> ActionHandler | None: - factory = AlertRuleTriggerAction._factory_registrations.by_action_service.get(type) - if factory is not None: - return factory.build_handler() - else: - metrics.incr(f"alert_rule_trigger.unhandled_type.{type}") - return None - def get_single_sentry_app_config(self) -> dict[str, Any] | None: value = self.sentry_app_config if isinstance(value, list): @@ -495,7 +452,7 @@ def register_type( integration_provider: str | None = None, ) -> Callable[[type[ActionHandler]], type[ActionHandler]]: """ - Register a factory for the decorated ActionHandler class, for a given service type. + Register a metadata factory for the decorated ActionHandler class, for a given service type. :param slug: A string representing the name of this type registration :param service_type: The action service type the decorated handler supports. @@ -505,12 +462,8 @@ def register_type( """ def inner(handler: type[ActionHandler]) -> type[ActionHandler]: - """ - :param handler: A subclass of `ActionHandler` that accepts the - `AlertRuleActionHandler` and `Incident`. - """ - factory = _AlertRuleActionHandlerClassFactory( - slug, service_type, supported_target_types, integration_provider, handler + factory = ActionHandlerFactory( + slug, service_type, supported_target_types, integration_provider ) cls.register_factory(factory) return handler diff --git a/src/sentry/integrations/discord/spec.py b/src/sentry/integrations/discord/spec.py index 82a88c15194b..144f64f016b2 100644 --- a/src/sentry/integrations/discord/spec.py +++ b/src/sentry/integrations/discord/spec.py @@ -1,19 +1,10 @@ from sentry import analytics -from sentry.incidents.endpoints.serializers.alert_rule import AlertRuleSerializerResponse -from sentry.incidents.endpoints.serializers.incident import DetailedIncidentSerializerResponse -from sentry.incidents.typings.metric_detector import ( - AlertContext, - MetricIssueContext, - NotificationContext, - OpenPeriodContext, -) from sentry.integrations.base import IntegrationProvider from sentry.integrations.messaging.spec import ( MessagingIdentityLinkViewSet, MessagingIntegrationSpec, ) from sentry.integrations.types import IntegrationProviderSlug -from sentry.models.organization import Organization from sentry.notifications.models.notificationaction import ActionService from sentry.rules.actions import IntegrationEventAction @@ -43,32 +34,6 @@ def identity_view_set(self) -> MessagingIdentityLinkViewSet: unlink_personal_identity=DiscordUnlinkIdentityView, ) - def send_incident_alert_notification( - self, - organization: Organization, - alert_context: AlertContext, - notification_context: NotificationContext, - metric_issue_context: MetricIssueContext, - open_period_context: OpenPeriodContext, - alert_rule_serialized_response: AlertRuleSerializerResponse, - incident_serialized_response: DetailedIncidentSerializerResponse, - notification_uuid: str | None = None, - ) -> bool: - from sentry.integrations.discord.actions.metric_alert import ( - send_incident_alert_notification, - ) - - return send_incident_alert_notification( - organization=organization, - alert_context=alert_context, - notification_context=notification_context, - metric_issue_context=metric_issue_context, - open_period_context=open_period_context, - alert_rule_serialized_response=alert_rule_serialized_response, - incident_serialized_response=incident_serialized_response, - notification_uuid=notification_uuid, - ) - @property def notify_service_action(self) -> type[IntegrationEventAction] | None: from sentry.integrations.discord.actions.issue_alert.notification import ( diff --git a/src/sentry/integrations/messaging/spec.py b/src/sentry/integrations/messaging/spec.py index 25848b097f6a..e800a3a7ab16 100644 --- a/src/sentry/integrations/messaging/spec.py +++ b/src/sentry/integrations/messaging/spec.py @@ -1,4 +1,3 @@ -import logging from abc import ABC, abstractmethod from dataclasses import dataclass @@ -7,34 +6,12 @@ from django.views.generic import View from sentry import analytics -from sentry.api.serializers import serialize -from sentry.incidents.action_handlers import ActionHandler, DefaultActionHandler -from sentry.incidents.endpoints.serializers.alert_rule import ( - AlertRuleSerializer, - AlertRuleSerializerResponse, -) -from sentry.incidents.endpoints.serializers.incident import ( - DetailedIncidentSerializer, - DetailedIncidentSerializerResponse, -) from sentry.incidents.models.alert_rule import ActionHandlerFactory, AlertRuleTriggerAction -from sentry.incidents.models.incident import Incident, IncidentStatus -from sentry.incidents.typings.metric_detector import ( - AlertContext, - MetricIssueContext, - NotificationContext, - OpenPeriodContext, -) from sentry.integrations.base import IntegrationProvider -from sentry.integrations.metric_alerts import get_metric_count_from_incident -from sentry.models.organization import Organization -from sentry.models.project import Project from sentry.notifications.models.notificationaction import ActionService, ActionTarget from sentry.rules import rules from sentry.rules.actions import IntegrationEventAction -logger = logging.getLogger("sentry.integrations.messaging.spec") - @dataclass(frozen=True) class MessagingIdentityLinkViewSet: @@ -145,20 +122,6 @@ def build_path(operation_slug: str, view_cls: type[View]) -> URLPattern: if view_cls is not None ] - @abstractmethod - def send_incident_alert_notification( - self, - organization: Organization, - alert_context: AlertContext, - notification_context: NotificationContext, - metric_issue_context: MetricIssueContext, - open_period_context: OpenPeriodContext, - alert_rule_serialized_response: AlertRuleSerializerResponse, - incident_serialized_response: DetailedIncidentSerializerResponse, - notification_uuid: str | None = None, - ) -> bool: - raise NotImplementedError - @property @abstractmethod def notify_service_action(self) -> type[IntegrationEventAction] | None: @@ -180,63 +143,6 @@ def notification_sent(self) -> type[analytics.Event] | None: raise NotImplementedError -class MessagingActionHandler(DefaultActionHandler): - def __init__(self, spec: MessagingIntegrationSpec): - super().__init__() - self._spec = spec - - @property - def provider(self) -> str: - return self._spec.provider_slug - - def send_alert( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ) -> None: - if metric_value is None: - metric_value = get_metric_count_from_incident(incident) - - alert_rule_serialized_response: AlertRuleSerializerResponse = serialize( - incident.alert_rule, None, AlertRuleSerializer() - ) - incident_serialized_response: DetailedIncidentSerializerResponse = serialize( - incident, None, DetailedIncidentSerializer() - ) - open_period_context = OpenPeriodContext.from_incident(incident=incident) - - notification_context = NotificationContext.from_alert_rule_trigger_action(action) - alert_context = AlertContext.from_alert_rule_incident(incident.alert_rule) - metric_issue_context = MetricIssueContext.from_legacy_models( - incident=incident, - new_status=new_status, - metric_value=metric_value, - ) - - success = self._spec.send_incident_alert_notification( - organization=incident.organization, - alert_context=alert_context, - notification_context=notification_context, - metric_issue_context=metric_issue_context, - open_period_context=open_period_context, - alert_rule_serialized_response=alert_rule_serialized_response, - incident_serialized_response=incident_serialized_response, - notification_uuid=notification_uuid, - ) - if success: - self.record_alert_sent_analytics( - organization_id=incident.organization.id, - project_id=project.id, - alert_id=incident.alert_rule.id, - external_id=action.target_identifier, - notification_uuid=notification_uuid, - ) - - class _MessagingHandlerFactory(ActionHandlerFactory): def __init__(self, spec: MessagingIntegrationSpec) -> None: super().__init__( @@ -245,7 +151,3 @@ def __init__(self, spec: MessagingIntegrationSpec) -> None: supported_target_types=[ActionTarget.SPECIFIC], integration_provider=spec.provider_slug, ) - self.spec = spec - - def build_handler(self) -> ActionHandler: - return MessagingActionHandler(self.spec) diff --git a/src/sentry/integrations/msteams/spec.py b/src/sentry/integrations/msteams/spec.py index f416c8d94232..fd1d2ef72801 100644 --- a/src/sentry/integrations/msteams/spec.py +++ b/src/sentry/integrations/msteams/spec.py @@ -1,19 +1,10 @@ from sentry import analytics -from sentry.incidents.endpoints.serializers.alert_rule import AlertRuleSerializerResponse -from sentry.incidents.endpoints.serializers.incident import DetailedIncidentSerializerResponse -from sentry.incidents.typings.metric_detector import ( - AlertContext, - MetricIssueContext, - NotificationContext, - OpenPeriodContext, -) from sentry.integrations.base import IntegrationProvider from sentry.integrations.messaging.spec import ( MessagingIdentityLinkViewSet, MessagingIntegrationSpec, ) from sentry.integrations.types import IntegrationProviderSlug -from sentry.models.organization import Organization from sentry.notifications.models.notificationaction import ActionService from sentry.rules.actions import IntegrationEventAction @@ -43,30 +34,6 @@ def identity_view_set(self) -> MessagingIdentityLinkViewSet: unlink_personal_identity=MsTeamsUnlinkIdentityView, ) - def send_incident_alert_notification( - self, - organization: Organization, - alert_context: AlertContext, - notification_context: NotificationContext, - metric_issue_context: MetricIssueContext, - open_period_context: OpenPeriodContext, - alert_rule_serialized_response: AlertRuleSerializerResponse, - incident_serialized_response: DetailedIncidentSerializerResponse, - notification_uuid: str | None = None, - ) -> bool: - from sentry.integrations.msteams.utils import send_incident_alert_notification - - return send_incident_alert_notification( - organization=organization, - alert_context=alert_context, - notification_context=notification_context, - metric_issue_context=metric_issue_context, - open_period_context=open_period_context, - alert_rule_serialized_response=alert_rule_serialized_response, - incident_serialized_response=incident_serialized_response, - notification_uuid=notification_uuid, - ) - @property def notify_service_action(self) -> type[IntegrationEventAction] | None: from sentry.integrations.msteams.actions.notification import MsTeamsNotifyServiceAction diff --git a/src/sentry/integrations/slack/spec.py b/src/sentry/integrations/slack/spec.py index b2b5c887a0d5..743a84f67fa6 100644 --- a/src/sentry/integrations/slack/spec.py +++ b/src/sentry/integrations/slack/spec.py @@ -1,19 +1,10 @@ from sentry import analytics -from sentry.incidents.endpoints.serializers.alert_rule import AlertRuleSerializerResponse -from sentry.incidents.endpoints.serializers.incident import DetailedIncidentSerializerResponse -from sentry.incidents.typings.metric_detector import ( - AlertContext, - MetricIssueContext, - NotificationContext, - OpenPeriodContext, -) from sentry.integrations.base import IntegrationProvider from sentry.integrations.messaging.spec import ( MessagingIdentityLinkViewSet, MessagingIntegrationSpec, ) from sentry.integrations.types import IntegrationProviderSlug -from sentry.models.organization import Organization from sentry.notifications.models.notificationaction import ActionService from sentry.rules.actions import IntegrationEventAction @@ -47,30 +38,6 @@ def identity_view_set(self) -> MessagingIdentityLinkViewSet: unlink_team_identity=SlackUnlinkTeamView, ) - def send_incident_alert_notification( - self, - organization: Organization, - alert_context: AlertContext, - notification_context: NotificationContext, - metric_issue_context: MetricIssueContext, - open_period_context: OpenPeriodContext, - alert_rule_serialized_response: AlertRuleSerializerResponse, - incident_serialized_response: DetailedIncidentSerializerResponse, - notification_uuid: str | None = None, - ) -> bool: - from sentry.integrations.slack.utils.notifications import send_incident_alert_notification - - return send_incident_alert_notification( - organization=organization, - alert_context=alert_context, - notification_context=notification_context, - metric_issue_context=metric_issue_context, - open_period_context=open_period_context, - alert_rule_serialized_response=alert_rule_serialized_response, - incident_serialized_response=incident_serialized_response, - notification_uuid=notification_uuid, - ) - @property def notify_service_action(self) -> type[IntegrationEventAction] | None: from sentry.integrations.slack.actions.notification import SlackNotifyServiceAction diff --git a/src/sentry/testutils/helpers/alert_rule.py b/src/sentry/testutils/helpers/alert_rule.py deleted file mode 100644 index 3ba4ce3bb3fa..000000000000 --- a/src/sentry/testutils/helpers/alert_rule.py +++ /dev/null @@ -1,28 +0,0 @@ -from collections.abc import Generator -from contextlib import contextmanager -from dataclasses import dataclass - -from sentry.incidents.models.alert_rule import AlertRuleTriggerAction, _FactoryRegistry - - -@dataclass(frozen=True) -class TemporaryAlertRuleTriggerActionRegistry: - _suspended_values: _FactoryRegistry - - @classmethod - def suspend(cls) -> "TemporaryAlertRuleTriggerActionRegistry": - obj = cls(AlertRuleTriggerAction._factory_registrations) - AlertRuleTriggerAction._factory_registrations = _FactoryRegistry() - return obj - - def restore(self) -> None: - AlertRuleTriggerAction._factory_registrations = self._suspended_values - - @classmethod - @contextmanager - def registry_patched(cls) -> Generator[None]: - suspended = cls.suspend() - try: - yield - finally: - suspended.restore() diff --git a/tests/sentry/incidents/models/test_alert_rule.py b/tests/sentry/incidents/models/test_alert_rule.py index 473621bb7794..073ff5cb9a3c 100644 --- a/tests/sentry/incidents/models/test_alert_rule.py +++ b/tests/sentry/incidents/models/test_alert_rule.py @@ -1,7 +1,3 @@ -from collections.abc import Generator -from unittest import mock -from unittest.mock import Mock - import pytest from django.core.cache import cache @@ -15,7 +11,6 @@ AlertRuleTriggerAction, ) from sentry.testutils.cases import TestCase -from sentry.testutils.helpers.alert_rule import TemporaryAlertRuleTriggerActionRegistry class IncidentGetForSubscriptionTest(TestCase): @@ -235,34 +230,6 @@ def test_specific(self) -> None: assert trigger.target == email -class AlertRuleTriggerActionActivateTest(TestCase): - @pytest.fixture(autouse=True) - def _setup_metric_patch(self) -> Generator[None]: - with mock.patch("sentry.incidents.models.alert_rule.metrics") as self.metrics: - yield - - def setUp(self) -> None: - self.suspended_registry = TemporaryAlertRuleTriggerActionRegistry.suspend() - - def tearDown(self) -> None: - self.suspended_registry.restore() - - def test_unhandled(self) -> None: - trigger = AlertRuleTriggerAction(type=AlertRuleTriggerAction.Type.EMAIL.value) - trigger.build_handler(type=AlertRuleTriggerAction.Type(trigger.type)) - self.metrics.incr.assert_called_once_with("alert_rule_trigger.unhandled_type.0") - - def test_handled(self) -> None: - mock_handler = Mock() - type = AlertRuleTriggerAction.Type.EMAIL - AlertRuleTriggerAction.register_type("something", type, [])(mock_handler) - - trigger = AlertRuleTriggerAction(type=AlertRuleTriggerAction.Type.EMAIL.value) - trigger.build_handler(type=AlertRuleTriggerAction.Type(trigger.type)) - mock_handler.assert_called_once_with() - assert not self.metrics.incr.called - - class AlertRuleActivityTest(TestCase): def test_simple(self) -> None: assert AlertRuleActivity.objects.all().count() == 0