Skip to content

feat(webhooks): Add standalone legacy webhook service module#115688

Merged
Christinarlong merged 7 commits into
masterfrom
christinarlong/legacy-webhook-service
May 18, 2026
Merged

feat(webhooks): Add standalone legacy webhook service module#115688
Christinarlong merged 7 commits into
masterfrom
christinarlong/legacy-webhook-service

Conversation

@Christinarlong
Copy link
Copy Markdown
Contributor

@Christinarlong Christinarlong commented May 15, 2026

Spin out

  • webhook payload building
  • url construction from project options
  • api client
    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 sense

Context

spec
PR 2 of the legacy webhook plugin migration. Depends on PR 1 (#115669) for feature flags

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>
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 15, 2026
Comment thread src/sentry/sentry_apps/services/legacy_webhook/tasks.py
Christinarlong and others added 3 commits May 18, 2026 11:52
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>
Comment thread src/sentry/sentry_apps/services/legacy_webhook/tasks.py Outdated
Comment thread src/sentry/sentry_apps/services/legacy_webhook/tasks.py
Comment thread src/sentry/sentry_apps/services/legacy_webhook/__init__.py
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>
@Christinarlong Christinarlong marked this pull request as ready for review May 18, 2026 20:08
@Christinarlong Christinarlong requested a review from a team as a code owner May 18, 2026 20:08
@Christinarlong Christinarlong requested a review from a team May 18, 2026 20:08
Copy link
Copy Markdown
Member

@ceorourke ceorourke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a list of strings? What are the strings if they're rules?

Copy link
Copy Markdown
Contributor Author

@Christinarlong Christinarlong May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The strings are the alert/rule label. So example would be alert me on every error from python-flask for this alert

@Christinarlong Christinarlong merged commit 573dbdd into master May 18, 2026
87 checks passed
@Christinarlong Christinarlong deleted the christinarlong/legacy-webhook-service branch May 18, 2026 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants