From 32d27ce41b24d7182bf6565a4bd113aff7b22b5d Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 4 Dec 2020 11:11:35 -0500 Subject: [PATCH 1/6] prop --- opentelemetry-api/CHANGELOG.md | 2 ++ .../opentelemetry/trace/propagation/textmap.py | 16 +++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index 84604f4bb27..f3766347af2 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -4,6 +4,8 @@ - Add `fields` to propagators ([#1374](https://github.com/open-telemetry/opentelemetry-python/pull/1374)) +- Return `None` for `Getters` in propagators if not found + ([#1374](https://github.com/open-telemetry/opentelemetry-python/pull/1374)) ## Version 0.16b0 diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py index fb92d371680..05ae4e057a1 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py @@ -29,7 +29,11 @@ class Getter(typing.Generic[TextMapPropagatorT]): """ - def get(self, carrier: TextMapPropagatorT, key: str) -> typing.List[str]: + def get( + self, + carrier: TextMapPropagatorT, + key: str + ) -> typing.Optional[typing.List[str]]: """Function that can retrieve zero or more values from the carrier. In the case that the value does not exist, returns an empty list. @@ -38,8 +42,8 @@ def get(self, carrier: TextMapPropagatorT, key: str) -> typing.List[str]: carrier: An object which contains values that are used to construct a Context. key: key of a field in carrier. - Returns: first value of the propagation key or an empty list if the - key doesn't exist. + Returns: first value of the propagation key or None if the key doesn't + exist. """ raise NotImplementedError() @@ -58,8 +62,10 @@ def keys(self, carrier: TextMapPropagatorT) -> typing.List[str]: class DictGetter(Getter[typing.Dict[str, CarrierValT]]): def get( self, carrier: typing.Dict[str, CarrierValT], key: str - ) -> typing.List[str]: - val = carrier.get(key, []) + ) -> typing.Optional[typing.List[str]]: + val = carrier.get(key, None) + if val is None: + return None if isinstance(val, typing.Iterable) and not isinstance(val, str): return list(val) return [val] From cd43131a18fd6b7f54e69113e4dc2374139d2a05 Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 4 Dec 2020 12:17:14 -0500 Subject: [PATCH 2/6] changelog --- opentelemetry-api/CHANGELOG.md | 4 ++-- .../src/opentelemetry/trace/propagation/tracecontext.py | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index f3766347af2..af02d068dc8 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -4,8 +4,8 @@ - Add `fields` to propagators ([#1374](https://github.com/open-telemetry/opentelemetry-python/pull/1374)) -- Return `None` for `Getters` in propagators if not found - ([#1374](https://github.com/open-telemetry/opentelemetry-python/pull/1374)) +- Return `None` for `DictGetter` if key not found + ([#1449](https://github.com/open-telemetry/opentelemetry-python/pull/1449)) ## Version 0.16b0 diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py index adcdb5e1255..146ba47772a 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/tracecontext.py @@ -91,7 +91,10 @@ def extract( return trace.set_span_in_context(trace.INVALID_SPAN, context) tracestate_headers = getter.get(carrier, self._TRACESTATE_HEADER_NAME) - tracestate = _parse_tracestate(tracestate_headers) + if tracestate_headers is None: + tracestate = None + else: + tracestate = _parse_tracestate(tracestate_headers) span_context = trace.SpanContext( trace_id=int(trace_id, 16), From a288834141d82f48d305199fc59f3557e6c0f5ed Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 4 Dec 2020 12:53:06 -0500 Subject: [PATCH 3/6] lint --- .../src/opentelemetry/baggage/propagation/__init__.py | 2 +- .../src/opentelemetry/trace/propagation/textmap.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py index ad47179c3de..75ba25bbe8d 100644 --- a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py @@ -101,7 +101,7 @@ def _format_baggage(baggage_entries: typing.Mapping[str, object]) -> str: def _extract_first_element( - items: typing.Iterable[textmap.TextMapPropagatorT], + items: typing.Optional[typing.Iterable[textmap.TextMapPropagatorT]], ) -> typing.Optional[textmap.TextMapPropagatorT]: if items is None: return None diff --git a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py index 05ae4e057a1..0612a740f64 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py @@ -30,9 +30,7 @@ class Getter(typing.Generic[TextMapPropagatorT]): """ def get( - self, - carrier: TextMapPropagatorT, - key: str + self, carrier: TextMapPropagatorT, key: str ) -> typing.Optional[typing.List[str]]: """Function that can retrieve zero or more values from the carrier. In the case that From df6e6ae543df3e3c7d90be851e8e47807006c4e9 Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 4 Dec 2020 13:17:40 -0500 Subject: [PATCH 4/6] typing --- opentelemetry-api/src/opentelemetry/trace/span.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index 2d0e34996c7..9ab1929df0d 100644 --- a/opentelemetry-api/src/opentelemetry/trace/span.py +++ b/opentelemetry-api/src/opentelemetry/trace/span.py @@ -175,8 +175,8 @@ def __new__( trace_id: int, span_id: int, is_remote: bool, - trace_flags: "TraceFlags" = DEFAULT_TRACE_OPTIONS, - trace_state: "TraceState" = DEFAULT_TRACE_STATE, + trace_flags: typing.Optional["TraceFlags"] = DEFAULT_TRACE_OPTIONS, + trace_state: typing.Optional["TraceState"] = DEFAULT_TRACE_STATE, ) -> "SpanContext": if trace_flags is None: trace_flags = DEFAULT_TRACE_OPTIONS From 9db39c6ccceb7a8b7530a7e008316d9810facd87 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 7 Dec 2020 11:15:02 -0500 Subject: [PATCH 5/6] test --- .../tests/trace/propagation/test_textmap.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 opentelemetry-api/tests/trace/propagation/test_textmap.py diff --git a/opentelemetry-api/tests/trace/propagation/test_textmap.py b/opentelemetry-api/tests/trace/propagation/test_textmap.py new file mode 100644 index 00000000000..8347af6cc95 --- /dev/null +++ b/opentelemetry-api/tests/trace/propagation/test_textmap.py @@ -0,0 +1,42 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +from opentelemetry.trace.propagation.textmap import DictGetter + + +class TestDictGetter(unittest.TestCase): + def test_get_none(self): + getter = DictGetter() + carrier = {} + val = getter.get(carrier, "test") + self.assertIsNone(val) + + def test_get_str(self): + getter = DictGetter() + carrier = {"test":"val"} + val = getter.get(carrier, "test") + self.assertEqual(val, ["val"]) + + def test_get_iter(self): + getter = DictGetter() + carrier = {"test":["val"]} + val = getter.get(carrier, "test") + self.assertEqual(val, ["val"]) + + def test_keys(self): + getter = DictGetter() + keys = getter.keys({"test":"val"}) + self.assertEqual(keys, ["test"]) From 86b1623ed346396d193532a27847d6310cbff353 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 8 Dec 2020 09:43:57 -0500 Subject: [PATCH 6/6] lint --- opentelemetry-api/tests/trace/propagation/test_textmap.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/tests/trace/propagation/test_textmap.py b/opentelemetry-api/tests/trace/propagation/test_textmap.py index 8347af6cc95..830f7ac2c1c 100644 --- a/opentelemetry-api/tests/trace/propagation/test_textmap.py +++ b/opentelemetry-api/tests/trace/propagation/test_textmap.py @@ -26,17 +26,17 @@ def test_get_none(self): def test_get_str(self): getter = DictGetter() - carrier = {"test":"val"} + carrier = {"test": "val"} val = getter.get(carrier, "test") self.assertEqual(val, ["val"]) def test_get_iter(self): getter = DictGetter() - carrier = {"test":["val"]} + carrier = {"test": ["val"]} val = getter.get(carrier, "test") self.assertEqual(val, ["val"]) def test_keys(self): getter = DictGetter() - keys = getter.keys({"test":"val"}) + keys = getter.keys({"test": "val"}) self.assertEqual(keys, ["test"])