Skip to content

Add headers and hooks to taskbroker client#587

Open
gricha wants to merge 4 commits intomainfrom
tasbroker-client-headers
Open

Add headers and hooks to taskbroker client#587
gricha wants to merge 4 commits intomainfrom
tasbroker-client-headers

Conversation

@gricha
Copy link
Copy Markdown
Member

@gricha gricha commented Apr 3, 2026

Relevant context: https://www.notion.so/sentry/RFC-Unified-ViewerContext-via-ContextVar-32f8b10e4b5d81988625cb5787035e02

I think if we want to implicitly pass VC across the wire, we'll need to do these.

to make this work fully implicitly i add hooks to both pack it into context as header on producer side and unpack it on the consumer side automatically.

@gricha gricha requested a review from markstory April 3, 2026 18:29
@gricha gricha requested a review from a team as a code owner April 3, 2026 18:29
gricha and others added 3 commits April 3, 2026 11:36
ExternalNamespace was missing context_hooks, so on_dispatch hooks
would not fire for tasks dispatched to external applications.

Also fixes black formatting issues.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
create_activation passes headers as MutableMapping[str, Any], not
dict[str, str]. Values are stringified after hooks run.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests verify:
- on_dispatch injects headers during create_activation
- No headers injected when no hooks registered
- Multiple hooks all get called
- on_execute receives activation headers during child_process execution

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@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.

self._producer_factory = producer_factory
self._router = router
self._metrics = metrics
self._context_hooks: list[ContextHook] = context_hooks or []
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Falsy or [] breaks hook list reference chain

Medium Severity

The context_hooks or [] pattern in TaskRegistry.__init__ and TaskNamespace.__init__ creates new disconnected empty lists when the incoming list is empty (since [] is falsy in Python). This breaks the reference chain from app.context_hooks through to namespace.context_hooks. The dispatch side reads hooks from self._namespace.context_hooks while the execution side reads from app.context_hooks — so any hooks appended to app.context_hooks after construction will be visible on execution but silently ignored on dispatch. Using if context_hooks is not None instead of the or pattern would preserve empty list references.

Additional Locations (1)
Fix in Cursor Fix in Web

gricha added a commit to getsentry/sentry that referenced this pull request Apr 3, 2026
Implement ViewerContextHook that propagates ViewerContext through
TaskBroker task headers. On dispatch, the hook reads from the
contextvar and injects org_id, user_id, and actor_type into task
headers. On execution, it restores the ViewerContext from headers
into a viewer_context_scope.

This enables implicit identity propagation through async tasks
without requiring callsites to manually pass user context.

Depends on getsentry/taskbroker#587 for the ContextHook protocol.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant