chore(sentry apps): Add context manager for comment webhook SLOs (#86739)

Author: Christinarlong

src/sentry/sentry_apps/tasks/sentry_apps.py

@@ -71,7 +71,7 @@
 retry_decorator = retry(
     on=(RequestException, ApiHostError, ApiTimeoutError),
     ignore=(ClientError,),
-    ignore_and_capture=(SentryAppSentryError, AssertionError),
+    ignore_and_capture=(SentryAppSentryError, AssertionError, ValueError),
 )
 
 # We call some models by a different name, publicly, than their class name.
@@ -227,15 +227,10 @@ def _process_resource_change(
         # Looks up the human name for the model. Defaults to the model name.
         name = RESOURCE_RENAMES.get(model.__name__, model.__name__.lower())
 
-    try:
-        event = SentryAppEventType(f"{name}.{action}")
-    except ValueError as e:
-        raise SentryAppSentryError(
-            message=f"{SentryAppWebhookFailureReason.INVALID_EVENT}",
-        ) from e
-
+    event = SentryAppEventType(f"{name}.{action}")
     with SentryAppInteractionEvent(
-        operation_type=SentryAppInteractionType.PREPARE_WEBHOOK, event_type=event
+        operation_type=SentryAppInteractionType.PREPARE_WEBHOOK,
+        event_type=event,
     ).capture():
         project_id: int | None = kwargs.get("project_id", None)
         group_id: int | None = kwargs.get("group_id", None)
@@ -386,11 +381,7 @@ def clear_region_cache(sentry_app_id: int, region_name: str) -> None:
 def workflow_notification(
     installation_id: int, issue_id: int, type: str, user_id: int | None, *args: Any, **kwargs: Any
 ) -> None:
-    try:
-        event = SentryAppEventType(f"issue.{type}")
-    except ValueError as e:
-        raise SentryAppSentryError(message=SentryAppWebhookFailureReason.INVALID_EVENT) from e
-
+    event = SentryAppEventType(f"issue.{type}")
     with SentryAppInteractionEvent(
         operation_type=SentryAppInteractionType.PREPARE_WEBHOOK,
         event_type=event,
@@ -417,22 +408,28 @@ def workflow_notification(
 def build_comment_webhook(
     installation_id: int, issue_id: int, type: str, user_id: int, *args: Any, **kwargs: Any
 ) -> None:
-    webhook_data = get_webhook_data(installation_id, issue_id, user_id)
-    install, _, user = webhook_data
-    data = kwargs.get("data", {})
-    project_slug = data.get("project_slug")
-    comment_id = data.get("comment_id")
-    payload = {
-        "comment_id": data.get("comment_id"),
-        "issue_id": issue_id,
-        "project_slug": data.get("project_slug"),
-        "timestamp": data.get("timestamp"),
-        "comment": data.get("comment"),
-    }
-    send_webhooks(installation=install, event=type, data=payload, actor=user)
-    # `type` is comment.created, comment.updated, or comment.deleted
+    event = SentryAppEventType(type)
+    with SentryAppInteractionEvent(
+        operation_type=SentryAppInteractionType.PREPARE_WEBHOOK,
+        event_type=event,
+    ).capture():
+        webhook_data = get_webhook_data(installation_id, issue_id, user_id)
+        install, _, user = webhook_data
+        data = kwargs.get("data", {})
+        project_slug = data.get("project_slug")
+        comment_id = data.get("comment_id")
+        payload = {
+            "comment_id": data.get("comment_id"),
+            "issue_id": issue_id,
+            "project_slug": data.get("project_slug"),
+            "timestamp": data.get("timestamp"),
+            "comment": data.get("comment"),
+        }
+
+    send_webhooks(installation=install, event=event, data=payload, actor=user)
+    # `event` is comment.created, comment.updated, or comment.deleted
     analytics.record(
-        type,
+        event,
         user_id=user_id,
         group_id=issue_id,
         project_slug=project_slug,
@@ -526,15 +523,9 @@ def notify_sentry_app(event: GroupEvent, futures: Sequence[RuleFuture]):
 
 
 def send_webhooks(installation: RpcSentryAppInstallation, event: str, **kwargs: Any) -> None:
-    try:
-        event = SentryAppEventType(event)
-    except ValueError as e:
-        raise SentryAppSentryError(
-            message=f"{SentryAppWebhookFailureReason.INVALID_EVENT}",
-        ) from e
-
     with SentryAppInteractionEvent(
-        operation_type=SentryAppInteractionType.SEND_WEBHOOK, event_type=event
+        operation_type=SentryAppInteractionType.SEND_WEBHOOK,
+        event_type=SentryAppEventType(event),
     ).capture():
         servicehook: ServiceHook
         try:

tests/sentry/sentry_apps/tasks/test_sentry_apps.py

@@ -1012,7 +1012,8 @@ def setUp(self):
             "project_slug": self.note.project.slug,
         }
 
-    def test_sends_comment_created_webhook(self, safe_urlopen):
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_sends_comment_created_webhook(self, mock_record, safe_urlopen):
         build_comment_webhook(
             self.install.id, self.issue.id, "comment.created", self.user.id, data=self.data
         )
@@ -1024,7 +1025,18 @@ def test_sends_comment_created_webhook(self, safe_urlopen):
         assert data["action"] == "created"
         assert data["data"]["issue_id"] == self.issue.id
 
-    def test_sends_comment_updated_webhook(self, safe_urlopen):
+        # SLO assertions
+        assert_success_metric(mock_record)
+        # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) -> SEND_WEBHOOK (success)
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
+        )
+
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_sends_comment_updated_webhook(self, mock_record, safe_urlopen):
         self.data.update(data={"text": "goodbye world"})
         build_comment_webhook(
             self.install.id, self.issue.id, "comment.updated", self.user.id, data=self.data
@@ -1037,7 +1049,18 @@ def test_sends_comment_updated_webhook(self, safe_urlopen):
         assert data["action"] == "updated"
         assert data["data"]["issue_id"] == self.issue.id
 
-    def test_sends_comment_deleted_webhook(self, safe_urlopen):
+        # SLO assertions
+        assert_success_metric(mock_record)
+        # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) -> SEND_WEBHOOK (success)
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
+        )
+
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_sends_comment_deleted_webhook(self, mock_record, safe_urlopen):
         self.note.delete()
         build_comment_webhook(
             self.install.id, self.issue.id, "comment.deleted", self.user.id, data=self.data
@@ -1050,6 +1073,16 @@ def test_sends_comment_deleted_webhook(self, safe_urlopen):
         assert data["action"] == "deleted"
         assert data["data"]["issue_id"] == self.issue.id
 
+        # SLO assertions
+        assert_success_metric(mock_record)
+        # PREPARE_WEBHOOK (success) -> SEND_WEBHOOK (success) -> SEND_WEBHOOK (success)
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=3
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=3
+        )
+
 
 @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance)
 class TestWorkflowNotification(TestCase):
@@ -1367,7 +1400,7 @@ def test_ignore_issue_alert(self, mock_record, safe_urlopen):
             self.sentry_app.update(status=SentryAppStatus.INTERNAL)
         data = {"issue": serialize(self.issue)}
         # event.alert is not in the servicehook events
-        with pytest.raises(SentryAppSentryError):
+        with pytest.raises(ValueError):
             for i in range(3):
                 send_webhooks(
                     installation=self.install, event="event.alert", data=data, actor=self.user