fix(celery): Propagate user-set headers#5581
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Documentation 📚
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ 1707 passed | ⏭️ 184 skipped | Total: 1891 | Pass Rate: 90.27% | Execution Time: 2m 15s 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ❌ Patch coverage is 0.00%. Project has 11872 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 39.98% 39.98% —%
==========================================
Files 189 189 —
Lines 19776 19779 +3
Branches 6762 6766 +4
==========================================
+ Hits 7907 7907 —
- Misses 11869 11872 +3
- Partials 391 391 —Generated by Codecov Action |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: User header propagation skipped when tracing is inactive
- Moved custom-header mirroring outside the tracing-headers guard and ensured the inner headers dict is created when needed so user headers propagate even with empty Sentry trace headers.
Or push these changes by commenting:
@cursor push df190b50ed
Preview (df190b50ed)
diff --git a/sentry_sdk/integrations/celery/__init__.py b/sentry_sdk/integrations/celery/__init__.py
--- a/sentry_sdk/integrations/celery/__init__.py
+++ b/sentry_sdk/integrations/celery/__init__.py
@@ -231,11 +231,11 @@
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
+ # 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":
+ updated_headers.setdefault("headers", {}).setdefault(key, value)
return updated_headers| # 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.
User header propagation skipped when tracing is inactive
Medium Severity
The new loop that copies user-provided headers into the inner updated_headers["headers"] dict is placed inside the if headers: block (line 191), so it only runs when Sentry produces non-empty trace propagation headers (i.e., when an active Sentry transaction exists). When tracing is disabled or there is no active transaction, headers is an empty dict, the block is skipped entirely, and user custom headers are never copied to the inner dict — leaving the original issue (#5566) unfixed for non-tracing deployments.
There was a problem hiding this comment.
please address this as well if applicable 🙏 @sentrivana
There was a problem hiding this comment.
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.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| # 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"]: |
There was a problem hiding this comment.
Bug: A KeyError occurs when using custom headers with apply_async if the SDK client's propagate_traces option is disabled, as updated_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 calling updated_headers.setdefault("headers", {}) unconditionally before the loop that preserves original headers.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: sentry_sdk/integrations/celery/__init__.py#L237
Potential issue: A `KeyError` will occur when calling a Celery task's `apply_async` with
custom headers under specific configuration. The new code attempts to access
`updated_headers["headers"]` to preserve user-provided headers. However, this dictionary
key is only created if Sentry trace headers are generated. If the Sentry SDK client has
`propagate_traces=False` while the Celery integration has `propagate_traces=True` (the
default), no Sentry headers are generated, the `updated_headers["headers"]` key is never
created, and the new code will raise a `KeyError` when it tries to access it, causing
the task submission to fail.
Did we get this right? 👍 / 👎 to inform future reviews.
| # 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.
please address this as well if applicable 🙏 @sentrivana
2 participants
Description
The SDK interferes with Celery logic that puts custom headers into
request.headersin the worker. Currently, we only make internal sentry headers available there. Make sure to copy over any user-set headers as well.Issues
Closes #5566
Reminders
tox -e linters.feat:,fix:,ref:,meta:)