feat(webhooks): Add payload validation during dual-write migration#116040
Merged
Conversation
…param Move _ParityChecker and _qualify from testutils/helpers/serializer_parity.py to sentry/utils/payload_comparison.py so it can be used by production code. Changes from the original: - Renamed _ParityChecker -> PayloadComparator, _qualify -> qualify - Added format_value parameter (defaults to repr, preserving current behavior) - Added describe_value() for PII-safe value formatting in production serializer_parity.py now imports from the new location; all existing callers of assert_serializer_parity are unaffected.
github-actions
Bot
added
the
Scope: Backend
Move _ParityChecker and _qualify from testutils/helpers/serializer_parity.py to sentry/utils/payload_comparison.py so it can be used by production code. Changes from the original: - Added format_value parameter (defaults to repr, preserving current behavior) - Added describe_value() for PII-safe value formatting in production serializer_parity.py now imports from the new location; all existing callers of assert_serializer_parity are unaffected.
Christinarlong
force-pushed
the
crl/webhook-payload-validation-usage
branch
from
8c1a7d6 to
c3948de
Compare
ParityChecker only recursed into nested structures when known_diffs or unreliable field paths referenced something inside. When known_diffs is empty (production validation case), differing nested dicts were reported as a single flat mismatch instead of drilling into specific fields. Now always recurses into Mapping and list types.
During the dual-write window (new path on, old path not disabled), build both old-path and new-path payloads and compare them using PayloadComparator with PII-safe formatting. Validation runs after both paths have dispatched and never blocks webhook delivery. Logs legacy_webhook.validation.match on parity, or legacy_webhook.validation.payload_mismatch per field difference with structural metadata only (no actual values).
…om_action create_rule_instance_from_action calls build_rule_action_blob which requires an integration_id mapping that webhooks don't have. Use _get_triggering_rule_name directly — it does the same workflow->rule label lookup without the integration_id requirement. Also fixes mypy: dict -> dict[str, Any] in test return types.
TypedDict is not assignable to dict[str, Any] in mypy. Use Mapping[str, Any] for old_payload (from get_group_data) and LegacyWebhookPayload for new_payload (from build_legacy_webhook_payload).
Christinarlong
force-pushed
the
crl/webhook-payload-validation-usage
branch
from
d566f1a to
581d107
Compare
Comment on lines
+26
to
+27
| old_payload = WebHooksPlugin().get_group_data(group, event, [rule_name]) | ||
| new_payload = build_legacy_webhook_payload(invocation) |
Member
There was a problem hiding this comment.
How expensive are these? Are these doing any queries that could add additional latency into our delivery process?
This may be another place we want to have an option to enable/disable these checks.
Contributor
Author
There was a problem hiding this comment.
I dont think these are expensive at all since we're just extracting fields for old and new payload. The more expensive thing would be the amount of recurring we're doing on the event payload. Yeah option makes sense.
Also this validation would happen after the old and new path webhooks have been sent out so it wouldn't affect webhook delivery
markstory
reviewed
May 25, 2026
…n log Change payload validation option from bool to float-based rollout rate so we can gradually increase validation coverage. Remove the logger.info call from the dry run path to avoid logging payload contents.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a90e775. Configure here.
leeandher
approved these changes
May 26, 2026
Member
leeandher
left a comment
leeandher
left a comment
There was a problem hiding this comment.
few comments, but makes sense!
…ndling - Add logging_context variable to avoid redeclaring shared context - Narrow try/except to only wrap compare_payloads call - Log all mismatches in a single warning instead of one per mismatch - Add test for comparison error exception handling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use the ParityChecker on when dual writing to validate the old and new path are outputting the same payload w/o PII exposed
relies on #116038