fix(code-review): Add cache to dedupe github webhook events by suejung-sentry · Pull Request #107734 · getsentry/sentry

suejung-sentry · 2026-02-05T23:03:56Z

This PR handles webhook delivery deduplication by introducing redis idempotency keys for the github webhook id.

GitHub guarantees "at-least-once" delivery so may send duplicate webhooks. We have seen anecdotally that seer can receive multiple requests for a single commit (from a pull_request.synchronize event) within 500 milliseconds of each other (redash).

It's unclear whether GitHub is delivering the webhook twice or something in our control-->regional forwarding queues is causing redelivery. In any case, it seems likely that the same payload is getting processed with the same github webhook id. So use that as the idempotency key.

I considered whether we should go for a lock instead. The downside of that is the lock would release after the function returns, which may happen sooner than the 500 milliseconds we are currently seeing dupes in.

So instead in this PR, we just say any webhook with the same webhook id delivered within the same 20 second window are not replayed and re-forwarded to seer.

I chose 20 second TTL to cover the 500 milliseconds period and any errant github retry+backoff behavior.

Redis should be able to handle this load which is one SET per github webhook that makes it past our "preflight" feature enablement filters (a max hour is around 2,000 code reviews, so say 1 request per second in a peak hour). Also the keys are small with a short TTL.

Closes CW-673

suejung-sentry · 2026-02-05T23:48:14Z

src/sentry/seer/code_review/webhooks/handlers.py

+
+
+def _get_webhook_seen_cluster() -> RedisCluster[str] | StrictRedis[str]:
+    return redis_clusters.get("default")


used similar pattern as done here

Thank you for adding references to the patterns you use 👍🏻

suejung-sentry · 2026-02-06T02:40:48Z

src/sentry/seer/code_review/webhooks/handlers.py


 logger = logging.getLogger(__name__)

+WEBHOOK_SEEN_TTL_SECONDS = 20


I picked 20 seconds because per the redash, if there ever is a dupe, they happen within 500 milliseconds of each other. I thought 20 seconds would comfortably cover that and any errant github redelivery behavior

Thank you for the source and speaking out your reasoning. Makes sense 👍🏻

linear · 2026-02-06T03:05:46Z

CW-673 [investigation] Seer processes duplicate requests despite idempotency check

vaind

While I'm not familiar with this part of the codebase, this looks reasonable to me and saves quite a few runs 👍 . Would be good to get another set of eyes on this though.

vaind · 2026-02-06T07:24:18Z

src/sentry/integrations/github/webhook.py

+            if github_delivery_id is not None:
+                github_delivery_id = str(github_delivery_id)
+            sentry_sdk.set_extra("github_delivery_id", github_delivery_id)


Does it make sense to add to context if it's None?

Suggested change

if github_delivery_id is not None:

github_delivery_id = str(github_delivery_id)

sentry_sdk.set_extra("github_delivery_id", github_delivery_id)

if github_delivery_id is not None:

github_delivery_id = str(github_delivery_id)

sentry_sdk.set_extra("github_delivery_id", github_delivery_id)

You could still have the filter in Sentry if you want to find events without it.

For the record, this header will never be missing (unless GitHub introduces a bug in their code).

vaind · 2026-02-06T07:30:57Z

tests/sentry/seer/code_review/webhooks/test_handlers.py

+    def test_same_delivery_id_second_seen_skipped(self) -> None:
+        """
+        Two deliveries with the same id, one after the other: the first marks seen and runs;
+        the second is already seen (key exists), so only one handler run.
+        """


Do we need both this test and test_webhook_already_seen_handler_not_invoked?

armenzg · 2026-02-06T12:08:46Z

src/sentry/integrations/github/webhook.py


    def __call__(self, event: Mapping[str, Any], **kwargs: Any) -> None:
        github_event = kwargs["github_event"]
+        github_delivery_id = kwargs.get("github_delivery_id")


As reference:

X-GitHub-Delivery: A globally unique identifier (GUID) to identify the event.

armenzg · 2026-02-06T12:10:58Z

src/sentry/integrations/github/webhook.py

+            if github_delivery_id is not None:
+                github_delivery_id = str(github_delivery_id)
+            sentry_sdk.set_extra("github_delivery_id", github_delivery_id)


You could still have the filter in Sentry if you want to find events without it.

For the record, this header will never be missing (unless GitHub introduces a bug in their code).

armenzg · 2026-02-06T12:13:14Z

src/sentry/seer/code_review/webhooks/handlers.py

+
+
+def _get_webhook_seen_cluster() -> RedisCluster[str] | StrictRedis[str]:
+    return redis_clusters.get("default")


Thank you for adding references to the patterns you use 👍🏻

armenzg · 2026-02-06T12:14:42Z

src/sentry/seer/code_review/webhooks/handlers.py


 logger = logging.getLogger(__name__)

+WEBHOOK_SEEN_TTL_SECONDS = 20


Thank you for the source and speaking out your reasoning. Makes sense 👍🏻

armenzg · 2026-02-06T12:21:21Z

tests/sentry/seer/code_review/webhooks/test_handlers.py

+        delivery_id = f"already-seen-{uuid4()}"
+        cluster = redis_clusters.get("default")
+        seen_key = f"{WEBHOOK_SEEN_KEY_PREFIX}{delivery_id}"
+        cluster.set(seen_key, "1", ex=WEBHOOK_SEEN_TTL_SECONDS, nx=True)


A more realistic test would be to simply call handle_webhook_event twice.
That way if the code managing the cluster changes the test would be exercise the logic within handle_webhook_event.

Perhaps delete this test and use test_same_delivery_id_second_seen_skipped since it does what I mention.

fix(code-review): Add lock to dedupe github webhook events

da51e64

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 5, 2026

vercel bot deployed to Preview February 5, 2026 23:06 View deployment

use redis idempotency key instead of lock

84149fb

suejung-sentry changed the title ~~fix(code-review): Add lock to dedupe github webhook events~~ fix(code-review): Add cache to dedupe github webhook events Feb 5, 2026

suejung-sentry commented Feb 5, 2026

View reviewed changes

vercel bot deployed to Preview February 5, 2026 23:48 View deployment

vercel bot deployed to Preview February 6, 2026 00:21 View deployment

fix tests

0230d2e

suejung-sentry force-pushed the sshin/dedupe-gh-webhooks branch from 724dc45 to 0230d2e Compare February 6, 2026 02:31

vercel bot deployed to Preview February 6, 2026 02:33 View deployment

suejung-sentry commented Feb 6, 2026

View reviewed changes

suejung-sentry marked this pull request as ready for review February 6, 2026 03:07

suejung-sentry requested a review from a team as a code owner February 6, 2026 03:07

vaind approved these changes Feb 6, 2026

View reviewed changes

armenzg approved these changes Feb 6, 2026

View reviewed changes



		def _get_webhook_seen_cluster() -> RedisCluster[str] \| StrictRedis[str]:
		return redis_clusters.get("default")


		logger = logging.getLogger(__name__)

		WEBHOOK_SEEN_TTL_SECONDS = 20

Uh oh!

Conversation

suejung-sentry commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linear bot commented Feb 6, 2026

Uh oh!

vaind left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

suejung-sentry commented Feb 5, 2026 •

edited

Loading