From 7a9b79a0c2e8ba64c1a65ce3ed5110b7c4b07f61 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 30 Aug 2021 11:06:43 +0200 Subject: [PATCH 1/3] Do not count skipped baggage entries Fixes #2071 --- CHANGELOG.md | 2 ++ .../baggage/propagation/__init__.py | 6 ++--- .../tests/baggage/test_baggage_propagation.py | 23 +++++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db3b4eb83f..8bee8bd3cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-semantic-conventions` Update to semantic conventions v1.6.1 ([#2077](https://github.com/open-telemetry/opentelemetry-python/pull/2077)) +- Fix propagation bug caused by counting skipped entries + ([#2071](https://github.com/open-telemetry/opentelemetry-python/pull/2071)) ## [1.5.0-0.24b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.5.0-0.24b0) - 2021-08-26 diff --git a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py index b484d9b2e8..9d170aae9a 100644 --- a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py @@ -54,9 +54,6 @@ def extract( baggage_entries = header.split(",") total_baggage_entries = self._MAX_PAIRS for entry in baggage_entries: - if total_baggage_entries <= 0: - return context - total_baggage_entries -= 1 if len(entry) > self._MAX_PAIR_LENGTH: continue try: @@ -68,6 +65,9 @@ def extract( unquote_plus(value).strip(), context=context, ) + total_baggage_entries -= 1 + if total_baggage_entries == 0: + break return context diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py index 009598db9f..962075ce60 100644 --- a/opentelemetry-api/tests/baggage/test_baggage_propagation.py +++ b/opentelemetry-api/tests/baggage/test_baggage_propagation.py @@ -118,6 +118,29 @@ def test_extract_unquote_plus(self): {"key/key": "value/value"}, ) + def test_header_max_entries_skip_invalid_entry(self): + + self.assertEqual( + self._extract( + ",".join( + [ + f"key{index}=value{index}" + if index != 2 + else ( + f"key{index}=" + f"value{'s' * (W3CBaggagePropagator._MAX_PAIR_LENGTH + 1)}" + ) + for index in range(W3CBaggagePropagator._MAX_PAIRS + 1) + ] + ) + ), + { + f"key{index}": f"value{index}" + for index in range(W3CBaggagePropagator._MAX_PAIRS + 1) + if index != 2 + }, + ) + def test_inject_no_baggage_entries(self): values = {} output = self._inject(values) From 75ae1bcd0902974b1ed1e7f0b31ab8304bce85a3 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 31 Aug 2021 14:06:10 +0200 Subject: [PATCH 2/3] Add exception raising test --- .../tests/baggage/test_baggage_propagation.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py index 962075ce60..dab20c342e 100644 --- a/opentelemetry-api/tests/baggage/test_baggage_propagation.py +++ b/opentelemetry-api/tests/baggage/test_baggage_propagation.py @@ -140,6 +140,26 @@ def test_header_max_entries_skip_invalid_entry(self): if index != 2 }, ) + self.assertEqual( + self._extract( + ",".join( + [ + f"key{index}=value{index}" + if index != 2 + else ( + f"key{index}x" + f"value{index}" + ) + for index in range(W3CBaggagePropagator._MAX_PAIRS + 1) + ] + ) + ), + { + f"key{index}": f"value{index}" + for index in range(W3CBaggagePropagator._MAX_PAIRS + 1) + if index != 2 + }, + ) def test_inject_no_baggage_entries(self): values = {} From 88b58782f0d37bf6fa3e6dc16057058ce322b8ce Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 31 Aug 2021 14:37:04 +0200 Subject: [PATCH 3/3] Fix lint --- opentelemetry-api/tests/baggage/test_baggage_propagation.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py index dab20c342e..caecc4615e 100644 --- a/opentelemetry-api/tests/baggage/test_baggage_propagation.py +++ b/opentelemetry-api/tests/baggage/test_baggage_propagation.py @@ -146,10 +146,7 @@ def test_header_max_entries_skip_invalid_entry(self): [ f"key{index}=value{index}" if index != 2 - else ( - f"key{index}x" - f"value{index}" - ) + else f"key{index}xvalue{index}" for index in range(W3CBaggagePropagator._MAX_PAIRS + 1) ] )