Fix TraceState.update to only update already existing keys (#5543)

* Fix TraceState.update dropping all entries at the 32-key limit

TraceState.update() builds a new entry list and passes it to the
constructor, which discards everything and returns an empty TraceState
when the list exceeds the 32-key W3C limit. As a result, upserting a new
key into a tracestate that already holds 32 entries silently wiped all 32
existing key/value pairs, contradicting update()'s documented contract
that "the same tracestate will be returned".

Guard the limit the way add() already does, but only for genuinely new
keys, so updates to existing keys at capacity keep working. Add tests for
both cases.

Signed-off-by: Md. Amdadul Bari Imad <amdadulbari@gmail.com>

* Preserve upsert in TraceState.update; guard only new key at 32-key limit

Address review feedback: instead of restricting update() to existing keys
(a breaking change for samplers relying on upsert), keep upsert behavior and
only return the tracestate unchanged when adding a *new* key would exceed the
32-key maximum. Fixes the silent drop of all entries at capacity.

* Document upsert behaviour of TraceState.update

Note in the update() docstring that it performs an upsert and that this
intentionally goes beyond the spec's update definition, as requested in
review. Docstring-only change; no behavior change.

* Fix precommit: format update() length guard onto one line

---------

Signed-off-by: Md. Amdadul Bari Imad <amdadulbari@gmail.com>
Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
Co-authored-by: Leighton Chen <lechen@microsoft.com>

Author: 3 people

.changelog/5543.fixed

@@ -0,0 +1 @@
+`opentelemetry-api`: fix `TraceState.update` dropping all entries when adding a new key at the 32-key limit

opentelemetry-api/src/opentelemetry/trace/span.py

@@ -314,20 +314,35 @@ def update(self, key: str, value: str) -> TraceState:
         """Updates a key-value pair in tracestate. The provided pair should
         adhere to w3c tracestate identifiers format.
 
+        Note:
+            This method performs an "upsert": if ``key`` is not already present
+            it is added (when the tracestate is below the 32-entry limit),
+            otherwise its value is updated. This upsert behaviour is intentional
+            but goes beyond what the OpenTelemetry specification defines for
+            ``update`` (https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#tracestate),
+            and is kept for backwards compatibility with callers that rely on it.
+
         Args:
             key: A valid tracestate key to update
             value: A valid tracestate value to update for key
 
         Returns:
             A new TraceState with the modifications applied.
 
-            If the provided key-value pair is invalid or results in tracestate
-            that violates tracecontext specification, they are discarded and
-            same tracestate will be returned.
+            If the provided pair is invalid, or adding a new key would exceed
+            the maximum of 32 key/value pairs, they are discarded and the same
+            tracestate is returned unchanged. Updating an existing key is always
+            allowed, even at the maximum.
         """
         if not _is_valid_pair(key, value):
             _logger.warning("Invalid key/value pair (%s, %s) found.", key, value)
             return self
+        # Adding a new key at the maximum would push the tracestate over the
+        # limit and cause the constructor to drop every entry. Return unchanged
+        # instead of silently discarding existing state.
+        if key not in self._dict and len(self._dict) >= _TRACECONTEXT_MAXIMUM_TRACESTATE_KEYS:
+            _logger.warning("There can't be more 32 key/value pairs.")
+            return self
         prev_state = self._dict.copy()
         prev_state.pop(key, None)
         new_state = [(key, value), *prev_state.items()]

opentelemetry-api/tests/trace/test_tracestate.py

@@ -51,6 +51,33 @@ def test_tracestate_update_invalid(self):
         self.assertNotEqual(new_state.get("a"), ",,2,,f")
         self.assertEqual(new_state.get("a"), "1")
 
+    def test_tracestate_update_new_key_added_below_capacity(self):
+        # update() keeps its upsert behavior: a key that is not present is added
+        # as long as there is room below the 32-entry limit.
+        small_state = TraceState([("a", "1")])
+        new_state = small_state.update("b", "2")
+        self.assertEqual(new_state.get("b"), "2")
+        self.assertEqual(new_state.get("a"), "1")
+
+    def test_tracestate_update_at_capacity_new_key_preserved(self):
+        # Guards the previous bug: adding a new key at the 32-entry limit used to
+        # push the list to 33 entries and cause the constructor to wipe them all.
+        # Now the tracestate is returned unchanged, preserving existing entries.
+        pairs = [(f"key{i}", f"value{i}") for i in range(32)]
+        state = TraceState(pairs)
+        new_state = state.update("newkey", "newvalue")
+        self.assertEqual(len(new_state), 32)
+        self.assertIsNone(new_state.get("newkey"))
+        self.assertEqual(new_state.get("key0"), "value0")
+
+    def test_tracestate_update_existing_key_at_capacity(self):
+        # Updating an existing key while at the limit stays within the limit.
+        pairs = [(f"key{i}", f"value{i}") for i in range(32)]
+        state = TraceState(pairs)
+        new_state = state.update("key0", "changed")
+        self.assertEqual(len(new_state), 32)
+        self.assertEqual(new_state.get("key0"), "changed")
+
     def test_tracestate_delete_preserved(self):
         state = TraceState([("a", "1"), ("b", "2"), ("c", "3")])
         new_state = state.delete("b")