fix(taskworker) Remove pickle usage from sentryapps task (#91372)

The `event` parameter of process_service_hook requires pickle. By
generating the payload before spawning a task, we don't have to use
pickle.

Refs SENTRY-3V47

Author: markstory

src/sentry/sentry_apps/tasks/service_hooks.py

@@ -1,5 +1,6 @@
 import logging
 from time import time
+from typing import Any
 
 from sentry.api.serializers import serialize
 from sentry.http import safe_urlopen
@@ -46,16 +47,24 @@ def get_payload_v0(event):
     ),
 )
 @retry
-def process_service_hook(servicehook_id, event, **kwargs):
+def process_service_hook(
+    servicehook_id: int,
+    event: Any,
+    payload: str | None = None,
+    project_id: int | None = None,
+    **kwargs,
+):
     try:
         servicehook = ServiceHook.objects.get(id=servicehook_id)
     except ServiceHook.DoesNotExist:
         return
 
-    if servicehook.version == 0:
-        payload = get_payload_v0(event)
-    else:
-        raise NotImplementedError
+    project_id = project_id or event.project_id
+    if not payload:
+        if servicehook.version == 0:
+            payload = json.dumps(get_payload_v0(event))
+        else:
+            raise NotImplementedError
 
     from sentry import tsdb
 
@@ -65,10 +74,8 @@ def process_service_hook(servicehook_id, event, **kwargs):
         "Content-Type": "application/json",
         "X-ServiceHook-Timestamp": str(int(time())),
         "X-ServiceHook-GUID": servicehook.guid,
-        "X-ServiceHook-Signature": servicehook.build_signature(json.dumps(payload)),
+        "X-ServiceHook-Signature": servicehook.build_signature(payload),
     }
 
-    safe_urlopen(
-        url=servicehook.url, data=json.dumps(payload), headers=headers, timeout=5, verify_ssl=False
-    )
-    logger.info("service_hook.success", extra={"project_id": event.project_id})
+    safe_urlopen(url=servicehook.url, data=payload, headers=headers, timeout=5, verify_ssl=False)
+    logger.info("service_hook.success", extra={"project_id": project_id})

src/sentry/tasks/post_process.py

@@ -72,7 +72,7 @@ class PostProcessJob(TypedDict, total=False):
     has_escalated: bool
 
 
-def _get_service_hooks(project_id):
+def _get_service_hooks(project_id: int) -> list[tuple[int, list[str]]]:
     from sentry.sentry_apps.models.servicehook import ServiceHook
 
     cache_key = f"servicehooks:1:{project_id}"
@@ -1180,7 +1180,7 @@ def process_service_hooks(job: PostProcessJob) -> None:
     if job["is_reprocessed"]:
         return
 
-    from sentry.sentry_apps.tasks.service_hooks import process_service_hook
+    from sentry.sentry_apps.tasks.service_hooks import get_payload_v0, process_service_hook
 
     event, has_alert = job["event"], job["has_alert"]
 
@@ -1189,10 +1189,17 @@ def process_service_hooks(job: PostProcessJob) -> None:
         if has_alert:
             allowed_events.add("event.alert")
 
-        if allowed_events:
-            for servicehook_id, events in _get_service_hooks(project_id=event.project_id):
-                if any(e in allowed_events for e in events):
-                    process_service_hook.delay(servicehook_id=servicehook_id, event=event)
+        # TODO: when we have multiple service hook versions, this will need to change.
+        payload = json.dumps(get_payload_v0(event))
+
+        for servicehook_id, events in _get_service_hooks(project_id=event.project_id):
+            if any(e in allowed_events for e in events):
+                process_service_hook.delay(
+                    servicehook_id=servicehook_id,
+                    project_id=event.project_id,
+                    event=event,
+                    payload=payload,
+                )
 
 
 def process_resource_change_bounds(job: PostProcessJob) -> None:

tests/sentry/tasks/test_post_process.py

@@ -99,6 +99,23 @@ def __eq__(self, other):
         return matching_id
 
 
+class ServiceHookPayloadMatcher:
+    def __init__(self, event: Event):
+        self.event = event
+
+    def __eq__(self, other: Any) -> bool:
+        assert isinstance(other, str), "other should be a string"
+        assert self.event.group
+
+        payload = json.loads(other)
+        assert payload["event"]["id"] == self.event.event_id
+        assert payload["group"]["id"] == str(self.event.group.id)
+        assert "url" in payload["group"]
+        assert "tags" in payload["event"]
+        assert "project" in payload
+        return True
+
+
 class BasePostProgressGroupMixin(BaseTestCase, metaclass=abc.ABCMeta):
     @abc.abstractmethod
     def create_event(self, data, project_id, assert_no_errors=True):
@@ -518,7 +535,10 @@ def test_service_hook_fires_on_new_event(self, mock_process_service_hook):
             )
 
         mock_process_service_hook.delay.assert_called_once_with(
-            servicehook_id=hook.id, event=EventMatcher(event)
+            servicehook_id=hook.id,
+            project_id=self.project.id,
+            event=EventMatcher(event),
+            payload=ServiceHookPayloadMatcher(event),
         )
 
     @patch("sentry.sentry_apps.tasks.service_hooks.process_service_hook")
@@ -547,7 +567,10 @@ def test_service_hook_fires_on_alert(self, mock_processor, mock_process_service_
             )
 
         mock_process_service_hook.delay.assert_called_once_with(
-            servicehook_id=hook.id, event=EventMatcher(event)
+            servicehook_id=hook.id,
+            project_id=self.project.id,
+            event=EventMatcher(event),
+            payload=ServiceHookPayloadMatcher(event),
         )
 
     @patch("sentry.sentry_apps.tasks.service_hooks.process_service_hook")