From 780a7822c930ea629aa2133a3f0ce34b724c2bfe Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 8 Dec 2020 10:02:10 -0500 Subject: [PATCH] Return none for Getter if key does not exist (#1449) --- opentelemetry-api/CHANGELOG.md | 2 + .../baggage/propagation/__init__.py | 2 +- .../trace/propagation/textmap.py | 14 ++++--- .../trace/propagation/tracecontext.py | 5 ++- .../src/opentelemetry/trace/span.py | 4 +- .../tests/trace/propagation/test_textmap.py | 42 +++++++++++++++++++ 6 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 opentelemetry-api/tests/trace/propagation/test_textmap.py diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index 84604f4bb27..af02d068dc8 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 `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/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 fb92d371680..0612a740f64 100644 --- a/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py +++ b/opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py @@ -29,7 +29,9 @@ 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 +40,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 +60,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] 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), diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index d41a0c3fbd4..6506f6f949d 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 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..830f7ac2c1c --- /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"])