From c35142d9e66955c7a235c8edfe3da60edfce709d Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 22 Jul 2026 00:57:52 -0500 Subject: [PATCH 1/2] Preserve W3C Baggage metadata and fix space percent-encoding Baggage entry metadata (the ;-delimited properties a W3C Baggage entry may carry) was previously used only for validation on extract and never stored, so it was silently dropped and could not be re-injected. set_baggage now accepts an optional metadata parameter; extract splits the value from its metadata and stores the metadata alongside the entry, and inject re-appends it verbatim. A new get_baggage_metadata accessor exposes it. Spaces were encoded with quote_plus/unquote_plus, emitting a space as + which spec-compliant receivers misread. Injection and extraction now use quote/unquote so a space is encoded as %20 per the W3C Baggage format. Existing set_baggage(name, value) calls are unaffected. --- .changelog/0000.fixed | 3 + .../src/opentelemetry/baggage/__init__.py | 72 +++++++++++++++++-- .../baggage/propagation/__init__.py | 38 +++++++--- .../tests/baggage/test_baggage.py | 30 ++++++++ .../propagators/test_w3cbaggagepropagator.py | 51 +++++++++++-- 5 files changed, 177 insertions(+), 17 deletions(-) create mode 100644 .changelog/0000.fixed diff --git a/.changelog/0000.fixed b/.changelog/0000.fixed new file mode 100644 index 00000000000..0a863444e66 --- /dev/null +++ b/.changelog/0000.fixed @@ -0,0 +1,3 @@ +W3C Baggage: preserve entry metadata (`;`-delimited properties) across +extract/inject and percent-encode spaces as `%20` instead of `+`. +`set_baggage` gains an optional `metadata` parameter. diff --git a/opentelemetry-api/src/opentelemetry/baggage/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/__init__.py index b1b36a990df..a04cdb2490b 100644 --- a/opentelemetry-api/src/opentelemetry/baggage/__init__.py +++ b/opentelemetry-api/src/opentelemetry/baggage/__init__.py @@ -15,6 +15,7 @@ ) _BAGGAGE_KEY = create_key("baggage") +_BAGGAGE_METADATA_KEY = create_key("baggage-metadata") _logger = getLogger(__name__) _KEY_PATTERN = compile(_KEY_FORMAT) @@ -51,8 +52,33 @@ def get_baggage(name: str, context: Context | None = None) -> object | None: return _get_baggage_value(context=context).get(name) +def get_baggage_metadata( + name: str, context: Context | None = None +) -> str | None: + """Provides access to the metadata associated with a name/value pair in + the Baggage. + + Metadata is the optional ``;``-delimited list of properties that a W3C + Baggage entry may carry (see + https://www.w3.org/TR/baggage/#definition). It is preserved verbatim so + that it can be re-appended when the Baggage is injected downstream. + + Args: + name: The name of the entry whose metadata to retrieve + context: The Context to use. If not set, uses current Context + + Returns: + The metadata string associated with the given name, or ``None`` if the + given name has no metadata or is not present. + """ + return _get_baggage_metadata_value(context=context).get(name) + + def set_baggage( - name: str, value: object, context: Context | None = None + name: str, + value: object, + context: Context | None = None, + metadata: str | None = None, ) -> Context: """Sets a value in the Baggage @@ -60,13 +86,31 @@ def set_baggage( name: The name of the value to set value: The value to set context: The Context to use. If not set, uses current Context + metadata: Optional ``;``-delimited W3C Baggage metadata (properties) + to associate with this entry. It is stored verbatim and + re-appended when the Baggage is injected. Returns: A Context with the value updated """ baggage = _get_baggage_value(context=context).copy() baggage[name] = value - return set_value(_BAGGAGE_KEY, baggage, context=context) + context = set_value(_BAGGAGE_KEY, baggage, context=context) + + existing_metadata = _get_baggage_metadata_value(context=context) + # Only touch the metadata store when there is something to record or an + # existing entry to clear, so entries set without metadata don't leave an + # empty metadata mapping behind in the Context. + if metadata is not None or name in existing_metadata: + baggage_metadata = existing_metadata.copy() + if metadata is None: + baggage_metadata.pop(name, None) + else: + baggage_metadata[name] = metadata + context = set_value( + _BAGGAGE_METADATA_KEY, baggage_metadata, context=context + ) + return context def remove_baggage(name: str, context: Context | None = None) -> Context: @@ -81,8 +125,16 @@ def remove_baggage(name: str, context: Context | None = None) -> Context: """ baggage = _get_baggage_value(context=context).copy() baggage.pop(name, None) + context = set_value(_BAGGAGE_KEY, baggage, context=context) - return set_value(_BAGGAGE_KEY, baggage, context=context) + existing_metadata = _get_baggage_metadata_value(context=context) + if name in existing_metadata: + baggage_metadata = existing_metadata.copy() + baggage_metadata.pop(name, None) + context = set_value( + _BAGGAGE_METADATA_KEY, baggage_metadata, context=context + ) + return context def clear(context: Context | None = None) -> Context: @@ -94,7 +146,10 @@ def clear(context: Context | None = None) -> Context: Returns: A Context with all baggage entries removed """ - return set_value(_BAGGAGE_KEY, {}, context=context) + context = set_value(_BAGGAGE_KEY, {}, context=context) + if _get_baggage_metadata_value(context=context): + context = set_value(_BAGGAGE_METADATA_KEY, {}, context=context) + return context def _get_baggage_value(context: Context | None = None) -> dict[str, object]: @@ -104,6 +159,15 @@ def _get_baggage_value(context: Context | None = None) -> dict[str, object]: return {} +def _get_baggage_metadata_value( + context: Context | None = None, +) -> dict[str, str]: + baggage_metadata = get_value(_BAGGAGE_METADATA_KEY, context=context) + if isinstance(baggage_metadata, dict): + return baggage_metadata + return {} + + def _is_valid_key(name: str) -> bool: return _KEY_PATTERN.fullmatch(str(name)) is not None diff --git a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py index 1d873c5f3cd..185198ec954 100644 --- a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py @@ -4,9 +4,14 @@ from collections.abc import Iterable, Iterator, Mapping from logging import getLogger from re import split -from urllib.parse import quote_plus, unquote_plus - -from opentelemetry.baggage import _is_valid_pair, get_all, set_baggage +from urllib.parse import quote, unquote + +from opentelemetry.baggage import ( + _is_valid_pair, + get_all, + get_baggage_metadata, + set_baggage, +) from opentelemetry.context import get_current from opentelemetry.context.context import Context from opentelemetry.propagators import textmap @@ -125,13 +130,20 @@ def extract( _logger.warning("Invalid baggage entry: `%s`", entry) continue - name = unquote_plus(name).strip() - value = unquote_plus(value).strip() + # A value may carry ;-delimited metadata (properties). The value + # is everything before the first `;`; the remainder is preserved + # verbatim so it can be re-appended on inject. + value, _, metadata = value.partition(";") + + name = unquote(name).strip() + value = unquote(value).strip() + metadata = metadata.strip() or None context = set_baggage( name, value, context=context, + metadata=metadata, ) return context @@ -153,7 +165,7 @@ def inject( baggage_string = ",".join( _apply_baggage_limits( - _encode_baggage_pairs(baggage_entries), + _encode_baggage_pairs(baggage_entries, context=context), max_pairs=self._MAX_PAIRS, max_pair_length=self._MAX_PAIR_LENGTH, max_header_length=self._MAX_HEADER_LENGTH, @@ -171,10 +183,20 @@ def fields(self) -> set[str]: def _encode_baggage_pairs( baggage_entries: Mapping[str, object], + context: Context | None = None, ) -> Iterator[str]: - """Yield URL-encoded 'key=value' pairs from baggage entries.""" + """Yield URL-encoded 'key=value' pairs from baggage entries. + + Spaces are encoded as ``%20`` (per RFC 3986 / the W3C Baggage format) + rather than ``+``. Any metadata associated with an entry is re-appended + verbatim after a ``;`` separator. + """ for key, value in baggage_entries.items(): - yield quote_plus(str(key)) + "=" + quote_plus(str(value)) + pair = quote(str(key), safe="") + "=" + quote(str(value), safe="") + metadata = get_baggage_metadata(str(key), context=context) + if metadata: + pair += ";" + metadata + yield pair def _extract_first_element( diff --git a/opentelemetry-api/tests/baggage/test_baggage.py b/opentelemetry-api/tests/baggage/test_baggage.py index 5ead8178f36..5bb4a991acf 100644 --- a/opentelemetry-api/tests/baggage/test_baggage.py +++ b/opentelemetry-api/tests/baggage/test_baggage.py @@ -10,6 +10,7 @@ clear, get_all, get_baggage, + get_baggage_metadata, remove_baggage, set_baggage, ) @@ -68,5 +69,34 @@ def test_clear_baggage(self): ctx = clear(context=ctx) self.assertEqual(get_all(context=ctx), {}) + def test_set_baggage_without_metadata(self): + ctx = set_baggage("test", "value") + self.assertIsNone(get_baggage_metadata("test", context=ctx)) + + def test_set_baggage_with_metadata(self): + ctx = set_baggage("test", "value", metadata="prop1;prop2=x") + self.assertEqual(get_baggage("test", context=ctx), "value") + self.assertEqual( + get_baggage_metadata("test", context=ctx), "prop1;prop2=x" + ) + # The value stays clean; metadata does not leak into get_all. + self.assertEqual(get_all(context=ctx), {"test": "value"}) + + def test_set_baggage_clears_metadata_when_overwritten(self): + ctx = set_baggage("test", "value", metadata="prop1") + self.assertEqual(get_baggage_metadata("test", context=ctx), "prop1") + ctx = set_baggage("test", "value2", context=ctx) + self.assertIsNone(get_baggage_metadata("test", context=ctx)) + + def test_remove_baggage_removes_metadata(self): + ctx = set_baggage("test", "value", metadata="prop1") + ctx = remove_baggage("test", context=ctx) + self.assertIsNone(get_baggage_metadata("test", context=ctx)) + + def test_clear_removes_metadata(self): + ctx = set_baggage("test", "value", metadata="prop1") + ctx = clear(context=ctx) + self.assertIsNone(get_baggage_metadata("test", context=ctx)) + def test__is_valid_value(self): self.assertTrue(_is_valid_value("GET%20%2Fapi%2F%2Freport")) diff --git a/opentelemetry-api/tests/propagators/test_w3cbaggagepropagator.py b/opentelemetry-api/tests/propagators/test_w3cbaggagepropagator.py index 178da653c5b..804681ba30b 100644 --- a/opentelemetry-api/tests/propagators/test_w3cbaggagepropagator.py +++ b/opentelemetry-api/tests/propagators/test_w3cbaggagepropagator.py @@ -8,7 +8,11 @@ from unittest.mock import Mock, patch from urllib.parse import quote_plus -from opentelemetry.baggage import get_all, set_baggage +from opentelemetry.baggage import ( + get_all, + get_baggage_metadata, + set_baggage, +) from opentelemetry.baggage.propagation import W3CBaggagePropagator from opentelemetry.context import get_current @@ -52,8 +56,32 @@ def test_invalid_header_with_space(self): def test_valid_header_with_properties(self): header = "key1=val1,key2=val2;prop=1;prop2;prop3=2" - expected = {"key1": "val1", "key2": "val2;prop=1;prop2;prop3=2"} - self.assertEqual(self._extract(header), expected) + # The ;-delimited metadata is stored separately from the value so the + # value stays clean and the metadata can be re-appended on inject. + expected = {"key1": "val1", "key2": "val2"} + context = self.propagator.extract({"baggage": [header]}) + self.assertEqual(get_all(context), expected) + self.assertEqual( + get_baggage_metadata("key2", context), "prop=1;prop2;prop3=2" + ) + self.assertIsNone(get_baggage_metadata("key1", context)) + + def test_metadata_round_trip(self): + """Entry metadata survives a full extract -> inject round trip.""" + header = "key1=val1;prop=1;prop2;prop3=2" + context = self.propagator.extract({"baggage": [header]}) + carrier = {} + self.propagator.inject(carrier, context=context) + self.assertEqual(carrier["baggage"], "key1=val1;prop=1;prop2;prop3=2") + + def test_inject_metadata_from_set_baggage(self): + """Metadata passed to set_baggage is re-appended on inject.""" + context = set_baggage( + "key1", "val1", context=get_current(), metadata="prop=1;prop2" + ) + carrier = {} + self.propagator.inject(carrier, context=context) + self.assertEqual(carrier["baggage"], "key1=val1;prop=1;prop2") def test_valid_header_with_url_escaped_values(self): header = "key1=val1,key2=val2%3Aval3,key3=val4%40%23%24val5" @@ -224,7 +252,20 @@ def test_inject_no_baggage_entries(self): self.assertEqual(None, output) def test_inject_space_entries(self): - self.assertEqual("key=val+ue", self._inject({"key": "val ue"})) + # A space must be percent-encoded as %20 (not "+") to be read + # correctly by spec-compliant W3C Baggage receivers. + self.assertEqual("key=val%20ue", self._inject({"key": "val ue"})) + + def test_extract_percent_encoded_space(self): + # A %20 in the header decodes back to a space. + self.assertEqual(self._extract("key=val%20ue"), {"key": "val ue"}) + + def test_space_round_trip(self): + # A value containing a space round-trips as %20 on the wire and back + # to a space after extraction. + header = self._inject({"key": "val ue"}) + self.assertEqual(header, "key=val%20ue") + self.assertEqual(self._extract(header), {"key": "val ue"}) def test_inject(self): values = { @@ -371,7 +412,7 @@ def test_inject_extract(self): context = self.propagator.extract(carrier) self.assertEqual( - carrier, {"baggage": "transaction=string+with+spaces"} + carrier, {"baggage": "transaction=string%20with%20spaces"} ) self.assertEqual( From 344ba660ef72a56266a249bac3362e2d4b95dfb7 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 22 Jul 2026 08:37:42 -0500 Subject: [PATCH 2/2] Rename changelog fragment to match PR number --- .changelog/{0000.fixed => 24.fixed} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{0000.fixed => 24.fixed} (100%) diff --git a/.changelog/0000.fixed b/.changelog/24.fixed similarity index 100% rename from .changelog/0000.fixed rename to .changelog/24.fixed