fix(webhooks): Route sentry app actions through send_alert_webhook_v2 in new path#115975
fix(webhooks): Route sentry app actions through send_alert_webhook_v2 in new path#115975Christinarlong wants to merge 2 commits into
Conversation
… in new path Sentry app actions stored as Action.Type.WEBHOOK were incorrectly going through send_legacy_webhooks_for_invocation, which sends unsigned LegacyWebhookPayload to project webhook URLs. These should instead go through send_alert_webhook_v2 which sends a signed AppPlatformEvent to the sentry app's own webhook URL. Routes based on target_identifier: "webhooks" goes to the legacy webhook helper, anything else is treated as a sentry app slug and dispatched via send_alert_webhook_v2.
github-actions
Bot
added
the
Scope: Backend
|
|
||
| def send_sentry_app_webhook_for_invocation(invocation: ActionInvocation) -> None: | ||
| event = invocation.event_data.event | ||
| assert isinstance(event, GroupEvent) |
There was a problem hiding this comment.
Does this assertion exist in the old code? If not, do we have a strong guarantee that this will always be true? Also would help to add a message in the assertion of why we expect this to be the case.
There was a problem hiding this comment.
yes this would be guarded in the old code by the same NOA utils. We also have the guard in the action code but for typing (since we're giving ActionInvocation) we need to add the check
There was a problem hiding this comment.
Got it. You may want to consider having the webhook handler execute method break down the ActionInvocation (or have it call into a helper/transformer), that gives you all the properly typed data you need instead of doing multiple instance checks throughout the code.
2 participants
If you have select sentry app that is-alertable as TRUE as the integration to notify in the Action step but have no alert rule action UI assoc. with it. Then we'll store that action as a ActionType.WEBHOOK instead of ActionType.Sentryapp.
That's annoying for us, since the legacy Rule future/after (notify_event_service) layer did some translation to figure out whether to dispatch to plugin or sentry app task. But since we're trying to sidestep the NOA layer by connecting directly to the Action we need to do some of the control flow ourselves/copy it over.
Also the steps to try and migrate sentry apps using webhook action types over to be actiontype.sentryapp would be insane and blow our timeline out so lets not do that.