Skip to content

feat(notifications): Instrument killswitch mechanism for notifications#114505

Merged
hobzcalvin merged 2 commits intomasterfrom
ISWF-2178
May 4, 2026
Merged

feat(notifications): Instrument killswitch mechanism for notifications#114505
hobzcalvin merged 2 commits intomasterfrom
ISWF-2178

Conversation

@hobzcalvin
Copy link
Copy Markdown
Contributor

resolves ISWF-2178

Adds notifications.platform.killswitch.sources option key which can be set in options automator with string values corresponding to notification sources we want to block sending. Blocks in notify_target, notify_async, and notify_target_async.

@hobzcalvin hobzcalvin requested review from a team as code owners April 30, 2026 22:13
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Apr 30, 2026

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 30, 2026
Even when the killswitch is added after the task is enqueued, the task
itself must short-circuit before sending.
"""
service = NotificationService(data=MockNotification(message="test"))
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.

Iirc, tasks don't run by default, so you may be missing a with task_runner() decorator invocation to ensure we actually flush the attempted send. See:

It could help to have an assertion that validates that we step through at least some of the dispatch code.

Similarly, we don't need to enqueue the task manually in this case since the invocation is deferred already.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

with tasks() returns TaskRunner which returns the same callable:

return TaskRunner()

and I can confirm the test breaks / alert() is called if the killswitch value doesn't match.

Adding assert that the killswitch log message fires, here and elsewhere.

notify_async is called in the test above; the enqueue is manual here to ensure that the killswitch inside notify_target_async also works:

if notification_data.source in sentry_options.get(KILLSWITCH_OPTION_KEY):

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.

Totally missed that, change looks good!

hobzcalvin added a commit that referenced this pull request May 4, 2026
While testing #114505 it was helpful to use e.g. `sentry notifications
send email --source 'data-export-success'` without having an actual
email backend set up. This turns the exception into a warning.
@hobzcalvin hobzcalvin merged commit 02cfbd6 into master May 4, 2026
82 checks passed
@hobzcalvin hobzcalvin deleted the ISWF-2178 branch May 4, 2026 20:08
cleptric pushed a commit that referenced this pull request May 5, 2026
While testing #114505 it was helpful to use e.g. `sentry notifications
send email --source 'data-export-success'` without having an actual
email backend set up. This turns the exception into a warning.
cleptric pushed a commit that referenced this pull request May 5, 2026
#114505)

resolves ISWF-2178

Adds `notifications.platform.killswitch.sources` option key which can be
set in options automator with string values corresponding to
notification sources we want to block sending. Blocks in
`notify_target`, `notify_async`, and `notify_target_async`.
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