Skip to content

ref(snuba): Port query subscriptions consumer to taskbroker raw mode#116288

Merged
untitaker merged 1 commit into
masterfrom
markusunterwaditzer/stream-1021-port-metrics-subscriptions-consumer-to-taskbroker
May 27, 2026
Merged

ref(snuba): Port query subscriptions consumer to taskbroker raw mode#116288
untitaker merged 1 commit into
masterfrom
markusunterwaditzer/stream-1021-port-metrics-subscriptions-consumer-to-taskbroker

Conversation

@untitaker
Copy link
Copy Markdown
Member

Adds taskbroker raw mode tasks for query subscription consumers (events, transactions, metrics, generic_metrics, eap).

ref STREAM-1021

@untitaker untitaker requested review from a team as code owners May 27, 2026 13:04
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 27, 2026

STREAM-1021

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 27, 2026
@untitaker untitaker force-pushed the markusunterwaditzer/stream-1021-port-metrics-subscriptions-consumer-to-taskbroker branch from fc6c689 to 8dab57b Compare May 27, 2026 13:17
@untitaker untitaker requested a review from a team as a code owner May 27, 2026 13:17
Comment on lines +128 to +129
from sentry.snuba.query_subscriptions.consumer import handle_message
from sentry.utils import metrics
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.

Circular import?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh i misinterpreted this. you're asking why the import is in the fn, not claiming there is a circular import. the honest answer is that this is vibecoded. i'll move it out in a followup.

_process_subscription_message(message_bytes, _d)


_register_subscription_tasks()
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 dynamically construct them when you're immediately calling the function? Can these tasks be statically defined?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this dynamic code mainly enforces that PRs adding new datasets also create the respective task for it.

@untitaker
Copy link
Copy Markdown
Member Author

fun fact: entity is part of the message. so technically, we only need one task here, and the task could dispatch based on entity. we can still spawn many taskbrokers that spawn this task.

the main reason we don't want this is because then all of our observability would break. our task timings would be muddled all together.

the other reason is that this creates a weird relationship between tasks and pools. many tasks of the same type would now run on separate pools, which isn't something we do today.

@untitaker untitaker merged commit 3df62b0 into master May 27, 2026
83 checks passed
@untitaker untitaker deleted the markusunterwaditzer/stream-1021-port-metrics-subscriptions-consumer-to-taskbroker branch May 27, 2026 13:37
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