Skip to content

feat(webhooks): Wire new service with feature-flagged routing#115747

Merged
Christinarlong merged 8 commits into
masterfrom
christinarlong/legacy-webhook-handler-wiring
May 19, 2026
Merged

feat(webhooks): Wire new service with feature-flagged routing#115747
Christinarlong merged 8 commits into
masterfrom
christinarlong/legacy-webhook-handler-wiring

Conversation

@Christinarlong
Copy link
Copy Markdown
Contributor

@Christinarlong Christinarlong commented May 18, 2026

Connects the dual write flags and new service together. See table below for what the NOA will do based on the flag setup.

new-path disable-old dry-run New path sends Old path sends Net result
OFF OFF N/A no yes Old path only (default)
ON OFF OFF yes yes Dual-write
ON OFF ON logs only yes Old path sends + new path logs
ON ON OFF yes no New path only (cutover)
ON ON ON logs only no Dry-run only (nothing sends)
OFF ON N/A no no Nothing fires

Context

PR 3 of the legacy webhook plugin migration. Depends on PR 2 (#115688).

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 18, 2026
Base automatically changed from christinarlong/legacy-webhook-service to master May 18, 2026 20:55
Christinarlong and others added 2 commits May 18, 2026 13:58
…routing

Wire the new webhook service into the invocation path with three
feature flags controlling the rollout:

- legacy-webhook-new-path ON → dispatches via new service
- legacy-webhook-disable-old-path ON → skips old plugin path
- Both OFF (default) → old path only, no behavior change

The dry-run flag is checked at the task level, so the handler
always calls send_legacy_webhooks_for_project when new-path is on.

Truth table:
  new-path OFF, disable-old OFF → old path only (default)
  new-path ON, disable-old OFF  → both paths (dual-write)
  new-path ON, disable-old ON   → new path only (cutover)
  new-path OFF, disable-old ON  → guard: old path still fires

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
…routing

Wire the new webhook service into the invocation path with feature
flags controlling the rollout:

- legacy-webhook-new-path ON → dispatches via new service
- legacy-webhook-disable-old-path ON → skips old plugin path
- legacy-webhook-dry-run ON → task logs instead of sending

Refactors:
- build_legacy_webhook_payload accepts ActionInvocation, derives
  group/event/triggering_rules internally
- send_legacy_webhooks_for_invocation accepts ActionInvocation
- Handler logic simplified: old path runs unless disable_old is on
- Tests run e2e through IssueAlertRegistryHandler with plugin
  enabled and self.tasks() for eager task execution

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
@Christinarlong Christinarlong force-pushed the christinarlong/legacy-webhook-handler-wiring branch from ca332ac to c229ff8 Compare May 18, 2026 22:02
…bhook_payload

The event from ActionInvocation.event_data is typed as GroupEvent | Activity,
but Activity lacks get_tag, message, tags, and event_id attributes. Activity
events are already filtered upstream in execute_via_group_type_registry, so
add an isinstance check to narrow the type for mypy.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
@Christinarlong Christinarlong marked this pull request as ready for review May 18, 2026 23:43
@Christinarlong Christinarlong requested a review from a team as a code owner May 18, 2026 23:43
@Christinarlong Christinarlong requested a review from a team May 18, 2026 23:43
…handler

Move feature-flagged routing from WebhookIssueAlertHandler.invoke_legacy_registry
up to WebhookActionHandler.execute so the new path calls
send_legacy_webhooks_for_invocation directly, bypassing the NOA registry
dispatch chain. The old path still falls through to
execute_via_group_type_registry when not disabled.
…ent events

Legacy webhooks only apply to error events. Add an isinstance check for
GroupEvent at the top of execute() so non-GroupEvent events (e.g. Activity)
are skipped before reaching either the new or old path.
Move the isinstance check so it only gates the new-path call, not the
entire function. Non-GroupEvent events (e.g. Activity) still reach
execute_via_group_type_registry which handles them in the old path.
Comment thread src/sentry/sentry_apps/services/legacy_webhook/service.py
…en available

_get_triggering_rule_name now checks AlertRuleWorkflow for a linked Rule
and uses Rule.label if found, matching the old path's behavior in
create_rule_instance_from_action. Falls back to workflow.name otherwise.
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4a1b5fd. Configure here.

Run the old path first and catch exceptions so the new path still
executes during dual-write. The new path runs second and can bubble
exceptions normally since nothing depends on it.
@Christinarlong Christinarlong merged commit 9de9bf8 into master May 19, 2026
62 checks passed
@Christinarlong Christinarlong deleted the christinarlong/legacy-webhook-handler-wiring branch May 19, 2026 18:24
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.

3 participants