oss_16_baggage_metadata_and_percent_encoding

.changelog/24.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.

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,22 +52,65 @@ 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
 
     Args:
         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
 

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(

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"))

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(