ref(hybridcloud): Read webhook bucket keys through one helper

src/sentry/integrations/middleware/hybrid_cloud/parser.py

@@ -2,6 +2,7 @@
 
 import logging
 from abc import ABC
+from collections.abc import Mapping
 from concurrent.futures import as_completed
 from typing import TYPE_CHECKING, Any, ClassVar
 
@@ -37,6 +38,7 @@
 from sentry.types.cell import Cell, find_cells_for_org_mappings, get_cell_by_name
 from sentry.utils import metrics
 from sentry.utils.concurrent import ContextPropagatingThreadPoolExecutor
+from sentry.utils.safe import get_path
 
 logger = logging.getLogger(__name__)
 if TYPE_CHECKING:
@@ -390,6 +392,18 @@ def mailbox_bucket_id(self, data: dict[str, Any]) -> int | None:
             "You must implement mailbox_bucket_id to use bucketed identifiers"
         )
 
+    @staticmethod
+    def bucket_key_at(data: Mapping[str, Any], *path: str) -> int | None:
+        """Read a bucket key out of `data`, or None if it is missing or not numeric.
+
+        Shared so every provider degrades the same way rather than raising out of the
+        parser on a body it did not expect.
+        """
+        try:
+            return int(get_path(data, *path))
+        except (TypeError, ValueError):
+            return None
+
     def _mailbox_event_type(self, data: dict[str, Any]) -> str | None:
         """Validation lives here, not in the subclass: the discriminator comes out of
         a body control has not verified — gitlab and bitbucket resolve their handlers

src/sentry/middleware/integrations/parsers/github.py

@@ -73,17 +73,9 @@ def _get_external_id(self, event: Mapping[str, Any]) -> str | None:
         return get_github_external_id(event)
 
     def mailbox_bucket_id(self, data: Mapping[str, Any]) -> int | None:
-        """Hash on repository ID to distribute webhooks across sub-mailboxes.
-
-        GitHub webhook payloads include repository.id for most event types.
-        Installation events are routed to control silo and don't reach this path.
-        """
-        repository = data.get("repository")
-        if isinstance(repository, dict):
-            repo_id = repository.get("id")
-            if isinstance(repo_id, int):
-                return repo_id
-        return None
+        """Every event type that reaches a cell carries `repository.id`; installation
+        events are handled on control and never get here."""
+        return self.bucket_key_at(data, "repository", "id")
 
     def mailbox_event_type(self, data: Mapping[str, Any]) -> str | None:
         return self.request.META.get(GITHUB_WEBHOOK_TYPE_HEADER)

src/sentry/middleware/integrations/parsers/gitlab.py

@@ -84,15 +84,8 @@ def get_response_from_gitlab_webhook(self) -> HttpResponseBase:
         )
 
     def mailbox_bucket_id(self, data: Mapping[str, Any]) -> int | None:
-        """
-        Used by get_mailbox to find the project.id a payload is for.
-        In high volume gitlab instances we shard messages by project for greater
-        delivery throughput.
-        """
-        project_id = data.get("project", {}).get("id", None)
-        if not project_id:
-            return None
-        return project_id
+        """Every event kind a cell processes names the project it belongs to."""
+        return self.bucket_key_at(data, "project", "id")
 
     def mailbox_event_type(self, data: Mapping[str, Any]) -> str | None:
         """Reads the body's `object_kind`, not the `X-Gitlab-Event` header the

src/sentry/middleware/integrations/parsers/jira.py

@@ -28,7 +28,6 @@
     parse_integration_from_request,
 )
 from sentry.shared_integrations.exceptions import ApiError
-from sentry.utils.safe import get_path
 
 logger = logging.getLogger(__name__)
 
@@ -97,7 +96,4 @@ def mailbox_bucket_id(self, data: Mapping[str, Any]) -> int | None:
         """The Connect descriptor registers only `jira:issue_updated`, so the issue is
         the only axis a Jira mailbox can be split on.
         """
-        try:
-            return int(get_path(data, "issue", "id"))
-        except (TypeError, ValueError):
-            return None
+        return self.bucket_key_at(data, "issue", "id")

src/sentry/middleware/integrations/parsers/jira_server.py

@@ -57,18 +57,8 @@ def get_response_from_issue_update_webhook(self) -> HttpResponseBase:
         )
 
     def mailbox_bucket_id(self, data: Mapping[str, Any]) -> int | None:
-        """
-        Used by get_mailbox to find the issue.id a payload is for.
-        In high volume jira_server instances we shard messages by issue for greater
-        delivery throughput.
-        """
-        issue_id = data.get("issue", {}).get("id", None)
-        if not issue_id:
-            return None
-        try:
-            return int(issue_id)
-        except ValueError:
-            return None
+        """Only changelog webhooks reach a cell, and each names its issue."""
+        return self.bucket_key_at(data, "issue", "id")
 
     def get_response(self) -> HttpResponseBase:
         if self.view_class == JiraServerIssueUpdatedWebhook:

src/sentry/middleware/integrations/parsers/vsts.py

@@ -14,7 +14,6 @@
 from sentry.integrations.types import IntegrationProviderSlug
 from sentry.integrations.vsts.webhooks import WorkItemWebhook, get_vsts_external_id
 from sentry.silo.base import control_silo_function
-from sentry.utils.safe import get_path
 
 logger = logging.getLogger(__name__)
 
@@ -65,7 +64,4 @@ def mailbox_bucket_id(self, data: Mapping[str, Any]) -> int | None:
         """The subscription is created for `workitem.updated` only, so the work item
         is the only axis a VSTS mailbox can be split on.
         """
-        try:
-            return int(get_path(data, "resource", "workItemId"))
-        except (TypeError, ValueError):
-            return None
+        return self.bucket_key_at(data, "resource", "workItemId")

tests/sentry/integrations/middleware/hybrid_cloud/test_base.py

@@ -184,6 +184,20 @@ def mailbox_bucket_id(self, data: dict[str, Any]) -> int | None:
         ):
             assert str(parser.get_mailbox(integration, {})) == f"test_provider:{integration.id}:77"
 
+    def test_bucket_key_at_coerces_or_falls_back(self) -> None:
+        at = BaseRequestParser.bucket_key_at
+
+        assert at({"issue": {"id": 10237}}, "issue", "id") == 10237
+        assert at({"issue": {"id": "10237"}}, "issue", "id") == 10237
+
+        # Anything unusable falls back rather than raising at the modulo.
+        assert at({}, "issue", "id") is None
+        assert at({"issue": {}}, "issue", "id") is None
+        assert at({"issue": "PROJ-1"}, "issue", "id") is None
+        assert at({"issue": {"id": None}}, "issue", "id") is None
+        assert at({"issue": {"id": "not-a-number"}}, "issue", "id") is None
+        assert at({"issue": {"id": ["10237"]}}, "issue", "id") is None
+
     def test_get_mailbox_always_bucket_skips_volume_check(self) -> None:
         class AlwaysBucketedParser(ExampleRequestParser):
             always_bucket = True

tests/sentry/middleware/integrations/parsers/test_github.py

@@ -284,7 +284,7 @@ def test_issue_deleted_routing(self) -> None:
                 "installation": {"id": "1"},
                 "issue": {"id": "1"},
                 "action": "deleted",
-                "repository": {"id": "1"},
+                "repository": {"id": 1},
             },
             content_type="application/json",
             headers={"X-GITHUB-EVENT": GithubWebhookType.ISSUE.value},
@@ -298,7 +298,7 @@ def test_issue_deleted_routing(self) -> None:
         assert len(responses.calls) == 0
         assert_webhook_payloads_for_mailbox(
             request=request,
-            mailbox_name=f"github:{integration.id}:issues",
+            mailbox_name=f"github:{integration.id}:1:issues",
             cell_names=[cell.name],
             destination_types={DestinationType.SENTRY_CELL: 1},
         )

tests/sentry/middleware/integrations/parsers/test_gitlab.py

@@ -58,6 +58,23 @@ def run_parser(self, request):
         parser = GitlabRequestParser(request=request, response_handler=self.get_response)
         return parser.get_response()
 
+    def test_mailbox_bucket_id(self) -> None:
+        request = self.factory.post(
+            self.path,
+            data=PUSH_EVENT,
+            content_type="application/json",
+            HTTP_X_GITLAB_TOKEN=WEBHOOK_TOKEN,
+            HTTP_X_GITLAB_EVENT="Push Hook",
+        )
+        parser = GitlabRequestParser(request=request, response_handler=self.get_response)
+
+        assert parser.mailbox_bucket_id({"project": {"id": 15}}) == 15
+        assert parser.mailbox_bucket_id({"project": {"id": "15"}}) == 15
+        assert parser.mailbox_bucket_id({}) is None
+        assert parser.mailbox_bucket_id({"project": {}}) is None
+        assert parser.mailbox_bucket_id({"project": "sentry"}) is None
+        assert parser.mailbox_bucket_id({"project": {"id": "sentry"}}) is None
+
     @override_settings(SILO_MODE=SiloMode.CONTROL)
     @override_cells(cell_config)
     def test_missing_x_gitlab_token(self) -> None: