diff --git a/src/sentry/integrations/discord/integration.py b/src/sentry/integrations/discord/integration.py index 1e38f22bd936..ad4610fcfbdc 100644 --- a/src/sentry/integrations/discord/integration.py +++ b/src/sentry/integrations/discord/integration.py @@ -97,7 +97,7 @@ def send_notification( except ApiError as e: translate_discord_api_error(e) - def send_notification_with_threading( + def send_notification_with_response( self, target: IntegrationNotificationTarget, payload: DiscordRenderable, diff --git a/src/sentry/integrations/msteams/integration.py b/src/sentry/integrations/msteams/integration.py index ebd885a02e5a..83f66028eae2 100644 --- a/src/sentry/integrations/msteams/integration.py +++ b/src/sentry/integrations/msteams/integration.py @@ -103,7 +103,7 @@ def send_notification( client = self.get_client() client.send_card(conversation_id=target.resource_id, card=payload) - def send_notification_with_threading( + def send_notification_with_response( self, target: IntegrationNotificationTarget, payload: AdaptiveCard, diff --git a/src/sentry/integrations/slack/integration.py b/src/sentry/integrations/slack/integration.py index 60ee18348a8b..49ebd4519ab1 100644 --- a/src/sentry/integrations/slack/integration.py +++ b/src/sentry/integrations/slack/integration.py @@ -122,7 +122,7 @@ def send_notification( translate_slack_api_error(e) return {} - def send_notification_with_threading( + def send_notification_with_response( self, target: IntegrationNotificationTarget, payload: SlackRenderable, diff --git a/src/sentry/integrations/slack/service.py b/src/sentry/integrations/slack/service.py index 8706c5b58744..085e04db5479 100644 --- a/src/sentry/integrations/slack/service.py +++ b/src/sentry/integrations/slack/service.py @@ -37,6 +37,10 @@ from sentry.models.group import Group from sentry.models.options.organization_option import OrganizationOption from sentry.notifications.additional_attachment_manager import get_additional_attachment +from sentry.notifications.models.notificationthread import ( + NotificationThread, + NotificationThreadObjectType, +) from sentry.notifications.notifications.activity.archive import ArchiveActivityNotification from sentry.notifications.notifications.activity.assigned import AssignedActivityNotification from sentry.notifications.notifications.activity.base import GroupActivityNotification @@ -58,6 +62,8 @@ from sentry.notifications.notifications.activity.unassigned import UnassignedActivityNotification from sentry.notifications.notifications.activity.unresolved import UnresolvedActivityNotification from sentry.notifications.notifications.base import BaseNotification +from sentry.notifications.platform.rollout import NotificationRolloutService +from sentry.notifications.platform.types import SLACK_PROVIDERS, NotificationSource from sentry.notifications.utils.open_period import open_period_start_for_group from sentry.silo.base import SiloMode from sentry.types.activity import ActivityType @@ -249,6 +255,25 @@ def _notify_all_threads_for_activity( group_ids=[group.id], ) + # Also query platform threads for issue alerts sent via notification platform. Only orgs in + # a platform rollout tier have them, so skip the query for everyone else. This checks the + # tier, not the per-send random rollout rate, so replies reach every existing thread. + platform_threads = NotificationThread.objects.none() + if NotificationRolloutService(organization=group.organization).has_feature_flag_access(): + platform_threads = NotificationThread.objects.filter( + object_type=NotificationThreadObjectType.GROUP, + object_id=group.id, + key_type=NotificationSource.ISSUE.value, + provider_key__in=SLACK_PROVIDERS, + ) + if use_open_period_start: + # The platform thread key stores the open period start as an ISO string + platform_threads = platform_threads.filter( + key_data__open_period_start=( + open_period_start.isoformat() if open_period_start else None + ) + ) + # We don't wrap this in a lifecycle because _handle_parent_notification is already wrapped in a lifecycle parent_notification_count = 0 for parent_notification in parent_notifications: @@ -307,6 +332,34 @@ def _notify_all_threads_for_activity( }, ) + # Send to platform threads + for platform_thread in platform_threads: + with MessagingInteractionEvent( + interaction_type=MessagingInteractionType.SEND_ACTIVITY_NOTIFICATION, + spec=SlackMessagingSpec(), + ).capture() as lifecycle: + lifecycle.add_extras( + { + "activity_id": activity.id, + "platform_thread_id": platform_thread.id, + "integration_id": client.integration_id, + "group_id": group.id, + "project_id": activity.project.id, + } + ) + try: + self._send_notification_to_slack_channel( + channel_id=platform_thread.target_id, + message_identifier=platform_thread.thread_identifier, + notification_to_send=notification_to_send, + client=client, + ) + except Exception as err: + if isinstance(err, SlackApiError): + record_lifecycle_termination_level(lifecycle, err) + else: + lifecycle.record_failure(err) + def _get_channel_id_from_parent_notification_notification_action( self, parent_notification: NotificationActionNotificationMessage, diff --git a/src/sentry/notifications/notification_action/platform_dispatch.py b/src/sentry/notifications/notification_action/platform_dispatch.py index 7bbbdf7c4f84..ffd717eb5b09 100644 --- a/src/sentry/notifications/notification_action/platform_dispatch.py +++ b/src/sentry/notifications/notification_action/platform_dispatch.py @@ -1,29 +1,50 @@ import logging +from typing import Any import sentry_sdk -from sentry import features, options -from sentry.constants import METRIC_ALERTS_THREAD_DEFAULT, ObjectStatus +from sentry import analytics, features, options +from sentry.analytics.events.alert_sent import AlertSentEvent +from sentry.analytics.events.base_notification_sent import BaseNotificationSent +from sentry.constants import ISSUE_ALERTS_THREAD_DEFAULT, METRIC_ALERTS_THREAD_DEFAULT, ObjectStatus from sentry.incidents.charts import build_metric_alert_chart +from sentry.incidents.grouptype import MetricIssue from sentry.incidents.models.incident import IncidentStatus +from sentry.incidents.typings.metric_detector import NotificationContext +from sentry.integrations.discord.analytics import DiscordIntegrationNotificationSent +from sentry.integrations.msteams.analytics import MSTeamsIntegrationNotificationSent from sentry.integrations.services.integration.service import integration_service +from sentry.integrations.slack.analytics import SlackIntegrationNotificationSent from sentry.models.options.organization_option import OrganizationOption +from sentry.models.organization import Organization +from sentry.notifications.models.notificationthread import NotificationThreadObjectType from sentry.notifications.notification_action.metric_alert_registry.handlers.utils import ( get_detector_serializer, ) -from sentry.notifications.notification_action.utils import metric_alert_notification_data_factory +from sentry.notifications.notification_action.utils import ( + issue_notification_data_factory, + metric_alert_notification_data_factory, +) +from sentry.notifications.platform.provider import SendFailure, SendSuccessResult from sentry.notifications.platform.service import NotificationService from sentry.notifications.platform.target import IntegrationNotificationTarget -from sentry.notifications.platform.templates.metric_alert import MetricAlertNotificationData from sentry.notifications.platform.threading import ThreadingOptions, ThreadKey from sentry.notifications.platform.types import ( SHADOW_PROVIDERS, + SLACK_PROVIDERS, + NotificationData, NotificationProviderKey, NotificationSource, NotificationTargetResourceType, ) from sentry.notifications.types import TEST_NOTIFICATION_ID from sentry.notifications.utils.issue_notification_context import IssueNotificationContext +from sentry.notifications.utils.open_period import open_period_start_for_group +from sentry.seer.entrypoints.operator import SeerAutofixOperator +from sentry.seer.entrypoints.slack.entrypoint import prepare_slack_thread_for_autofix_updates +from sentry.seer.entrypoints.types import SeerEntrypointKey +from sentry.uptime.grouptype import UptimeDomainCheckFailure +from sentry.utils import metrics from sentry.workflow_engine.types import ActionInvocation logger = logging.getLogger(__name__) @@ -38,6 +59,32 @@ } ) +# The legacy issue alert analytics per provider: (provider name, sent event, metric instance) +LEGACY_ISSUE_ALERT_ANALYTICS: dict[ + NotificationProviderKey, tuple[str, type[BaseNotificationSent], str] +] = { + NotificationProviderKey.SLACK: ( + "slack", + SlackIntegrationNotificationSent, + "slack.notification", + ), + NotificationProviderKey.SLACK_STAGING: ( + "slack", + SlackIntegrationNotificationSent, + "slack.notification", + ), + NotificationProviderKey.DISCORD: ( + "discord", + DiscordIntegrationNotificationSent, + "discord.notifications", + ), + NotificationProviderKey.MSTEAMS: ( + "msteams", + MSTeamsIntegrationNotificationSent, + "msteams.notification", + ), +} + def get_platform_provider( invocation: ActionInvocation, source: NotificationSource @@ -60,13 +107,12 @@ def get_platform_provider( return provider -def send_metric_alert(context: IssueNotificationContext, provider: NotificationProviderKey) -> None: - organization = context.organization - notification_context = context.notification_context - metric_issue_context = context.metric_issue_context - open_period_context = context.open_period_context - - # Match the legacy send: skip actions whose channel or integration is gone, rather than +def _build_target( + provider: NotificationProviderKey, + organization: Organization, + notification_context: NotificationContext, +) -> IntegrationNotificationTarget | None: + # Match the legacy sends: skip actions whose channel or integration is gone, rather than # raising and retrying on every alert. if ( notification_context.integration_id is None @@ -81,7 +127,7 @@ def send_metric_alert(context: IssueNotificationContext, provider: NotificationP "provider": str(provider), }, ) - return + return None integration_context = integration_service.organization_context( organization_id=organization.id, integration_id=notification_context.integration_id ) @@ -103,9 +149,9 @@ def send_metric_alert(context: IssueNotificationContext, provider: NotificationP "integration_status": integration_status, }, ) - return + return None - target = IntegrationNotificationTarget( + return IntegrationNotificationTarget( provider_key=provider, resource_type=NotificationTargetResourceType.CHANNEL, resource_id=notification_context.target_identifier, @@ -113,6 +159,53 @@ def send_metric_alert(context: IssueNotificationContext, provider: NotificationP organization_id=organization.id, ) + +def _should_thread( + provider: NotificationProviderKey, + organization: Organization, + key: str, + default: bool, +) -> bool: + # for notifications that have threads (just slack) + return provider in SLACK_PROVIDERS and OrganizationOption.objects.get_value( + organization=organization, key=key, default=default + ) + + +def _send( + data: NotificationData, + target: IntegrationNotificationTarget, + threading_options: ThreadingOptions | None, + action_id: int, + group_id: int, +) -> SendSuccessResult | SendFailure: + service = NotificationService(data=data) + result = service.notify_target(target=target, threading_options=threading_options) + + if isinstance(result, SendFailure): + logger.warning( + "notification_action.platform_dispatch.send_failed", + extra={ + "action_id": action_id, + "group_id": group_id, + "provider": target.provider_key, + "status": result.status.value, + }, + ) + + return result + + +def send_metric_alert(context: IssueNotificationContext, provider: NotificationProviderKey) -> None: + organization = context.organization + notification_context = context.notification_context + metric_issue_context = context.metric_issue_context + open_period_context = context.open_period_context + + target = _build_target(provider, organization, notification_context) + if target is None: + return + # for notifications that include a chart chart_url = None if provider in CHART_PROVIDERS and features.has( @@ -141,15 +234,9 @@ def send_metric_alert(context: IssueNotificationContext, provider: NotificationP chart_url=chart_url, ) - # for notifications that have threads (just slack) threading_options = None - if provider in ( - NotificationProviderKey.SLACK, - NotificationProviderKey.SLACK_STAGING, - ) and OrganizationOption.objects.get_value( - organization=organization, - key="sentry:metric_alerts_thread_flag", - default=METRIC_ALERTS_THREAD_DEFAULT, + if _should_thread( + provider, organization, "sentry:metric_alerts_thread_flag", METRIC_ALERTS_THREAD_DEFAULT ): threading_options = ThreadingOptions( thread_key=ThreadKey( @@ -159,20 +246,114 @@ def send_metric_alert(context: IssueNotificationContext, provider: NotificationP "group_id": metric_issue_context.id, "open_period_start": open_period_context.date_started.isoformat(), }, + object_type=NotificationThreadObjectType.GROUP, + object_id=metric_issue_context.id, ), reply_broadcast=(metric_issue_context.new_status == IncidentStatus.CRITICAL), ) - errors = NotificationService[MetricAlertNotificationData](data=data).notify_sync( - targets=[target], threading_options=threading_options + _send(data, target, threading_options, notification_context.id, metric_issue_context.id) + + +def send_issue_alert(invocation: ActionInvocation, provider: NotificationProviderKey) -> None: + group = invocation.event_data.group + organization = group.project.organization + + target = _build_target( + provider, organization, NotificationContext.from_action_model(invocation.action) ) - for failure in errors.get(provider, []): - logger.warning( - "notification_action.platform_dispatch.send_failed", - extra={ - "action_id": notification_context.id, - "group_id": metric_issue_context.id, - "provider": provider, - "status": failure.status.value, - }, + if target is None: + return + + # Same thread as the legacy send: one per action and issue, plus the open period for uptime + # and metric issues. + threading_options = None + if _should_thread( + provider, organization, "sentry:issue_alerts_thread_flag", ISSUE_ALERTS_THREAD_DEFAULT + ): + key_data: dict[str, Any] = {"action_id": invocation.action.id, "group_id": group.id} + if group.issue_type.type_id in (UptimeDomainCheckFailure.type_id, MetricIssue.type_id): + open_period_start = open_period_start_for_group(group) + key_data["open_period_start"] = ( + open_period_start.isoformat() if open_period_start else None + ) + threading_options = ThreadingOptions( + thread_key=ThreadKey( + key_type=NotificationSource.ISSUE, + key_data=key_data, + object_type=NotificationThreadObjectType.GROUP, + object_id=group.id, + ), + reply_broadcast=True, + ) + + result = _send( + issue_notification_data_factory(invocation), + target, + threading_options, + invocation.action.id, + group.id, + ) + _record_legacy_issue_alert_sent(invocation, provider, target.resource_id) + + # Cache the thread for Seer autofix updates + if not ( + isinstance(result, SendSuccessResult) + and result.provider_message_id + and provider in SLACK_PROVIDERS + and SeerAutofixOperator.has_access( + organization=organization, entrypoint_key=SeerEntrypointKey.SLACK + ) + ): + return + + # A threaded reply returns its own ts; Seer needs the root ts, like the legacy send. + thread_ts = result.thread_identifier or result.provider_message_id + prepare_slack_thread_for_autofix_updates( + thread_ts=thread_ts, + channel_id=target.resource_id, + group=group, + organization_id=organization.id, + integration_id=target.integration_id, + ) + + +def _record_legacy_issue_alert_sent( + invocation: ActionInvocation, provider: NotificationProviderKey, channel_id: str +) -> None: + """ + Keep the legacy issue alert analytics and metric during the migration, recorded whether or not + the send succeeded like the legacy sends. Remove with the legacy path. + """ + provider_name, sent_event_class, metric_instance = LEGACY_ISSUE_ALERT_ANALYTICS[provider] + group = invocation.event_data.group + metrics.incr( + "notifications.sent", + instance=metric_instance, + tags={"issue_type": group.issue_type.slug}, + skip_internal=False, + ) + try: + analytics.record( + sent_event_class( + organization_id=group.project.organization_id, + project_id=group.project_id, + group_id=group.id, + notification_uuid=invocation.notification_uuid, + alert_id=invocation.action.id, + category="issue_alert", + ) + ) + analytics.record( + AlertSentEvent( + provider=provider_name, + alert_id=invocation.action.id, + alert_type="issue_alert", + organization_id=group.project.organization_id, + project_id=group.project_id, + external_id=channel_id, + notification_uuid=invocation.notification_uuid, + ) ) + except Exception as e: + sentry_sdk.capture_exception(e) diff --git a/src/sentry/notifications/notification_action/types.py b/src/sentry/notifications/notification_action/types.py index 2cdafa035587..09389b0acea4 100644 --- a/src/sentry/notifications/notification_action/types.py +++ b/src/sentry/notifications/notification_action/types.py @@ -343,8 +343,17 @@ def invoke_legacy_registry(cls, invocation: ActionInvocation) -> None: 2. activate_downstream_actions 3. execute_futures (also in post_process process_rules) """ + from sentry.notifications.notification_action.platform_dispatch import ( + get_platform_provider, + send_issue_alert, + ) from sentry.notifications.notification_action.utils import issue_notification_data_factory + provider = get_platform_provider(invocation, NotificationSource.ISSUE) + if provider is not None: + send_issue_alert(invocation, provider) + return + with shadow_read( invocation, NotificationSource.ISSUE, diff --git a/src/sentry/notifications/platform/provider.py b/src/sentry/notifications/platform/provider.py index 23bb68736023..cc42ccf9d389 100644 --- a/src/sentry/notifications/platform/provider.py +++ b/src/sentry/notifications/platform/provider.py @@ -51,6 +51,9 @@ class SendSuccessResult: is_threaded: bool = False """Whether the notification was sent with threading.""" + thread_identifier: str | None = None + """The existing thread's root identifier when sent as a reply, None for a new message.""" + @dataclass(frozen=True) class SendFailure: @@ -88,7 +91,7 @@ def send_notification( self, target: IntegrationNotificationTarget, payload: RenderableT ) -> ResponseT | None: ... - def send_notification_with_threading( + def send_notification_with_response( self, target: IntegrationNotificationTarget, payload: RenderableT, diff --git a/src/sentry/notifications/platform/service.py b/src/sentry/notifications/platform/service.py index 7b8e16c17c0d..ddabeedcc988 100644 --- a/src/sentry/notifications/platform/service.py +++ b/src/sentry/notifications/platform/service.py @@ -314,6 +314,8 @@ def _handle_threading_result( target_id=target.resource_id, thread_identifier=result.provider_message_id, provider_data=None, + object_type=threading_options.thread_key.object_type, + object_id=threading_options.thread_key.object_id, ) ThreadingService.store_new_thread( threading_config=threading_config, diff --git a/src/sentry/notifications/platform/slack/provider.py b/src/sentry/notifications/platform/slack/provider.py index 2e6c83823c1e..65edc2d8d83e 100644 --- a/src/sentry/notifications/platform/slack/provider.py +++ b/src/sentry/notifications/platform/slack/provider.py @@ -191,12 +191,16 @@ def _send_with_threading( ) try: - response = slack_target.integration_installation.send_notification_with_threading( + response = slack_target.integration_installation.send_notification_with_response( target=slack_target.target, payload=renderable, threading_context=provider_threading_ctx, ) - return SendSuccessResult(provider_message_id=response.get("ts"), is_threaded=True) + return SendSuccessResult( + provider_message_id=response.get("ts"), + is_threaded=True, + thread_identifier=provider_threading_ctx.thread_ts, + ) except IntegrationError as e: return integration_error_result(e, is_threaded=True) diff --git a/src/sentry/notifications/platform/threading.py b/src/sentry/notifications/platform/threading.py index 2fb8610892f7..45234b982b7b 100644 --- a/src/sentry/notifications/platform/threading.py +++ b/src/sentry/notifications/platform/threading.py @@ -3,13 +3,16 @@ import hashlib import logging from dataclasses import dataclass -from typing import Any, TypedDict +from typing import Any, NotRequired, TypedDict import pydantic from django.db import router, transaction from sentry.notifications.models.notificationrecord import NotificationRecord -from sentry.notifications.models.notificationthread import NotificationThread +from sentry.notifications.models.notificationthread import ( + NotificationThread, + NotificationThreadObjectType, +) from sentry.notifications.platform.types import NotificationProviderKey, NotificationSource from sentry.utils import json @@ -29,6 +32,16 @@ class ThreadKey(pydantic.BaseModel): e.g. for NOA: {"action_id": ..., "group_id": ..., "open_period_start": ...} """ + object_type: NotificationThreadObjectType | None = None + """ + The kind of object this thread is about (e.g. GROUP for issue and metric alerts). Stored with + object_id so every thread for an object can be found without the full key. Not part of the + thread key hash. + """ + + object_id: int | None = None + """The ID of the object this thread is about, interpreted by object_type.""" + class Config: frozen = True use_enum_values = True @@ -92,6 +105,10 @@ class ThreadingConfig(ThreadingLookup): # Optional provider-specific metadata provider_data: dict[str, Any] | None + # The object this thread is about (see ThreadKey.object_type and ThreadKey.object_id) + object_type: NotRequired[NotificationThreadObjectType | None] + object_id: NotRequired[int | None] + class ThreadingService: """ @@ -163,6 +180,8 @@ def _create_thread_and_record( "thread_identifier": threading_config["thread_identifier"], "key_type": threading_config["key_type"], "key_data": threading_config["key_data"], + "object_type": threading_config.get("object_type"), + "object_id": threading_config.get("object_id"), "provider_data": threading_config["provider_data"] or {}, }, ) diff --git a/src/sentry/notifications/platform/types.py b/src/sentry/notifications/platform/types.py index 3c0b73e0aa97..cbb3b0d3f0fa 100644 --- a/src/sentry/notifications/platform/types.py +++ b/src/sentry/notifications/platform/types.py @@ -189,6 +189,8 @@ class NotificationProviderKey(StrEnum): DISCORD = ExternalProviderEnum.DISCORD +SLACK_PROVIDERS = frozenset({NotificationProviderKey.SLACK, NotificationProviderKey.SLACK_STAGING}) + SHADOW_PROVIDERS: dict[str, NotificationProviderKey] = { "slack": NotificationProviderKey.SLACK, "slack_staging": NotificationProviderKey.SLACK_STAGING, diff --git a/tests/sentry/integrations/slack/service/test_slack_service.py b/tests/sentry/integrations/slack/service/test_slack_service.py index 1600354bb1ab..38a25ed2ed27 100644 --- a/tests/sentry/integrations/slack/service/test_slack_service.py +++ b/tests/sentry/integrations/slack/service/test_slack_service.py @@ -14,14 +14,20 @@ from sentry.integrations.types import EventLifecycleOutcome from sentry.issues.grouptype import GroupCategory from sentry.models.activity import Activity +from sentry.models.group import Group from sentry.models.groupopenperiod import get_latest_open_period from sentry.models.options.organization_option import OrganizationOption from sentry.notifications.models.notificationaction import ActionTarget from sentry.notifications.models.notificationmessage import NotificationMessage +from sentry.notifications.models.notificationthread import NotificationThreadObjectType +from sentry.notifications.platform.threading import ThreadingService +from sentry.notifications.platform.types import NotificationProviderKey, NotificationSource +from sentry.notifications.utils.open_period import open_period_start_for_group from sentry.silo.base import SiloMode from sentry.testutils.cases import TestCase from sentry.testutils.helpers import with_feature from sentry.testutils.helpers.datetime import freeze_time +from sentry.testutils.helpers.options import override_options from sentry.testutils.silo import assume_test_silo_mode from sentry.types.activity import ActivityType from sentry.uptime.grouptype import UptimeDomainCheckFailure @@ -251,6 +257,95 @@ def test_handle_parent_notification_with_open_period( mock_send_notification.assert_called_once() assert mock_get_channel_id.call_args.args[0].id == parent_notification_2_message.id + def create_platform_threads(self, group: Group) -> None: + open_period_start = open_period_start_for_group(group) + assert open_period_start is not None + for thread_ts, period_start in [ + ("111.111", (open_period_start - timedelta(days=1)).isoformat()), + ("222.222", open_period_start.isoformat()), + ]: + ThreadingService.store_new_thread( + threading_config={ + "key_type": NotificationSource.ISSUE, + "key_data": { + "action_id": self.action.id, + "group_id": group.id, + "open_period_start": period_start, + }, + "provider_key": NotificationProviderKey.SLACK, + "target_id": "C1234567890", + "thread_identifier": thread_ts, + "provider_data": None, + "object_type": NotificationThreadObjectType.GROUP, + "object_id": group.id, + }, + external_message_id=thread_ts, + ) + + @with_feature( + [ + "organizations:slack-threads-refactor-uptime", + "organizations:notification-platform.internal-testing", + ] + ) + # A 0% rate would skip every new send, but replies still reach threads sent at a higher rate + @override_options({"notifications.platform-rollout.internal-testing": {"issue": 0.0}}) + @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + @mock.patch( + "sentry.integrations.slack.service.SlackService._send_notification_to_slack_channel" + ) + def test_platform_threads_filtered_to_current_open_period( + self, mock_send_notification, mock_record + ) -> None: + group = self.create_group(type=UptimeDomainCheckFailure.type_id) + activity = Activity.objects.create( + group=group, + project=self.project, + type=ActivityType.SET_IGNORED.value, + user_id=self.user.id, + data={"ignoreUntilEscalating": True}, + ) + self.create_platform_threads(group) + + with mock.patch.object( + type(group), + "issue_category", + new_callable=mock.PropertyMock, + return_value=GroupCategory.OUTAGE, + ): + self.service.notify_all_threads_for_activity(activity=activity) + + mock_send_notification.assert_called_once() + assert mock_send_notification.call_args.kwargs["message_identifier"] == "222.222" + + @with_feature("organizations:slack-threads-refactor-uptime") + @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") + @mock.patch( + "sentry.integrations.slack.service.SlackService._send_notification_to_slack_channel" + ) + def test_platform_threads_skipped_outside_platform_rollout( + self, mock_send_notification, mock_record + ) -> None: + group = self.create_group(type=UptimeDomainCheckFailure.type_id) + activity = Activity.objects.create( + group=group, + project=self.project, + type=ActivityType.SET_IGNORED.value, + user_id=self.user.id, + data={"ignoreUntilEscalating": True}, + ) + self.create_platform_threads(group) + + with mock.patch.object( + type(group), + "issue_category", + new_callable=mock.PropertyMock, + return_value=GroupCategory.OUTAGE, + ): + self.service.notify_all_threads_for_activity(activity=activity) + + mock_send_notification.assert_not_called() + @with_feature("organizations:slack-threads-refactor-uptime") @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @mock.patch( diff --git a/tests/sentry/notifications/notification_action/test_platform_dispatch.py b/tests/sentry/notifications/notification_action/test_platform_dispatch.py index 680352cda442..85b37b667a1f 100644 --- a/tests/sentry/notifications/notification_action/test_platform_dispatch.py +++ b/tests/sentry/notifications/notification_action/test_platform_dispatch.py @@ -6,24 +6,40 @@ from slack_sdk.web import SlackResponse +from sentry.analytics.events.alert_sent import AlertSentEvent from sentry.constants import ObjectStatus +from sentry.grouping.grouptype import ErrorGroupType +from sentry.integrations.slack.analytics import SlackIntegrationNotificationSent from sentry.integrations.types import IntegrationProviderSlug from sentry.notifications.models.notificationaction import ActionTarget +from sentry.notifications.models.notificationthread import ( + NotificationThread, + NotificationThreadObjectType, +) +from sentry.notifications.notification_action.issue_alert_registry import ( + DiscordIssueAlertHandler, + SlackIssueAlertHandler, +) from sentry.notifications.notification_action.metric_alert_registry import SlackMetricAlertHandler +from sentry.notifications.platform.provider import SendSuccessResult from sentry.notifications.platform.service import NotificationService from sentry.notifications.platform.types import NotificationProviderKey from sentry.silo.base import SiloMode +from sentry.testutils.helpers.analytics import assert_any_analytics_event from sentry.testutils.helpers.datetime import freeze_time from sentry.testutils.helpers.features import with_feature from sentry.testutils.helpers.options import override_options from sentry.testutils.silo import assume_test_silo_mode +from sentry.testutils.skips import requires_snuba from sentry.workflow_engine.models import Action -from sentry.workflow_engine.types import ActionInvocation +from sentry.workflow_engine.types import ActionInvocation, WorkflowEventData from tests.sentry.notifications.notification_action.test_metric_alert_registry_handlers import ( MetricAlertHandlerBase, ) +from tests.sentry.workflow_engine.test_base import BaseWorkflowTest PLATFORM_DISPATCH_PATH = "sentry.notifications.notification_action.platform_dispatch" +SLACK_ISSUE_LEGACY_CLIENT = "sentry.integrations.slack.actions.notification.SlackSdkClient" SLACK_HANDLER_PATH = "sentry.notifications.notification_action.metric_alert_registry.handlers.slack_metric_alert_handler" @@ -125,6 +141,11 @@ def test_follow_up_replies_in_thread(self, mock_slack_client: mock.MagicMock) -> assert follow_up["thread_ts"] == "123.456" assert follow_up["reply_broadcast"] is True + thread = NotificationThread.objects.get() + assert thread.object_type == NotificationThreadObjectType.GROUP + assert thread.object_id == self.event_data.group.id + assert thread.key_data["group_id"] == self.event_data.group.id + @mock.patch("sentry.integrations.slack.integration.SlackSdkClient") def test_metric_thread_flag_off_posts_top_level( self, mock_slack_client: mock.MagicMock @@ -141,13 +162,13 @@ def test_metric_thread_flag_off_posts_top_level( assert "thread_ts" not in client.chat_postMessage.call_args_list[1].kwargs - @mock.patch.object(NotificationService, "notify_sync", return_value={}) + @mock.patch.object(NotificationService, "notify_target", return_value=SendSuccessResult()) def test_slack_staging_targets_staging_provider(self, mock_notify: mock.MagicMock) -> None: action = self.create_slack_action(Action.Type.SLACK_STAGING) SlackMetricAlertHandler.invoke_legacy_registry(self.invocation(action)) - [target] = mock_notify.call_args.kwargs["targets"] + target = mock_notify.call_args.kwargs["target"] assert target.provider_key == NotificationProviderKey.SLACK_STAGING @override_options({"notifications.platform.alert-providers": {"metric-alert": []}}) @@ -180,7 +201,7 @@ def test_logs_when_integration_id_missing(self, mock_logger: mock.MagicMock) -> @mock.patch(f"{PLATFORM_DISPATCH_PATH}.logger") def test_logs_when_target_identifier_missing(self, mock_logger: mock.MagicMock) -> None: - # Create action without target_identifier + # The config schema rejects a missing target_identifier, so drop it after saving action = self.create_slack_action(Action.Type.SLACK) action.config["target_identifier"] = None @@ -207,3 +228,193 @@ def test_logs_when_integration_inactive(self, mock_logger: mock.MagicMock) -> No assert call_args[1]["extra"]["provider"] == "slack" assert call_args[1]["extra"]["has_integration"] is True assert call_args[1]["extra"]["integration_status"] == 1 + + +ISSUE_PLATFORM_OPTIONS = { + "notifications.platform-rollout.internal-testing": {"issue": 1.0}, + "notifications.platform.alert-providers": {"issue": ["slack", "discord"]}, +} + + +@requires_snuba +@with_feature("organizations:notification-platform.internal-testing") +class IssueAlertPlatformDispatchTest(BaseWorkflowTest): + def setUp(self) -> None: + super().setUp() + self.enterContext(override_options(ISSUE_PLATFORM_OPTIONS)) + self.workflow = self.create_workflow(organization=self.organization) + self.detector = self.create_detector(project=self.project, type=ErrorGroupType.slug) + event = self.store_event( + data={"message": "oh no", "level": "error"}, project_id=self.project.id + ) + assert event.group is not None + self.group = event.group + self.event = event.for_group(self.group) + + def create_channel_action(self, action_type: str, provider: str) -> Action: + integration, _ = self.create_provider_integration_for( + provider=provider, + organization=self.organization, + user=self.user, + name=f"test-{provider}", + external_id=f"{provider}-workspace", + ) + return self.create_action( + type=action_type, + integration_id=integration.id, + config={ + "target_identifier": "channel123", + "target_display": "Channel 123", + "target_type": ActionTarget.SPECIFIC, + }, + ) + + def invocation(self, action: Action) -> ActionInvocation: + return ActionInvocation( + event_data=WorkflowEventData(event=self.event, group=self.group), + action=action, + detector=self.detector, + notification_uuid=str(uuid.uuid4()), + workflow_id=self.workflow.id, + ) + + @mock.patch(SLACK_ISSUE_LEGACY_CLIENT) + @mock.patch("sentry.integrations.slack.integration.SlackSdkClient") + def test_slack_sends_to_channel( + self, mock_slack_client: mock.MagicMock, mock_legacy_client: mock.MagicMock + ) -> None: + action = self.create_channel_action(Action.Type.SLACK, IntegrationProviderSlug.SLACK) + client = mock_slack_client.return_value + client.chat_postMessage.return_value = slack_response(client, "123.456") + + SlackIssueAlertHandler.invoke_legacy_registry(self.invocation(action)) + + mock_legacy_client.return_value.chat_postMessage.assert_not_called() + client.chat_postMessage.assert_called_once() + call_kwargs = client.chat_postMessage.call_args.kwargs + assert call_kwargs["channel"] == "channel123" + assert "thread_ts" not in call_kwargs + assert "oh no" in call_kwargs["text"] + + @mock.patch("sentry.integrations.slack.integration.SlackSdkClient") + def test_slack_follow_up_replies_in_thread_and_broadcasts( + self, mock_slack_client: mock.MagicMock + ) -> None: + action = self.create_channel_action(Action.Type.SLACK, IntegrationProviderSlug.SLACK) + client = mock_slack_client.return_value + client.chat_postMessage.side_effect = [ + slack_response(client, "123.456"), + slack_response(client, "123.789"), + ] + + SlackIssueAlertHandler.invoke_legacy_registry(self.invocation(action)) + SlackIssueAlertHandler.invoke_legacy_registry(self.invocation(action)) + + follow_up = client.chat_postMessage.call_args_list[1].kwargs + assert follow_up["thread_ts"] == "123.456" + assert follow_up["reply_broadcast"] is True + + thread = NotificationThread.objects.get() + assert thread.object_type == NotificationThreadObjectType.GROUP + assert thread.object_id == self.group.id + assert thread.key_data["group_id"] == self.group.id + + @mock.patch("sentry.analytics.record") + @mock.patch(f"{PLATFORM_DISPATCH_PATH}.metrics") + @mock.patch("sentry.integrations.slack.integration.SlackSdkClient") + def test_slack_records_legacy_analytics( + self, + mock_slack_client: mock.MagicMock, + mock_metrics: mock.MagicMock, + mock_record: mock.MagicMock, + ) -> None: + action = self.create_channel_action(Action.Type.SLACK, IntegrationProviderSlug.SLACK) + client = mock_slack_client.return_value + client.chat_postMessage.return_value = slack_response(client, "123.456") + invocation = self.invocation(action) + + SlackIssueAlertHandler.invoke_legacy_registry(invocation) + + mock_metrics.incr.assert_called_once_with( + "notifications.sent", + instance="slack.notification", + tags={"issue_type": self.group.issue_type.slug}, + skip_internal=False, + ) + assert_any_analytics_event( + mock_record, + SlackIntegrationNotificationSent( + category="issue_alert", + organization_id=self.organization.id, + project_id=self.project.id, + group_id=self.group.id, + notification_uuid=invocation.notification_uuid, + alert_id=action.id, + ), + ) + assert_any_analytics_event( + mock_record, + AlertSentEvent( + provider="slack", + alert_id=action.id, + alert_type="issue_alert", + organization_id=self.organization.id, + project_id=self.project.id, + external_id="channel123", + notification_uuid=invocation.notification_uuid, + ), + ) + + @mock.patch(f"{PLATFORM_DISPATCH_PATH}.prepare_slack_thread_for_autofix_updates") + @mock.patch(f"{PLATFORM_DISPATCH_PATH}.SeerAutofixOperator.has_access", return_value=True) + @mock.patch("sentry.integrations.slack.integration.SlackSdkClient") + def test_slack_follow_up_caches_root_thread_for_seer( + self, + mock_slack_client: mock.MagicMock, + mock_has_access: mock.MagicMock, + mock_prepare_thread: mock.MagicMock, + ) -> None: + action = self.create_channel_action(Action.Type.SLACK, IntegrationProviderSlug.SLACK) + client = mock_slack_client.return_value + client.chat_postMessage.side_effect = [ + slack_response(client, "123.456"), + slack_response(client, "123.789"), + ] + + SlackIssueAlertHandler.invoke_legacy_registry(self.invocation(action)) + SlackIssueAlertHandler.invoke_legacy_registry(self.invocation(action)) + + assert [c.kwargs["thread_ts"] for c in mock_prepare_thread.call_args_list] == [ + "123.456", + "123.456", + ] + + @mock.patch(f"{PLATFORM_DISPATCH_PATH}.prepare_slack_thread_for_autofix_updates") + @mock.patch(f"{PLATFORM_DISPATCH_PATH}.SeerAutofixOperator.has_access", return_value=True) + @mock.patch("sentry.integrations.slack.integration.SlackSdkClient") + def test_slack_thread_flag_off_caches_message_for_seer( + self, + mock_slack_client: mock.MagicMock, + mock_has_access: mock.MagicMock, + mock_prepare_thread: mock.MagicMock, + ) -> None: + self.organization.update_option("sentry:issue_alerts_thread_flag", False) + action = self.create_channel_action(Action.Type.SLACK, IntegrationProviderSlug.SLACK) + client = mock_slack_client.return_value + client.chat_postMessage.return_value = slack_response(client, "123.456") + + SlackIssueAlertHandler.invoke_legacy_registry(self.invocation(action)) + + assert "thread_ts" not in client.chat_postMessage.call_args.kwargs + mock_prepare_thread.assert_called_once() + assert mock_prepare_thread.call_args.kwargs["thread_ts"] == "123.456" + + @mock.patch("sentry.integrations.discord.integration.DiscordClient") + def test_discord_sends_to_channel(self, mock_discord_client: mock.MagicMock) -> None: + action = self.create_channel_action(Action.Type.DISCORD, IntegrationProviderSlug.DISCORD) + + DiscordIssueAlertHandler.invoke_legacy_registry(self.invocation(action)) + + send_message = mock_discord_client.return_value.send_message + send_message.assert_called_once() + assert send_message.call_args.kwargs["channel_id"] == "channel123" diff --git a/tests/sentry/notifications/platform/slack/test_provider.py b/tests/sentry/notifications/platform/slack/test_provider.py index 260c017cea6c..399330e4d9cb 100644 --- a/tests/sentry/notifications/platform/slack/test_provider.py +++ b/tests/sentry/notifications/platform/slack/test_provider.py @@ -193,6 +193,7 @@ def test_send_success(self, mock_slack_client: Mock) -> None: assert isinstance(result, SendSuccessResult) assert result.provider_message_id == "1234567890.123456" + assert result.is_threaded is False mock_client_instance.chat_postMessage.assert_called_once_with( channel="C1234567890", blocks=renderable["blocks"], diff --git a/tests/sentry/notifications/platform/test_threading.py b/tests/sentry/notifications/platform/test_threading.py index cd8c3ef112f3..80641b3447e9 100644 --- a/tests/sentry/notifications/platform/test_threading.py +++ b/tests/sentry/notifications/platform/test_threading.py @@ -1,7 +1,10 @@ import hashlib from sentry.notifications.models.notificationrecord import NotificationRecord -from sentry.notifications.models.notificationthread import NotificationThread +from sentry.notifications.models.notificationthread import ( + NotificationThread, + NotificationThreadObjectType, +) from sentry.notifications.platform.threading import ( ThreadingConfig, ThreadingLookup, @@ -178,6 +181,8 @@ def test_store_new_thread_creates_thread_and_record(self) -> None: assert thread.key_type == self.key_type assert thread.key_data == self.key_data assert thread.provider_data == {} + assert thread.object_type is None + assert thread.object_id is None assert record.id is not None assert record.thread_id == thread.id @@ -185,6 +190,27 @@ def test_store_new_thread_creates_thread_and_record(self) -> None: assert record.target_id == self.target_id assert record.message_id == self.message_id + def test_store_new_thread_stores_object(self) -> None: + thread, _ = ThreadingService.store_new_thread( + threading_config={ + **self.threading_config, + "object_type": NotificationThreadObjectType.GROUP, + "object_id": 456, + }, + external_message_id=self.message_id, + ) + + assert thread.object_type == NotificationThreadObjectType.GROUP + assert thread.object_id == 456 + # The object is for lookups only and doesn't change the thread key + assert thread.thread_key == self.thread_key + assert ( + NotificationThread.objects.get( + object_type=NotificationThreadObjectType.GROUP, object_id=456 + ) + == thread + ) + def test_store_new_thread_uses_existing_thread_on_race(self) -> None: """If two workers race, the second get_or_create returns the first's thread.""" first_thread, _ = ThreadingService.store_new_thread(