diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 167a52e5ac..aa3059dfa8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,8 +6,7 @@ on: - 'release/*' pull_request: env: - CORE_REPO_SHA: f69e12fba8d0afd587dd21adbedfe751153aa73c - + CORE_REPO_SHA: master jobs: build: diff --git a/instrumentation/opentelemetry-instrumentation-asgi/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-asgi/CHANGELOG.md index 686b8cf830..93399696b5 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-asgi/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Return `None` for `CarrierGetter` if key not found + ([#1374](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/233)) + ## Version 0.12b0 Released 2020-08-14 diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 1c442889a1..6391636035 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -34,7 +34,9 @@ class CarrierGetter(DictGetter): - def get(self, carrier: dict, key: str) -> typing.List[str]: + def get( + self, carrier: dict, key: str + ) -> typing.Optional[typing.List[str]]: """Getter implementation to retrieve a HTTP header value from the ASGI scope. @@ -43,14 +45,17 @@ def get(self, carrier: dict, key: str) -> typing.List[str]: key: header name in scope Returns: A list with a single string with the header value if it exists, - else an empty list. + else None. """ headers = carrier.get("headers") - return [ + decoded = [ _value.decode("utf8") for (_key, _value) in headers if _key.decode("utf8") == key ] + if not decoded: + return None + return decoded carrier_getter = CarrierGetter() @@ -82,11 +87,12 @@ def collect_request_attributes(scope): http_method = scope.get("method") if http_method: result["http.method"] = http_method - http_host_value = ",".join(carrier_getter.get(scope, "host")) - if http_host_value: - result["http.server_name"] = http_host_value + + http_host_value_list = carrier_getter.get(scope, "host") + if http_host_value_list: + result["http.server_name"] = ",".join(http_host_value_list) http_user_agent = carrier_getter.get(scope, "user-agent") - if len(http_user_agent) > 0: + if http_user_agent: result["http.user_agent"] = http_user_agent[0] if "client" in scope and scope["client"] is not None: diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index cf8944ce6d..298d8ca458 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -164,7 +164,7 @@ def test_basic_asgi_call(self): outputs = self.get_all_output() self.validate_outputs(outputs) - def test_wsgi_not_recording(self): + def test_asgi_not_recording(self): mock_tracer = mock.Mock() mock_span = mock.Mock() mock_span.is_recording.return_value = False @@ -312,8 +312,12 @@ def setUp(self): def test_request_attributes(self): self.scope["query_string"] = b"foo=bar" + headers = [] + headers.append(("host".encode("utf8"), "test".encode("utf8"))) + self.scope["headers"] = headers attrs = otel_asgi.collect_request_attributes(self.scope) + self.assertDictEqual( attrs, { @@ -324,6 +328,7 @@ def test_request_attributes(self): "http.url": "http://127.0.0.1/?foo=bar", "host.port": 80, "http.scheme": "http", + "http.server_name": "test", "http.flavor": "1.0", "net.peer.ip": "127.0.0.1", "net.peer.port": 32767, diff --git a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py index cdf5b00618..495cbe96a1 100644 --- a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py @@ -80,7 +80,9 @@ def add(x, y): class CarrierGetter(DictGetter): def get(self, carrier, key): - value = getattr(carrier, key, []) + value = getattr(carrier, key, None) + if value is None: + return None if isinstance(value, str) or not isinstance(value, Iterable): value = (value,) return value diff --git a/instrumentation/opentelemetry-instrumentation-celery/tests/test_getter.py b/instrumentation/opentelemetry-instrumentation-celery/tests/test_getter.py new file mode 100644 index 0000000000..a534309750 --- /dev/null +++ b/instrumentation/opentelemetry-instrumentation-celery/tests/test_getter.py @@ -0,0 +1,44 @@ +# 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. + +from unittest import TestCase, mock + +from opentelemetry.instrumentation.celery import CarrierGetter + + +class TestCarrierGetter(TestCase): + def test_get_none(self): + getter = CarrierGetter() + carrier = {} + val = getter.get(carrier, "test") + self.assertIsNone(val) + + def test_get_str(self): + mock_obj = mock.Mock() + getter = CarrierGetter() + mock_obj.test = "val" + val = getter.get(mock_obj, "test") + self.assertEqual(val, ("val",)) + + def test_get_iter(self): + mock_obj = mock.Mock() + getter = CarrierGetter() + mock_obj.test = ["val"] + val = getter.get(mock_obj, "test") + self.assertEqual(val, ["val"]) + + def test_keys(self): + getter = CarrierGetter() + keys = getter.keys({}) + self.assertEqual(keys, []) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-wsgi/CHANGELOG.md index 68bd25237c..ee3678f89e 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-wsgi/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Return `None` for `CarrierGetter` if key not found + ([#1374](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/233)) + ## Version 0.13b0 Released 2020-09-17 diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index e1ef92c6ba..b8de5c1820 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -68,7 +68,9 @@ def hello(): class CarrierGetter(DictGetter): - def get(self, carrier: dict, key: str) -> typing.List[str]: + def get( + self, carrier: dict, key: str + ) -> typing.Optional[typing.List[str]]: """Getter implementation to retrieve a HTTP header value from the PEP3333-conforming WSGI environ @@ -77,13 +79,13 @@ def get(self, carrier: dict, key: str) -> typing.List[str]: key: header name in environ object Returns: A list with a single string with the header value if it exists, - else an empty list. + else None. """ environ_key = "HTTP_" + key.upper().replace("-", "_") value = carrier.get(environ_key) if value is not None: return [value] - return [] + return None def keys(self, carrier): return [] diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_getter.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_getter.py new file mode 100644 index 0000000000..630e5fc579 --- /dev/null +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_getter.py @@ -0,0 +1,36 @@ +# 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. + +from unittest import TestCase, mock + +from opentelemetry.instrumentation.wsgi import CarrierGetter + + +class TestCarrierGetter(TestCase): + def test_get_none(self): + getter = CarrierGetter() + carrier = {} + val = getter.get(carrier, "test") + self.assertIsNone(val) + + def test_get_(self): + getter = CarrierGetter() + carrier = {"HTTP_TEST_KEY": "val"} + val = getter.get(carrier, "test-key") + self.assertEqual(val, ["val"]) + + def test_keys(self): + getter = CarrierGetter() + keys = getter.keys({}) + self.assertEqual(keys, [])