feat(webhooks): Add standalone legacy webhook service module#115688
Merged
Conversation
Create the new webhook service that will replace the legacy plugin: - LegacyWebhookClient: HTTP client extending BaseApiClient with identical behavior to the plugin's WebhookApiClient (no SSL verify, no redirects, 5s timeout, JSON POST) - build_legacy_webhook_payload: exact clone of WebHooksPlugin.get_group_data() - send_legacy_webhooks_for_project: reads webhooks:urls ProjectOption, fans out one Celery task per URL - log_legacy_webhook_dry_run: builds payload and logs without sending - send_legacy_webhook_task: instrumented Celery task with retry config All code is dead (no callers yet) — wiring happens in a follow-up PR. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Check the legacy-webhook-dry-run feature flag inside the task instead of the service layer. This tests more of the system end-to-end: the task fires, looks up the org, checks the flag, and logs instead of sending when dry-run is enabled. - Remove log_legacy_webhook_dry_run from service.py - Add organization_id param to send_legacy_webhook_task - Task checks legacy-webhook-dry-run flag and logs if enabled - Update tests accordingly Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
- Pass project_id instead of organization_id to the task; look up the organization via Project.objects.get_from_cache(id=project_id) - Switch retry to on=(Exception,) to fix Warden comment - Log the full payload in dry-run mode, not just keys - Keep delayed import in service.py to avoid circular dependency (tasks.py imports LegacyWebhookPayload from service.py) Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
- Remove project_id param from task; look up organization via Group.objects.get(id=payload["id"]) instead - Restore ignore and silenced_exceptions for RestrictedIPAddress, ConnectionError, ReadTimeout, ApiError - Update tests to match simplified task signature Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Return early with a warning log if the group has been deleted between dispatch and task execution. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
ceorourke
approved these changes
May 18, 2026
Member
ceorourke
left a comment
There was a problem hiding this comment.
Seems reasonable esp compared to the current implementation. I left some nits but idk if they are going through, it's being weirdly slow.
| culprit: str | None | ||
| message: str | ||
| url: str | ||
| triggering_rules: list[str] |
Member
There was a problem hiding this comment.
Why is this a list of strings? What are the strings if they're rules?
Contributor
Author
There was a problem hiding this comment.
The strings are the alert/rule label. So example would be alert me on every error from python-flask for this alert
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
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.
Spin out
into separate files under
src/sentry/sentry_apps/services/legacy_webhook/<- honestly could change the location but wasn't sure which of sentry_apps/ , integrations/ or a whole new module made the most senseContext
spec
PR 2 of the legacy webhook plugin migration. Depends on PR 1 (#115669) for feature flags