-
Notifications
You must be signed in to change notification settings - Fork 585
fix(celery): Propagate user-set headers #5581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -231,6 +231,12 @@ def _update_celery_task_headers( | |
| if key.startswith("sentry-"): | ||
| updated_headers["headers"][key] = value | ||
|
|
||
| # Preserve user-provided custom headers in the inner "headers" dict | ||
| # so they survive to task.request.headers on the worker (celery#4875). | ||
| for key, value in original_headers.items(): | ||
| if key != "headers" and key not in updated_headers["headers"]: | ||
| updated_headers["headers"][key] = value | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. User header propagation skipped when tracing is inactiveMedium Severity The new loop that copies user-provided headers into the inner
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please address this as well if applicable 🙏 @sentrivana
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I can tell this is ok. Celery itself has logic in there that actually moves the headers to the correct location as long as there are no headers. If tracing is disabled in the SDK, the SDK won't pre-populate the headers, and the default Celery logic will kick in and take care of it. Only the case where the SDK actually populates some of the headers is problematic, since it causes Celery to skip the moving logic entirely. So we need to make sure that we either populate all or nothing. |
||
|
|
||
| return updated_headers | ||
|
|
||
|
|
||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: A
KeyErroroccurs when using custom headers withapply_asyncif the SDK client'spropagate_tracesoption is disabled, asupdated_headers["headers"]is accessed before it is created.Severity: HIGH
Suggested Fix
Ensure the
updated_headers["headers"]dictionary exists before the new loop that attempts to access it. This can be done by callingupdated_headers.setdefault("headers", {})unconditionally before the loop that preserves original headers.Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.