chore(sentry apps): Add slos for metric alert webhooks (#86748)

Author: Christinarlong

src/sentry/integrations/services/integration/impl.py

@@ -41,10 +41,15 @@
 )
 from sentry.rules.actions.notify_event_service import find_alert_rule_action_ui_component
 from sentry.sentry_apps.api.serializers.app_platform_event import AppPlatformEvent
+from sentry.sentry_apps.metrics import (
+    SentryAppEventType,
+    SentryAppInteractionEvent,
+    SentryAppInteractionType,
+)
 from sentry.sentry_apps.models.sentry_app import SentryApp
 from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
 from sentry.shared_integrations.exceptions import ApiError
-from sentry.utils import json, metrics
+from sentry.utils import json
 from sentry.utils.sentry_apps import send_and_save_webhook_request
 
 if TYPE_CHECKING:
@@ -368,44 +373,40 @@ def send_incident_alert_notification(
         notification_uuid: str | None = None,
     ) -> bool:
         try:
-            sentry_app = SentryApp.objects.get(id=sentry_app_id)
-        except SentryApp.DoesNotExist:
-            logger.info(
-                "metric_alert_webhook.missing_sentryapp",
-                extra={
-                    "sentry_app_id": sentry_app_id,
-                    "organization_id": organization_id,
-                },
-            )
+            new_status_str = INCIDENT_STATUS[IncidentStatus(new_status)].lower()
+            event = SentryAppEventType(f"metric_alert.{new_status_str}")
+        except ValueError as e:
+            sentry_sdk.capture_exception(e)
             return False
 
-        metrics.incr("notifications.sent", instance=sentry_app.slug, skip_internal=False)
+        with SentryAppInteractionEvent(
+            operation_type=SentryAppInteractionType.PREPARE_WEBHOOK,
+            event_type=event,
+        ).capture() as lifecycle:
+            try:
+                sentry_app = SentryApp.objects.get(id=sentry_app_id)
+            except SentryApp.DoesNotExist as e:
+                sentry_sdk.capture_exception(e)
+                lifecycle.record_failure(e)
+                return False
 
-        try:
-            install = SentryAppInstallation.objects.get(
-                organization_id=organization_id,
-                sentry_app=sentry_app,
-                status=SentryAppInstallationStatus.INSTALLED,
-            )
-        except SentryAppInstallation.DoesNotExist:
-            logger.info(
-                "metric_alert_webhook.missing_installation",
-                extra={
-                    "action": action_id,
-                    "incident": incident_id,
-                    "organization_id": organization_id,
-                    "sentry_app_id": sentry_app.id,
-                },
-                exc_info=True,
-            )
-            return False
+            try:
+                install = SentryAppInstallation.objects.get(
+                    organization_id=organization_id,
+                    sentry_app=sentry_app,
+                    status=SentryAppInstallationStatus.INSTALLED,
+                )
+            except SentryAppInstallation.DoesNotExist as e:
+                sentry_sdk.capture_exception(e)
+                lifecycle.record_failure(e)
+                return False
 
-        app_platform_event = AppPlatformEvent(
-            resource="metric_alert",
-            action=INCIDENT_STATUS[IncidentStatus(new_status)].lower(),
-            install=install,
-            data=json.loads(incident_attachment_json),
-        )
+            app_platform_event = AppPlatformEvent(
+                resource="metric_alert",
+                action=new_status_str,
+                install=install,
+                data=json.loads(incident_attachment_json),
+            )
 
         # Can raise errors if client returns >= 400
         send_and_save_webhook_request(

tests/sentry/incidents/action_handlers/test_sentry_app.py

@@ -5,6 +5,13 @@
 from sentry.incidents.action_handlers import SentryAppActionHandler
 from sentry.incidents.models.alert_rule import AlertRuleTriggerAction
 from sentry.incidents.models.incident import IncidentStatus
+from sentry.integrations.types import EventLifecycleOutcome
+from sentry.sentry_apps.metrics import SentryAppWebhookHaltReason
+from sentry.testutils.asserts import (
+    assert_count_of_metric,
+    assert_halt_metric,
+    assert_success_metric,
+)
 from sentry.testutils.helpers.datetime import freeze_time
 from sentry.utils import json
 
@@ -63,7 +70,8 @@ def run_test(self, incident, method):
         )
 
     @responses.activate
-    def test_rule_snoozed(self):
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_rule_snoozed(self, mock_record):
         alert_rule = self.create_alert_rule()
         incident = self.create_incident(alert_rule=alert_rule, status=IncidentStatus.CLOSED.value)
         self.snooze_rule(alert_rule=alert_rule)
@@ -88,11 +96,82 @@ def test_rule_snoozed(self):
 
         assert len(responses.calls) == 0
 
-    def test_fire_metric_alert(self):
+        # SLO asserts
+        # PREPARE_WEBHOOK (never got to this point)
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=0
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=0
+        )
+
+    @responses.activate
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_rule_bad_response(self, mock_record):
+        alert_rule = self.create_alert_rule()
+        incident = self.create_incident(alert_rule=alert_rule, status=IncidentStatus.CLOSED.value)
+
+        responses.add(
+            method=responses.POST,
+            url="https://example.com/webhook",
+            status=400,
+            content_type="application/json",
+            body=json.dumps({"ok": "true"}),
+        )
+
+        metric_value = 1000
+        with self.tasks():
+            self.handler.fire(
+                self.action, incident, self.project, metric_value, IncidentStatus(incident.status)
+            )
+
+        assert len(responses.calls) == 1
+
+        # SLO asserts
+        assert_halt_metric(
+            mock_record=mock_record,
+            error_msg=f"send_and_save_webhook_request.{SentryAppWebhookHaltReason.GOT_CLIENT_ERROR}_{400}",
+        )
+
+        # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (halt)
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=1
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1
+        )
+
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_fire_metric_alert(self, mock_record):
         self.run_fire_test()
 
-    def test_resolve_metric_alert(self):
+        # SLO asserts
+        assert_success_metric(mock_record)
+
+        # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success)
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
+        )
+
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_resolve_metric_alert(self, mock_record):
         self.run_fire_test("resolve")
+        # SLO asserts
+        assert_success_metric(mock_record)
+
+        # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success)
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
+        )
 
 
 @freeze_time()
@@ -169,14 +248,39 @@ def run_test(self, incident, method):
             {"name": "teamId", "value": 1},
         ]
 
-    def test_fire_metric_alert(self):
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_fire_metric_alert(self, mock_record):
         self.run_fire_test()
 
-    def test_resolve_metric_alert(self):
+        # SLO asserts
+        assert_success_metric(mock_record)
+
+        # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success)
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
+        )
+
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_resolve_metric_alert(self, mock_record):
         self.run_fire_test("resolve")
 
+        # SLO asserts
+        assert_success_metric(mock_record)
+
+        # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success)
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
+        )
+
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
     @patch("sentry.analytics.record")
-    def test_alert_sent_recorded(self, mock_record):
+    def test_alert_sent_recorded(self, mock_record, mock_record_event):
         self.run_fire_test()
         mock_record.assert_called_with(
             "alert.sent",
@@ -188,3 +292,14 @@ def test_alert_sent_recorded(self, mock_record):
             external_id=str(self.action.sentry_app_id),
             notification_uuid="",
         )
+
+        # SLO asserts
+        assert_success_metric(mock_record_event)
+
+        # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success)
+        assert_count_of_metric(
+            mock_record=mock_record_event, outcome=EventLifecycleOutcome.STARTED, outcome_count=2
+        )
+        assert_count_of_metric(
+            mock_record=mock_record_event, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
+        )

tests/sentry/integrations/services/test_integration.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 from datetime import datetime, timezone
+from unittest.mock import patch
 
 from sentry.constants import ObjectStatus
 from sentry.integrations.base import IntegrationFeatures
@@ -16,8 +17,10 @@
     serialize_integration,
     serialize_organization_integration,
 )
-from sentry.integrations.types import ExternalProviders
+from sentry.integrations.types import EventLifecycleOutcome, ExternalProviders
+from sentry.sentry_apps.models.sentry_app import SentryApp
 from sentry.silo.base import SiloMode
+from sentry.testutils.asserts import assert_count_of_metric, assert_failure_metric
 from sentry.testutils.cases import TestCase
 from sentry.testutils.helpers.datetime import freeze_time
 from sentry.testutils.silo import all_silo_test, assume_test_silo_mode
@@ -326,14 +329,28 @@ def test_update_organization_integrations(self) -> None:
             assert oi.config == new_config
             assert oi.grace_period_end is None
 
-    def test_send_incident_alert_missing_sentryapp(self) -> None:
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_send_incident_alert_missing_sentryapp(self, mock_record) -> None:
         result = integration_service.send_incident_alert_notification(
             sentry_app_id=9876,  # does not exist
             action_id=1,
             incident_id=1,
-            new_status=0,
+            new_status=10,
             metric_value=100,
             organization_id=self.organization.id,
             incident_attachment_json="{}",
         )
         assert not result
+
+        # SLO asserts
+        assert_failure_metric(
+            mock_record, SentryApp.DoesNotExist("SentryApp matching query does not exist.")
+        )
+
+        # PREPARE_WEBHOOK (failure)
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1
+        )