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

This reverts commit 81d0180.

Co-authored-by: markstory <24086+markstory@users.noreply.github.com>

Author: 2 people

src/sentry/sentry_apps/tasks/service_hooks.py

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

src/sentry/tasks/post_process.py

@@ -72,7 +72,7 @@ class PostProcessJob(TypedDict, total=False):
     has_escalated: bool
 
 
-def _get_service_hooks(project_id: int) -> list[tuple[int, list[str]]]:
+def _get_service_hooks(project_id):
     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 get_payload_v0, process_service_hook
+    from sentry.sentry_apps.tasks.service_hooks import process_service_hook
 
     event, has_alert = job["event"], job["has_alert"]
 
@@ -1189,17 +1189,10 @@ def process_service_hooks(job: PostProcessJob) -> None:
         if has_alert:
             allowed_events.add("event.alert")
 
-        # 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,
-                )
+        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)
 
 
 def process_resource_change_bounds(job: PostProcessJob) -> None:

tests/sentry/tasks/test_post_process.py

@@ -99,23 +99,6 @@ 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):
@@ -535,10 +518,7 @@ 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,
-            project_id=self.project.id,
-            event=EventMatcher(event),
-            payload=ServiceHookPayloadMatcher(event),
+            servicehook_id=hook.id, event=EventMatcher(event)
         )
 
     @patch("sentry.sentry_apps.tasks.service_hooks.process_service_hook")
@@ -567,10 +547,7 @@ 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,
-            project_id=self.project.id,
-            event=EventMatcher(event),
-            payload=ServiceHookPayloadMatcher(event),
+            servicehook_id=hook.id, event=EventMatcher(event)
         )
 
     @patch("sentry.sentry_apps.tasks.service_hooks.process_service_hook")