diff --git a/src/sentry/incidents/action_handlers.py b/src/sentry/incidents/action_handlers.py index 6c15ada1a251..8fb78f4f32ff 100644 --- a/src/sentry/incidents/action_handlers.py +++ b/src/sentry/incidents/action_handlers.py @@ -2,7 +2,6 @@ import abc import logging -from collections.abc import Sequence from typing import Any from urllib.parse import urlencode @@ -14,54 +13,33 @@ from sentry import analytics, features from sentry.analytics.events.alert_sent import AlertSentEvent -from sentry.api.serializers import serialize from sentry.charts.types import ChartSize from sentry.constants import CRASH_RATE_ALERT_AGGREGATE_ALIAS from sentry.incidents.charts import build_metric_alert_chart -from sentry.incidents.endpoints.serializers.alert_rule import ( - AlertRuleSerializer, - AlertRuleSerializerResponse, -) -from sentry.incidents.endpoints.serializers.incident import ( - DetailedIncidentSerializer, - DetailedIncidentSerializerResponse, - IncidentSerializer, -) +from sentry.incidents.endpoints.serializers.alert_rule import AlertRuleSerializerResponse +from sentry.incidents.endpoints.serializers.incident import DetailedIncidentSerializerResponse from sentry.incidents.endpoints.serializers.utils import get_fake_id_from_object_id from sentry.incidents.models.alert_rule import ( AlertRuleDetectionType, AlertRuleThresholdType, AlertRuleTriggerAction, ) -from sentry.incidents.models.incident import ( - INCIDENT_STATUS, - Incident, - IncidentStatus, - TriggerStatus, -) +from sentry.incidents.models.incident import INCIDENT_STATUS, IncidentStatus, TriggerStatus from sentry.incidents.typings.metric_detector import ( AlertContext, MetricIssueContext, - NotificationContext, OpenPeriodContext, ) -from sentry.integrations.metric_alerts import get_metric_count_from_incident -from sentry.integrations.types import ExternalProviders, IntegrationProviderSlug +from sentry.integrations.types import IntegrationProviderSlug from sentry.models.organization import Organization -from sentry.models.organizationmember import OrganizationMember from sentry.models.project import Project -from sentry.models.rulesnooze import RuleSnooze -from sentry.models.team import Team -from sentry.notifications.types import NotificationSettingEnum -from sentry.notifications.utils.participants import get_notification_recipients from sentry.snuba.metrics import format_mri_field, is_mri_field from sentry.snuba.utils import build_query_strings -from sentry.types.actor import Actor, ActorType from sentry.users.models.user import User from sentry.users.services.user import RpcUser from sentry.users.services.user.service import user_service from sentry.users.services.user_option import RpcUserOption, user_option_service -from sentry.utils.email import MessageBuilder, get_email_addresses +from sentry.utils.email import MessageBuilder from sentry.workflow_engine.endpoints.serializers.detector_serializer import ( DetectorSerializerResponse, ) @@ -76,30 +54,6 @@ class ActionHandler(metaclass=abc.ABCMeta): def provider(self) -> str: raise NotImplementedError - @abc.abstractmethod - def fire( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ) -> None: - pass - - @abc.abstractmethod - def resolve( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ) -> None: - pass - def record_alert_sent_analytics( self, organization_id: int, @@ -125,55 +79,7 @@ def record_alert_sent_analytics( class DefaultActionHandler(ActionHandler): - def fire( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ) -> None: - if not RuleSnooze.objects.is_snoozed_for_all(alert_rule=incident.alert_rule): - self.send_alert( - action=action, - incident=incident, - project=project, - metric_value=metric_value, - new_status=new_status, - notification_uuid=notification_uuid, - ) - - def resolve( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ) -> None: - if not RuleSnooze.objects.is_snoozed_for_all(alert_rule=incident.alert_rule): - self.send_alert( - action=action, - incident=incident, - project=project, - metric_value=metric_value, - new_status=new_status, - notification_uuid=notification_uuid, - ) - - @abc.abstractmethod - def send_alert( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ) -> None: - pass + pass @AlertRuleTriggerAction.register_type( @@ -186,146 +92,6 @@ class EmailActionHandler(ActionHandler): def provider(self) -> str: return "email" - def _get_targets( - self, action: AlertRuleTriggerAction, incident: Incident, project: Project - ) -> set[int]: - target = action.target - if not target: - return set() - - if RuleSnooze.objects.is_snoozed_for_all(alert_rule=incident.alert_rule): - return set() - - if action.target_type == AlertRuleTriggerAction.TargetType.USER.value: - assert isinstance(target, OrganizationMember) - if RuleSnooze.objects.is_snoozed_for_user( - user_id=target.user_id, alert_rule=incident.alert_rule - ): - return set() - - if target.user_id: - return {target.user_id} - return set() - - elif action.target_type == AlertRuleTriggerAction.TargetType.TEAM.value: - assert isinstance(target, Team) - out = get_notification_recipients( - recipients=list( - Actor(id=member.user_id, actor_type=ActorType.USER) - for member in target.member_set - ), - type=NotificationSettingEnum.ISSUE_ALERTS, - organization_id=incident.organization_id, - project_ids=[project.id], - actor_type=ActorType.USER, - ) - users = out[ExternalProviders.EMAIL] - - snoozed_users = RuleSnooze.objects.filter( - alert_rule=incident.alert_rule, user_id__in=[user.id for user in users] - ).values_list("user_id", flat=True) - return {user.id for user in users if user.id not in snoozed_users} - - return set() - - def get_targets( - self, action: AlertRuleTriggerAction, incident: Incident, project: Project - ) -> Sequence[tuple[int, str]]: - return list( - get_email_addresses( - self._get_targets(action, incident, project), project=project - ).items() - ) - - def fire( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ) -> None: - self._email_users( - action=action, - incident=incident, - project=project, - metric_value=metric_value, - new_status=new_status, - trigger_status=TriggerStatus.ACTIVE, - notification_uuid=notification_uuid, - ) - - def resolve( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ) -> None: - self._email_users( - action=action, - incident=incident, - project=project, - metric_value=metric_value, - new_status=new_status, - trigger_status=TriggerStatus.RESOLVED, - notification_uuid=notification_uuid, - ) - - def _email_users( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - trigger_status: TriggerStatus, - notification_uuid: str | None = None, - ): - alert_rule_serialized_response: AlertRuleSerializerResponse = serialize( - incident.alert_rule, None, AlertRuleSerializer() - ) - incident_serialized_response: DetailedIncidentSerializerResponse = serialize( - incident, None, DetailedIncidentSerializer() - ) - - metric_issue_context = MetricIssueContext.from_legacy_models( - incident=incident, - new_status=new_status, - metric_value=metric_value, - ) - open_period_context = OpenPeriodContext.from_incident(incident) - alert_context = AlertContext.from_alert_rule_incident( - incident.alert_rule, alert_rule_threshold=action.alert_rule_trigger.alert_threshold - ) - targets = [ - (user_id, email) for user_id, email in self.get_targets(action, incident, project) - ] - - users_sent_to = email_users( - metric_issue_context=metric_issue_context, - open_period_context=open_period_context, - alert_context=alert_context, - alert_rule_serialized_response=alert_rule_serialized_response, - incident_serialized_response=incident_serialized_response, - trigger_status=trigger_status, - targets=targets, - project=project, - notification_uuid=notification_uuid, - ) - - for user_id in users_sent_to: - self.record_alert_sent_analytics( - organization_id=incident.organization.id, - project_id=project.id, - alert_id=incident.alert_rule.id, - external_id=user_id, - notification_uuid=notification_uuid, - ) - @AlertRuleTriggerAction.register_type( IntegrationProviderSlug.PAGERDUTY.value, @@ -338,44 +104,6 @@ class PagerDutyActionHandler(DefaultActionHandler): def provider(self) -> str: return IntegrationProviderSlug.PAGERDUTY.value - def send_alert( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ): - from sentry.integrations.pagerduty.utils import send_incident_alert_notification - - if metric_value is None: - metric_value = get_metric_count_from_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 = send_incident_alert_notification( - notification_context=notification_context, - alert_context=alert_context, - metric_issue_context=metric_issue_context, - organization=incident.organization, - 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, - ) - @AlertRuleTriggerAction.register_type( "opsgenie", @@ -388,44 +116,6 @@ class OpsgenieActionHandler(DefaultActionHandler): def provider(self) -> str: return "opsgenie" - def send_alert( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ): - from sentry.integrations.opsgenie.utils import send_incident_alert_notification - - if metric_value is None: - metric_value = get_metric_count_from_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 = send_incident_alert_notification( - notification_context=notification_context, - alert_context=alert_context, - metric_issue_context=metric_issue_context, - organization=incident.organization, - 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, - ) - @AlertRuleTriggerAction.register_type( "sentry_app", @@ -437,40 +127,6 @@ class SentryAppActionHandler(DefaultActionHandler): def provider(self) -> str: return "sentry_app" - def send_alert( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ): - from sentry.rules.actions.notify_event_service import send_incident_alert_notification - - if metric_value is None: - metric_value = get_metric_count_from_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, - ) - - incident_serialized_response = serialize(incident, serializer=IncidentSerializer()) - - send_incident_alert_notification( - notification_context=notification_context, - alert_context=alert_context, - metric_issue_context=metric_issue_context, - incident_serialized_response=incident_serialized_response, - organization=incident.organization, - project_id=project.id, - notification_uuid=notification_uuid, - ) - def format_duration(minutes): """ diff --git a/src/sentry/incidents/logic.py b/src/sentry/incidents/logic.py index 9f52f777f2da..8aa604cc6f47 100644 --- a/src/sentry/incidents/logic.py +++ b/src/sentry/incidents/logic.py @@ -48,9 +48,7 @@ IncidentProject, IncidentStatus, IncidentStatusMethod, - IncidentTrigger, IncidentType, - TriggerStatus, ) from sentry.incidents.utils.constants import INCIDENTS_SNUBA_SUBSCRIPTION_TYPE from sentry.integrations.services.integration import RpcIntegration, integration_service @@ -245,12 +243,6 @@ def update_incident_status( except Exception as e: sentry_sdk.capture_exception(e) - if status == IncidentStatus.CLOSED and ( - status_method == IncidentStatusMethod.MANUAL - or status_method == IncidentStatusMethod.RULE_UPDATED - ): - _trigger_incident_triggers(incident) - return incident @@ -473,10 +465,6 @@ def get_metric_issue_aggregates( return aggregated_result[0] -def get_incident_activity(incident: Incident) -> Iterable[IncidentActivity]: - return IncidentActivity.objects.filter(incident=incident).select_related("incident") - - class AlertRuleNameAlreadyUsedError(Exception): pass @@ -1246,43 +1234,6 @@ def get_triggers_for_alert_rule(alert_rule: AlertRule) -> QuerySet[AlertRuleTrig return AlertRuleTrigger.objects.filter(alert_rule=alert_rule) -def _trigger_incident_triggers(incident: Incident) -> None: - incident_triggers = IncidentTrigger.objects.filter(incident=incident) - triggers = get_triggers_for_alert_rule(incident.alert_rule) - actions = deduplicate_trigger_actions(triggers=list(triggers)) - with transaction.atomic(router.db_for_write(AlertRuleTrigger)): - for trigger in incident_triggers: - trigger.status = TriggerStatus.RESOLVED.value - trigger.save() - - for action in actions: - for project in incident.projects.all(): - _schedule_trigger_action( - action_id=action.id, - incident_id=incident.id, - project_id=project.id, - method="resolve", - new_status=IncidentStatus.CLOSED.value, - ) - - -def _schedule_trigger_action( - action_id: int, incident_id: int, project_id: int, method: str, new_status: int -) -> None: - from sentry.incidents.tasks import handle_trigger_action - - transaction.on_commit( - lambda: handle_trigger_action.delay( - action_id=action_id, - incident_id=incident_id, - project_id=project_id, - method=method, - new_status=new_status, - ), - using=router.db_for_write(AlertRuleTrigger), - ) - - def _sort_by_priority_list( triggers: Collection[AlertRuleTrigger], ) -> list[AlertRuleTrigger]: diff --git a/src/sentry/incidents/models/alert_rule.py b/src/sentry/incidents/models/alert_rule.py index 954d12f28922..1ac63d1e8560 100644 --- a/src/sentry/incidents/models/alert_rule.py +++ b/src/sentry/incidents/models/alert_rule.py @@ -26,7 +26,7 @@ from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey from sentry.db.models.manager.base import BaseManager from sentry.db.models.manager.base_query_set import BaseQuerySet -from sentry.incidents.models.incident import Incident, IncidentStatus, IncidentTrigger +from sentry.incidents.models.incident import IncidentTrigger from sentry.models.organization import Organization from sentry.models.organizationmember import OrganizationMember from sentry.models.project import Project @@ -480,46 +480,6 @@ def build_handler(type: ActionService) -> ActionHandler | None: metrics.incr(f"alert_rule_trigger.unhandled_type.{type}") return None - def fire( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ) -> None: - handler = AlertRuleTriggerAction.build_handler(AlertRuleTriggerAction.Type(self.type)) - if handler: - return handler.fire( - action=action, - incident=incident, - project=project, - new_status=new_status, - metric_value=metric_value, - notification_uuid=notification_uuid, - ) - - def resolve( - self, - action: AlertRuleTriggerAction, - incident: Incident, - project: Project, - metric_value: int | float | None, - new_status: IncidentStatus, - notification_uuid: str | None = None, - ) -> None: - handler = AlertRuleTriggerAction.build_handler(AlertRuleTriggerAction.Type(self.type)) - if handler: - return handler.resolve( - action=action, - incident=incident, - project=project, - metric_value=metric_value, - new_status=new_status, - notification_uuid=notification_uuid, - ) - def get_single_sentry_app_config(self) -> dict[str, Any] | None: value = self.sentry_app_config if isinstance(value, list): diff --git a/src/sentry/incidents/tasks.py b/src/sentry/incidents/tasks.py index 309dc4183bfe..e5d8a4732132 100644 --- a/src/sentry/incidents/tasks.py +++ b/src/sentry/incidents/tasks.py @@ -38,25 +38,6 @@ def handle_snuba_query_update( SubscriptionProcessor.process(subscription, subscription_update) -@instrumented_task( - name="sentry.incidents.tasks.handle_trigger_action", - namespace=alerts_tasks, - retry=Retry(times=5, delay=60), - processing_deadline_duration=60, - silo_mode=SiloMode.CELL, -) -def handle_trigger_action( - action_id: int, - incident_id: int, - project_id: int, - method: str, - new_status: int, - metric_value: float | int | None = None, - **kwargs: Any, -) -> None: - pass - - @instrumented_task( name="sentry.incidents.tasks.auto_resolve_snapshot_incidents", namespace=alerts_tasks, diff --git a/tests/sentry/incidents/models/test_alert_rule.py b/tests/sentry/incidents/models/test_alert_rule.py index 5d23d6edec05..473621bb7794 100644 --- a/tests/sentry/incidents/models/test_alert_rule.py +++ b/tests/sentry/incidents/models/test_alert_rule.py @@ -1,4 +1,3 @@ -import unittest from collections.abc import Generator from unittest import mock from unittest.mock import Mock @@ -15,7 +14,6 @@ AlertRuleTrigger, AlertRuleTriggerAction, ) -from sentry.incidents.models.incident import IncidentStatus from sentry.testutils.cases import TestCase from sentry.testutils.helpers.alert_rule import TemporaryAlertRuleTriggerActionRegistry @@ -237,50 +235,6 @@ def test_specific(self) -> None: assert trigger.target == email -class AlertRuleTriggerActionActivateBaseTest: - method: str - - def setUp(self) -> None: - self.suspended_registry = TemporaryAlertRuleTriggerActionRegistry.suspend() - - def tearDown(self) -> None: - self.suspended_registry.restore() - - def test_no_handler(self) -> None: - trigger = AlertRuleTriggerAction(type=AlertRuleTriggerAction.Type.EMAIL.value) - result = trigger.fire( - Mock(), Mock(), Mock(), metric_value=123, new_status=IncidentStatus.CRITICAL - ) # type: ignore[func-returns-value] - - # TODO(RyanSkonnord): Remove assertion (see test_handler) - assert result is None - - def test_handler(self) -> None: - mock_handler = Mock() - mock_method = getattr(mock_handler.return_value, self.method) - mock_method.return_value = "test" - type = AlertRuleTriggerAction.Type.EMAIL - AlertRuleTriggerAction.register_type("something", type, [])(mock_handler) - trigger = AlertRuleTriggerAction(type=type.value) - result = getattr(trigger, self.method)( - Mock(), Mock(), Mock(), metric_value=123, new_status=IncidentStatus.CRITICAL - ) - - # TODO(RyanSkonnord): Don't assert on return value. - # All concrete ActionHandlers return None from their fire and resolve - # methods. It seems that this return value's only purpose is to spy on - # whether the AlertRuleTriggerAction produced a handler. - assert result == mock_method.return_value - - -class AlertRuleTriggerActionFireTest(AlertRuleTriggerActionActivateBaseTest, unittest.TestCase): - method = "fire" - - -class AlertRuleTriggerActionResolveTest(AlertRuleTriggerActionActivateBaseTest, unittest.TestCase): - method = "resolve" - - class AlertRuleTriggerActionActivateTest(TestCase): @pytest.fixture(autouse=True) def _setup_metric_patch(self) -> Generator[None]: