Skip to content

fix(tracing): drop trailing comma when serializing baggage without third-party items - #8119

Draft
alphasafal wants to merge 1 commit into
getsentry:masterfrom
alphasafal:fix/baggage-trailing-comma
Draft

alphasafal wants to merge 1 commit into
getsentry:masterfrom
alphasafal:fix/baggage-trailing-comma

Conversation

@alphasafal

@alphasafal alphasafal commented Oct 25, 2026 •

Copy link
Copy Markdown

Fixes #8120

Summary

Baggage.serialize(include_third_party=True) appends third_party_items unconditionally. When there are no third-party items (third_party_items == ""), it appends an empty segment, so ",".join(...) leaves a trailing comma in the serialized baggage header:

sentry-trace_id=...,sentry-public_key=...,sentry-sample_rate=0.5,

Per the W3C Baggage spec a trailing comma is an empty list-member (malformed). Sentry's own parser tolerates it, but a strict W3C / OpenTelemetry baggage parser on a downstream consumer can reject or mishandle the header.

Where it's reachable

The Celery integration calls combined.serialize(include_third_party=True) (sentry_sdk/integrations/celery/__init__.py) when merging incoming and Sentry baggage. Whenever the merged baggage has only Sentry items and no third-party ones — the common Sentry → Celery → Sentry propagation case — the outgoing baggage header gets the stray comma.

Fix

Only append third_party_items when it is non-empty:

if include_third_party and self.third_party_items:
    items.append(self.third_party_items)

The output is unchanged when third-party items are present.

Test

Adds test_serialize_no_trailing_comma_without_third_party_items in tests/tracing/test_baggage.py: it builds a baggage with only Sentry items and asserts the serialized header does not end with , and equals the plain serialize() output. It fails before this change (trailing comma) and passes after.

…ird-party items

Baggage.serialize(include_third_party=True) appended third_party_items
unconditionally, so when there were no third-party items it added an empty
segment and the serialized header ended with a comma (e.g.
"sentry-trace_id=...,sentry-public_key=...,"). This is reachable from the
Celery integration, which calls combined.serialize(include_third_party=True)
whenever the merged baggage has only Sentry items.

Only append third_party_items when it is non-empty; the output is unchanged
when third-party items are present. Adds a regression test.
@alphasafal
alphasafal requested a review from a team as a code owner October 25, 2026 18:25
Copilot AI balanced review requested due to automatic review settings October 25, 2026 18:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@alphasafal

Copy link
Copy Markdown
Author

Ran the baggage tests locally against this branch — tests/tracing/test_baggage.py: 4 passed, including the new test_serialize_no_trailing_comma_without_third_party_items, with no change to the existing cases.

@chrikrah chrikrah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@alphasafal CONTRIBUTING auto-closes outside PRs without a linked, maintainer-acknowledged issue. Otherwise, from my perspective this can be merged at e8f8ace.

# probe_celery.py: _update_celery_task_headers({"baggage": "sentry-trace_id=771a43a4192642f0b136d5159a501700,sentry-sample_rate=0.5"}, span, False)
$ python probe_celery.py   # e8f8ace with the base condition (if include_third_party:)
'sentry-trace_id=771a43a4192642f0b136d5159a501700,sentry-sample_rand=0.395125,sentry-environment=production,sentry-transaction=t,sentry-sample_rate=0.5,sentry-sampled=true,'
$ python probe_celery.py   # e8f8ace
'sentry-trace_id=771a43a4192642f0b136d5159a501700,sentry-sample_rand=0.051087,sentry-environment=production,sentry-transaction=t,sentry-sample_rate=0.5,sentry-sampled=true'

$ pytest tests/tracing/test_baggage.py tests/integrations/celery/test_update_celery_task_headers.py -q --no-cov
42 passed, 4 warnings in 4.42s                       # e8f8ace, Python 3.12
1 failed, 41 passed, 4 warnings in 1.11s             # base condition restored, test kept
FAILED tests/tracing/test_baggage.py::test_serialize_no_trailing_comma_without_third_party_items

# test_span_with_transaction_custom_headers, the one Celery merge test, sends custom=value
# and expects combined_baggage.serialize(include_third_party=True): it passes on both

non-blocking: add a Celery test without third-party baggage?

@alphasafal

Copy link
Copy Markdown
Author

@chrikrah thanks for running the Celery probe and checking it against the base condition. I've opened #8120 for the bug and linked it in the description, and I'll add a Celery test with no third-party baggage so that path is covered directly.

@alphasafal
alphasafal marked this pull request as draft October 26, 2026 02:12

This branch has not been deployed

No deployments
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.

Baggage.serialize(include_third_party=True) leaves a trailing comma when there are no third-party items

3 participants