From f5970062f5873038553c17aab6f066446e4501ef Mon Sep 17 00:00:00 2001 From: "Md. Amdadul Bari Imad" Date: Thu, 13 Aug 2026 04:38:53 +0000 Subject: [PATCH 1/2] 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 --- .changelog/5543.fixed | 1 + .../src/opentelemetry/trace/span.py | 14 +++++++---- .../tests/trace/test_tracestate.py | 23 +++++++++++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 .changelog/5543.fixed diff --git a/.changelog/5543.fixed b/.changelog/5543.fixed new file mode 100644 index 00000000000..f4511ba7b63 --- /dev/null +++ b/.changelog/5543.fixed @@ -0,0 +1 @@ +`opentelemetry-api`: fix `TraceState.update` to only update already existing keys diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index 745f4f5ff08..cf322a28d82 100644 --- a/opentelemetry-api/src/opentelemetry/trace/span.py +++ b/opentelemetry-api/src/opentelemetry/trace/span.py @@ -311,8 +311,8 @@ def add(self, key: str, value: str) -> TraceState: return TraceState(new_state) 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. + """Updates an existing key's value in tracestate. The provided pair + should adhere to w3c tracestate identifiers format. Args: key: A valid tracestate key to update @@ -321,13 +321,17 @@ def update(self, key: str, value: str) -> TraceState: 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 key is not already present, or the provided pair is invalid, + the same tracestate is returned unchanged. Adding a new key is done + with ``add()``. """ if not _is_valid_pair(key, value): _logger.warning("Invalid key/value pair (%s, %s) found.", key, value) return self + if key not in self._dict: + # update() only updates an existing key (see the TraceState API spec); + # adding a new key is the job of add(). Return unchanged otherwise. + return self prev_state = self._dict.copy() prev_state.pop(key, None) new_state = [(key, value), *prev_state.items()] diff --git a/opentelemetry-api/tests/trace/test_tracestate.py b/opentelemetry-api/tests/trace/test_tracestate.py index 555cef16bde..e365efc8b7e 100644 --- a/opentelemetry-api/tests/trace/test_tracestate.py +++ b/opentelemetry-api/tests/trace/test_tracestate.py @@ -51,6 +51,29 @@ 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_missing_key_returns_unchanged(self): + # update() only updates an existing key; for a key that is not present it + # returns the same tracestate unchanged (and never adds the key). This also + # guards the previous bug where updating a new key at the 32-entry limit + # wiped all 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") + + small_state = TraceState([("a", "1")]) + self.assertIsNone(small_state.update("b", "2").get("b")) + + 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") From c913c4eb9a4b0105dcdf25f27e06384f65173c86 Mon Sep 17 00:00:00 2001 From: amdadulbari Date: Tue, 18 Aug 2026 04:59:36 +0000 Subject: [PATCH 2/2] 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. --- .changelog/5543.fixed | 2 +- .../src/opentelemetry/trace/span.py | 22 ++++++++++++------- .../tests/trace/test_tracestate.py | 20 ++++++++++------- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/.changelog/5543.fixed b/.changelog/5543.fixed index f4511ba7b63..ecbb79bd01c 100644 --- a/.changelog/5543.fixed +++ b/.changelog/5543.fixed @@ -1 +1 @@ -`opentelemetry-api`: fix `TraceState.update` to only update already existing keys +`opentelemetry-api`: fix `TraceState.update` dropping all entries when adding a new key at the 32-key limit diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index cf322a28d82..7d3462bebe8 100644 --- a/opentelemetry-api/src/opentelemetry/trace/span.py +++ b/opentelemetry-api/src/opentelemetry/trace/span.py @@ -311,8 +311,8 @@ def add(self, key: str, value: str) -> TraceState: return TraceState(new_state) def update(self, key: str, value: str) -> TraceState: - """Updates an existing key's value in tracestate. The provided pair - should adhere to w3c tracestate identifiers format. + """Updates a key-value pair in tracestate. The provided pair should + adhere to w3c tracestate identifiers format. Args: key: A valid tracestate key to update @@ -321,16 +321,22 @@ def update(self, key: str, value: str) -> TraceState: Returns: A new TraceState with the modifications applied. - If the key is not already present, or the provided pair is invalid, - the same tracestate is returned unchanged. Adding a new key is done - with ``add()``. + 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 - if key not in self._dict: - # update() only updates an existing key (see the TraceState API spec); - # adding a new key is the job of add(). Return unchanged otherwise. + # 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) diff --git a/opentelemetry-api/tests/trace/test_tracestate.py b/opentelemetry-api/tests/trace/test_tracestate.py index e365efc8b7e..62e6f5fe29b 100644 --- a/opentelemetry-api/tests/trace/test_tracestate.py +++ b/opentelemetry-api/tests/trace/test_tracestate.py @@ -51,11 +51,18 @@ 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_missing_key_returns_unchanged(self): - # update() only updates an existing key; for a key that is not present it - # returns the same tracestate unchanged (and never adds the key). This also - # guards the previous bug where updating a new key at the 32-entry limit - # wiped all existing entries. + 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") @@ -63,9 +70,6 @@ def test_tracestate_update_missing_key_returns_unchanged(self): self.assertIsNone(new_state.get("newkey")) self.assertEqual(new_state.get("key0"), "value0") - small_state = TraceState([("a", "1")]) - self.assertIsNone(small_state.update("b", "2").get("b")) - 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)]