Skip to content

feat(incidents): Add a BROKEN state to QuerySubscription; include it in Detector APIs#115244

Draft
kcons wants to merge 5 commits intomasterfrom
kcons/dontjustfail
Draft

feat(incidents): Add a BROKEN state to QuerySubscription; include it in Detector APIs#115244
kcons wants to merge 5 commits intomasterfrom
kcons/dontjustfail

Conversation

@kcons
Copy link
Copy Markdown
Member

@kcons kcons commented May 8, 2026

Rather than trying continuously to create/update broken Snuba subscriptions, this introduces a 'BROKEN' state and adds methods to DataSourceHandler to allow the Detector APIs to indicate when the data source is broken.
This should allow us to make it clear in the UI that a particular Detector isn't in a good state, and potentially allow us to ask users to resolve the issue.

@kcons kcons requested review from a team as code owners May 8, 2026 22:44
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 8, 2026
Comment thread src/sentry/snuba/tasks.py
@kcons kcons requested a review from a team as a code owner May 8, 2026 23:46
@kcons kcons marked this pull request as draft May 8, 2026 23:47
with transaction.atomic(router.db_for_write(QuerySubscription)):
subscription.update(status=QuerySubscription.Status.UPDATING.value)
subscription.update(
status=QuerySubscription.Status.UPDATING.value, date_updated=timezone.now()
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.

possibly overkill but there is an auto_now=True field you can set on the django model

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.

I really like this concept - though it would be great if we weren't even allowing the detectors to be created if the subscription couldn't be created. I know sometimes the query ages out and that just causes previously valid subscriptions to become invalid though. Maybe as a follow up we can tighten up the ones that are making it through creation that are failing. I'm curious as to how we'll surface the error messages to users in a helpful way.

@kcons
Copy link
Copy Markdown
Member Author

kcons commented May 9, 2026

I really like this concept - though it would be great if we weren't even allowing the detectors to be created if the subscription couldn't be created. I know sometimes the query ages out and that just causes previously valid subscriptions to become invalid though. Maybe as a follow up we can tighten up the ones that are making it through creation that are failing. I'm curious as to how we'll surface the error messages to users in a helpful way.

Yeah, fixing our validation is some deep EAP stuff that also needs to happen for this to be actually useful. If you can submit the same broken query again, that's.. not great. I have a PR brewing for it, but I'm not sure I'll be able to see it through.

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