diff --git a/src/sentry/notifications/platform/metrics.py b/src/sentry/notifications/platform/metrics.py new file mode 100644 index 00000000000000..ea1657cde4b753 --- /dev/null +++ b/src/sentry/notifications/platform/metrics.py @@ -0,0 +1,36 @@ +from collections.abc import Mapping +from dataclasses import dataclass +from enum import StrEnum + +from sentry.integrations.types import EventLifecycleOutcome +from sentry.integrations.utils.metrics import EventLifecycleMetric +from sentry.notifications.platform.types import NotificationProviderKey + + +class NotificationInteractionType(StrEnum): + """Actions involved in notifications""" + + NOTIFY_TARGET_SYNC = "notify_target_sync" + + +@dataclass +class NotificationEventLifecycleMetric(EventLifecycleMetric): + interaction_type: NotificationInteractionType + # The template/source of the notification + notification_source: str + # The sender of the notification + notification_provider: NotificationProviderKey | None + + def get_metric_key(self, outcome: EventLifecycleOutcome) -> str: + tokens = ("notifications", "slo", str(outcome)) + return ".".join(tokens) + + def get_metric_tags(self) -> Mapping[str, str]: + tags = { + "interaction_type": self.interaction_type, + "notification_source": self.notification_source, + } + + if self.notification_provider: + tags["notification_provider"] = self.notification_provider + return tags diff --git a/src/sentry/notifications/platform/service.py b/src/sentry/notifications/platform/service.py index adae1ff5cf7ca8..454f313e46e3e4 100644 --- a/src/sentry/notifications/platform/service.py +++ b/src/sentry/notifications/platform/service.py @@ -1,12 +1,22 @@ import logging +from collections import defaultdict +from collections.abc import Mapping from typing import Final +from sentry.notifications.platform.metrics import ( + NotificationEventLifecycleMetric, + NotificationInteractionType, +) +from sentry.notifications.platform.provider import NotificationProvider from sentry.notifications.platform.registry import provider_registry, template_registry from sentry.notifications.platform.types import ( NotificationData, + NotificationProviderKey, NotificationStrategy, NotificationTarget, + NotificationTemplate, ) +from sentry.shared_integrations.exceptions import ApiError logger = logging.getLogger(__name__) @@ -15,42 +25,60 @@ class NotificationServiceError(Exception): pass +def _render_template[T: NotificationData, RenderableT]( + data: T, template: NotificationTemplate[T], provider: type[NotificationProvider[RenderableT]] +) -> RenderableT: + rendered_template = template.render(data=data) + renderer = provider.get_renderer(data=data, category=template.category) + return renderer.render(data=data, rendered_template=rendered_template) + + class NotificationService[T: NotificationData]: def __init__(self, *, data: T): self.data: Final[T] = data - # TODO(ecosystem): Eventually this should be converted to spawn a task with the business logic below def notify_target(self, *, target: NotificationTarget) -> None: """ - Send a notification directly to a target. + Send a notification directly to a target synchronously. NOTE: This method ignores notification settings. When possible, consider using a strategy instead of - using this method directly to prevent unwanted noise associated with your notifications. + using this method directly to prevent unwanted noise associated with your notifications. + NOTE: Use this method when you care about the notification sending result and delivering that back to the user. + Otherwise, we generally reccomend using the async version. """ if not self.data: raise NotificationServiceError( "Notification service must be initialized with data before sending!" ) - # Step 1: Get the provider, and validate the target against it - provider = provider_registry.get(target.provider_key) - provider.validate_target(target=target) + with NotificationEventLifecycleMetric( + interaction_type=NotificationInteractionType.NOTIFY_TARGET_SYNC, + notification_source=self.data.source, + notification_provider=target.provider_key, + ).capture() as lifecycle: + # Step 1: Get the provider, and validate the target against it + provider = provider_registry.get(target.provider_key) + provider.validate_target(target=target) - # Step 2: Render the template - template_cls = template_registry.get(self.data.source) - template = template_cls() - rendered_template = template.render(data=self.data) - renderer = provider.get_renderer(data=self.data, category=template.category) - renderable = renderer.render(data=self.data, rendered_template=rendered_template) + # Step 2: Render the template + template_cls = template_registry.get(self.data.source) + template = template_cls() + renderable = _render_template(data=self.data, template=template, provider=provider) - # Step 3: Send the notification - provider.send(target=target, renderable=renderable) + # Step 3: Send the notification + try: + provider.send(target=target, renderable=renderable) + except ApiError as e: + lifecycle.record_failure(failure_reason=e, create_issue=False) + raise + return None def notify( self, *, strategy: NotificationStrategy | None = None, targets: list[NotificationTarget] | None = None, - ) -> None: + sync_send: bool = False, + ) -> None | Mapping[NotificationProviderKey, list[ApiError]]: if not strategy and not targets: raise NotificationServiceError( "Must provide either a strategy or targets. Strategy is preferred." @@ -63,7 +91,27 @@ def notify( targets = strategy.get_targets() if not targets: logger.info("Strategy '%s' did not yield targets", strategy.__class__.__name__) - return + return None + + if sync_send: + errors = defaultdict(list) + for target in targets: + try: + self.notify_target(target=target) + except ApiError as e: + errors[target.provider_key].append(e) + return errors + + else: + for target in targets: + notify_target_async(target=target) + return None + - for target in targets: - self.notify_target(target=target) +def notify_target_async(*, target: NotificationTarget) -> None: + """ + Send a notification directly to a target asynchronously. + NOTE: This method ignores notification settings. When possible, consider using a strategy instead of + using this method directly to prevent unwanted noise associated with your notifications. + """ + raise NotImplementedError diff --git a/tests/sentry/notifications/platform/test_service.py b/tests/sentry/notifications/platform/test_service.py index 2060fa013cdaaf..99963c826a758c 100644 --- a/tests/sentry/notifications/platform/test_service.py +++ b/tests/sentry/notifications/platform/test_service.py @@ -27,7 +27,7 @@ def setUp(self) -> None: def test_basic_notify(self) -> None: service = NotificationService(data=MockNotification(message="this is a test notification")) - service.notify(targets=[self.target]) + service.notify(targets=[self.target], sync_send=True) @mock.patch("sentry.notifications.platform.service.logger") def test_validation_on_notify(self, mock_logger: mock.MagicMock) -> None: