From 5d6df0cd7836ae9b830680cc2b96dac43d345cad Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Sat, 8 Feb 2020 15:52:23 +0100 Subject: [PATCH 01/36] ext-wsgi duplicated as ext-asgi --- ext/opentelemetry-ext-asgi/CHANGELOG.md | 23 ++ ext/opentelemetry-ext-asgi/README.rst | 60 ++++ ext/opentelemetry-ext-asgi/setup.cfg | 49 +++ ext/opentelemetry-ext-asgi/setup.py | 26 ++ .../src/opentelemetry/ext/wsgi/__init__.py | 219 +++++++++++++ .../src/opentelemetry/ext/wsgi/version.py | 15 + ext/opentelemetry-ext-asgi/tests/__init__.py | 0 .../tests/test_wsgi_middleware.py | 297 ++++++++++++++++++ 8 files changed, 689 insertions(+) create mode 100644 ext/opentelemetry-ext-asgi/CHANGELOG.md create mode 100644 ext/opentelemetry-ext-asgi/README.rst create mode 100644 ext/opentelemetry-ext-asgi/setup.cfg create mode 100644 ext/opentelemetry-ext-asgi/setup.py create mode 100644 ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/__init__.py create mode 100644 ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/version.py create mode 100644 ext/opentelemetry-ext-asgi/tests/__init__.py create mode 100644 ext/opentelemetry-ext-asgi/tests/test_wsgi_middleware.py diff --git a/ext/opentelemetry-ext-asgi/CHANGELOG.md b/ext/opentelemetry-ext-asgi/CHANGELOG.md new file mode 100644 index 0000000000..62d8a5baf0 --- /dev/null +++ b/ext/opentelemetry-ext-asgi/CHANGELOG.md @@ -0,0 +1,23 @@ +# Changelog + +## Unreleased + +## 0.3a0 + +Released 2019-12-11 + +- Support new semantic conventions + ([#299](https://github.com/open-telemetry/opentelemetry-python/pull/299)) +- Updates for core library changes + +## 0.2a0 + +Released 2019-10-29 + +- Updates for core library changes + +## 0.1a0 + +Released 2019-09-30 + +- Initial release diff --git a/ext/opentelemetry-ext-asgi/README.rst b/ext/opentelemetry-ext-asgi/README.rst new file mode 100644 index 0000000000..82641bcaa4 --- /dev/null +++ b/ext/opentelemetry-ext-asgi/README.rst @@ -0,0 +1,60 @@ +OpenTelemetry WSGI Middleware +============================= + +|pypi| + +.. |pypi| image:: https://badge.fury.io/py/opentelemetry-ext-wsgi.svg + :target: https://pypi.org/project/opentelemetry-ext-wsgi/ + + +This library provides a WSGI middleware that can be used on any WSGI framework +(such as Django / Flask) to track requests timing through OpenTelemetry. + +Installation +------------ + +:: + + pip install opentelemetry-ext-wsgi + + +Usage (Flask) +------------- + +.. code-block:: python + + from flask import Flask + from opentelemetry.ext.wsgi import OpenTelemetryMiddleware + + app = Flask(__name__) + app.wsgi_app = OpenTelemetryMiddleware(app.wsgi_app) + + @app.route("/") + def hello(): + return "Hello!" + + if __name__ == "__main__": + app.run(debug=True) + + +Usage (Django) +-------------- + +Modify the application's ``wsgi.py`` file as shown below. + +.. code-block:: python + + import os + from opentelemetry.ext.wsgi import OpenTelemetryMiddleware + from django.core.wsgi import get_wsgi_application + + os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'application.settings') + + application = get_wsgi_application() + application = OpenTelemetryMiddleware(application) + +References +---------- + +* `OpenTelemetry Project `_ +* `WSGI `_ diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg new file mode 100644 index 0000000000..1db49209be --- /dev/null +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -0,0 +1,49 @@ +# Copyright 2019, 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. +# +[metadata] +name = opentelemetry-ext-wsgi +description = WSGI Middleware for OpenTelemetry +long_description = file: README.rst +long_description_content_type = text/x-rst +author = OpenTelemetry Authors +author_email = cncf-opentelemetry-contributors@lists.cncf.io +url = https://github.com/open-telemetry/opentelemetry-python/ext/opentelemetry-ext-wsgi +platforms = any +license = Apache-2.0 +classifiers = + Development Status :: 3 - Alpha + Intended Audience :: Developers + License :: OSI Approved :: Apache Software License + Programming Language :: Python + Programming Language :: Python :: 3 + Programming Language :: Python :: 3.4 + Programming Language :: Python :: 3.5 + Programming Language :: Python :: 3.6 + Programming Language :: Python :: 3.7 + +[options] +python_requires = >=3.4 +package_dir= + =src +packages=find_namespace: +install_requires = + opentelemetry-api + +[options.extras_require] +test = + opentelemetry-ext-testutil + +[options.packages.find] +where = src diff --git a/ext/opentelemetry-ext-asgi/setup.py b/ext/opentelemetry-ext-asgi/setup.py new file mode 100644 index 0000000000..3f8ef9cc5f --- /dev/null +++ b/ext/opentelemetry-ext-asgi/setup.py @@ -0,0 +1,26 @@ +# Copyright 2019, 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 os + +import setuptools + +BASE_DIR = os.path.dirname(__file__) +VERSION_FILENAME = os.path.join( + BASE_DIR, "src", "opentelemetry", "ext", "wsgi", "version.py" +) +PACKAGE_INFO = {} +with open(VERSION_FILENAME) as f: + exec(f.read(), PACKAGE_INFO) + +setuptools.setup(version=PACKAGE_INFO["__version__"]) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/__init__.py new file mode 100644 index 0000000000..e6751f34ce --- /dev/null +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/__init__.py @@ -0,0 +1,219 @@ +# Copyright 2019, 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. + +""" +The opentelemetry-ext-wsgi package provides a WSGI middleware that can be used +on any WSGI framework (such as Django / Flask) to track requests timing through +OpenTelemetry. +""" + +import functools +import typing +import wsgiref.util as wsgiref_util + +from opentelemetry import propagators, trace +from opentelemetry.ext.wsgi.version import __version__ +from opentelemetry.trace.status import Status, StatusCanonicalCode + +_HTTP_VERSION_PREFIX = "HTTP/" + + +def get_header_from_environ( + environ: dict, header_name: str +) -> typing.List[str]: + """Retrieve a HTTP header value from the PEP3333-conforming WSGI environ. + + Returns: + A list with a single string with the header value if it exists, else an empty list. + """ + environ_key = "HTTP_" + header_name.upper().replace("-", "_") + value = environ.get(environ_key) + if value is not None: + return [value] + return [] + + +def setifnotnone(dic, key, value): + if value is not None: + dic[key] = value + + +def http_status_to_canonical_code(code: int, allow_redirect: bool = True): + # pylint:disable=too-many-branches,too-many-return-statements + if code < 100: + return StatusCanonicalCode.UNKNOWN + if code <= 299: + return StatusCanonicalCode.OK + if code <= 399: + if allow_redirect: + return StatusCanonicalCode.OK + return StatusCanonicalCode.DEADLINE_EXCEEDED + if code <= 499: + if code == 401: # HTTPStatus.UNAUTHORIZED: + return StatusCanonicalCode.UNAUTHENTICATED + if code == 403: # HTTPStatus.FORBIDDEN: + return StatusCanonicalCode.PERMISSION_DENIED + if code == 404: # HTTPStatus.NOT_FOUND: + return StatusCanonicalCode.NOT_FOUND + if code == 429: # HTTPStatus.TOO_MANY_REQUESTS: + return StatusCanonicalCode.RESOURCE_EXHAUSTED + return StatusCanonicalCode.INVALID_ARGUMENT + if code <= 599: + if code == 501: # HTTPStatus.NOT_IMPLEMENTED: + return StatusCanonicalCode.UNIMPLEMENTED + if code == 503: # HTTPStatus.SERVICE_UNAVAILABLE: + return StatusCanonicalCode.UNAVAILABLE + if code == 504: # HTTPStatus.GATEWAY_TIMEOUT: + return StatusCanonicalCode.DEADLINE_EXCEEDED + return StatusCanonicalCode.INTERNAL + return StatusCanonicalCode.UNKNOWN + + +def collect_request_attributes(environ): + """Collects HTTP request attributes from the PEP3333-conforming + WSGI environ and returns a dictionary to be used as span creation attributes.""" + + result = { + "component": "http", + "http.method": environ["REQUEST_METHOD"], + "http.server_name": environ["SERVER_NAME"], + "http.scheme": environ["wsgi.url_scheme"], + "host.port": int(environ["SERVER_PORT"]), + } + + setifnotnone(result, "http.host", environ.get("HTTP_HOST")) + target = environ.get("RAW_URI") + if target is None: # Note: `"" or None is None` + target = environ.get("REQUEST_URI") + if target is not None: + result["http.target"] = target + else: + result["http.url"] = wsgiref_util.request_uri(environ) + + remote_addr = environ.get("REMOTE_ADDR") + if remote_addr: + result["net.peer.ip"] = remote_addr + remote_host = environ.get("REMOTE_HOST") + if remote_host and remote_host != remote_addr: + result["net.peer.name"] = remote_host + + setifnotnone(result, "net.peer.port", environ.get("REMOTE_PORT")) + flavor = environ.get("SERVER_PROTOCOL", "") + if flavor.upper().startswith(_HTTP_VERSION_PREFIX): + flavor = flavor[len(_HTTP_VERSION_PREFIX) :] + if flavor: + result["http.flavor"] = flavor + + return result + + +def add_response_attributes( + span, start_response_status, response_headers +): # pylint: disable=unused-argument + """Adds HTTP response attributes to span using the arguments + passed to a PEP3333-conforming start_response callable.""" + + status_code, status_text = start_response_status.split(" ", 1) + span.set_attribute("http.status_text", status_text) + + try: + status_code = int(status_code) + except ValueError: + span.set_status( + Status( + StatusCanonicalCode.UNKNOWN, + "Non-integer HTTP status: " + repr(status_code), + ) + ) + else: + span.set_attribute("http.status_code", status_code) + span.set_status(Status(http_status_to_canonical_code(status_code))) + + +def get_default_span_name(environ): + """Calculates a (generic) span name for an incoming HTTP request based on the PEP3333 conforming WSGI environ.""" + + # TODO: Update once + # https://github.com/open-telemetry/opentelemetry-specification/issues/270 + # is resolved + return environ.get("PATH_INFO", "/") + + +class OpenTelemetryMiddleware: + """The WSGI application middleware. + + This class is a PEP 3333 conforming WSGI middleware that starts and + annotates spans for any requests it is invoked with. + + Args: + wsgi: The WSGI application callable to forward requests to. + """ + + def __init__(self, wsgi): + self.wsgi = wsgi + self.tracer = trace.tracer_source().get_tracer(__name__, __version__) + + @staticmethod + def _create_start_response(span, start_response): + @functools.wraps(start_response) + def _start_response(status, response_headers, *args, **kwargs): + add_response_attributes(span, status, response_headers) + return start_response(status, response_headers, *args, **kwargs) + + return _start_response + + def __call__(self, environ, start_response): + """The WSGI application + + Args: + environ: A WSGI environment. + start_response: The WSGI start_response callable. + """ + + parent_span = propagators.extract(get_header_from_environ, environ) + span_name = get_default_span_name(environ) + + span = self.tracer.start_span( + span_name, + parent_span, + kind=trace.SpanKind.SERVER, + attributes=collect_request_attributes(environ), + ) + + try: + with self.tracer.use_span(span): + start_response = self._create_start_response( + span, start_response + ) + iterable = self.wsgi(environ, start_response) + return _end_span_after_iterating(iterable, span, self.tracer) + except: # noqa + # TODO Set span status (cf. https://github.com/open-telemetry/opentelemetry-python/issues/292) + span.end() + raise + + +# Put this in a subfunction to not delay the call to the wrapped +# WSGI application (instrumentation should change the application +# behavior as little as possible). +def _end_span_after_iterating(iterable, span, tracer): + try: + with tracer.use_span(span): + for yielded in iterable: + yield yielded + finally: + close = getattr(iterable, "close", None) + if close: + close() + span.end() diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/version.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/version.py new file mode 100644 index 0000000000..2f792fff80 --- /dev/null +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/version.py @@ -0,0 +1,15 @@ +# Copyright 2019, 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. + +__version__ = "0.4.dev0" diff --git a/ext/opentelemetry-ext-asgi/tests/__init__.py b/ext/opentelemetry-ext-asgi/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/ext/opentelemetry-ext-asgi/tests/test_wsgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_wsgi_middleware.py new file mode 100644 index 0000000000..1912dd0079 --- /dev/null +++ b/ext/opentelemetry-ext-asgi/tests/test_wsgi_middleware.py @@ -0,0 +1,297 @@ +# Copyright 2019, 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 sys +import unittest +import unittest.mock as mock +import wsgiref.util as wsgiref_util +from urllib.parse import urlsplit + +import opentelemetry.ext.wsgi as otel_wsgi +from opentelemetry import trace as trace_api +from opentelemetry.ext.testutil.wsgitestutil import WsgiTestBase + + +class Response: + def __init__(self): + self.iter = iter([b"*"]) + self.close_calls = 0 + + def __iter__(self): + return self + + def __next__(self): + return next(self.iter) + + def close(self): + self.close_calls += 1 + + +def simple_wsgi(environ, start_response): + assert isinstance(environ, dict) + start_response("200 OK", [("Content-Type", "text/plain")]) + return [b"*"] + + +def create_iter_wsgi(response): + def iter_wsgi(environ, start_response): + assert isinstance(environ, dict) + start_response("200 OK", [("Content-Type", "text/plain")]) + return response + + return iter_wsgi + + +def create_gen_wsgi(response): + def gen_wsgi(environ, start_response): + result = create_iter_wsgi(response)(environ, start_response) + yield from result + getattr(result, "close", lambda: None)() + + return gen_wsgi + + +def error_wsgi(environ, start_response): + assert isinstance(environ, dict) + try: + raise ValueError + except ValueError: + exc_info = sys.exc_info() + start_response("200 OK", [("Content-Type", "text/plain")], exc_info) + exc_info = None + return [b"*"] + + +class TestWsgiApplication(WsgiTestBase): + def validate_response(self, response, error=None): + while True: + try: + value = next(response) + self.assertEqual(value, b"*") + except StopIteration: + break + + self.assertEqual(self.status, "200 OK") + self.assertEqual( + self.response_headers, [("Content-Type", "text/plain")] + ) + if error: + self.assertIs(self.exc_info[0], error) + self.assertIsInstance(self.exc_info[1], error) + self.assertIsNotNone(self.exc_info[2]) + else: + self.assertIsNone(self.exc_info) + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + self.assertEqual(span_list[0].name, "/") + self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) + self.assertEqual( + span_list[0].attributes, + { + "component": "http", + "http.method": "GET", + "http.server_name": "127.0.0.1", + "http.scheme": "http", + "host.port": 80, + "http.host": "127.0.0.1", + "http.flavor": "1.0", + "http.url": "http://127.0.0.1/", + "http.status_text": "OK", + "http.status_code": 200, + }, + ) + + def test_basic_wsgi_call(self): + app = otel_wsgi.OpenTelemetryMiddleware(simple_wsgi) + response = app(self.environ, self.start_response) + self.validate_response(response) + + def test_wsgi_iterable(self): + original_response = Response() + iter_wsgi = create_iter_wsgi(original_response) + app = otel_wsgi.OpenTelemetryMiddleware(iter_wsgi) + response = app(self.environ, self.start_response) + # Verify that start_response has been called + self.assertTrue(self.status) + self.validate_response(response) + + # Verify that close has been called exactly once + self.assertEqual(1, original_response.close_calls) + + def test_wsgi_generator(self): + original_response = Response() + gen_wsgi = create_gen_wsgi(original_response) + app = otel_wsgi.OpenTelemetryMiddleware(gen_wsgi) + response = app(self.environ, self.start_response) + # Verify that start_response has not been called + self.assertIsNone(self.status) + self.validate_response(response) + + # Verify that close has been called exactly once + self.assertEqual(original_response.close_calls, 1) + + def test_wsgi_exc_info(self): + app = otel_wsgi.OpenTelemetryMiddleware(error_wsgi) + response = app(self.environ, self.start_response) + self.validate_response(response, error=ValueError) + + +class TestWsgiAttributes(unittest.TestCase): + def setUp(self): + self.environ = {} + wsgiref_util.setup_testing_defaults(self.environ) + self.span = mock.create_autospec(trace_api.Span, spec_set=True) + + def test_request_attributes(self): + self.environ["QUERY_STRING"] = "foo=bar" + + attrs = otel_wsgi.collect_request_attributes(self.environ) + self.assertDictEqual( + attrs, + { + "component": "http", + "http.method": "GET", + "http.host": "127.0.0.1", + "http.url": "http://127.0.0.1/?foo=bar", + "host.port": 80, + "http.scheme": "http", + "http.server_name": "127.0.0.1", + "http.flavor": "1.0", + }, + ) + + def validate_url(self, expected_url, raw=False, has_host=True): + parts = urlsplit(expected_url) + expected = { + "http.scheme": parts.scheme, + "host.port": parts.port or (80 if parts.scheme == "http" else 443), + "http.server_name": parts.hostname, # Not true in the general case, but for all tests. + } + if raw: + expected["http.target"] = expected_url.split(parts.netloc, 1)[1] + else: + expected["http.url"] = expected_url + if has_host: + expected["http.host"] = parts.hostname + + attrs = otel_wsgi.collect_request_attributes(self.environ) + self.assertGreaterEqual( + attrs.items(), expected.items(), expected_url + " expected." + ) + + def test_request_attributes_with_partial_raw_uri(self): + self.environ["RAW_URI"] = "/#top" + self.validate_url("http://127.0.0.1/#top", raw=True) + + def test_request_attributes_with_partial_raw_uri_and_nonstandard_port( + self, + ): + self.environ["RAW_URI"] = "/?" + del self.environ["HTTP_HOST"] + self.environ["SERVER_PORT"] = "8080" + self.validate_url("http://127.0.0.1:8080/?", raw=True, has_host=False) + + def test_https_uri_port(self): + del self.environ["HTTP_HOST"] + self.environ["SERVER_PORT"] = "443" + self.environ["wsgi.url_scheme"] = "https" + self.validate_url("https://127.0.0.1/", has_host=False) + + self.environ["SERVER_PORT"] = "8080" + self.validate_url("https://127.0.0.1:8080/", has_host=False) + + self.environ["SERVER_PORT"] = "80" + self.validate_url("https://127.0.0.1:80/", has_host=False) + + def test_http_uri_port(self): + del self.environ["HTTP_HOST"] + self.environ["SERVER_PORT"] = "80" + self.environ["wsgi.url_scheme"] = "http" + self.validate_url("http://127.0.0.1/", has_host=False) + + self.environ["SERVER_PORT"] = "8080" + self.validate_url("http://127.0.0.1:8080/", has_host=False) + + self.environ["SERVER_PORT"] = "443" + self.validate_url("http://127.0.0.1:443/", has_host=False) + + def test_request_attributes_with_nonstandard_port_and_no_host(self): + del self.environ["HTTP_HOST"] + self.environ["SERVER_PORT"] = "8080" + self.validate_url("http://127.0.0.1:8080/", has_host=False) + + self.environ["SERVER_PORT"] = "443" + self.validate_url("http://127.0.0.1:443/", has_host=False) + + def test_request_attributes_with_conflicting_nonstandard_port(self): + self.environ[ + "HTTP_HOST" + ] += ":8080" # Note that we do not correct SERVER_PORT + expected = { + "http.host": "127.0.0.1:8080", + "http.url": "http://127.0.0.1:8080/", + "host.port": 80, + } + self.assertGreaterEqual( + otel_wsgi.collect_request_attributes(self.environ).items(), + expected.items(), + ) + + def test_request_attributes_with_faux_scheme_relative_raw_uri(self): + self.environ["RAW_URI"] = "//127.0.0.1/?" + self.validate_url("http://127.0.0.1//127.0.0.1/?", raw=True) + + def test_request_attributes_pathless(self): + self.environ["RAW_URI"] = "" + expected = {"http.target": ""} + self.assertGreaterEqual( + otel_wsgi.collect_request_attributes(self.environ).items(), + expected.items(), + ) + + def test_request_attributes_with_full_request_uri(self): + self.environ["HTTP_HOST"] = "127.0.0.1:8080" + self.environ["REQUEST_METHOD"] = "CONNECT" + self.environ[ + "REQUEST_URI" + ] = "127.0.0.1:8080" # Might happen in a CONNECT request + expected = { + "http.host": "127.0.0.1:8080", + "http.target": "127.0.0.1:8080", + } + self.assertGreaterEqual( + otel_wsgi.collect_request_attributes(self.environ).items(), + expected.items(), + ) + + def test_response_attributes(self): + otel_wsgi.add_response_attributes(self.span, "404 Not Found", {}) + expected = ( + mock.call("http.status_code", 404), + mock.call("http.status_text", "Not Found"), + ) + self.assertEqual(self.span.set_attribute.call_count, len(expected)) + self.span.set_attribute.assert_has_calls(expected, any_order=True) + + def test_response_attributes_invalid_status_code(self): + otel_wsgi.add_response_attributes(self.span, "Invalid Status Code", {}) + self.assertEqual(self.span.set_attribute.call_count, 1) + self.span.set_attribute.assert_called_with( + "http.status_text", "Status Code" + ) + + +if __name__ == "__main__": + unittest.main() From e2b85ad084fed1d7fb7a034be72914dcdffd3c94 Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Sat, 8 Feb 2020 16:08:38 +0100 Subject: [PATCH 02/36] Rename WSGI files --- .../src/opentelemetry/ext/{wsgi => asgi}/__init__.py | 0 .../src/opentelemetry/ext/{wsgi => asgi}/version.py | 0 .../tests/{test_wsgi_middleware.py => test_asgi_middleware.py} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename ext/opentelemetry-ext-asgi/src/opentelemetry/ext/{wsgi => asgi}/__init__.py (100%) rename ext/opentelemetry-ext-asgi/src/opentelemetry/ext/{wsgi => asgi}/version.py (100%) rename ext/opentelemetry-ext-asgi/tests/{test_wsgi_middleware.py => test_asgi_middleware.py} (100%) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py similarity index 100% rename from ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/__init__.py rename to ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/version.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py similarity index 100% rename from ext/opentelemetry-ext-asgi/src/opentelemetry/ext/wsgi/version.py rename to ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py diff --git a/ext/opentelemetry-ext-asgi/tests/test_wsgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py similarity index 100% rename from ext/opentelemetry-ext-asgi/tests/test_wsgi_middleware.py rename to ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py From f729c3d9c14b0e8356342851a874713f2e8be2bc Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Sat, 8 Feb 2020 16:09:17 +0100 Subject: [PATCH 03/36] WSGI to ASGI --- ext/opentelemetry-ext-asgi/CHANGELOG.md | 20 - ext/opentelemetry-ext-asgi/README.rst | 33 +- ext/opentelemetry-ext-asgi/setup.cfg | 9 +- ext/opentelemetry-ext-asgi/setup.py | 2 +- .../src/opentelemetry/ext/asgi/__init__.py | 204 +++++----- .../tests/test_asgi_middleware.py | 373 +++++++----------- 6 files changed, 246 insertions(+), 395 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/CHANGELOG.md b/ext/opentelemetry-ext-asgi/CHANGELOG.md index 62d8a5baf0..1512c42162 100644 --- a/ext/opentelemetry-ext-asgi/CHANGELOG.md +++ b/ext/opentelemetry-ext-asgi/CHANGELOG.md @@ -1,23 +1,3 @@ # Changelog ## Unreleased - -## 0.3a0 - -Released 2019-12-11 - -- Support new semantic conventions - ([#299](https://github.com/open-telemetry/opentelemetry-python/pull/299)) -- Updates for core library changes - -## 0.2a0 - -Released 2019-10-29 - -- Updates for core library changes - -## 0.1a0 - -Released 2019-09-30 - -- Initial release diff --git a/ext/opentelemetry-ext-asgi/README.rst b/ext/opentelemetry-ext-asgi/README.rst index 82641bcaa4..09e8bcc1fa 100644 --- a/ext/opentelemetry-ext-asgi/README.rst +++ b/ext/opentelemetry-ext-asgi/README.rst @@ -1,13 +1,13 @@ -OpenTelemetry WSGI Middleware +OpenTelemetry ASGI Middleware ============================= |pypi| -.. |pypi| image:: https://badge.fury.io/py/opentelemetry-ext-wsgi.svg - :target: https://pypi.org/project/opentelemetry-ext-wsgi/ +.. |pypi| image:: https://badge.fury.io/py/opentelemetry-ext-asgi.svg + :target: https://pypi.org/project/opentelemetry-ext-asgi/ -This library provides a WSGI middleware that can be used on any WSGI framework +This library provides a ASGI middleware that can be used on any ASGI framework (such as Django / Flask) to track requests timing through OpenTelemetry. Installation @@ -15,22 +15,22 @@ Installation :: - pip install opentelemetry-ext-wsgi + pip install opentelemetry-ext-asgi -Usage (Flask) +Usage (Quart) ------------- .. code-block:: python - from flask import Flask - from opentelemetry.ext.wsgi import OpenTelemetryMiddleware + from quart import Quart + from opentelemetry.ext.asgi import OpenTelemetryMiddleware - app = Flask(__name__) - app.wsgi_app = OpenTelemetryMiddleware(app.wsgi_app) + app = Quart(__name__) + app.asgi_app = OpenTelemetryMiddleware(app.asgi_app) @app.route("/") - def hello(): + async def hello(): return "Hello!" if __name__ == "__main__": @@ -40,21 +40,22 @@ Usage (Flask) Usage (Django) -------------- -Modify the application's ``wsgi.py`` file as shown below. +Modify the application's ``asgi.py`` file as shown below. .. code-block:: python import os - from opentelemetry.ext.wsgi import OpenTelemetryMiddleware - from django.core.wsgi import get_wsgi_application + import django + from channels.routing import get_default_application + from opentelemetry.ext.asgi import OpenTelemetryMiddleware os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'application.settings') + django.setup() - application = get_wsgi_application() + application = get_default_application() application = OpenTelemetryMiddleware(application) References ---------- * `OpenTelemetry Project `_ -* `WSGI `_ diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index 1db49209be..903ad7e662 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -13,13 +13,13 @@ # limitations under the License. # [metadata] -name = opentelemetry-ext-wsgi -description = WSGI Middleware for OpenTelemetry +name = opentelemetry-ext-asgi +description = ASGI Middleware for OpenTelemetry long_description = file: README.rst long_description_content_type = text/x-rst author = OpenTelemetry Authors author_email = cncf-opentelemetry-contributors@lists.cncf.io -url = https://github.com/open-telemetry/opentelemetry-python/ext/opentelemetry-ext-wsgi +url = https://github.com/open-telemetry/opentelemetry-python/ext/opentelemetry-ext-asgi platforms = any license = Apache-2.0 classifiers = @@ -28,9 +28,6 @@ classifiers = License :: OSI Approved :: Apache Software License Programming Language :: Python Programming Language :: Python :: 3 - Programming Language :: Python :: 3.4 - Programming Language :: Python :: 3.5 - Programming Language :: Python :: 3.6 Programming Language :: Python :: 3.7 [options] diff --git a/ext/opentelemetry-ext-asgi/setup.py b/ext/opentelemetry-ext-asgi/setup.py index 3f8ef9cc5f..42c82506eb 100644 --- a/ext/opentelemetry-ext-asgi/setup.py +++ b/ext/opentelemetry-ext-asgi/setup.py @@ -17,7 +17,7 @@ BASE_DIR = os.path.dirname(__file__) VERSION_FILENAME = os.path.join( - BASE_DIR, "src", "opentelemetry", "ext", "wsgi", "version.py" + BASE_DIR, "src", "opentelemetry", "ext", "asgi", "version.py" ) PACKAGE_INFO = {} with open(VERSION_FILENAME) as f: diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index e6751f34ce..4f520e679b 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -13,40 +13,36 @@ # limitations under the License. """ -The opentelemetry-ext-wsgi package provides a WSGI middleware that can be used -on any WSGI framework (such as Django / Flask) to track requests timing through -OpenTelemetry. +The opentelemetry-ext-asgi package provides an ASGI middleware that can be used +on any ASGI framework (such as Django-channels / Quart) to track requests +timing through OpenTelemetry. """ -import functools +from functools import wraps import typing -import wsgiref.util as wsgiref_util +import operator +from asgiref.compatibility import guarantee_single_callable from opentelemetry import propagators, trace -from opentelemetry.ext.wsgi.version import __version__ +from opentelemetry.ext.asgi.version import __version__ # noqa from opentelemetry.trace.status import Status, StatusCanonicalCode _HTTP_VERSION_PREFIX = "HTTP/" -def get_header_from_environ( - environ: dict, header_name: str +def get_header_from_scope( + scope: dict, header_name: str ) -> typing.List[str]: - """Retrieve a HTTP header value from the PEP3333-conforming WSGI environ. + """Retrieve a HTTP header value from the ASGI scope. Returns: A list with a single string with the header value if it exists, else an empty list. """ - environ_key = "HTTP_" + header_name.upper().replace("-", "_") - value = environ.get(environ_key) - if value is not None: - return [value] - return [] - - -def setifnotnone(dic, key, value): - if value is not None: - dic[key] = value + headers = scope.get('headers') + return [ + value.decode('utf8') for (key,value) in headers + if key.decode('utf8') == header_name + ] def http_status_to_canonical_code(code: int, allow_redirect: bool = True): @@ -80,53 +76,39 @@ def http_status_to_canonical_code(code: int, allow_redirect: bool = True): return StatusCanonicalCode.UNKNOWN -def collect_request_attributes(environ): - """Collects HTTP request attributes from the PEP3333-conforming - WSGI environ and returns a dictionary to be used as span creation attributes.""" +def collect_request_attributes(scope): + """Collects HTTP request attributes from the ASGI scope and returns a + dictionary to be used as span creation attributes.""" + + port = scope.get("server")[1] + server_host = ( + scope.get("server")[0] + (":" + str(port) if port != 80 else "") + ) + http_url = scope.get("scheme") + "://" + server_host + scope.get("path") + if scope.get("query_string"): + http_url = http_url + ("?" + scope.get("query_string").decode("utf8")) result = { - "component": "http", - "http.method": environ["REQUEST_METHOD"], - "http.server_name": environ["SERVER_NAME"], - "http.scheme": environ["wsgi.url_scheme"], - "host.port": int(environ["SERVER_PORT"]), + "component": scope.get("type"), + "http.method": scope.get("method"), + "http.server_name": scope.get("server")[0], + "http.scheme": scope.get("scheme"), + "http.host": server_host, + "host.port": port, + "http.flavor": scope.get("http_version"), + "http.target": scope.get("path"), + "http.url": http_url, } - setifnotnone(result, "http.host", environ.get("HTTP_HOST")) - target = environ.get("RAW_URI") - if target is None: # Note: `"" or None is None` - target = environ.get("REQUEST_URI") - if target is not None: - result["http.target"] = target - else: - result["http.url"] = wsgiref_util.request_uri(environ) - - remote_addr = environ.get("REMOTE_ADDR") - if remote_addr: - result["net.peer.ip"] = remote_addr - remote_host = environ.get("REMOTE_HOST") - if remote_host and remote_host != remote_addr: - result["net.peer.name"] = remote_host - - setifnotnone(result, "net.peer.port", environ.get("REMOTE_PORT")) - flavor = environ.get("SERVER_PROTOCOL", "") - if flavor.upper().startswith(_HTTP_VERSION_PREFIX): - flavor = flavor[len(_HTTP_VERSION_PREFIX) :] - if flavor: - result["http.flavor"] = flavor + if "client" in scope: + result["net.peer.ip"] = scope.get("client")[0] + result["net.peer.port"] = scope.get("client")[1] return result -def add_response_attributes( - span, start_response_status, response_headers -): # pylint: disable=unused-argument - """Adds HTTP response attributes to span using the arguments - passed to a PEP3333-conforming start_response callable.""" - - status_code, status_text = start_response_status.split(" ", 1) - span.set_attribute("http.status_text", status_text) - +def set_status_code(span, status_code): + """Adds HTTP response attributes to span using the status_code argument.""" try: status_code = int(status_code) except ValueError: @@ -141,79 +123,69 @@ def add_response_attributes( span.set_status(Status(http_status_to_canonical_code(status_code))) -def get_default_span_name(environ): - """Calculates a (generic) span name for an incoming HTTP request based on the PEP3333 conforming WSGI environ.""" +def get_default_span_name(scope): + """Calculates a (generic) span name for an incoming HTTP request based on the ASGI scope.""" # TODO: Update once # https://github.com/open-telemetry/opentelemetry-specification/issues/270 # is resolved - return environ.get("PATH_INFO", "/") + return scope.get("path", "/") class OpenTelemetryMiddleware: - """The WSGI application middleware. + """The ASGI application middleware. - This class is a PEP 3333 conforming WSGI middleware that starts and - annotates spans for any requests it is invoked with. + This class is an ASGI middleware that starts and annotates spans for any + requests it is invoked with. Args: - wsgi: The WSGI application callable to forward requests to. + app: The ASGI application callable to forward requests to. """ - def __init__(self, wsgi): - self.wsgi = wsgi + def __init__(self, app): + self.app = guarantee_single_callable(app) self.tracer = trace.tracer_source().get_tracer(__name__, __version__) - @staticmethod - def _create_start_response(span, start_response): - @functools.wraps(start_response) - def _start_response(status, response_headers, *args, **kwargs): - add_response_attributes(span, status, response_headers) - return start_response(status, response_headers, *args, **kwargs) - - return _start_response - - def __call__(self, environ, start_response): - """The WSGI application + async def __call__(self, scope, receive, send): + """The ASGI application Args: - environ: A WSGI environment. - start_response: The WSGI start_response callable. + scope: A ASGI environment. + receive: An awaitable callable yielding dictionaries + send: An awaitable callable taking a single dictionary as argument. """ - parent_span = propagators.extract(get_header_from_environ, environ) - span_name = get_default_span_name(environ) - - span = self.tracer.start_span( - span_name, - parent_span, - kind=trace.SpanKind.SERVER, - attributes=collect_request_attributes(environ), - ) - - try: - with self.tracer.use_span(span): - start_response = self._create_start_response( - span, start_response - ) - iterable = self.wsgi(environ, start_response) - return _end_span_after_iterating(iterable, span, self.tracer) - except: # noqa - # TODO Set span status (cf. https://github.com/open-telemetry/opentelemetry-python/issues/292) - span.end() - raise - - -# Put this in a subfunction to not delay the call to the wrapped -# WSGI application (instrumentation should change the application -# behavior as little as possible). -def _end_span_after_iterating(iterable, span, tracer): - try: - with tracer.use_span(span): - for yielded in iterable: - yield yielded - finally: - close = getattr(iterable, "close", None) - if close: - close() - span.end() + parent_span = propagators.extract(get_header_from_scope, scope) + span_name = get_default_span_name(scope) + + with self.tracer.start_as_current_span( + span_name, parent_span, kind=trace.SpanKind.SERVER, + attributes=collect_request_attributes(scope)) as connection_span: + + @wraps(receive) + async def wrapped_receive(): + with self.tracer.start_as_current_span(span_name + " (unknown-receive)") as receive_span: + payload = await receive() + if payload['type'] == "websocket.receive": + set_status_code(receive_span, 200) + receive_span.set_attribute("http.status_text", payload['text']) + + receive_span.update_name(span_name + " (" + payload['type'] + ")") + receive_span.set_attribute('type', payload['type']) + return payload + + @wraps(send) + async def wrapped_send(payload): + with self.tracer.start_as_current_span(span_name + " (unknown-send)") as send_span: + if payload['type'] == "http.response.start": + status_code = payload['status'] + set_status_code(send_span, status_code) + elif payload['type'] == "websocket.send": + set_status_code(send_span, 200) + send_span.set_attribute("http.status_text", payload['text']) + + send_span.update_name(span_name + " (" + payload['type'] + ")") + send_span.set_attribute('type', payload['type']) + await send(payload) + + await self.app(scope, wrapped_receive, wrapped_send) diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 1912dd0079..47fb461af3 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -15,282 +15,183 @@ import sys import unittest import unittest.mock as mock -import wsgiref.util as wsgiref_util from urllib.parse import urlsplit -import opentelemetry.ext.wsgi as otel_wsgi +import opentelemetry.ext.asgi as otel_asgi from opentelemetry import trace as trace_api -from opentelemetry.ext.testutil.wsgitestutil import WsgiTestBase - - -class Response: - def __init__(self): - self.iter = iter([b"*"]) - self.close_calls = 0 - - def __iter__(self): - return self - - def __next__(self): - return next(self.iter) - - def close(self): - self.close_calls += 1 - - -def simple_wsgi(environ, start_response): - assert isinstance(environ, dict) - start_response("200 OK", [("Content-Type", "text/plain")]) - return [b"*"] - - -def create_iter_wsgi(response): - def iter_wsgi(environ, start_response): - assert isinstance(environ, dict) - start_response("200 OK", [("Content-Type", "text/plain")]) - return response - - return iter_wsgi - - -def create_gen_wsgi(response): - def gen_wsgi(environ, start_response): - result = create_iter_wsgi(response)(environ, start_response) - yield from result - getattr(result, "close", lambda: None)() - - return gen_wsgi - - -def error_wsgi(environ, start_response): - assert isinstance(environ, dict) - try: - raise ValueError - except ValueError: - exc_info = sys.exc_info() - start_response("200 OK", [("Content-Type", "text/plain")], exc_info) - exc_info = None - return [b"*"] - - -class TestWsgiApplication(WsgiTestBase): - def validate_response(self, response, error=None): - while True: - try: - value = next(response) - self.assertEqual(value, b"*") - except StopIteration: - break - - self.assertEqual(self.status, "200 OK") - self.assertEqual( - self.response_headers, [("Content-Type", "text/plain")] - ) +from opentelemetry.ext.testutil.asgitestutil import ( + AsgiTestBase, setup_testing_defaults +) + + +async def simple_asgi(scope, receive, send): + assert isinstance(scope, dict) + assert scope.get('type') == "http" + payload = await receive() + if payload.get('type') == "http.request": + await send({ + 'type': 'http.response.start', + 'status': 200, + 'headers': [ + [b'Content-Type', b'text/plain'], + ], + }) + await send({ + 'type': 'http.response.body', + 'body': b"*" + }) + + +async def error_asgi(scope, receive, send): + assert isinstance(scope, dict) + assert scope.get('type') == "http" + payload = await receive() + if payload.get('type') == "http.request": + try: + raise ValueError + except ValueError: + scope['hack_exc_info'] = sys.exc_info() + await send({ + 'type': 'http.response.start', + 'status': 200, + 'headers': [ + [b'Content-Type', b'text/plain'], + ], + }) + await send({ + 'type': 'http.response.body', + 'body': b"*" + }) + + +class TestAsgiApplication(AsgiTestBase): + def validate_outputs(self, outputs, error=None): + # Check for expected outputs + self.assertEqual(len(outputs), 2) + response_start = outputs[0] + response_body = outputs[1] + self.assertEqual(response_start['type'], 'http.response.start') + self.assertEqual(response_body['type'], 'http.response.body') + + # Check http response body + self.assertEqual(response_body['body'], b"*") + + # Check http response start + self.assertEqual(response_start['status'], 200) + self.assertEqual(response_start['headers'], [[b'Content-Type', b'text/plain']]) + + exc_info = self.scope.get('hack_exc_info') if error: - self.assertIs(self.exc_info[0], error) - self.assertIsInstance(self.exc_info[1], error) - self.assertIsNotNone(self.exc_info[2]) + self.assertIs(exc_info[0], error) + self.assertIsInstance(exc_info[1], error) + self.assertIsNotNone(exc_info[2]) else: - self.assertIsNone(self.exc_info) + self.assertIsNone(exc_info) + # Check spans span_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "/") - self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) - self.assertEqual( - span_list[0].attributes, + self.assertEqual(len(span_list), 4) + expected = [ { - "component": "http", - "http.method": "GET", - "http.server_name": "127.0.0.1", - "http.scheme": "http", - "host.port": 80, - "http.host": "127.0.0.1", - "http.flavor": "1.0", - "http.url": "http://127.0.0.1/", - "http.status_text": "OK", - "http.status_code": 200, + 'name': "/ (http.request)", + 'kind': trace_api.SpanKind.INTERNAL, + 'attributes': { + "type": "http.request", + }, }, - ) - - def test_basic_wsgi_call(self): - app = otel_wsgi.OpenTelemetryMiddleware(simple_wsgi) - response = app(self.environ, self.start_response) - self.validate_response(response) - - def test_wsgi_iterable(self): - original_response = Response() - iter_wsgi = create_iter_wsgi(original_response) - app = otel_wsgi.OpenTelemetryMiddleware(iter_wsgi) - response = app(self.environ, self.start_response) - # Verify that start_response has been called - self.assertTrue(self.status) - self.validate_response(response) - - # Verify that close has been called exactly once - self.assertEqual(1, original_response.close_calls) - - def test_wsgi_generator(self): - original_response = Response() - gen_wsgi = create_gen_wsgi(original_response) - app = otel_wsgi.OpenTelemetryMiddleware(gen_wsgi) - response = app(self.environ, self.start_response) - # Verify that start_response has not been called - self.assertIsNone(self.status) - self.validate_response(response) - # Verify that close has been called exactly once - self.assertEqual(original_response.close_calls, 1) + { + 'name': "/ (http.response.start)", + 'kind': trace_api.SpanKind.INTERNAL, + 'attributes': { + "http.status_code": 200, + "type": "http.response.start", + }, + }, + { + 'name': "/ (http.response.body)", + 'kind': trace_api.SpanKind.INTERNAL, + 'attributes': { + "type": "http.response.body", + }, + }, + { + 'name': "/", + 'kind': trace_api.SpanKind.SERVER, + 'attributes': { + "component": "http", + "http.method": "GET", + "http.server_name": "127.0.0.1", + "http.scheme": "http", + "host.port": 80, + "http.host": "127.0.0.1", + "http.flavor": "1.0", + "http.target": "/", + "http.url": "http://127.0.0.1/", + "net.peer.ip": "127.0.0.1", + "net.peer.port": 32767, + }, + }, + ] + for span, expected in zip(span_list, expected): + self.assertEqual(span.name, expected['name']) + self.assertEqual(span.kind, expected['kind']) + self.assertEqual(span.attributes, expected['attributes']) + + def test_basic_asgi_call(self): + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs(outputs) def test_wsgi_exc_info(self): - app = otel_wsgi.OpenTelemetryMiddleware(error_wsgi) - response = app(self.environ, self.start_response) - self.validate_response(response, error=ValueError) + app = otel_asgi.OpenTelemetryMiddleware(error_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs(outputs, error=ValueError) -class TestWsgiAttributes(unittest.TestCase): +class TestAsgiAttributes(unittest.TestCase): def setUp(self): - self.environ = {} - wsgiref_util.setup_testing_defaults(self.environ) + self.scope = {} + setup_testing_defaults(self.scope) self.span = mock.create_autospec(trace_api.Span, spec_set=True) def test_request_attributes(self): - self.environ["QUERY_STRING"] = "foo=bar" + self.scope["query_string"] = b"foo=bar" - attrs = otel_wsgi.collect_request_attributes(self.environ) + attrs = otel_asgi.collect_request_attributes(self.scope) self.assertDictEqual( attrs, { "component": "http", "http.method": "GET", "http.host": "127.0.0.1", + "http.target": "/", "http.url": "http://127.0.0.1/?foo=bar", "host.port": 80, "http.scheme": "http", "http.server_name": "127.0.0.1", "http.flavor": "1.0", + "net.peer.ip": "127.0.0.1", + "net.peer.port": 32767 }, ) - def validate_url(self, expected_url, raw=False, has_host=True): - parts = urlsplit(expected_url) - expected = { - "http.scheme": parts.scheme, - "host.port": parts.port or (80 if parts.scheme == "http" else 443), - "http.server_name": parts.hostname, # Not true in the general case, but for all tests. - } - if raw: - expected["http.target"] = expected_url.split(parts.netloc, 1)[1] - else: - expected["http.url"] = expected_url - if has_host: - expected["http.host"] = parts.hostname - - attrs = otel_wsgi.collect_request_attributes(self.environ) - self.assertGreaterEqual( - attrs.items(), expected.items(), expected_url + " expected." - ) - - def test_request_attributes_with_partial_raw_uri(self): - self.environ["RAW_URI"] = "/#top" - self.validate_url("http://127.0.0.1/#top", raw=True) - - def test_request_attributes_with_partial_raw_uri_and_nonstandard_port( - self, - ): - self.environ["RAW_URI"] = "/?" - del self.environ["HTTP_HOST"] - self.environ["SERVER_PORT"] = "8080" - self.validate_url("http://127.0.0.1:8080/?", raw=True, has_host=False) - - def test_https_uri_port(self): - del self.environ["HTTP_HOST"] - self.environ["SERVER_PORT"] = "443" - self.environ["wsgi.url_scheme"] = "https" - self.validate_url("https://127.0.0.1/", has_host=False) - - self.environ["SERVER_PORT"] = "8080" - self.validate_url("https://127.0.0.1:8080/", has_host=False) - - self.environ["SERVER_PORT"] = "80" - self.validate_url("https://127.0.0.1:80/", has_host=False) - - def test_http_uri_port(self): - del self.environ["HTTP_HOST"] - self.environ["SERVER_PORT"] = "80" - self.environ["wsgi.url_scheme"] = "http" - self.validate_url("http://127.0.0.1/", has_host=False) - - self.environ["SERVER_PORT"] = "8080" - self.validate_url("http://127.0.0.1:8080/", has_host=False) - - self.environ["SERVER_PORT"] = "443" - self.validate_url("http://127.0.0.1:443/", has_host=False) - - def test_request_attributes_with_nonstandard_port_and_no_host(self): - del self.environ["HTTP_HOST"] - self.environ["SERVER_PORT"] = "8080" - self.validate_url("http://127.0.0.1:8080/", has_host=False) - - self.environ["SERVER_PORT"] = "443" - self.validate_url("http://127.0.0.1:443/", has_host=False) - - def test_request_attributes_with_conflicting_nonstandard_port(self): - self.environ[ - "HTTP_HOST" - ] += ":8080" # Note that we do not correct SERVER_PORT - expected = { - "http.host": "127.0.0.1:8080", - "http.url": "http://127.0.0.1:8080/", - "host.port": 80, - } - self.assertGreaterEqual( - otel_wsgi.collect_request_attributes(self.environ).items(), - expected.items(), - ) - - def test_request_attributes_with_faux_scheme_relative_raw_uri(self): - self.environ["RAW_URI"] = "//127.0.0.1/?" - self.validate_url("http://127.0.0.1//127.0.0.1/?", raw=True) - - def test_request_attributes_pathless(self): - self.environ["RAW_URI"] = "" - expected = {"http.target": ""} - self.assertGreaterEqual( - otel_wsgi.collect_request_attributes(self.environ).items(), - expected.items(), - ) - - def test_request_attributes_with_full_request_uri(self): - self.environ["HTTP_HOST"] = "127.0.0.1:8080" - self.environ["REQUEST_METHOD"] = "CONNECT" - self.environ[ - "REQUEST_URI" - ] = "127.0.0.1:8080" # Might happen in a CONNECT request - expected = { - "http.host": "127.0.0.1:8080", - "http.target": "127.0.0.1:8080", - } - self.assertGreaterEqual( - otel_wsgi.collect_request_attributes(self.environ).items(), - expected.items(), - ) - def test_response_attributes(self): - otel_wsgi.add_response_attributes(self.span, "404 Not Found", {}) + otel_asgi.set_status_code(self.span, 404) expected = ( mock.call("http.status_code", 404), - mock.call("http.status_text", "Not Found"), ) - self.assertEqual(self.span.set_attribute.call_count, len(expected)) + self.assertEqual(self.span.set_attribute.call_count, 1) + self.assertEqual(self.span.set_attribute.call_count, 1) self.span.set_attribute.assert_has_calls(expected, any_order=True) def test_response_attributes_invalid_status_code(self): - otel_wsgi.add_response_attributes(self.span, "Invalid Status Code", {}) - self.assertEqual(self.span.set_attribute.call_count, 1) - self.span.set_attribute.assert_called_with( - "http.status_text", "Status Code" - ) + otel_asgi.set_status_code(self.span, "Invalid Status Code") + self.assertEqual(self.span.set_status.call_count, 1) if __name__ == "__main__": From 782e9b1e91bee1d8454ef777a4a4093ddfb5e731 Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Sat, 8 Feb 2020 16:10:52 +0100 Subject: [PATCH 04/36] Fix ext/testutil --- .../src/opentelemetry/test/asgitestutil.py | 58 +++++++++++++++++++ .../src/opentelemetry/test/spantestutil.py | 31 ++++++++++ .../src/opentelemetry/test/wsgitestutil.py | 24 +------- 3 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 tests/util/src/opentelemetry/test/asgitestutil.py create mode 100644 tests/util/src/opentelemetry/test/spantestutil.py diff --git a/tests/util/src/opentelemetry/test/asgitestutil.py b/tests/util/src/opentelemetry/test/asgitestutil.py new file mode 100644 index 0000000000..7b84d2b52d --- /dev/null +++ b/tests/util/src/opentelemetry/test/asgitestutil.py @@ -0,0 +1,58 @@ +import asyncio +from asgiref.testing import ApplicationCommunicator +from opentelemetry.ext.testutil.spantestutil import SpanTestBase + + +def setup_testing_defaults(scope): + scope.update({ + 'client': ('127.0.0.1', 32767), + 'headers': [], + 'http_version': '1.0', + 'method': 'GET', + 'path': '/', + 'query_string': b'', + 'scheme': 'http', + 'server': ('127.0.0.1', 80), + 'type': 'http' + }) + + +class AsgiTestBase(SpanTestBase): + def setUp(self): + super().setUp() + + self.scope = {} + setup_testing_defaults(self.scope) + self.communicator = None + + def tearDown(self): + if self.communicator: + asyncio.get_event_loop().run_until_complete( + self.communicator.wait() + ) + + def seed_app(self, app): + self.communicator = ApplicationCommunicator(app, self.scope) + + def send_input(self, payload): + asyncio.get_event_loop().run_until_complete( + self.communicator.send_input(payload) + ) + + def send_default_request(self): + self.send_input({'type': 'http.request', 'body': b''}) + + def get_output(self): + output = asyncio.get_event_loop().run_until_complete( + self.communicator.receive_output(0) + ) + return output + + def get_all_output(self): + outputs = [] + while True: + try: + outputs.append(self.get_output()) + except asyncio.TimeoutError as e: + break + return outputs diff --git a/tests/util/src/opentelemetry/test/spantestutil.py b/tests/util/src/opentelemetry/test/spantestutil.py new file mode 100644 index 0000000000..b82fb630c0 --- /dev/null +++ b/tests/util/src/opentelemetry/test/spantestutil.py @@ -0,0 +1,31 @@ +import unittest +from importlib import reload + +from opentelemetry import trace as trace_api +from opentelemetry.sdk.trace import TracerSource, export +from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( + InMemorySpanExporter, +) + +_MEMORY_EXPORTER = None + + +class SpanTestBase(unittest.TestCase): + @classmethod + def setUpClass(cls): + global _MEMORY_EXPORTER # pylint:disable=global-statement + trace_api.set_preferred_tracer_source_implementation( + lambda T: TracerSource() + ) + tracer_source = trace_api.tracer_source() + _MEMORY_EXPORTER = InMemorySpanExporter() + span_processor = export.SimpleExportSpanProcessor(_MEMORY_EXPORTER) + tracer_source.add_span_processor(span_processor) + + @classmethod + def tearDownClass(cls): + reload(trace_api) + + def setUp(self): + self.memory_exporter = _MEMORY_EXPORTER + self.memory_exporter.clear() diff --git a/tests/util/src/opentelemetry/test/wsgitestutil.py b/tests/util/src/opentelemetry/test/wsgitestutil.py index cdce28b907..d93ff2ea96 100644 --- a/tests/util/src/opentelemetry/test/wsgitestutil.py +++ b/tests/util/src/opentelemetry/test/wsgitestutil.py @@ -1,7 +1,6 @@ import io -import unittest import wsgiref.util as wsgiref_util -from importlib import reload +from opentelemetry.ext.testutil.spantestutil import SpanTestBase from opentelemetry import trace as trace_api from opentelemetry.sdk.trace import TracerProvider, export @@ -25,24 +24,3 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): reload(trace_api) - - def setUp(self): - - self.memory_exporter = _MEMORY_EXPORTER - self.memory_exporter.clear() - - self.write_buffer = io.BytesIO() - self.write = self.write_buffer.write - - self.environ = {} - wsgiref_util.setup_testing_defaults(self.environ) - - self.status = None - self.response_headers = None - self.exc_info = None - - def start_response(self, status, response_headers, exc_info=None): - self.status = status - self.response_headers = response_headers - self.exc_info = exc_info - return self.write From a343b7dcf7fcc53df28ade4017acf4d44d899370 Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Sat, 8 Feb 2020 16:37:24 +0100 Subject: [PATCH 05/36] Black reformatting --- .../src/opentelemetry/ext/asgi/__init__.py | 62 ++++++---- .../tests/test_asgi_middleware.py | 106 ++++++++---------- .../src/opentelemetry/test/asgitestutil.py | 28 ++--- 3 files changed, 101 insertions(+), 95 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 4f520e679b..0b7fab697b 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -30,18 +30,17 @@ _HTTP_VERSION_PREFIX = "HTTP/" -def get_header_from_scope( - scope: dict, header_name: str -) -> typing.List[str]: +def get_header_from_scope(scope: dict, header_name: str) -> typing.List[str]: """Retrieve a HTTP header value from the ASGI scope. Returns: A list with a single string with the header value if it exists, else an empty list. """ - headers = scope.get('headers') + headers = scope.get("headers") return [ - value.decode('utf8') for (key,value) in headers - if key.decode('utf8') == header_name + value.decode("utf8") + for (key, value) in headers + if key.decode("utf8") == header_name ] @@ -81,8 +80,8 @@ def collect_request_attributes(scope): dictionary to be used as span creation attributes.""" port = scope.get("server")[1] - server_host = ( - scope.get("server")[0] + (":" + str(port) if port != 80 else "") + server_host = scope.get("server")[0] + ( + ":" + str(port) if port != 80 else "" ) http_url = scope.get("scheme") + "://" + server_host + scope.get("path") if scope.get("query_string"): @@ -159,33 +158,48 @@ async def __call__(self, scope, receive, send): span_name = get_default_span_name(scope) with self.tracer.start_as_current_span( - span_name, parent_span, kind=trace.SpanKind.SERVER, - attributes=collect_request_attributes(scope)) as connection_span: + span_name, + parent_span, + kind=trace.SpanKind.SERVER, + attributes=collect_request_attributes(scope), + ) as connection_span: @wraps(receive) async def wrapped_receive(): - with self.tracer.start_as_current_span(span_name + " (unknown-receive)") as receive_span: + with self.tracer.start_as_current_span( + span_name + " (unknown-receive)" + ) as receive_span: payload = await receive() - if payload['type'] == "websocket.receive": + if payload["type"] == "websocket.receive": set_status_code(receive_span, 200) - receive_span.set_attribute("http.status_text", payload['text']) - - receive_span.update_name(span_name + " (" + payload['type'] + ")") - receive_span.set_attribute('type', payload['type']) + receive_span.set_attribute( + "http.status_text", payload["text"] + ) + + receive_span.update_name( + span_name + " (" + payload["type"] + ")" + ) + receive_span.set_attribute("type", payload["type"]) return payload @wraps(send) async def wrapped_send(payload): - with self.tracer.start_as_current_span(span_name + " (unknown-send)") as send_span: - if payload['type'] == "http.response.start": - status_code = payload['status'] + with self.tracer.start_as_current_span( + span_name + " (unknown-send)" + ) as send_span: + if payload["type"] == "http.response.start": + status_code = payload["status"] set_status_code(send_span, status_code) - elif payload['type'] == "websocket.send": + elif payload["type"] == "websocket.send": set_status_code(send_span, 200) - send_span.set_attribute("http.status_text", payload['text']) - - send_span.update_name(span_name + " (" + payload['type'] + ")") - send_span.set_attribute('type', payload['type']) + send_span.set_attribute( + "http.status_text", payload["text"] + ) + + send_span.update_name( + span_name + " (" + payload["type"] + ")" + ) + send_span.set_attribute("type", payload["type"]) await send(payload) await self.app(scope, wrapped_receive, wrapped_send) diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 47fb461af3..e2e5b6f9db 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -20,48 +20,43 @@ import opentelemetry.ext.asgi as otel_asgi from opentelemetry import trace as trace_api from opentelemetry.ext.testutil.asgitestutil import ( - AsgiTestBase, setup_testing_defaults + AsgiTestBase, + setup_testing_defaults, ) async def simple_asgi(scope, receive, send): assert isinstance(scope, dict) - assert scope.get('type') == "http" + assert scope.get("type") == "http" payload = await receive() - if payload.get('type') == "http.request": - await send({ - 'type': 'http.response.start', - 'status': 200, - 'headers': [ - [b'Content-Type', b'text/plain'], - ], - }) - await send({ - 'type': 'http.response.body', - 'body': b"*" - }) + if payload.get("type") == "http.request": + await send( + { + "type": "http.response.start", + "status": 200, + "headers": [[b"Content-Type", b"text/plain"]], + } + ) + await send({"type": "http.response.body", "body": b"*"}) async def error_asgi(scope, receive, send): assert isinstance(scope, dict) - assert scope.get('type') == "http" + assert scope.get("type") == "http" payload = await receive() - if payload.get('type') == "http.request": + if payload.get("type") == "http.request": try: raise ValueError except ValueError: - scope['hack_exc_info'] = sys.exc_info() - await send({ - 'type': 'http.response.start', - 'status': 200, - 'headers': [ - [b'Content-Type', b'text/plain'], - ], - }) - await send({ - 'type': 'http.response.body', - 'body': b"*" - }) + scope["hack_exc_info"] = sys.exc_info() + await send( + { + "type": "http.response.start", + "status": 200, + "headers": [[b"Content-Type", b"text/plain"]], + } + ) + await send({"type": "http.response.body", "body": b"*"}) class TestAsgiApplication(AsgiTestBase): @@ -70,17 +65,19 @@ def validate_outputs(self, outputs, error=None): self.assertEqual(len(outputs), 2) response_start = outputs[0] response_body = outputs[1] - self.assertEqual(response_start['type'], 'http.response.start') - self.assertEqual(response_body['type'], 'http.response.body') + self.assertEqual(response_start["type"], "http.response.start") + self.assertEqual(response_body["type"], "http.response.body") # Check http response body - self.assertEqual(response_body['body'], b"*") + self.assertEqual(response_body["body"], b"*") # Check http response start - self.assertEqual(response_start['status'], 200) - self.assertEqual(response_start['headers'], [[b'Content-Type', b'text/plain']]) + self.assertEqual(response_start["status"], 200) + self.assertEqual( + response_start["headers"], [[b"Content-Type", b"text/plain"]] + ) - exc_info = self.scope.get('hack_exc_info') + exc_info = self.scope.get("hack_exc_info") if error: self.assertIs(exc_info[0], error) self.assertIsInstance(exc_info[1], error) @@ -93,32 +90,27 @@ def validate_outputs(self, outputs, error=None): self.assertEqual(len(span_list), 4) expected = [ { - 'name': "/ (http.request)", - 'kind': trace_api.SpanKind.INTERNAL, - 'attributes': { - "type": "http.request", - }, + "name": "/ (http.request)", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"type": "http.request"}, }, - { - 'name': "/ (http.response.start)", - 'kind': trace_api.SpanKind.INTERNAL, - 'attributes': { + "name": "/ (http.response.start)", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": { "http.status_code": 200, "type": "http.response.start", }, }, { - 'name': "/ (http.response.body)", - 'kind': trace_api.SpanKind.INTERNAL, - 'attributes': { - "type": "http.response.body", - }, + "name": "/ (http.response.body)", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"type": "http.response.body"}, }, { - 'name': "/", - 'kind': trace_api.SpanKind.SERVER, - 'attributes': { + "name": "/", + "kind": trace_api.SpanKind.SERVER, + "attributes": { "component": "http", "http.method": "GET", "http.server_name": "127.0.0.1", @@ -134,9 +126,9 @@ def validate_outputs(self, outputs, error=None): }, ] for span, expected in zip(span_list, expected): - self.assertEqual(span.name, expected['name']) - self.assertEqual(span.kind, expected['kind']) - self.assertEqual(span.attributes, expected['attributes']) + self.assertEqual(span.name, expected["name"]) + self.assertEqual(span.kind, expected["kind"]) + self.assertEqual(span.attributes, expected["attributes"]) def test_basic_asgi_call(self): app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) @@ -176,15 +168,13 @@ def test_request_attributes(self): "http.server_name": "127.0.0.1", "http.flavor": "1.0", "net.peer.ip": "127.0.0.1", - "net.peer.port": 32767 + "net.peer.port": 32767, }, ) def test_response_attributes(self): otel_asgi.set_status_code(self.span, 404) - expected = ( - mock.call("http.status_code", 404), - ) + expected = (mock.call("http.status_code", 404),) self.assertEqual(self.span.set_attribute.call_count, 1) self.assertEqual(self.span.set_attribute.call_count, 1) self.span.set_attribute.assert_has_calls(expected, any_order=True) diff --git a/tests/util/src/opentelemetry/test/asgitestutil.py b/tests/util/src/opentelemetry/test/asgitestutil.py index 7b84d2b52d..1461f3e846 100644 --- a/tests/util/src/opentelemetry/test/asgitestutil.py +++ b/tests/util/src/opentelemetry/test/asgitestutil.py @@ -4,17 +4,19 @@ def setup_testing_defaults(scope): - scope.update({ - 'client': ('127.0.0.1', 32767), - 'headers': [], - 'http_version': '1.0', - 'method': 'GET', - 'path': '/', - 'query_string': b'', - 'scheme': 'http', - 'server': ('127.0.0.1', 80), - 'type': 'http' - }) + scope.update( + { + "client": ("127.0.0.1", 32767), + "headers": [], + "http_version": "1.0", + "method": "GET", + "path": "/", + "query_string": b"", + "scheme": "http", + "server": ("127.0.0.1", 80), + "type": "http", + } + ) class AsgiTestBase(SpanTestBase): @@ -38,9 +40,9 @@ def send_input(self, payload): asyncio.get_event_loop().run_until_complete( self.communicator.send_input(payload) ) - + def send_default_request(self): - self.send_input({'type': 'http.request', 'body': b''}) + self.send_input({"type": "http.request", "body": b""}) def get_output(self): output = asyncio.get_event_loop().run_until_complete( From d6ac1e23fa110dc36991e0f1bfcd3e7ba05e2979 Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Sat, 8 Feb 2020 16:48:40 +0100 Subject: [PATCH 06/36] Isort reformatting --- .../src/opentelemetry/ext/asgi/__init__.py | 5 +++-- tests/util/src/opentelemetry/test/asgitestutil.py | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 0b7fab697b..e36dc515fc 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -18,9 +18,10 @@ timing through OpenTelemetry. """ -from functools import wraps -import typing import operator +import typing +from functools import wraps + from asgiref.compatibility import guarantee_single_callable from opentelemetry import propagators, trace diff --git a/tests/util/src/opentelemetry/test/asgitestutil.py b/tests/util/src/opentelemetry/test/asgitestutil.py index 1461f3e846..6149efe257 100644 --- a/tests/util/src/opentelemetry/test/asgitestutil.py +++ b/tests/util/src/opentelemetry/test/asgitestutil.py @@ -1,5 +1,7 @@ import asyncio + from asgiref.testing import ApplicationCommunicator + from opentelemetry.ext.testutil.spantestutil import SpanTestBase From 8a8f7265503f5628d806f4076839d78d7e3c5efd Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Sat, 8 Feb 2020 17:02:55 +0100 Subject: [PATCH 07/36] Flake8 reformatting --- .../src/opentelemetry/ext/asgi/__init__.py | 2 +- ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py | 2 +- tests/util/src/opentelemetry/test/asgitestutil.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index e36dc515fc..816bf5c1f6 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -163,7 +163,7 @@ async def __call__(self, scope, receive, send): parent_span, kind=trace.SpanKind.SERVER, attributes=collect_request_attributes(scope), - ) as connection_span: + ): @wraps(receive) async def wrapped_receive(): diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index e2e5b6f9db..3d9477163f 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -137,7 +137,7 @@ def test_basic_asgi_call(self): outputs = self.get_all_output() self.validate_outputs(outputs) - def test_wsgi_exc_info(self): + def test_asgi_exc_info(self): app = otel_asgi.OpenTelemetryMiddleware(error_asgi) self.seed_app(app) self.send_default_request() diff --git a/tests/util/src/opentelemetry/test/asgitestutil.py b/tests/util/src/opentelemetry/test/asgitestutil.py index 6149efe257..4f5fb94f0b 100644 --- a/tests/util/src/opentelemetry/test/asgitestutil.py +++ b/tests/util/src/opentelemetry/test/asgitestutil.py @@ -57,6 +57,6 @@ def get_all_output(self): while True: try: outputs.append(self.get_output()) - except asyncio.TimeoutError as e: + except asyncio.TimeoutError: break return outputs From ac7cb7298f4a13eb3bb2fc22e90da8a171f57d2a Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Sat, 8 Feb 2020 17:10:25 +0100 Subject: [PATCH 08/36] Pylint reformatting --- ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 3d9477163f..480305b1ae 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -15,7 +15,6 @@ import sys import unittest import unittest.mock as mock -from urllib.parse import urlsplit import opentelemetry.ext.asgi as otel_asgi from opentelemetry import trace as trace_api From 2d32ef97c518787360d8fd4ac309ebfc80faf5ff Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Sat, 8 Feb 2020 17:18:29 +0100 Subject: [PATCH 09/36] Documentation --- docs-requirements.txt | 1 + docs/opentelemetry.ext.asgi.rst | 10 ++++++++++ ext/opentelemetry-ext-asgi/setup.cfg | 3 ++- scripts/coverage.sh | 1 + 4 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 docs/opentelemetry.ext.asgi.rst diff --git a/docs-requirements.txt b/docs-requirements.txt index 55b647402f..2788761006 100644 --- a/docs-requirements.txt +++ b/docs-requirements.txt @@ -3,6 +3,7 @@ sphinx-rtd-theme~=0.4 sphinx-autodoc-typehints~=1.10.2 # Required by ext packages +asgiref~=3.2.3 ddtrace>=0.34.0 aiohttp ~= 3.0 Deprecated>=1.2.6 diff --git a/docs/opentelemetry.ext.asgi.rst b/docs/opentelemetry.ext.asgi.rst new file mode 100644 index 0000000000..2d8f4e99c7 --- /dev/null +++ b/docs/opentelemetry.ext.asgi.rst @@ -0,0 +1,10 @@ +opentelemetry.ext.asgi package +========================================== + +Module contents +--------------- + +.. automodule:: opentelemetry.ext.asgi + :members: + :undoc-members: + :show-inheritance: diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index 903ad7e662..5a28247990 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -31,12 +31,13 @@ classifiers = Programming Language :: Python :: 3.7 [options] -python_requires = >=3.4 +python_requires = >=3.7 package_dir= =src packages=find_namespace: install_requires = opentelemetry-api + asgiref [options.extras_require] test = diff --git a/scripts/coverage.sh b/scripts/coverage.sh index 248e5faea8..7ea759a929 100755 --- a/scripts/coverage.sh +++ b/scripts/coverage.sh @@ -20,6 +20,7 @@ coverage erase cov opentelemetry-api cov opentelemetry-sdk cov ext/opentelemetry-ext-datadog +cov ext/opentelemetry-ext-asgi cov ext/opentelemetry-ext-flask cov ext/opentelemetry-ext-requests cov ext/opentelemetry-ext-jaeger From 103ff1dc0b8972765bbb0fd1c3eaaac74a40714d Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Wed, 19 Feb 2020 21:07:41 +0100 Subject: [PATCH 10/36] ASGI only runs under Python 3.5+, fixes to tox/coverage Signed-off-by: Emil Madsen --- ext/opentelemetry-ext-asgi/setup.cfg | 4 +++- scripts/coverage.sh | 3 +++ tox.ini | 10 ++++++++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index 5a28247990..fdbc0dee22 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -28,10 +28,12 @@ classifiers = License :: OSI Approved :: Apache Software License Programming Language :: Python Programming Language :: Python :: 3 + Programming Language :: Python :: 3.5 + Programming Language :: Python :: 3.6 Programming Language :: Python :: 3.7 [options] -python_requires = >=3.7 +python_requires = >=3.5 package_dir= =src packages=find_namespace: diff --git a/scripts/coverage.sh b/scripts/coverage.sh index 7ea759a929..f6d8827488 100755 --- a/scripts/coverage.sh +++ b/scripts/coverage.sh @@ -32,6 +32,9 @@ cov docs/examples/opentelemetry-example-app # aiohttp is only supported on Python 3.5+. if [ ${PYTHON_VERSION_INFO[1]} -gt 4 ]; then cov ext/opentelemetry-ext-aiohttp-client +# ext-asgi is only supported on Python 3.5+. +if [ ${PYTHON_VERSION_INFO[1]} -gt 4 ]; then + cov ext/opentelemetry-ext-asgi fi coverage report --show-missing diff --git a/tox.ini b/tox.ini index 04ea2b1650..529480a6e6 100644 --- a/tox.ini +++ b/tox.ini @@ -83,6 +83,10 @@ envlist = py3{4,5,6,7,8}-test-ext-pymysql pypy3-test-ext-pymysql + ; opentelemetry-ext-asgi + py3{4,5,6,7,8}-test-ext-asgi + pypy3-test-ext-asgi + ; opentelemetry-ext-wsgi py3{4,5,6,7,8}-test-ext-wsgi pypy3-test-ext-wsgi @@ -152,6 +156,7 @@ changedir = test-ext-pymongo: ext/opentelemetry-ext-pymongo/tests test-ext-psycopg2: ext/opentelemetry-ext-psycopg2/tests test-ext-pymysql: ext/opentelemetry-ext-pymysql/tests + test-ext-asgi: ext/opentelemetry-ext-asgi/tests test-ext-wsgi: ext/opentelemetry-ext-wsgi/tests test-ext-zipkin: ext/opentelemetry-ext-zipkin/tests test-ext-flask: ext/opentelemetry-ext-flask/tests @@ -187,9 +192,10 @@ commands_pre = grpc: pip install {toxinidir}/ext/opentelemetry-ext-grpc[test] - wsgi,flask,django: pip install {toxinidir}/tests/util - wsgi,flask,django: pip install {toxinidir}/ext/opentelemetry-ext-wsgi + wsgi,flask,django,asgi: pip install {toxinidir}/tests/util + wsgi,flask,django,asgi: pip install {toxinidir}/ext/opentelemetry-ext-wsgi flask,django: pip install {toxinidir}/opentelemetry-auto-instrumentation + asgi: pip install {toxinidir}/ext/opentelemetry-ext-asgi flask: pip install {toxinidir}/ext/opentelemetry-ext-flask[test] dbapi: pip install {toxinidir}/ext/opentelemetry-ext-dbapi[test] From 4bbc7a6665da77c18673985e4846ab661f900fa7 Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Fri, 21 Feb 2020 17:44:31 +0100 Subject: [PATCH 11/36] Added callback to override default span-name, default span name to HTTP METHOD Signed-off-by: Emil Madsen --- .../src/opentelemetry/ext/asgi/__init__.py | 18 ++++----- .../tests/test_asgi_middleware.py | 37 ++++++++++++++++--- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 816bf5c1f6..2297998d67 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -124,12 +124,8 @@ def set_status_code(span, status_code): def get_default_span_name(scope): - """Calculates a (generic) span name for an incoming HTTP request based on the ASGI scope.""" - - # TODO: Update once - # https://github.com/open-telemetry/opentelemetry-specification/issues/270 - # is resolved - return scope.get("path", "/") + """Default implementation for name_callback, returns HTTP {METHOD_NAME}.""" + return "HTTP " + scope.get("method") class OpenTelemetryMiddleware: @@ -140,11 +136,15 @@ class OpenTelemetryMiddleware: Args: app: The ASGI application callable to forward requests to. + name_callback: Callback which calculates a generic span name for an + incoming HTTP request based on the ASGI scope. + Optional: Defaults to get_default_span_name. """ - def __init__(self, app): + def __init__(self, app, name_callback=None): self.app = guarantee_single_callable(app) self.tracer = trace.tracer_source().get_tracer(__name__, __version__) + self.name_callback = name_callback or get_default_span_name async def __call__(self, scope, receive, send): """The ASGI application @@ -156,10 +156,10 @@ async def __call__(self, scope, receive, send): """ parent_span = propagators.extract(get_header_from_scope, scope) - span_name = get_default_span_name(scope) + span_name = self.name_callback(scope) with self.tracer.start_as_current_span( - span_name, + span_name + " (connection)", parent_span, kind=trace.SpanKind.SERVER, attributes=collect_request_attributes(scope), diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 480305b1ae..fd860f432d 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -59,7 +59,9 @@ async def error_asgi(scope, receive, send): class TestAsgiApplication(AsgiTestBase): - def validate_outputs(self, outputs, error=None): + def validate_outputs(self, outputs, error=None, modifiers=None): + # Ensure modifiers is a list + modifiers = modifiers or [] # Check for expected outputs self.assertEqual(len(outputs), 2) response_start = outputs[0] @@ -89,12 +91,12 @@ def validate_outputs(self, outputs, error=None): self.assertEqual(len(span_list), 4) expected = [ { - "name": "/ (http.request)", + "name": "HTTP GET (http.request)", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.request"}, }, { - "name": "/ (http.response.start)", + "name": "HTTP GET (http.response.start)", "kind": trace_api.SpanKind.INTERNAL, "attributes": { "http.status_code": 200, @@ -102,12 +104,12 @@ def validate_outputs(self, outputs, error=None): }, }, { - "name": "/ (http.response.body)", + "name": "HTTP GET (http.response.body)", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.response.body"}, }, { - "name": "/", + "name": "HTTP GET (connection)", "kind": trace_api.SpanKind.SERVER, "attributes": { "component": "http", @@ -124,12 +126,17 @@ def validate_outputs(self, outputs, error=None): }, }, ] + # Run our expected modifiers + for modifier in modifiers: + expected = modifier(expected) + # Check that output matches for span, expected in zip(span_list, expected): self.assertEqual(span.name, expected["name"]) self.assertEqual(span.kind, expected["kind"]) self.assertEqual(span.attributes, expected["attributes"]) def test_basic_asgi_call(self): + """Test that spans are emitted as expected.""" app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) self.seed_app(app) self.send_default_request() @@ -137,12 +144,32 @@ def test_basic_asgi_call(self): self.validate_outputs(outputs) def test_asgi_exc_info(self): + """Test that exception information is emitted as expected.""" app = otel_asgi.OpenTelemetryMiddleware(error_asgi) self.seed_app(app) self.send_default_request() outputs = self.get_all_output() self.validate_outputs(outputs, error=ValueError) + def test_override_span_name(self): + """Test that span_names can be overwritten by our callback function.""" + span_name = "Dymaxion" + def get_predefined_span_name(scope): + return span_name + def update_expected_span_name(expected): + for entry in expected: + entry['name'] = " ".join( + [span_name] + entry['name'].split(' ')[-1:] + ) + return expected + app = otel_asgi.OpenTelemetryMiddleware( + simple_asgi, name_callback=get_predefined_span_name + ) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs(outputs, modifiers=[update_expected_span_name]) + class TestAsgiAttributes(unittest.TestCase): def setUp(self): From 4833891ea7e2a8c7d61f6444273b554571c918f9 Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Fri, 21 Feb 2020 18:12:54 +0100 Subject: [PATCH 12/36] Set send_span name immediately, instead of updating it --- .../src/opentelemetry/ext/asgi/__init__.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 2297998d67..8bdc67a3c0 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -186,7 +186,7 @@ async def wrapped_receive(): @wraps(send) async def wrapped_send(payload): with self.tracer.start_as_current_span( - span_name + " (unknown-send)" + span_name + " (" + payload["type"] + ")" ) as send_span: if payload["type"] == "http.response.start": status_code = payload["status"] @@ -197,9 +197,6 @@ async def wrapped_send(payload): "http.status_text", payload["text"] ) - send_span.update_name( - span_name + " (" + payload["type"] + ")" - ) send_span.set_attribute("type", payload["type"]) await send(payload) From 924fe3546d6c62fa3a06637fb3377ebd06530a02 Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Fri, 21 Feb 2020 19:17:36 +0100 Subject: [PATCH 13/36] Changed span-names to asgi.{scope["type"]}.send/receive --- .../src/opentelemetry/ext/asgi/__init__.py | 11 +++-------- .../tests/test_asgi_middleware.py | 8 ++++---- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 8bdc67a3c0..c6b6eb8c7c 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -159,7 +159,7 @@ async def __call__(self, scope, receive, send): span_name = self.name_callback(scope) with self.tracer.start_as_current_span( - span_name + " (connection)", + span_name + " (asgi.connection)", parent_span, kind=trace.SpanKind.SERVER, attributes=collect_request_attributes(scope), @@ -168,7 +168,7 @@ async def __call__(self, scope, receive, send): @wraps(receive) async def wrapped_receive(): with self.tracer.start_as_current_span( - span_name + " (unknown-receive)" + span_name + " (asgi." + scope["type"] + ".receive)" ) as receive_span: payload = await receive() if payload["type"] == "websocket.receive": @@ -176,17 +176,13 @@ async def wrapped_receive(): receive_span.set_attribute( "http.status_text", payload["text"] ) - - receive_span.update_name( - span_name + " (" + payload["type"] + ")" - ) receive_span.set_attribute("type", payload["type"]) return payload @wraps(send) async def wrapped_send(payload): with self.tracer.start_as_current_span( - span_name + " (" + payload["type"] + ")" + span_name + " (asgi." + scope["type"] + ".send)" ) as send_span: if payload["type"] == "http.response.start": status_code = payload["status"] @@ -196,7 +192,6 @@ async def wrapped_send(payload): send_span.set_attribute( "http.status_text", payload["text"] ) - send_span.set_attribute("type", payload["type"]) await send(payload) diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index fd860f432d..e85efb953d 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -91,12 +91,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): self.assertEqual(len(span_list), 4) expected = [ { - "name": "HTTP GET (http.request)", + "name": "HTTP GET (asgi.http.receive)", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.request"}, }, { - "name": "HTTP GET (http.response.start)", + "name": "HTTP GET (asgi.http.send)", "kind": trace_api.SpanKind.INTERNAL, "attributes": { "http.status_code": 200, @@ -104,12 +104,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): }, }, { - "name": "HTTP GET (http.response.body)", + "name": "HTTP GET (asgi.http.send)", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.response.body"}, }, { - "name": "HTTP GET (connection)", + "name": "HTTP GET (asgi.connection)", "kind": trace_api.SpanKind.SERVER, "attributes": { "component": "http", From 5a13e0ce95cfcf60c77ee667c70d521b5ceb0ff6 Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Fri, 21 Feb 2020 19:44:42 +0100 Subject: [PATCH 14/36] Handle scope["server"] = None, by defaulting to 0.0.0.0:80 --- .../src/opentelemetry/ext/asgi/__init__.py | 10 ++++------ .../tests/test_asgi_middleware.py | 20 ++++++++++++++++++- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index c6b6eb8c7c..75a2b0d9f2 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -79,11 +79,9 @@ def http_status_to_canonical_code(code: int, allow_redirect: bool = True): def collect_request_attributes(scope): """Collects HTTP request attributes from the ASGI scope and returns a dictionary to be used as span creation attributes.""" - - port = scope.get("server")[1] - server_host = scope.get("server")[0] + ( - ":" + str(port) if port != 80 else "" - ) + server = scope.get("server") or ['0.0.0.0', 80] + port = server[1] + server_host = server[0] + (":" + str(port) if port != 80 else "") http_url = scope.get("scheme") + "://" + server_host + scope.get("path") if scope.get("query_string"): http_url = http_url + ("?" + scope.get("query_string").decode("utf8")) @@ -91,7 +89,7 @@ def collect_request_attributes(scope): result = { "component": scope.get("type"), "http.method": scope.get("method"), - "http.server_name": scope.get("server")[0], + "http.server_name": server[0], "http.scheme": scope.get("scheme"), "http.host": server_host, "host.port": port, diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index e85efb953d..3c2022464a 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -133,7 +133,7 @@ def validate_outputs(self, outputs, error=None, modifiers=None): for span, expected in zip(span_list, expected): self.assertEqual(span.name, expected["name"]) self.assertEqual(span.kind, expected["kind"]) - self.assertEqual(span.attributes, expected["attributes"]) + self.assertDictEqual(dict(span.attributes), expected["attributes"]) def test_basic_asgi_call(self): """Test that spans are emitted as expected.""" @@ -170,6 +170,24 @@ def update_expected_span_name(expected): outputs = self.get_all_output() self.validate_outputs(outputs, modifiers=[update_expected_span_name]) + def test_behavior_with_scope_server_as_none(self): + """Test that middleware is ok when server is none in scope.""" + def update_expected_server(expected): + expected[3]['attributes'].update({ + 'http.server_name': '0.0.0.0', + 'http.host': '0.0.0.0', + 'host.port': 80, + 'http.url': 'http://0.0.0.0/' + }) + return expected + self.scope["server"] = None + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs(outputs, modifiers=[update_expected_server]) + + class TestAsgiAttributes(unittest.TestCase): def setUp(self): From 0df9c6269c8c553300ccbcb1583e079dfc448fe5 Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Fri, 21 Feb 2020 20:43:38 +0100 Subject: [PATCH 15/36] Set http.server_name based on Host header --- .../src/opentelemetry/ext/asgi/__init__.py | 6 ++++-- .../tests/test_asgi_middleware.py | 18 +++++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 75a2b0d9f2..0af34b1d15 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -89,7 +89,6 @@ def collect_request_attributes(scope): result = { "component": scope.get("type"), "http.method": scope.get("method"), - "http.server_name": server[0], "http.scheme": scope.get("scheme"), "http.host": server_host, "host.port": port, @@ -97,8 +96,11 @@ def collect_request_attributes(scope): "http.target": scope.get("path"), "http.url": http_url, } + http_host_value = ",".join(get_header_from_scope(scope, "host")) + if http_host_value: + result['http.server_name'] = http_host_value - if "client" in scope: + if "client" in scope and scope["client"] is not None: result["net.peer.ip"] = scope.get("client")[0] result["net.peer.port"] = scope.get("client")[1] diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 3c2022464a..866c0220f5 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -114,7 +114,6 @@ def validate_outputs(self, outputs, error=None, modifiers=None): "attributes": { "component": "http", "http.method": "GET", - "http.server_name": "127.0.0.1", "http.scheme": "http", "host.port": 80, "http.host": "127.0.0.1", @@ -174,7 +173,6 @@ def test_behavior_with_scope_server_as_none(self): """Test that middleware is ok when server is none in scope.""" def update_expected_server(expected): expected[3]['attributes'].update({ - 'http.server_name': '0.0.0.0', 'http.host': '0.0.0.0', 'host.port': 80, 'http.url': 'http://0.0.0.0/' @@ -187,6 +185,21 @@ def update_expected_server(expected): outputs = self.get_all_output() self.validate_outputs(outputs, modifiers=[update_expected_server]) + def test_host_header(self): + """Test that host header is converted to http.server_name.""" + hostname = b"server_name_1" + def update_expected_server(expected): + expected[3]['attributes'].update({ + 'http.server_name': hostname.decode('utf8') + }) + return expected + self.scope["headers"].append([b'host', hostname]) + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs(outputs, modifiers=[update_expected_server]) + class TestAsgiAttributes(unittest.TestCase): @@ -209,7 +222,6 @@ def test_request_attributes(self): "http.url": "http://127.0.0.1/?foo=bar", "host.port": 80, "http.scheme": "http", - "http.server_name": "127.0.0.1", "http.flavor": "1.0", "net.peer.ip": "127.0.0.1", "net.peer.port": 32767, From c88f152383f487040d1dbf9e752329cf8e507729 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Wed, 20 May 2020 18:13:14 -0400 Subject: [PATCH 16/36] fix tests --- .../tests/test_asgi_middleware.py | 35 +++++++++++------- scripts/coverage.sh | 1 - .../src/opentelemetry/test/asgitestutil.py | 2 +- .../src/opentelemetry/test/spantestutil.py | 10 ++--- .../src/opentelemetry/test/wsgitestutil.py | 37 +++++++++---------- tox.ini | 2 +- 6 files changed, 46 insertions(+), 41 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 866c0220f5..e66f741cf9 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -18,7 +18,7 @@ import opentelemetry.ext.asgi as otel_asgi from opentelemetry import trace as trace_api -from opentelemetry.ext.testutil.asgitestutil import ( +from opentelemetry.test.asgitestutil import ( AsgiTestBase, setup_testing_defaults, ) @@ -153,14 +153,18 @@ def test_asgi_exc_info(self): def test_override_span_name(self): """Test that span_names can be overwritten by our callback function.""" span_name = "Dymaxion" + + # pylint:disable=unused-argument def get_predefined_span_name(scope): return span_name + def update_expected_span_name(expected): for entry in expected: - entry['name'] = " ".join( - [span_name] + entry['name'].split(' ')[-1:] + entry["name"] = " ".join( + [span_name] + entry["name"].split(" ")[-1:] ) return expected + app = otel_asgi.OpenTelemetryMiddleware( simple_asgi, name_callback=get_predefined_span_name ) @@ -171,13 +175,17 @@ def update_expected_span_name(expected): def test_behavior_with_scope_server_as_none(self): """Test that middleware is ok when server is none in scope.""" + def update_expected_server(expected): - expected[3]['attributes'].update({ - 'http.host': '0.0.0.0', - 'host.port': 80, - 'http.url': 'http://0.0.0.0/' - }) + expected[3]["attributes"].update( + { + "http.host": "0.0.0.0", + "host.port": 80, + "http.url": "http://0.0.0.0/", + } + ) return expected + self.scope["server"] = None app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) self.seed_app(app) @@ -188,12 +196,14 @@ def update_expected_server(expected): def test_host_header(self): """Test that host header is converted to http.server_name.""" hostname = b"server_name_1" + def update_expected_server(expected): - expected[3]['attributes'].update({ - 'http.server_name': hostname.decode('utf8') - }) + expected[3]["attributes"].update( + {"http.server_name": hostname.decode("utf8")} + ) return expected - self.scope["headers"].append([b'host', hostname]) + + self.scope["headers"].append([b"host", hostname]) app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) self.seed_app(app) self.send_default_request() @@ -201,7 +211,6 @@ def update_expected_server(expected): self.validate_outputs(outputs, modifiers=[update_expected_server]) - class TestAsgiAttributes(unittest.TestCase): def setUp(self): self.scope = {} diff --git a/scripts/coverage.sh b/scripts/coverage.sh index f6d8827488..8e09ae23a3 100755 --- a/scripts/coverage.sh +++ b/scripts/coverage.sh @@ -20,7 +20,6 @@ coverage erase cov opentelemetry-api cov opentelemetry-sdk cov ext/opentelemetry-ext-datadog -cov ext/opentelemetry-ext-asgi cov ext/opentelemetry-ext-flask cov ext/opentelemetry-ext-requests cov ext/opentelemetry-ext-jaeger diff --git a/tests/util/src/opentelemetry/test/asgitestutil.py b/tests/util/src/opentelemetry/test/asgitestutil.py index 4f5fb94f0b..871209bdde 100644 --- a/tests/util/src/opentelemetry/test/asgitestutil.py +++ b/tests/util/src/opentelemetry/test/asgitestutil.py @@ -2,7 +2,7 @@ from asgiref.testing import ApplicationCommunicator -from opentelemetry.ext.testutil.spantestutil import SpanTestBase +from opentelemetry.test.spantestutil import SpanTestBase def setup_testing_defaults(scope): diff --git a/tests/util/src/opentelemetry/test/spantestutil.py b/tests/util/src/opentelemetry/test/spantestutil.py index b82fb630c0..70d33547e8 100644 --- a/tests/util/src/opentelemetry/test/spantestutil.py +++ b/tests/util/src/opentelemetry/test/spantestutil.py @@ -2,7 +2,7 @@ from importlib import reload from opentelemetry import trace as trace_api -from opentelemetry.sdk.trace import TracerSource, export +from opentelemetry.sdk.trace import TracerProvider, export from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( InMemorySpanExporter, ) @@ -14,13 +14,11 @@ class SpanTestBase(unittest.TestCase): @classmethod def setUpClass(cls): global _MEMORY_EXPORTER # pylint:disable=global-statement - trace_api.set_preferred_tracer_source_implementation( - lambda T: TracerSource() - ) - tracer_source = trace_api.tracer_source() + trace_api.set_tracer_provider(TracerProvider()) + tracer_provider = trace_api.get_tracer_provider() _MEMORY_EXPORTER = InMemorySpanExporter() span_processor = export.SimpleExportSpanProcessor(_MEMORY_EXPORTER) - tracer_source.add_span_processor(span_processor) + tracer_provider.add_span_processor(span_processor) @classmethod def tearDownClass(cls): diff --git a/tests/util/src/opentelemetry/test/wsgitestutil.py b/tests/util/src/opentelemetry/test/wsgitestutil.py index d93ff2ea96..a2a5d7eea4 100644 --- a/tests/util/src/opentelemetry/test/wsgitestutil.py +++ b/tests/util/src/opentelemetry/test/wsgitestutil.py @@ -1,26 +1,25 @@ import io import wsgiref.util as wsgiref_util -from opentelemetry.ext.testutil.spantestutil import SpanTestBase -from opentelemetry import trace as trace_api -from opentelemetry.sdk.trace import TracerProvider, export -from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( - InMemorySpanExporter, -) +from opentelemetry.test.spantestutil import SpanTestBase -_MEMORY_EXPORTER = None +class WsgiTestBase(SpanTestBase): + def setUp(self): + super().setUp() -class WsgiTestBase(unittest.TestCase): - @classmethod - def setUpClass(cls): - global _MEMORY_EXPORTER # pylint:disable=global-statement - trace_api.set_tracer_provider(TracerProvider()) - tracer_provider = trace_api.get_tracer_provider() - _MEMORY_EXPORTER = InMemorySpanExporter() - span_processor = export.SimpleExportSpanProcessor(_MEMORY_EXPORTER) - tracer_provider.add_span_processor(span_processor) + self.write_buffer = io.BytesIO() + self.write = self.write_buffer.write - @classmethod - def tearDownClass(cls): - reload(trace_api) + self.environ = {} + wsgiref_util.setup_testing_defaults(self.environ) + + self.status = None + self.response_headers = None + self.exc_info = None + + def start_response(self, status, response_headers, exc_info=None): + self.status = status + self.response_headers = response_headers + self.exc_info = exc_info + return self.write diff --git a/tox.ini b/tox.ini index 529480a6e6..942c10bc9c 100644 --- a/tox.ini +++ b/tox.ini @@ -193,7 +193,7 @@ commands_pre = grpc: pip install {toxinidir}/ext/opentelemetry-ext-grpc[test] wsgi,flask,django,asgi: pip install {toxinidir}/tests/util - wsgi,flask,django,asgi: pip install {toxinidir}/ext/opentelemetry-ext-wsgi + wsgi,flask,django: pip install {toxinidir}/ext/opentelemetry-ext-wsgi flask,django: pip install {toxinidir}/opentelemetry-auto-instrumentation asgi: pip install {toxinidir}/ext/opentelemetry-ext-asgi flask: pip install {toxinidir}/ext/opentelemetry-ext-flask[test] From f0ef937189d3512228dd525ad0966969d60c85c2 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Wed, 20 May 2020 18:14:05 -0400 Subject: [PATCH 17/36] fix propagation --- .../src/opentelemetry/ext/asgi/__init__.py | 90 ++++++++++--------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 0af34b1d15..19e305642e 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -24,7 +24,7 @@ from asgiref.compatibility import guarantee_single_callable -from opentelemetry import propagators, trace +from opentelemetry import context, propagators, trace from opentelemetry.ext.asgi.version import __version__ # noqa from opentelemetry.trace.status import Status, StatusCanonicalCode @@ -79,7 +79,7 @@ def http_status_to_canonical_code(code: int, allow_redirect: bool = True): def collect_request_attributes(scope): """Collects HTTP request attributes from the ASGI scope and returns a dictionary to be used as span creation attributes.""" - server = scope.get("server") or ['0.0.0.0', 80] + server = scope.get("server") or ["0.0.0.0", 80] port = server[1] server_host = server[0] + (":" + str(port) if port != 80 else "") http_url = scope.get("scheme") + "://" + server_host + scope.get("path") @@ -98,7 +98,7 @@ def collect_request_attributes(scope): } http_host_value = ",".join(get_header_from_scope(scope, "host")) if http_host_value: - result['http.server_name'] = http_host_value + result["http.server_name"] = http_host_value if "client" in scope and scope["client"] is not None: result["net.peer.ip"] = scope.get("client")[0] @@ -143,7 +143,7 @@ class OpenTelemetryMiddleware: def __init__(self, app, name_callback=None): self.app = guarantee_single_callable(app) - self.tracer = trace.tracer_source().get_tracer(__name__, __version__) + self.tracer = trace.get_tracer(__name__, __version__) self.name_callback = name_callback or get_default_span_name async def __call__(self, scope, receive, send): @@ -155,44 +155,48 @@ async def __call__(self, scope, receive, send): send: An awaitable callable taking a single dictionary as argument. """ - parent_span = propagators.extract(get_header_from_scope, scope) + token = context.attach( + propagators.extract(get_header_from_scope, scope) + ) span_name = self.name_callback(scope) - with self.tracer.start_as_current_span( - span_name + " (asgi.connection)", - parent_span, - kind=trace.SpanKind.SERVER, - attributes=collect_request_attributes(scope), - ): - - @wraps(receive) - async def wrapped_receive(): - with self.tracer.start_as_current_span( - span_name + " (asgi." + scope["type"] + ".receive)" - ) as receive_span: - payload = await receive() - if payload["type"] == "websocket.receive": - set_status_code(receive_span, 200) - receive_span.set_attribute( - "http.status_text", payload["text"] - ) - receive_span.set_attribute("type", payload["type"]) - return payload - - @wraps(send) - async def wrapped_send(payload): - with self.tracer.start_as_current_span( - span_name + " (asgi." + scope["type"] + ".send)" - ) as send_span: - if payload["type"] == "http.response.start": - status_code = payload["status"] - set_status_code(send_span, status_code) - elif payload["type"] == "websocket.send": - set_status_code(send_span, 200) - send_span.set_attribute( - "http.status_text", payload["text"] - ) - send_span.set_attribute("type", payload["type"]) - await send(payload) - - await self.app(scope, wrapped_receive, wrapped_send) + try: + with self.tracer.start_as_current_span( + span_name + " (asgi.connection)", + kind=trace.SpanKind.SERVER, + attributes=collect_request_attributes(scope), + ): + + @wraps(receive) + async def wrapped_receive(): + with self.tracer.start_as_current_span( + span_name + " (asgi." + scope["type"] + ".receive)" + ) as receive_span: + payload = await receive() + if payload["type"] == "websocket.receive": + set_status_code(receive_span, 200) + receive_span.set_attribute( + "http.status_text", payload["text"] + ) + receive_span.set_attribute("type", payload["type"]) + return payload + + @wraps(send) + async def wrapped_send(payload): + with self.tracer.start_as_current_span( + span_name + " (asgi." + scope["type"] + ".send)" + ) as send_span: + if payload["type"] == "http.response.start": + status_code = payload["status"] + set_status_code(send_span, status_code) + elif payload["type"] == "websocket.send": + set_status_code(send_span, 200) + send_span.set_attribute( + "http.status_text", payload["text"] + ) + send_span.set_attribute("type", payload["type"]) + await send(payload) + + await self.app(scope, wrapped_receive, wrapped_send) + finally: + context.detach(token) From b4356ac49d3a3d34b424f23b03e95441a548daeb Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Wed, 20 May 2020 18:25:10 -0400 Subject: [PATCH 18/36] update changelog --- ext/opentelemetry-ext-asgi/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/opentelemetry-ext-asgi/CHANGELOG.md b/ext/opentelemetry-ext-asgi/CHANGELOG.md index 1512c42162..7936dd50fc 100644 --- a/ext/opentelemetry-ext-asgi/CHANGELOG.md +++ b/ext/opentelemetry-ext-asgi/CHANGELOG.md @@ -1,3 +1,5 @@ # Changelog ## Unreleased + +- Add ASGI middleware ([#716](https://github.com/open-telemetry/opentelemetry-python/pull/716)) From c1d3cbed797fe1aa151d3b1c3ffcffffec036cb1 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Wed, 20 May 2020 21:20:15 -0400 Subject: [PATCH 19/36] add http.user_agent --- .../src/opentelemetry/ext/asgi/__init__.py | 3 +++ .../tests/test_asgi_middleware.py | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 19e305642e..d90e49ff43 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -99,6 +99,9 @@ def collect_request_attributes(scope): http_host_value = ",".join(get_header_from_scope(scope, "host")) if http_host_value: result["http.server_name"] = http_host_value + http_user_agent = get_header_from_scope(scope, "user-agent") + if len(http_user_agent) > 0: + result["http.user_agent"] = http_user_agent[0] if "client" in scope and scope["client"] is not None: result["net.peer.ip"] = scope.get("client")[0] diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index e66f741cf9..9a2dd7c7ab 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -210,6 +210,22 @@ def update_expected_server(expected): outputs = self.get_all_output() self.validate_outputs(outputs, modifiers=[update_expected_server]) + def test_user_agent(self): + """Test that host header is converted to http.server_name.""" + user_agent = b"test-agent" + + def update_expected_user_agent(expected): + expected[3]["attributes"].update( + {"http.user_agent": user_agent.decode("utf8")} + ) + return expected + + self.scope["headers"].append([b"user-agent", user_agent]) + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs(outputs, modifiers=[update_expected_user_agent]) class TestAsgiAttributes(unittest.TestCase): def setUp(self): From 353bf271a5e8fb4abedf479e60f28a87c96e4406 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Wed, 20 May 2020 21:20:31 -0400 Subject: [PATCH 20/36] update version --- .../src/opentelemetry/ext/asgi/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py index 2f792fff80..fe3f666cdf 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py @@ -12,4 +12,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -__version__ = "0.4.dev0" +__version__ = "0.8.dev0" From 0e48edd7bbadcc977e69888f9266c3ead7156f63 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Wed, 20 May 2020 21:55:45 -0400 Subject: [PATCH 21/36] add websocket, disable for lifespan --- .../src/opentelemetry/ext/asgi/__init__.py | 2 + .../tests/test_asgi_middleware.py | 42 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index d90e49ff43..f8fb80effb 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -157,6 +157,8 @@ async def __call__(self, scope, receive, send): receive: An awaitable callable yielding dictionaries send: An awaitable callable taking a single dictionary as argument. """ + if scope.get("type") not in ("http", "websocket"): + return await self.app(scope, receive, send) token = context.attach( propagators.extract(get_header_from_scope, scope) diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 9a2dd7c7ab..726a0185da 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -227,6 +227,48 @@ def update_expected_user_agent(expected): outputs = self.get_all_output() self.validate_outputs(outputs, modifiers=[update_expected_user_agent]) + def test_websocket(self): + async def app_asgi(scope, receive, send): + payload = await receive() + if payload.get("type") == "http.request": + await send( + { + "type": "http.response.start", + "status": 200, + "headers": [[b"Content-Type", b"text/plain"]], + } + ) + await send({"type": "http.response.body", "body": b"*"}) + + self.scope["type"] = "websocket" + app = otel_asgi.OpenTelemetryMiddleware(app_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 4) + expected = [ + "HTTP GET (asgi.websocket.receive)", + "HTTP GET (asgi.websocket.send)", + "HTTP GET (asgi.websocket.send)", + ] + actual = [span.name for span in span_list] + self.assertListEqual(actual[:3], expected) + + def test_lifespan(self): + async def app_asgi(scope, receive, send): + await receive() + await send({}) + + self.scope["type"] = "lifespan" + app = otel_asgi.OpenTelemetryMiddleware(app_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 0) + + class TestAsgiAttributes(unittest.TestCase): def setUp(self): self.scope = {} From 46f35653c957ffcff266ee502845f8fd732bdf8e Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Thu, 21 May 2020 10:25:49 -0400 Subject: [PATCH 22/36] Apply suggestions from code review Co-authored-by: alrex --- docs/opentelemetry.ext.asgi.rst | 2 +- ext/opentelemetry-ext-asgi/setup.cfg | 2 +- .../src/opentelemetry/ext/asgi/version.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/opentelemetry.ext.asgi.rst b/docs/opentelemetry.ext.asgi.rst index 2d8f4e99c7..82c9833069 100644 --- a/docs/opentelemetry.ext.asgi.rst +++ b/docs/opentelemetry.ext.asgi.rst @@ -1,5 +1,5 @@ opentelemetry.ext.asgi package -========================================== +============================== Module contents --------------- diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index fdbc0dee22..d183de5688 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -1,4 +1,4 @@ -# Copyright 2019, OpenTelemetry Authors +# 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. diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py index 2f792fff80..fe3f666cdf 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py @@ -12,4 +12,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -__version__ = "0.4.dev0" +__version__ = "0.8.dev0" From 429b8e2f04c0b28991db45533a3900d125cf85af Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Thu, 21 May 2020 10:27:42 -0400 Subject: [PATCH 23/36] add py38 --- ext/opentelemetry-ext-asgi/setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index d183de5688..fa8ab65da1 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -31,6 +31,7 @@ classifiers = Programming Language :: Python :: 3.5 Programming Language :: Python :: 3.6 Programming Language :: Python :: 3.7 + Programming Language :: Python :: 3.8 [options] python_requires = >=3.5 From 83139f024f99e8bd9e303c8bf5b8923edf96034a Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Thu, 21 May 2020 11:49:55 -0400 Subject: [PATCH 24/36] remove py34 --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 942c10bc9c..e3a26e12dc 100644 --- a/tox.ini +++ b/tox.ini @@ -84,7 +84,7 @@ envlist = pypy3-test-ext-pymysql ; opentelemetry-ext-asgi - py3{4,5,6,7,8}-test-ext-asgi + py3{5,6,7,8}-test-ext-asgi pypy3-test-ext-asgi ; opentelemetry-ext-wsgi From 3759a51b93df7091246d5f0cefe4cd66c496cff5 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Thu, 21 May 2020 12:23:25 -0400 Subject: [PATCH 25/36] lint fix --- ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 726a0185da..d43662361c 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -229,6 +229,7 @@ def update_expected_user_agent(expected): def test_websocket(self): async def app_asgi(scope, receive, send): + assert scope["type"] == "websocket" payload = await receive() if payload.get("type") == "http.request": await send( @@ -244,7 +245,6 @@ async def app_asgi(scope, receive, send): app = otel_asgi.OpenTelemetryMiddleware(app_asgi) self.seed_app(app) self.send_default_request() - outputs = self.get_all_output() span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 4) expected = [ @@ -257,6 +257,7 @@ async def app_asgi(scope, receive, send): def test_lifespan(self): async def app_asgi(scope, receive, send): + assert scope["type"] == "lifespan" await receive() await send({}) @@ -264,7 +265,6 @@ async def app_asgi(scope, receive, send): app = otel_asgi.OpenTelemetryMiddleware(app_asgi) self.seed_app(app) self.send_default_request() - outputs = self.get_all_output() span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 0) From a4cce67d38569cbfef0ff31578096ffc7cb72a41 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Thu, 21 May 2020 12:53:22 -0400 Subject: [PATCH 26/36] move doc --- docs/{opentelemetry.ext.asgi.rst => ext/asgi/asgi.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/{opentelemetry.ext.asgi.rst => ext/asgi/asgi.rst} (100%) diff --git a/docs/opentelemetry.ext.asgi.rst b/docs/ext/asgi/asgi.rst similarity index 100% rename from docs/opentelemetry.ext.asgi.rst rename to docs/ext/asgi/asgi.rst From da0a00b30cc6b2092c7e2662159430b75e5ee882 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Thu, 21 May 2020 16:12:24 -0400 Subject: [PATCH 27/36] method or path --- .../src/opentelemetry/ext/asgi/__init__.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index f8fb80effb..ee42c42263 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -127,8 +127,15 @@ def set_status_code(span, status_code): def get_default_span_name(scope): - """Default implementation for name_callback, returns HTTP {METHOD_NAME}.""" - return "HTTP " + scope.get("method") + """Default implementation for name_callback, returns HTTP {METHOD_NAME or PATH}.""" + span_name = "HTTP" + + method_or_path = scope.get("method") or scope.get("path") + + if method_or_path: + return span_name + " " + method_or_path + + return span_name class OpenTelemetryMiddleware: From 16f37f704281ab4541c05a7658cd8160e468d1f1 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Thu, 21 May 2020 18:23:41 -0400 Subject: [PATCH 28/36] websocket tests --- .../src/opentelemetry/ext/asgi/__init__.py | 27 ++---- .../tests/test_asgi_middleware.py | 85 +++++++++++-------- 2 files changed, 59 insertions(+), 53 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index ee42c42263..62a6241712 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -28,8 +28,6 @@ from opentelemetry.ext.asgi.version import __version__ # noqa from opentelemetry.trace.status import Status, StatusCanonicalCode -_HTTP_VERSION_PREFIX = "HTTP/" - def get_header_from_scope(scope: dict, header_name: str) -> typing.List[str]: """Retrieve a HTTP header value from the ASGI scope. @@ -88,7 +86,6 @@ def collect_request_attributes(scope): result = { "component": scope.get("type"), - "http.method": scope.get("method"), "http.scheme": scope.get("scheme"), "http.host": server_host, "host.port": port, @@ -96,6 +93,9 @@ def collect_request_attributes(scope): "http.target": scope.get("path"), "http.url": http_url, } + http_method = scope.get("method") + if http_method: + result["http.method"] = http_method http_host_value = ",".join(get_header_from_scope(scope, "host")) if http_host_value: result["http.server_name"] = http_host_value @@ -127,15 +127,10 @@ def set_status_code(span, status_code): def get_default_span_name(scope): - """Default implementation for name_callback, returns HTTP {METHOD_NAME or PATH}.""" - span_name = "HTTP" - + """Default implementation for name_callback""" method_or_path = scope.get("method") or scope.get("path") - if method_or_path: - return span_name + " " + method_or_path - - return span_name + return method_or_path class OpenTelemetryMiddleware: @@ -174,7 +169,7 @@ async def __call__(self, scope, receive, send): try: with self.tracer.start_as_current_span( - span_name + " (asgi.connection)", + span_name + " asgi", kind=trace.SpanKind.SERVER, attributes=collect_request_attributes(scope), ): @@ -182,30 +177,24 @@ async def __call__(self, scope, receive, send): @wraps(receive) async def wrapped_receive(): with self.tracer.start_as_current_span( - span_name + " (asgi." + scope["type"] + ".receive)" + span_name + " asgi." + scope["type"] + ".receive" ) as receive_span: payload = await receive() if payload["type"] == "websocket.receive": set_status_code(receive_span, 200) - receive_span.set_attribute( - "http.status_text", payload["text"] - ) receive_span.set_attribute("type", payload["type"]) return payload @wraps(send) async def wrapped_send(payload): with self.tracer.start_as_current_span( - span_name + " (asgi." + scope["type"] + ".send)" + span_name + " asgi." + scope["type"] + ".send" ) as send_span: if payload["type"] == "http.response.start": status_code = payload["status"] set_status_code(send_span, status_code) elif payload["type"] == "websocket.send": set_status_code(send_span, 200) - send_span.set_attribute( - "http.status_text", payload["text"] - ) send_span.set_attribute("type", payload["type"]) await send(payload) diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index d43662361c..73c4b35430 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -24,9 +24,7 @@ ) -async def simple_asgi(scope, receive, send): - assert isinstance(scope, dict) - assert scope.get("type") == "http" +async def http_app(scope, receive, send): payload = await receive() if payload.get("type") == "http.request": await send( @@ -39,6 +37,28 @@ async def simple_asgi(scope, receive, send): await send({"type": "http.response.body", "body": b"*"}) +async def websocket_app(scope, receive, send): + while True: + payload = await receive() + if payload.get("type") == "websocket.connect": + await send({"type": "websocket.accept"}) + + if payload.get("type") == "websocket.receive": + if payload.get("text") == "ping": + await send({"type": "websocket.send", "text": "pong"}) + + if payload.get("type") == "websocket.disconnect": + break + + +async def simple_asgi(scope, receive, send): + assert isinstance(scope, dict) + if scope["type"] == "http": + await http_app(scope, receive, send) + elif scope["type"] == "websocket": + await websocket_app(scope, receive, send) + + async def error_asgi(scope, receive, send): assert isinstance(scope, dict) assert scope.get("type") == "http" @@ -91,12 +111,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): self.assertEqual(len(span_list), 4) expected = [ { - "name": "HTTP GET (asgi.http.receive)", + "name": "GET asgi.http.receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.request"}, }, { - "name": "HTTP GET (asgi.http.send)", + "name": "GET asgi.http.send", "kind": trace_api.SpanKind.INTERNAL, "attributes": { "http.status_code": 200, @@ -104,12 +124,12 @@ def validate_outputs(self, outputs, error=None, modifiers=None): }, }, { - "name": "HTTP GET (asgi.http.send)", + "name": "GET asgi.http.send", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"type": "http.response.body"}, }, { - "name": "HTTP GET (asgi.connection)", + "name": "GET asgi", "kind": trace_api.SpanKind.SERVER, "attributes": { "component": "http", @@ -228,41 +248,38 @@ def update_expected_user_agent(expected): self.validate_outputs(outputs, modifiers=[update_expected_user_agent]) def test_websocket(self): - async def app_asgi(scope, receive, send): - assert scope["type"] == "websocket" - payload = await receive() - if payload.get("type") == "http.request": - await send( - { - "type": "http.response.start", - "status": 200, - "headers": [[b"Content-Type", b"text/plain"]], - } - ) - await send({"type": "http.response.body", "body": b"*"}) - - self.scope["type"] = "websocket" - app = otel_asgi.OpenTelemetryMiddleware(app_asgi) + self.scope = { + "type": "websocket", + "http_version": "1.1", + "scheme": "ws", + "path": "/", + "query_string": b"", + "headers": [], + "client": ("127.0.0.1", 32767), + "server": ("127.0.0.1", 80), + } + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) self.seed_app(app) - self.send_default_request() + self.send_input({"type": "websocket.connect"}) + self.send_input({"type": "websocket.receive", "text": "ping"}) + self.send_input({"type": "websocket.disconnect"}) + outputs = self.get_all_output() span_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(span_list), 4) + self.assertEqual(len(span_list), 6) expected = [ - "HTTP GET (asgi.websocket.receive)", - "HTTP GET (asgi.websocket.send)", - "HTTP GET (asgi.websocket.send)", + "/ asgi.websocket.receive", + "/ asgi.websocket.send", + "/ asgi.websocket.receive", + "/ asgi.websocket.send", + "/ asgi.websocket.receive", + "/ asgi", ] actual = [span.name for span in span_list] - self.assertListEqual(actual[:3], expected) + self.assertListEqual(actual, expected) def test_lifespan(self): - async def app_asgi(scope, receive, send): - assert scope["type"] == "lifespan" - await receive() - await send({}) - self.scope["type"] = "lifespan" - app = otel_asgi.OpenTelemetryMiddleware(app_asgi) + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) self.seed_app(app) self.send_default_request() span_list = self.memory_exporter.get_finished_spans() From 3ac4620e708e0ffe5295ff748718f837ddf87184 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Thu, 21 May 2020 21:59:18 -0400 Subject: [PATCH 29/36] handle empty attributes --- .../src/opentelemetry/ext/asgi/__init__.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 62a6241712..941cf7a363 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -80,8 +80,12 @@ def collect_request_attributes(scope): server = scope.get("server") or ["0.0.0.0", 80] port = server[1] server_host = server[0] + (":" + str(port) if port != 80 else "") - http_url = scope.get("scheme") + "://" + server_host + scope.get("path") - if scope.get("query_string"): + http_url = ( + scope.get("scheme") + "://" + server_host + scope.get("path", "") + if scope.get("scheme") and server_host and scope.get("path") + else None + ) + if scope.get("query_string") and http_url: http_url = http_url + ("?" + scope.get("query_string").decode("utf8")) result = { @@ -107,6 +111,9 @@ def collect_request_attributes(scope): result["net.peer.ip"] = scope.get("client")[0] result["net.peer.port"] = scope.get("client")[1] + # remove None values + result = {k: v for k, v in result.items() if v is not None} + return result From 9f3d5bbbbd920b9a20e03d903c575723908dbb8b Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Thu, 21 May 2020 22:17:32 -0400 Subject: [PATCH 30/36] fix lint --- ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 73c4b35430..f994b89d68 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -26,6 +26,7 @@ async def http_app(scope, receive, send): payload = await receive() + assert scope["type"] == "http" if payload.get("type") == "http.request": await send( { @@ -38,6 +39,7 @@ async def http_app(scope, receive, send): async def websocket_app(scope, receive, send): + assert scope["type"] == "websocket" while True: payload = await receive() if payload.get("type") == "websocket.connect": @@ -263,7 +265,7 @@ def test_websocket(self): self.send_input({"type": "websocket.connect"}) self.send_input({"type": "websocket.receive", "text": "ping"}) self.send_input({"type": "websocket.disconnect"}) - outputs = self.get_all_output() + self.get_all_output() span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 6) expected = [ From 9e042283fcfcca586994aa87ad4a891ea7bdfc1c Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Tue, 26 May 2020 19:34:24 -0400 Subject: [PATCH 31/36] Apply suggestions from code review Co-authored-by: Florimond Manca Co-authored-by: alrex --- ext/opentelemetry-ext-asgi/README.rst | 2 +- ext/opentelemetry-ext-asgi/setup.cfg | 2 +- ext/opentelemetry-ext-asgi/setup.py | 2 +- .../src/opentelemetry/ext/asgi/__init__.py | 10 +++++----- .../src/opentelemetry/ext/asgi/version.py | 2 +- .../tests/test_asgi_middleware.py | 2 +- tests/util/src/opentelemetry/test/asgitestutil.py | 4 ++-- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/README.rst b/ext/opentelemetry-ext-asgi/README.rst index 09e8bcc1fa..78055ffe07 100644 --- a/ext/opentelemetry-ext-asgi/README.rst +++ b/ext/opentelemetry-ext-asgi/README.rst @@ -8,7 +8,7 @@ OpenTelemetry ASGI Middleware This library provides a ASGI middleware that can be used on any ASGI framework -(such as Django / Flask) to track requests timing through OpenTelemetry. +(such as Django, Starlette, FastAPI or Quart) to track requests timing through OpenTelemetry. Installation ------------ diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index fa8ab65da1..e0790217c4 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -23,7 +23,7 @@ url = https://github.com/open-telemetry/opentelemetry-python/ext/opentelemetry-e platforms = any license = Apache-2.0 classifiers = - Development Status :: 3 - Alpha + Development Status :: 4 - Beta Intended Audience :: Developers License :: OSI Approved :: Apache Software License Programming Language :: Python diff --git a/ext/opentelemetry-ext-asgi/setup.py b/ext/opentelemetry-ext-asgi/setup.py index 42c82506eb..8bf19bd422 100644 --- a/ext/opentelemetry-ext-asgi/setup.py +++ b/ext/opentelemetry-ext-asgi/setup.py @@ -1,4 +1,4 @@ -# Copyright 2019, OpenTelemetry Authors +# 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. diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 941cf7a363..e6e0acba32 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -1,4 +1,4 @@ -# Copyright 2019, OpenTelemetry Authors +# 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. @@ -81,7 +81,7 @@ def collect_request_attributes(scope): port = server[1] server_host = server[0] + (":" + str(port) if port != 80 else "") http_url = ( - scope.get("scheme") + "://" + server_host + scope.get("path", "") + scope.get("scheme", "http") + "://" + server_host + scope.get("path", "") if scope.get("scheme") and server_host and scope.get("path") else None ) @@ -89,7 +89,7 @@ def collect_request_attributes(scope): http_url = http_url + ("?" + scope.get("query_string").decode("utf8")) result = { - "component": scope.get("type"), + "component": scope["type"] "http.scheme": scope.get("scheme"), "http.host": server_host, "host.port": port, @@ -186,14 +186,14 @@ async def wrapped_receive(): with self.tracer.start_as_current_span( span_name + " asgi." + scope["type"] + ".receive" ) as receive_span: - payload = await receive() + message = await receive() if payload["type"] == "websocket.receive": set_status_code(receive_span, 200) receive_span.set_attribute("type", payload["type"]) return payload @wraps(send) - async def wrapped_send(payload): + async def wrapped_send(message): with self.tracer.start_as_current_span( span_name + " asgi." + scope["type"] + ".send" ) as send_span: diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py index fe3f666cdf..bcf6a35777 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/version.py @@ -1,4 +1,4 @@ -# Copyright 2019, OpenTelemetry Authors +# 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. diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index f994b89d68..26b247a531 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -1,4 +1,4 @@ -# Copyright 2019, OpenTelemetry Authors +# 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. diff --git a/tests/util/src/opentelemetry/test/asgitestutil.py b/tests/util/src/opentelemetry/test/asgitestutil.py index 871209bdde..096c32b1dd 100644 --- a/tests/util/src/opentelemetry/test/asgitestutil.py +++ b/tests/util/src/opentelemetry/test/asgitestutil.py @@ -38,9 +38,9 @@ def tearDown(self): def seed_app(self, app): self.communicator = ApplicationCommunicator(app, self.scope) - def send_input(self, payload): + def send(self, message): asyncio.get_event_loop().run_until_complete( - self.communicator.send_input(payload) + self.communicator.send_input(message) ) def send_default_request(self): From edc60444d6f2b3b7c95baf2f9ff142b0bbe86364 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Tue, 26 May 2020 20:55:24 -0400 Subject: [PATCH 32/36] more review updates --- ext/opentelemetry-ext-asgi/README.rst | 13 +++--- .../src/opentelemetry/ext/asgi/__init__.py | 33 +++++++------- .../tests/test_asgi_middleware.py | 43 ++++++++++++++----- .../src/opentelemetry/test/asgitestutil.py | 16 ++++++- .../src/opentelemetry/test/spantestutil.py | 14 ++++++ .../src/opentelemetry/test/wsgitestutil.py | 14 ++++++ 6 files changed, 98 insertions(+), 35 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/README.rst b/ext/opentelemetry-ext-asgi/README.rst index 78055ffe07..bc286e5c8b 100644 --- a/ext/opentelemetry-ext-asgi/README.rst +++ b/ext/opentelemetry-ext-asgi/README.rst @@ -37,24 +37,23 @@ Usage (Quart) app.run(debug=True) -Usage (Django) --------------- +Usage (Django 3.0) +------------------ Modify the application's ``asgi.py`` file as shown below. .. code-block:: python import os - import django - from channels.routing import get_default_application + from django.core.asgi import get_asgi_application from opentelemetry.ext.asgi import OpenTelemetryMiddleware - os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'application.settings') - django.setup() + os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'asgi_example.settings') - application = get_default_application() + application = get_asgi_application() application = OpenTelemetryMiddleware(application) + References ---------- diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index e6e0acba32..6a7c829d96 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -20,6 +20,7 @@ import operator import typing +import urllib from functools import wraps from asgiref.compatibility import guarantee_single_callable @@ -80,16 +81,16 @@ def collect_request_attributes(scope): server = scope.get("server") or ["0.0.0.0", 80] port = server[1] server_host = server[0] + (":" + str(port) if port != 80 else "") - http_url = ( - scope.get("scheme", "http") + "://" + server_host + scope.get("path", "") - if scope.get("scheme") and server_host and scope.get("path") - else None - ) + full_path = scope.get("root_path", "") + scope.get("path", "") + http_url = scope.get("scheme", "http") + "://" + server_host + full_path if scope.get("query_string") and http_url: - http_url = http_url + ("?" + scope.get("query_string").decode("utf8")) + if isinstance(scope["query_string"], bytes): + http_url = http_url + ("?" + scope.get("query_string").decode("utf8")) + else: + http_url = http_url + ("?" + urllib.parse.unquote(scope.get("query_string"))) result = { - "component": scope["type"] + "component": scope["type"], "http.scheme": scope.get("scheme"), "http.host": server_host, "host.port": port, @@ -166,7 +167,7 @@ async def __call__(self, scope, receive, send): receive: An awaitable callable yielding dictionaries send: An awaitable callable taking a single dictionary as argument. """ - if scope.get("type") not in ("http", "websocket"): + if scope["type"] not in ("http", "websocket"): return await self.app(scope, receive, send) token = context.attach( @@ -187,23 +188,23 @@ async def wrapped_receive(): span_name + " asgi." + scope["type"] + ".receive" ) as receive_span: message = await receive() - if payload["type"] == "websocket.receive": + if message["type"] == "websocket.receive": set_status_code(receive_span, 200) - receive_span.set_attribute("type", payload["type"]) - return payload + receive_span.set_attribute("type", message["type"]) + return message @wraps(send) async def wrapped_send(message): with self.tracer.start_as_current_span( span_name + " asgi." + scope["type"] + ".send" ) as send_span: - if payload["type"] == "http.response.start": - status_code = payload["status"] + if message["type"] == "http.response.start": + status_code = message["status"] set_status_code(send_span, status_code) - elif payload["type"] == "websocket.send": + elif message["type"] == "websocket.send": set_status_code(send_span, 200) - send_span.set_attribute("type", payload["type"]) - await send(payload) + send_span.set_attribute("type", message["type"]) + await send(message) await self.app(scope, wrapped_receive, wrapped_send) finally: diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 26b247a531..efcd6ec474 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -15,6 +15,7 @@ import sys import unittest import unittest.mock as mock +import urllib import opentelemetry.ext.asgi as otel_asgi from opentelemetry import trace as trace_api @@ -25,9 +26,9 @@ async def http_app(scope, receive, send): - payload = await receive() + message = await receive() assert scope["type"] == "http" - if payload.get("type") == "http.request": + if message.get("type") == "http.request": await send( { "type": "http.response.start", @@ -41,15 +42,15 @@ async def http_app(scope, receive, send): async def websocket_app(scope, receive, send): assert scope["type"] == "websocket" while True: - payload = await receive() - if payload.get("type") == "websocket.connect": + message = await receive() + if message.get("type") == "websocket.connect": await send({"type": "websocket.accept"}) - if payload.get("type") == "websocket.receive": - if payload.get("text") == "ping": + if message.get("type") == "websocket.receive": + if message.get("text") == "ping": await send({"type": "websocket.send", "text": "pong"}) - if payload.get("type") == "websocket.disconnect": + if message.get("type") == "websocket.disconnect": break @@ -63,9 +64,9 @@ async def simple_asgi(scope, receive, send): async def error_asgi(scope, receive, send): assert isinstance(scope, dict) - assert scope.get("type") == "http" - payload = await receive() - if payload.get("type") == "http.request": + assert scope["type"] == "http" + message = await receive() + if message.get("type") == "http.request": try: raise ValueError except ValueError: @@ -294,7 +295,7 @@ def setUp(self): setup_testing_defaults(self.scope) self.span = mock.create_autospec(trace_api.Span, spec_set=True) - def test_request_attributes(self): + def test_query_string_utf8(self): self.scope["query_string"] = b"foo=bar" attrs = otel_asgi.collect_request_attributes(self.scope) @@ -314,6 +315,26 @@ def test_request_attributes(self): }, ) + def test_query_string_percent_encoding(self): + self.scope["query_string"] = urllib.parse.quote(b"foo=bar") + + attrs = otel_asgi.collect_request_attributes(self.scope) + self.assertDictEqual( + attrs, + { + "component": "http", + "http.method": "GET", + "http.host": "127.0.0.1", + "http.target": "/", + "http.url": "http://127.0.0.1/?foo=bar", + "host.port": 80, + "http.scheme": "http", + "http.flavor": "1.0", + "net.peer.ip": "127.0.0.1", + "net.peer.port": 32767, + }, + ) + def test_response_attributes(self): otel_asgi.set_status_code(self.span, 404) expected = (mock.call("http.status_code", 404),) diff --git a/tests/util/src/opentelemetry/test/asgitestutil.py b/tests/util/src/opentelemetry/test/asgitestutil.py index 096c32b1dd..7d23039bf3 100644 --- a/tests/util/src/opentelemetry/test/asgitestutil.py +++ b/tests/util/src/opentelemetry/test/asgitestutil.py @@ -1,3 +1,17 @@ +# 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 asyncio from asgiref.testing import ApplicationCommunicator @@ -38,7 +52,7 @@ def tearDown(self): def seed_app(self, app): self.communicator = ApplicationCommunicator(app, self.scope) - def send(self, message): + def send_input(self, message): asyncio.get_event_loop().run_until_complete( self.communicator.send_input(message) ) diff --git a/tests/util/src/opentelemetry/test/spantestutil.py b/tests/util/src/opentelemetry/test/spantestutil.py index 70d33547e8..4ae4669a04 100644 --- a/tests/util/src/opentelemetry/test/spantestutil.py +++ b/tests/util/src/opentelemetry/test/spantestutil.py @@ -1,3 +1,17 @@ +# 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 importlib import reload diff --git a/tests/util/src/opentelemetry/test/wsgitestutil.py b/tests/util/src/opentelemetry/test/wsgitestutil.py index a2a5d7eea4..349db8f694 100644 --- a/tests/util/src/opentelemetry/test/wsgitestutil.py +++ b/tests/util/src/opentelemetry/test/wsgitestutil.py @@ -1,3 +1,17 @@ +# 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 io import wsgiref.util as wsgiref_util From 3849da12470e872bb3ece5727e797c92557ca675 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Tue, 26 May 2020 21:03:16 -0400 Subject: [PATCH 33/36] better handling for query_string --- .../src/opentelemetry/ext/asgi/__init__.py | 10 +++--- .../tests/test_asgi_middleware.py | 31 ++++++++----------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py index 6a7c829d96..69c30848da 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -83,11 +83,11 @@ def collect_request_attributes(scope): server_host = server[0] + (":" + str(port) if port != 80 else "") full_path = scope.get("root_path", "") + scope.get("path", "") http_url = scope.get("scheme", "http") + "://" + server_host + full_path - if scope.get("query_string") and http_url: - if isinstance(scope["query_string"], bytes): - http_url = http_url + ("?" + scope.get("query_string").decode("utf8")) - else: - http_url = http_url + ("?" + urllib.parse.unquote(scope.get("query_string"))) + query_string = scope.get("query_string") + if query_string and http_url: + if isinstance(query_string, bytes): + query_string = query_string.decode("utf8") + http_url = http_url + ("?" + urllib.parse.unquote(query_string)) result = { "component": scope["type"], diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index efcd6ec474..4d1296cb38 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -295,7 +295,7 @@ def setUp(self): setup_testing_defaults(self.scope) self.span = mock.create_autospec(trace_api.Span, spec_set=True) - def test_query_string_utf8(self): + def test_request_attributes(self): self.scope["query_string"] = b"foo=bar" attrs = otel_asgi.collect_request_attributes(self.scope) @@ -315,25 +315,20 @@ def test_query_string_utf8(self): }, ) - def test_query_string_percent_encoding(self): - self.scope["query_string"] = urllib.parse.quote(b"foo=bar") + def test_query_string(self): + self.scope["query_string"] = b"foo=bar" + attrs = otel_asgi.collect_request_attributes(self.scope) + self.assertEqual(attrs["http.url"], "http://127.0.0.1/?foo=bar") + def test_query_string_percent_bytes(self): + self.scope["query_string"] = b"foo%3Dbar" attrs = otel_asgi.collect_request_attributes(self.scope) - self.assertDictEqual( - attrs, - { - "component": "http", - "http.method": "GET", - "http.host": "127.0.0.1", - "http.target": "/", - "http.url": "http://127.0.0.1/?foo=bar", - "host.port": 80, - "http.scheme": "http", - "http.flavor": "1.0", - "net.peer.ip": "127.0.0.1", - "net.peer.port": 32767, - }, - ) + self.assertEqual(attrs["http.url"], "http://127.0.0.1/?foo=bar") + + def test_query_string_percent_str(self): + self.scope["query_string"] = "foo%3Dbar" + attrs = otel_asgi.collect_request_attributes(self.scope) + self.assertEqual(attrs["http.url"], "http://127.0.0.1/?foo=bar") def test_response_attributes(self): otel_asgi.set_status_code(self.span, 404) From bb8506bfefae8461c28e1050ee04d2a0d98033aa Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Tue, 26 May 2020 21:17:11 -0400 Subject: [PATCH 34/36] added versions for install_requires --- ext/opentelemetry-ext-asgi/setup.cfg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index e0790217c4..4cd964cedf 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -39,8 +39,8 @@ package_dir= =src packages=find_namespace: install_requires = - opentelemetry-api - asgiref + opentelemetry-api == 0.8.dev0 + asgiref ~= 3.2.3 [options.extras_require] test = From d888a6fd0ebb0dca553603e9d485503a45efe0a7 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Tue, 26 May 2020 21:26:56 -0400 Subject: [PATCH 35/36] at least asgiref 3.0 --- docs-requirements.txt | 2 +- ext/opentelemetry-ext-asgi/setup.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs-requirements.txt b/docs-requirements.txt index 2788761006..358316fa7e 100644 --- a/docs-requirements.txt +++ b/docs-requirements.txt @@ -3,7 +3,7 @@ sphinx-rtd-theme~=0.4 sphinx-autodoc-typehints~=1.10.2 # Required by ext packages -asgiref~=3.2.3 +asgiref~=3.0 ddtrace>=0.34.0 aiohttp ~= 3.0 Deprecated>=1.2.6 diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index 4cd964cedf..f834737735 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -40,7 +40,7 @@ package_dir= packages=find_namespace: install_requires = opentelemetry-api == 0.8.dev0 - asgiref ~= 3.2.3 + asgiref ~= 3.0 [options.extras_require] test = From 25b0534bd2386a562124f263033db2c2f9210c7d Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Tue, 26 May 2020 21:46:18 -0400 Subject: [PATCH 36/36] remove unused import --- ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 4d1296cb38..edc90444a7 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -15,7 +15,6 @@ import sys import unittest import unittest.mock as mock -import urllib import opentelemetry.ext.asgi as otel_asgi from opentelemetry import trace as trace_api