GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — fix(tracing): drop trailing comma when serializing baggage without third-party items by alphasafal · Pull Request #8119 · getsentry/sentry-python

Conversation

alphasafal commented Oct 25, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

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_sentry_only_baggage in tests/tracing/test_baggage.py: it verifies that baggage containing only Sentry items serializes to the expected header, with identical output whether third-party items are included or not.

Validation on Python 3.13.7:

TESTPATH=tests/tracing/test_baggage.py uv run tox -e py3.13-common -- --no-cov: 4 passed.
Restoring the original if include_third_party: condition makes test_sentry_only_baggage fail on the trailing comma (1 failed, 3 passed); restoring the fix gives 4 passed.
Ruff lint passed for tests and sentry_sdk; formatting checks passed for the changed test files.
uv run --group typing mypy sentry_sdk: no issues in 203 source files.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          fix(tracing): drop trailing comma when serializing baggage without th…

e8f8ace

…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 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 reviewed

Oct 25, 2026

Copilot AI left a comment

Copy link

Copy Markdown

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

alphasafal commented Oct 25, 2026

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

chrikrah reviewed

Oct 25, 2026

View reviewed changes

chrikrah left a comment

Copy link

Copy Markdown

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

alphasafal mentioned this pull request

Oct 26, 2026

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

Closed

alphasafal commented Oct 26, 2026

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

alphasafal marked this pull request as draft

October 26, 2026 02:12


          test(celery): cover baggage propagation without third-party items

0bc5679

Adds a Celery header test where the incoming baggage has only Sentry items.
The outgoing baggage must not end with a trailing comma.

alphasafal commented Oct 27, 2026

Copy link

Copy Markdown

Contributor Author

Added the Celery test for baggage without third-party items in 0bc5679. It passes with the fix and fails on the base condition. Marking this ready for review. @chrikrah could a maintainer also acknowledge #8120 so this isn't auto-closed?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

alphasafal marked this pull request as ready for review

October 27, 2026 04:52

chrikrah reviewed

Oct 28, 2026

View reviewed changes

chrikrah left a comment

Copy link

Copy Markdown

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.

@alphasafal the Celery test fails on the base condition; nothing blocks a merge.

$ pytest tests/tracing/test_baggage.py tests/integrations/celery/test_update_celery_task_headers.py -q --no-cov -p no:cacheprovider   # export of 0bc5679, sentry_sdk imported from it, Python 3.12.3, celery 5.6.3
43 passed, 4 warnings
2 failed, 41 passed, 4 warnings    # base condition `if include_third_party:` restored, tests kept
FAILED tests/tracing/test_baggage.py::test_serialize_no_trailing_comma_without_third_party_items
FAILED tests/integrations/celery/test_update_celery_task_headers.py::test_span_with_transaction_custom_headers_without_third_party_baggage

My 10-25 auto-close warning was wrong: validate-pr.yml only runs on open or reopen, so #8120 needs nothing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

pabloDeputter reviewed

Oct 28, 2026

View reviewed changes

Comment thread

tests/integrations/celery/test_update_celery_task_headers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

pabloDeputter reviewed

Oct 28, 2026

View reviewed changes

Comment thread

tests/tracing/test_baggage.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

pabloDeputter reviewed

Oct 28, 2026

View reviewed changes

Comment thread

tests/tracing/test_baggage.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

pabloDeputter reviewed

Oct 28, 2026

View reviewed changes

Comment thread

tests/tracing/test_baggage.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

pabloDeputter reviewed

Oct 28, 2026

View reviewed changes

Comment thread

tests/tracing/test_baggage.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

pabloDeputter suggested changes

Oct 28, 2026

View reviewed changes

alphasafal and others added 3 commits

October 28, 2026 15:36


          test(tracing): simplify baggage regression coverage

1f53399


          Merge branch 'master' into fix/baggage-trailing-comma

9060c2c


          test(tracing): update baggage repr expectations without trailing comma

b6fb51d

pabloDeputter approved these changes

Oct 28, 2026

View reviewed changes

pabloDeputter left a comment

Copy link

Copy Markdown

Contributor

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.

Thanks for picking this up! Looks good to me now 😃

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

pabloDeputter merged commit 63707d7 into getsentry:master

Oct 28, 2026

165 checks passed

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Nov 4, 2026

publish: getsentry/[email protected] getsentry/publish#10182

Closed

publish: getsentry/[email protected] getsentry/publish#10240

Closed

Reviewers

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

+2 more reviewers

chrikrah

chrikrah left review comments

pabloDeputter

pabloDeputter approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

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

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

alphasafal commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Where it's reachable

Fix

Test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

alphasafal commented Oct 25, 2026

Uh oh!

chrikrah left a comment

Choose a reason for hiding this comment

Uh oh!

alphasafal commented Oct 26, 2026

Uh oh!

alphasafal commented Oct 27, 2026

Uh oh!

chrikrah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pabloDeputter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alphasafal commented Oct 25, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.