From 48ebb60d3ad82e2151a493be04b35b5c08a04124 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Wed, 3 Jun 2020 21:44:12 -0700 Subject: [PATCH 01/20] Have a basic starlette instrumentation working. --- ext/opentelemetry-ext-asgi/setup.cfg | 5 +- .../CHANGELOG.md | 9 +++ .../README.rst | 60 +++++++++++++++++++ .../setup.cfg | 52 ++++++++++++++++ .../setup.py | 26 ++++++++ .../instrumentation/starlette/__init__.py | 57 ++++++++++++++++++ .../instrumentation/starlette/version.py | 15 +++++ .../tests/__init__.py | 0 .../tests/test_starlette_instrumentation.py | 48 +++++++++++++++ tox.ini | 11 +++- 10 files changed, 278 insertions(+), 5 deletions(-) create mode 100644 ext/opentelemetry-instrumentation-starlette/CHANGELOG.md create mode 100644 ext/opentelemetry-instrumentation-starlette/README.rst create mode 100644 ext/opentelemetry-instrumentation-starlette/setup.cfg create mode 100644 ext/opentelemetry-instrumentation-starlette/setup.py create mode 100644 ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py create mode 100644 ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/version.py create mode 100644 ext/opentelemetry-instrumentation-starlette/tests/__init__.py create mode 100644 ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index 2be30ce41c7..4ce67e02d7a 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -5,8 +5,7 @@ # 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 +# # 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 @@ -44,7 +43,7 @@ install_requires = [options.extras_require] test = - opentelemetry-ext-testutil + opentelemetry-test [options.packages.find] where = src diff --git a/ext/opentelemetry-instrumentation-starlette/CHANGELOG.md b/ext/opentelemetry-instrumentation-starlette/CHANGELOG.md new file mode 100644 index 00000000000..886cbff8f19 --- /dev/null +++ b/ext/opentelemetry-instrumentation-starlette/CHANGELOG.md @@ -0,0 +1,9 @@ +# Changelog + +## Unreleased + +## 0.8b0 + +Released 2020-05-27 + +- Add ASGI middleware ([#716](https://github.com/open-telemetry/opentelemetry-python/pull/716)) diff --git a/ext/opentelemetry-instrumentation-starlette/README.rst b/ext/opentelemetry-instrumentation-starlette/README.rst new file mode 100644 index 00000000000..c5da75441cf --- /dev/null +++ b/ext/opentelemetry-instrumentation-starlette/README.rst @@ -0,0 +1,60 @@ +OpenTelemetry ASGI Middleware +============================= + +|pypi| + +.. |pypi| image:: https://badge.fury.io/py/opentelemetry-instrumentation-starlette.svg + :target: https://pypi.org/project/opentelemetry-instrumentation-starlette/ + + +This library provides a ASGI middleware that can be used on any ASGI framework +(such as Django, Starlette, FastAPI or Quart) to track requests timing through OpenTelemetry. + +Installation +------------ + +:: + + pip install opentelemetry-instrumentation-starlette + + +Usage (Quart) +------------- + +.. code-block:: python + + from quart import Quart + from opentelemetry.instrumentation.starlette import OpenTelemetryMiddleware + + app = Quart(__name__) + app.starlette_app = OpenTelemetryMiddleware(app.starlette_app) + + @app.route("/") + async def hello(): + return "Hello!" + + if __name__ == "__main__": + app.run(debug=True) + + +Usage (Django 3.0) +------------------ + +Modify the application's ``starlette.py`` file as shown below. + +.. code-block:: python + + import os + from django.core.starlette import get_starlette_application + from opentelemetry.instrumentation.starlette import OpenTelemetryMiddleware + + os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'starlette_example.settings') + + application = get_starlette_application() + application = OpenTelemetryMiddleware(application) + + +References +---------- + +* `OpenTelemetry Project `_ diff --git a/ext/opentelemetry-instrumentation-starlette/setup.cfg b/ext/opentelemetry-instrumentation-starlette/setup.cfg new file mode 100644 index 00000000000..9d19ede0c2a --- /dev/null +++ b/ext/opentelemetry-instrumentation-starlette/setup.cfg @@ -0,0 +1,52 @@ +# 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. +# +[metadata] +name = opentelemetry-instrumentation-starlette +description = ASGI Middleware for OpenTelemetry +long_description = file: README.rst +long_description_content_type = tinstrumentation/x-rst +author = OpenTelemetry Authors +author_email = cncf-opentelemetry-contributors@lists.cncf.io +url = https://github.com/open-telemetry/opentelemetry-python/instrumentation/opentelemetry-instrumentation-starlette +platforms = any +license = Apache-2.0 +classifiers = + Development Status :: 4 - Beta + Intended Audience :: Developers + 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 + Programming Language :: Python :: 3.8 + +[options] +python_requires = >=3.5 +package_dir= + =src +packages=find_namespace: +install_requires = + opentelemetry-api == 0.9.dev0 + opentelemetry-ext-asgi == 0.9.dev0 + +[options.extras_require] +test = + opentelemetry-test + starlette + requests # needed for testclient + +[options.packages.find] +where = src diff --git a/ext/opentelemetry-instrumentation-starlette/setup.py b/ext/opentelemetry-instrumentation-starlette/setup.py new file mode 100644 index 00000000000..6390d909a7a --- /dev/null +++ b/ext/opentelemetry-instrumentation-starlette/setup.py @@ -0,0 +1,26 @@ +# 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 os + +import setuptools + +BASE_DIR = os.path.dirname(__file__) +VERSION_FILENAME = os.path.join( + BASE_DIR, "src", "opentelemetry", "instrumentation", "starlette", "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-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py new file mode 100644 index 00000000000..d93c6dafa35 --- /dev/null +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -0,0 +1,57 @@ +# 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. + +""" +The opentelemetry-instrumentation-starlette package provides an ASGI middleware that can be used +on any ASGI framework (such as Django-channels / Quart) to track requests +timing through OpenTelemetry. +""" + +from opentelemetry.ext.asgi import OpenTelemetryMiddleware +from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.instrumentation.starlette.version import __version__ # noqa + +from starlette.applications import Starlette +from starlette.middleware import Middleware +from starlette.routing import Match + + +# class StarletteInstrumentor(BaseInstrumentor): +# """An instrumentor for starlette +# +# See `BaseInstrumentor` +# """ +# +# def _instrument(self, ) + + +class _InstrumentedStarlette(Starlette): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.add_middleware( + OpenTelemetryMiddleware, name_callback=_get_route_name + ) + + +def _get_route_name(scope): + """Callback to retrieve the starlette route being served. + """ + app = scope["app"] + for route in app.routes: + match, _ = route.matches(scope) + if match == Match.FULL: + return route.path + # method only exists for http, if websocket + # leave it blank. + return scope.get("method", "") diff --git a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/version.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/version.py new file mode 100644 index 00000000000..603bf0b7e5f --- /dev/null +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/version.py @@ -0,0 +1,15 @@ +# 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. + +__version__ = "0.9.dev0" diff --git a/ext/opentelemetry-instrumentation-starlette/tests/__init__.py b/ext/opentelemetry-instrumentation-starlette/tests/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py b/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py new file mode 100644 index 00000000000..1631205ca3c --- /dev/null +++ b/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py @@ -0,0 +1,48 @@ +# 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 sys +import unittest +import unittest.mock as mock + +import opentelemetry.instrumentation.starlette as otel_starlette +from opentelemetry.test.test_base import TestBase +from starlette.responses import PlainTextResponse +from starlette.routing import Route +from starlette.testclient import TestClient + + +class TestStarletteApplication(TestBase): + def setUp(self): + super().setUp() + self._app = self._create_instrumented_app() + self._client = TestClient(self._app) + + def test_basic_starlette_call(self): + response = self._client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEquals(len(spans), 3) + for span in spans: + self.assertIn("foobar", span.name) + + @staticmethod + def _create_instrumented_app(): + """Create an instrumented starlette application""" + + def home(request): + return PlainTextResponse("hi") + + return otel_starlette._InstrumentedStarlette( + routes=[Route("/foobar", home)] + ) diff --git a/tox.ini b/tox.ini index 026d01cc0c1..ee4b0542fc6 100644 --- a/tox.ini +++ b/tox.ini @@ -48,6 +48,10 @@ envlist = py3{4,5,6,7,8}-test-ext-requests pypy3-test-ext-requests + ; opentelemetry-instrumentation-starlette + py3{4,5,6,7,8}-test-instrunentation-starlette + pypy3-test-instrumentation-starlette + ; opentelemetry-ext-jinja2 py3{4,5,6,7,8}-test-ext-jinja2 pypy3-test-ext-jinja2 @@ -174,6 +178,7 @@ changedir = test-ext-sqlalchemy: ext/opentelemetry-ext-sqlalchemy/tests test-ext-redis: ext/opentelemetry-ext-redis/tests test-ext-system-metrics: ext/opentelemetry-ext-system-metrics/tests + test-instrumentation-starlette: ext/opentelemetry-instrumentation-starlette/tests commands_pre = ; Install without -e to test the actual installation @@ -190,10 +195,10 @@ commands_pre = grpc: pip install {toxinidir}/ext/opentelemetry-ext-grpc[test] - wsgi,flask,django,asgi: pip install {toxinidir}/tests/util + wsgi,flask,django,asgi,starlette: pip install {toxinidir}/tests/util 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 + asgi,starlette: pip install {toxinidir}/ext/opentelemetry-ext-asgi boto: pip install {toxinidir}/opentelemetry-auto-instrumentation boto: pip install {toxinidir}/ext/opentelemetry-ext-boto[test] @@ -222,6 +227,8 @@ commands_pre = requests: pip install {toxinidir}/opentelemetry-auto-instrumentation {toxinidir}/ext/opentelemetry-ext-requests[test] + starlette: pip install {toxinidir}/opentelemetry-auto-instrumentation {toxinidir}/ext/opentelemetry-instrumentation-starlette[test] + jinja2: pip install {toxinidir}/opentelemetry-auto-instrumentation {toxinidir}/ext/opentelemetry-ext-jinja2[test] aiohttp-client: pip install {toxinidir}/opentelemetry-sdk {toxinidir}/ext/opentelemetry-ext-aiohttp-client From 537029ee8256e0894bca7c663f644cec8d7db3ad Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Mon, 8 Jun 2020 20:45:22 -0700 Subject: [PATCH 02/20] fix typo --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index ee4b0542fc6..bee548b3852 100644 --- a/tox.ini +++ b/tox.ini @@ -49,7 +49,7 @@ envlist = pypy3-test-ext-requests ; opentelemetry-instrumentation-starlette - py3{4,5,6,7,8}-test-instrunentation-starlette + py3{4,5,6,7,8}-test-instrumentation-starlette pypy3-test-instrumentation-starlette ; opentelemetry-ext-jinja2 From e89c37d390101c10d8789cdd1540c67f845f3fe7 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Mon, 8 Jun 2020 21:53:27 -0700 Subject: [PATCH 03/20] Fleshing out instrumentor interface. --- .../instrumentation/starlette/__init__.py | 35 +++++++++++++------ tox.ini | 2 +- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index d93c6dafa35..42d896cd877 100644 --- a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -19,24 +19,39 @@ """ from opentelemetry.ext.asgi import OpenTelemetryMiddleware -from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.starlette.version import __version__ # noqa -from starlette.applications import Starlette +from starlette import applications from starlette.middleware import Middleware from starlette.routing import Match -# class StarletteInstrumentor(BaseInstrumentor): -# """An instrumentor for starlette -# -# See `BaseInstrumentor` -# """ -# -# def _instrument(self, ) +class StarletteInstrumentor(BaseInstrumentor): + """An instrumentor for starlette + + See `BaseInstrumentor` + """ + + @staticmethod + def instrument_app(app: applications.Starlette): + """Instrument a previously instrumented Starlette application. + """ + if not getattr(app, "_is_instrumented_by_opentelemetry", False): + app.add_middleware( + OpenTelemetryMiddleware, name_callback=_get_route_name + ) + app._is_instrumented_by_opentelemetry = True + + def _instrument(self, **kwargs): + self._original_starlette = applications.Starlette + applications.Starlette = _InstrumentedStarlette + + def _uninstrument(self, **kwargs): + application.Starlette = self._original_starlette -class _InstrumentedStarlette(Starlette): +class _InstrumentedStarlette(applications.Starlette): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.add_middleware( diff --git a/tox.ini b/tox.ini index aa9f26df7b2..8cf5767efc5 100644 --- a/tox.ini +++ b/tox.ini @@ -225,7 +225,7 @@ commands_pre = requests: pip install {toxinidir}/ext/opentelemetry-ext-requests[test] - starlette: {toxinidir}/ext/opentelemetry-instrumentation-starlette[test] + starlette: pip install {toxinidir}/ext/opentelemetry-instrumentation-starlette[test] jinja2: pip install {toxinidir}/ext/opentelemetry-ext-jinja2[test] From 26464dbf25f0ff8e4eb833ffeedabbbf28c4d330 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Tue, 9 Jun 2020 08:59:52 -0700 Subject: [PATCH 04/20] more changes. --- .../setup.cfg | 2 +- .../setup.py | 7 ++- .../instrumentation/starlette/__init__.py | 1 - opentelemetry-instrumentation/README.rst | 28 ++++++++++++ .../opentelemetry/instrumentation/__init__.py | 43 ------------------- 5 files changed, 35 insertions(+), 46 deletions(-) delete mode 100644 opentelemetry-instrumentation/src/opentelemetry/instrumentation/__init__.py diff --git a/ext/opentelemetry-instrumentation-starlette/setup.cfg b/ext/opentelemetry-instrumentation-starlette/setup.cfg index 9d19ede0c2a..6d7210a84fe 100644 --- a/ext/opentelemetry-instrumentation-starlette/setup.cfg +++ b/ext/opentelemetry-instrumentation-starlette/setup.cfg @@ -44,7 +44,7 @@ install_requires = [options.extras_require] test = - opentelemetry-test + opentelemetry-test == 0.9.dev0 starlette requests # needed for testclient diff --git a/ext/opentelemetry-instrumentation-starlette/setup.py b/ext/opentelemetry-instrumentation-starlette/setup.py index 6390d909a7a..0232a6f448c 100644 --- a/ext/opentelemetry-instrumentation-starlette/setup.py +++ b/ext/opentelemetry-instrumentation-starlette/setup.py @@ -17,7 +17,12 @@ BASE_DIR = os.path.dirname(__file__) VERSION_FILENAME = os.path.join( - BASE_DIR, "src", "opentelemetry", "instrumentation", "starlette", "version.py" + BASE_DIR, + "src", + "opentelemetry", + "instrumentation", + "starlette", + "version.py", ) PACKAGE_INFO = {} with open(VERSION_FILENAME) as f: diff --git a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index 42d896cd877..05f1db3c319 100644 --- a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -21,7 +21,6 @@ from opentelemetry.ext.asgi import OpenTelemetryMiddleware from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.starlette.version import __version__ # noqa - from starlette import applications from starlette.middleware import Middleware from starlette.routing import Match diff --git a/opentelemetry-instrumentation/README.rst b/opentelemetry-instrumentation/README.rst index d52ed037b75..f01b689b7ea 100644 --- a/opentelemetry-instrumentation/README.rst +++ b/opentelemetry-instrumentation/README.rst @@ -13,6 +13,34 @@ Installation pip install opentelemetry-instrumentation + +This package provides a couple of commands that help automatically instruments a program: + +opentelemetry-instrument +------------------------ + +:: + + opentelemetry-instrument python program.py + +The code in ``program.py`` needs to use one of the packages for which there is +an OpenTelemetry integration. For a list of the available integrations please +check :doc:`here <../../index>`. + + +opentelemetry-bootstrap +----------------------- + +:: + + opentelemetry-bootstrap --action=install|requirements + +This commands inspects the active Python site-packages and figures out which +instrumentation packages the user might want to install. By default it prints out +a list of the suggested instrumentation packages which can be added to a requirements.txt +file. It also supports installing the suggested packages when run with :code:`--action=install` +flag. + References ---------- diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/__init__.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/__init__.py deleted file mode 100644 index 149a9b46c93..00000000000 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/__init__.py +++ /dev/null @@ -1,43 +0,0 @@ -# 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. - -""" - -This package provides a couple of commands that help automatically instruments a program: - -opentelemetry-instrument ------------------------------------ - -:: - - opentelemetry-instrument python program.py - -The code in ``program.py`` needs to use one of the packages for which there is -an OpenTelemetry integration. For a list of the available integrations please -check :doc:`here <../../index>`. - - -opentelemetry-bootstrap ------------------------- - -:: - - opentelemetry-bootstrap --action=install|requirements - -This commands inspects the active Python site-packages and figures out which -instrumentation packages the user might want to install. By default it prints out -a list of the suggested instrumentation packages which can be added to a requirements.txt -file. It also supports installing the suggested packages when run with :code:`--action=install` -flag. -""" From 1300812d7ada9204cb69e4f8beece08d8c38e15c Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Tue, 9 Jun 2020 13:51:58 -0700 Subject: [PATCH 05/20] tox: rename instrumentation to instrumentation-base tox does exact match on fields delimited by a dash. Thus, any instrumentation that includes "instrumentation" in the name would collide with testing of the "opentelemetry-instrumentation" package. Renaming the tox environment resolves the issue. --- tox.ini | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tox.ini b/tox.ini index 8cf5767efc5..f23a734e049 100644 --- a/tox.ini +++ b/tox.ini @@ -13,8 +13,8 @@ envlist = pypy3-test-sdk ; opentelemetry-instrumentation - py3{5,6,7,8}-test-instrumentation - pypy3-test-instrumentation + py3{5,6,7,8}-test-instrumentation-base + pypy3-test-instrumentation-base ; opentelemetry-example-app py3{4,5,6,7,8}-test-example-app @@ -150,7 +150,8 @@ setenv = changedir = test-api: opentelemetry-api/tests test-sdk: opentelemetry-sdk/tests - test-instrumentation: opentelemetry-instrumentation/tests + instrumentation-base: opentelemetry-instrumentation/tests + starlette: ext/opentelemetry-instrumentation-starlette/tests test-ext-grpc: ext/opentelemetry-ext-grpc/tests test-ext-aiohttp-client: ext/opentelemetry-ext-aiohttp-client/tests test-ext-requests: ext/opentelemetry-ext-requests/tests @@ -178,7 +179,6 @@ changedir = test-ext-sqlalchemy: ext/opentelemetry-ext-sqlalchemy/tests test-ext-redis: ext/opentelemetry-ext-redis/tests test-ext-system-metrics: ext/opentelemetry-ext-system-metrics/tests - test-instrumentation-starlette: ext/opentelemetry-instrumentation-starlette/tests commands_pre = ; Install without -e to test the actual installation @@ -237,7 +237,7 @@ commands_pre = opentracing-shim: pip install {toxinidir}/ext/opentelemetry-ext-opentracing-shim datadog: pip install {toxinidir}/opentelemetry-sdk {toxinidir}/ext/opentelemetry-ext-datadog - + zipkin: pip install {toxinidir}/ext/opentelemetry-ext-zipkin sqlalchemy: pip install {toxinidir}/ext/opentelemetry-ext-sqlalchemy @@ -344,7 +344,7 @@ commands_pre = -e {toxinidir}/ext/opentelemetry-ext-sqlalchemy \ -e {toxinidir}/ext/opentelemetry-ext-redis \ -e {toxinidir}/ext/opentelemetry-ext-system-metrics \ - -e {toxinidir}/ext/opentelemetry-ext-opencensusexporter + -e {toxinidir}/ext/opentelemetry-ext-opencensusexporter \ docker-compose up -d python check_availability.py commands = From 16c99219c73fe1e646c8a042ae5265966d6906e6 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Tue, 9 Jun 2020 15:27:18 -0700 Subject: [PATCH 06/20] fixing lint --- dev-requirements.txt | 3 ++- .../src/opentelemetry/instrumentation/starlette/__init__.py | 2 +- .../tests/test_starlette_instrumentation.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 253c0895072..93d8466be17 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -10,4 +10,5 @@ pytest!=5.2.3 pytest-cov>=2.8 readme-renderer~=24.0 httpretty~=1.0 -opentracing~=2.2.0 \ No newline at end of file +opentracing~=2.2.0 +starlette \ No newline at end of file diff --git a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index 05f1db3c319..e0981ac3160 100644 --- a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -47,7 +47,7 @@ def _instrument(self, **kwargs): applications.Starlette = _InstrumentedStarlette def _uninstrument(self, **kwargs): - application.Starlette = self._original_starlette + applications.Starlette = self._original_starlette class _InstrumentedStarlette(applications.Starlette): diff --git a/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py b/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py index 1631205ca3c..441242b86b6 100644 --- a/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py +++ b/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py @@ -30,7 +30,7 @@ def setUp(self): self._client = TestClient(self._app) def test_basic_starlette_call(self): - response = self._client.get("/foobar") + self._client.get("/foobar") spans = self.memory_exporter.get_finished_spans() self.assertEquals(len(spans), 3) for span in spans: From 342d3b681a64fb5d3827983591de62e4fb503672 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Tue, 9 Jun 2020 20:36:09 -0700 Subject: [PATCH 07/20] Adding entry point for starlette instrumentation --- ext/opentelemetry-instrumentation-starlette/setup.cfg | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ext/opentelemetry-instrumentation-starlette/setup.cfg b/ext/opentelemetry-instrumentation-starlette/setup.cfg index 6d7210a84fe..59d63c6f165 100644 --- a/ext/opentelemetry-instrumentation-starlette/setup.cfg +++ b/ext/opentelemetry-instrumentation-starlette/setup.cfg @@ -42,6 +42,10 @@ install_requires = opentelemetry-api == 0.9.dev0 opentelemetry-ext-asgi == 0.9.dev0 +[options.entry_points] +opentelemetry_instrumentor = + starlette = opentelemetry.instrumentation.starlette:StarletteInstrumentor + [options.extras_require] test = opentelemetry-test == 0.9.dev0 From 6f7e875120cfd55754a3af054d5505b145cdb2b9 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Tue, 9 Jun 2020 21:05:28 -0700 Subject: [PATCH 08/20] removing unused middleware --- .../src/opentelemetry/instrumentation/starlette/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index e0981ac3160..f198233fca4 100644 --- a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -22,7 +22,6 @@ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.starlette.version import __version__ # noqa from starlette import applications -from starlette.middleware import Middleware from starlette.routing import Match From 1e6e5d84c2e246f7c64aaf9ffa6061edd5c02d0b Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 9 Jun 2020 15:53:52 -0700 Subject: [PATCH 09/20] Add SumObserver and UpDownSumObserver instruments (#789) --- docs/examples/basic_meter/observer.py | 1 + opentelemetry-api/CHANGELOG.md | 2 + .../src/opentelemetry/metrics/__init__.py | 24 ++++ .../tests/metrics/test_metrics.py | 14 +- opentelemetry-sdk/CHANGELOG.md | 2 + .../src/opentelemetry/sdk/metrics/__init__.py | 74 ++++++++-- .../sdk/metrics/export/aggregate.py | 28 ++++ .../sdk/metrics/export/batcher.py | 5 + .../tests/metrics/test_metrics.py | 132 ++++++++++++++++++ 9 files changed, 267 insertions(+), 15 deletions(-) diff --git a/docs/examples/basic_meter/observer.py b/docs/examples/basic_meter/observer.py index b61b9e4db80..d3aa02168d4 100644 --- a/docs/examples/basic_meter/observer.py +++ b/docs/examples/basic_meter/observer.py @@ -60,6 +60,7 @@ def get_ram_usage_callback(observer): description="RAM memory usage", unit="1", value_type=float, + observer_type=ValueObserver, label_keys=(), ) diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index b1610ab2c3d..670e3d7a7f5 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -10,6 +10,8 @@ ([#552](https://github.com/open-telemetry/opentelemetry-python/pull/552)) - Rename Observer to ValueObserver ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) +- Add SumObserver and UpDownSumObserver in metrics + ([#789](https://github.com/open-telemetry/opentelemetry-python/pull/789)) ## 0.8b0 diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index da47356e058..569930d6f3b 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -189,6 +189,30 @@ def observe(self, value: ValueT, labels: Dict[str, str]) -> None: """ +class SumObserver(Observer): + """No-op implementation of ``SumObserver``.""" + + def observe(self, value: ValueT, labels: Dict[str, str]) -> None: + """Captures ``value`` to the sumobserver. + + Args: + value: The value to capture to this sumobserver metric. + labels: Labels associated to ``value``. + """ + + +class UpDownSumObserver(Observer): + """No-op implementation of ``UpDownSumObserver``.""" + + def observe(self, value: ValueT, labels: Dict[str, str]) -> None: + """Captures ``value`` to the updownsumobserver. + + Args: + value: The value to capture to this updownsumobserver metric. + labels: Labels associated to ``value``. + """ + + class ValueObserver(Observer): """No-op implementation of ``ValueObserver``.""" diff --git a/opentelemetry-api/tests/metrics/test_metrics.py b/opentelemetry-api/tests/metrics/test_metrics.py index 897c7492e42..b3cbdefd15a 100644 --- a/opentelemetry-api/tests/metrics/test_metrics.py +++ b/opentelemetry-api/tests/metrics/test_metrics.py @@ -56,6 +56,18 @@ def test_bound_valuerecorder(self): bound_valuerecorder = metrics.BoundValueRecorder() bound_valuerecorder.record(1) - def test_observer(self): + def test_default_observer(self): observer = metrics.DefaultObserver() observer.observe(1, {}) + + def test_sum_observer(self): + observer = metrics.SumObserver() + observer.observe(1, {}) + + def test_updown_sum_observer(self): + observer = metrics.UpDownSumObserver() + observer.observe(1, {}) + + def test_value_observer(self): + observer = metrics.ValueObserver() + observer.observe(1, {}) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 713580dbd6b..23007dde5a8 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -12,6 +12,8 @@ ([#775](https://github.com/open-telemetry/opentelemetry-python/pull/775)) - Rename Observer to ValueObserver ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) +- Add SumObserver, UpDownSumObserver and LastValueAggregator in metrics + ([#789](https://github.com/open-telemetry/opentelemetry-python/pull/789)) ## 0.8b0 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 507e00d8ead..7156f68c165 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -105,12 +105,16 @@ def record(self, value: metrics_api.ValueT) -> None: class Metric(metrics_api.Metric): - """Base class for all metric types. + """Base class for all synchronous metric types. - Also known as metric instrument. This is the class that is used to - represent a metric that is to be continuously recorded and tracked. Each - metric has a set of bound metrics that are created from the metric. See - `BaseBoundInstrument` for information on bound metric instruments. + This is the class that is used to represent a metric that is to be + synchronously recorded and tracked. Synchronous instruments are called + inside a request, meaning they have an associated distributed context + (i.e. Span context, correlation context). Multiple metric events may occur + for a synchronous instrument within a give collection interval. + + Each metric has a set of bound metrics that are created from the metric. + See `BaseBoundInstrument` for information on bound metric instruments. """ BOUND_INSTR_TYPE = BaseBoundInstrument @@ -190,8 +194,14 @@ def record( UPDATE_FUNCTION = record -class ValueObserver(metrics_api.ValueObserver): - """See `opentelemetry.metrics.ValueObserver`.""" +class Observer(metrics_api.Observer): + """Base class for all asynchronous metric types. + + Also known as Observers, observer metric instruments are asynchronous in + that they are reported by a callback, once per collection interval, and + lack context. They are permitted to report only one value per distinct + label set per period. + """ def __init__( self, @@ -218,15 +228,10 @@ def __init__( def observe( self, value: metrics_api.ValueT, labels: Dict[str, str] ) -> None: - if not self.enabled: - return - if not isinstance(value, self.value_type): - logger.warning( - "Invalid value passed for %s.", self.value_type.__name__ - ) + key = get_labels_as_key(labels) + if not self._validate_observe(value, key): return - key = get_labels_as_key(labels) if key not in self.aggregators: # TODO: how to cleanup aggregators? self.aggregators[key] = self.meter.batcher.aggregator_for( @@ -235,6 +240,20 @@ def observe( aggregator = self.aggregators[key] aggregator.update(value) + # pylint: disable=W0613 + def _validate_observe( + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], + ) -> bool: + if not self.enabled: + return False + if not isinstance(value, self.value_type): + logger.warning( + "Invalid value passed for %s.", self.value_type.__name__ + ) + return False + + return True + def run(self) -> bool: try: self.callback(self) @@ -252,6 +271,33 @@ def __repr__(self): ) +class SumObserver(Observer, metrics_api.SumObserver): + """See `opentelemetry.metrics.SumObserver`.""" + + def _validate_observe( + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], + ) -> bool: + if not super()._validate_observe(value, key): + return False + # Must be non-decreasing because monotonic + if ( + key in self.aggregators + and self.aggregators[key].current is not None + ): + if value < self.aggregators[key].current: + logger.warning("Value passed must be non-decreasing.") + return False + return True + + +class UpDownSumObserver(Observer, metrics_api.UpDownSumObserver): + """See `opentelemetry.metrics.UpDownSumObserver`.""" + + +class ValueObserver(Observer, metrics_api.ValueObserver): + """See `opentelemetry.metrics.ValueObserver`.""" + + class Record: """Container class used for processing in the `Batcher`""" diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py index 1745d854e9d..ad728d8c502 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py @@ -125,6 +125,34 @@ def merge(self, other): ) +class LastValueAggregator(Aggregator): + """Aggregator that stores last value results.""" + + def __init__(self): + super().__init__() + self._lock = threading.Lock() + self.last_update_timestamp = None + + def update(self, value): + with self._lock: + self.current = value + self.last_update_timestamp = time_ns() + + def take_checkpoint(self): + with self._lock: + self.checkpoint = self.current + self.current = None + + def merge(self, other): + last = self.checkpoint.last + self.last_update_timestamp = get_latest_timestamp( + self.last_update_timestamp, other.last_update_timestamp + ) + if self.last_update_timestamp == other.last_update_timestamp: + last = other.checkpoint.last + self.checkpoint = last + + class ValueObserverAggregator(Aggregator): """Same as MinMaxSumCount but also with last value.""" diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index db3675ecd61..c0405d1ffb8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -18,6 +18,8 @@ from opentelemetry.metrics import ( Counter, InstrumentT, + SumObserver, + UpDownSumObserver, ValueObserver, ValueRecorder, ) @@ -25,6 +27,7 @@ from opentelemetry.sdk.metrics.export.aggregate import ( Aggregator, CounterAggregator, + LastValueAggregator, MinMaxSumCountAggregator, ValueObserverAggregator, ) @@ -54,6 +57,8 @@ def aggregator_for(self, instrument_type: Type[InstrumentT]) -> Aggregator: # pylint:disable=R0201 if issubclass(instrument_type, Counter): return CounterAggregator() + if issubclass(instrument_type, (SumObserver, UpDownSumObserver)): + return LastValueAggregator() if issubclass(instrument_type, ValueRecorder): return MinMaxSumCountAggregator() if issubclass(instrument_type, ValueObserver): diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 4c2d691549d..02cf2c93548 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -290,6 +290,138 @@ def test_record(self): ) +class TestSumObserver(unittest.TestCase): + def test_observe(self): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.SumObserver( + None, "name", "desc", "unit", int, meter, ("key",), True + ) + labels = {"key": "value"} + key_labels = tuple(sorted(labels.items())) + values = (37, 42, 60, 100) + for val in values: + observer.observe(val, labels) + + self.assertEqual(observer.aggregators[key_labels].current, values[-1]) + + def test_observe_disabled(self): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.SumObserver( + None, "name", "desc", "unit", int, meter, ("key",), False + ) + labels = {"key": "value"} + observer.observe(37, labels) + self.assertEqual(len(observer.aggregators), 0) + + @mock.patch("opentelemetry.sdk.metrics.logger") + def test_observe_incorrect_type(self, logger_mock): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.SumObserver( + None, "name", "desc", "unit", int, meter, ("key",), True + ) + labels = {"key": "value"} + observer.observe(37.0, labels) + self.assertEqual(len(observer.aggregators), 0) + self.assertTrue(logger_mock.warning.called) + + @mock.patch("opentelemetry.sdk.metrics.logger") + def test_observe_non_decreasing_error(self, logger_mock): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.SumObserver( + None, "name", "desc", "unit", int, meter, ("key",), True + ) + labels = {"key": "value"} + observer.observe(37, labels) + observer.observe(14, labels) + self.assertEqual(len(observer.aggregators), 1) + self.assertTrue(logger_mock.warning.called) + + def test_run(self): + meter = metrics.MeterProvider().get_meter(__name__) + + callback = mock.Mock() + observer = metrics.SumObserver( + callback, "name", "desc", "unit", int, meter, (), True + ) + + self.assertTrue(observer.run()) + callback.assert_called_once_with(observer) + + @mock.patch("opentelemetry.sdk.metrics.logger") + def test_run_exception(self, logger_mock): + meter = metrics.MeterProvider().get_meter(__name__) + + callback = mock.Mock() + callback.side_effect = Exception("We have a problem!") + + observer = metrics.SumObserver( + callback, "name", "desc", "unit", int, meter, (), True + ) + + self.assertFalse(observer.run()) + self.assertTrue(logger_mock.warning.called) + + +class TestUpDownSumObserver(unittest.TestCase): + def test_observe(self): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.UpDownSumObserver( + None, "name", "desc", "unit", int, meter, ("key",), True + ) + labels = {"key": "value"} + key_labels = tuple(sorted(labels.items())) + values = (37, 42, 14, 30) + for val in values: + observer.observe(val, labels) + + self.assertEqual(observer.aggregators[key_labels].current, values[-1]) + + def test_observe_disabled(self): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.UpDownSumObserver( + None, "name", "desc", "unit", int, meter, ("key",), False + ) + labels = {"key": "value"} + observer.observe(37, labels) + self.assertEqual(len(observer.aggregators), 0) + + @mock.patch("opentelemetry.sdk.metrics.logger") + def test_observe_incorrect_type(self, logger_mock): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.UpDownSumObserver( + None, "name", "desc", "unit", int, meter, ("key",), True + ) + labels = {"key": "value"} + observer.observe(37.0, labels) + self.assertEqual(len(observer.aggregators), 0) + self.assertTrue(logger_mock.warning.called) + + def test_run(self): + meter = metrics.MeterProvider().get_meter(__name__) + + callback = mock.Mock() + observer = metrics.UpDownSumObserver( + callback, "name", "desc", "unit", int, meter, (), True + ) + + self.assertTrue(observer.run()) + callback.assert_called_once_with(observer) + + @mock.patch("opentelemetry.sdk.metrics.logger") + def test_run_exception(self, logger_mock): + meter = metrics.MeterProvider().get_meter(__name__) + + callback = mock.Mock() + callback.side_effect = Exception("We have a problem!") + + observer = metrics.UpDownSumObserver( + callback, "name", "desc", "unit", int, meter, (), True + ) + + self.assertFalse(observer.run()) + self.assertTrue(logger_mock.warning.called) + + class TestValueObserver(unittest.TestCase): def test_observe(self): meter = metrics.MeterProvider().get_meter(__name__) From bee354a07db81309049f3010a8351006345c7186 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 9 Jun 2020 16:33:51 -0700 Subject: [PATCH 10/20] Add start_pipeline to MeterProvider in SDK, atexit moved to MeterProvider (#791) --- docs/examples/basic_meter/basic_metrics.py | 17 +++--- .../basic_meter/calling_conventions.py | 6 +-- docs/examples/basic_meter/observer.py | 5 +- .../cloud_monitoring/basic_metrics.py | 10 ++-- .../opencensus-exporter-metrics/collector.py | 3 +- .../opentelemetry/ext/prometheus/__init__.py | 8 ++- .../ext/system_metrics/__init__.py | 3 ++ opentelemetry-sdk/CHANGELOG.md | 2 + .../src/opentelemetry/sdk/metrics/__init__.py | 52 +++++++++++++++++-- .../sdk/metrics/export/controller.py | 25 ++++----- .../tests/metrics/export/test_export.py | 1 - .../tests/metrics/test_metrics.py | 24 +++++++++ 12 files changed, 111 insertions(+), 45 deletions(-) diff --git a/docs/examples/basic_meter/basic_metrics.py b/docs/examples/basic_meter/basic_metrics.py index b9ff8d87417..e65aa788b83 100644 --- a/docs/examples/basic_meter/basic_metrics.py +++ b/docs/examples/basic_meter/basic_metrics.py @@ -26,9 +26,6 @@ from opentelemetry import metrics from opentelemetry.sdk.metrics import Counter, MeterProvider, ValueRecorder from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter -from opentelemetry.sdk.metrics.export.controller import PushController - -stateful = True print( "Starting example, values will be printed to the console every 5 seconds." @@ -37,7 +34,11 @@ # Stateful determines whether how metrics are collected: if true, metrics # accumulate over the process lifetime. If false, metrics are reset at the # beginning of each collection interval. -metrics.set_meter_provider(MeterProvider(stateful)) +stateful = True + +# Sets the global MeterProvider instance +metrics.set_meter_provider(MeterProvider()) + # The Meter is responsible for creating and recording metrics. Each meter has a # unique name, which we set as the module's name here. meter = metrics.get_meter(__name__) @@ -45,9 +46,9 @@ # Exporter to export metrics to the console exporter = ConsoleMetricsExporter() -# A PushController collects metrics created from meter and exports it via the -# exporter every interval -controller = PushController(meter=meter, exporter=exporter, interval=5) +# start_pipeline will notify the MeterProvider to begin collecting/exporting +# metrics with the given meter, exporter and interval in seconds +metrics.get_meter_provider().start_pipeline(meter, exporter, 5) # Metric instruments allow to capture measurements requests_counter = meter.create_metric( @@ -77,7 +78,7 @@ # Update the metric instruments using the direct calling convention requests_counter.add(25, staging_labels) requests_size.record(100, staging_labels) -time.sleep(5) +time.sleep(10) requests_counter.add(50, staging_labels) requests_size.record(5000, staging_labels) diff --git a/docs/examples/basic_meter/calling_conventions.py b/docs/examples/basic_meter/calling_conventions.py index f8cc3dddbb1..3615f60d7d4 100644 --- a/docs/examples/basic_meter/calling_conventions.py +++ b/docs/examples/basic_meter/calling_conventions.py @@ -21,13 +21,11 @@ from opentelemetry import metrics from opentelemetry.sdk.metrics import Counter, MeterProvider, ValueRecorder from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter -from opentelemetry.sdk.metrics.export.controller import PushController # Use the meter type provided by the SDK package metrics.set_meter_provider(MeterProvider()) meter = metrics.get_meter(__name__) -exporter = ConsoleMetricsExporter() -controller = PushController(meter=meter, exporter=exporter, interval=5) +metrics.get_meter_provider().start_pipeline(meter, ConsoleMetricsExporter(), 5) requests_counter = meter.create_metric( name="requests", @@ -62,7 +60,7 @@ # You can record metrics directly using the metric instrument. You pass in # labels that you would like to record for. requests_counter.add(25, labels) -time.sleep(5) +time.sleep(10) print("Updating using a bound instrument...") # You can record metrics with bound metric instruments. Bound metric diff --git a/docs/examples/basic_meter/observer.py b/docs/examples/basic_meter/observer.py index d3aa02168d4..076c416c0ab 100644 --- a/docs/examples/basic_meter/observer.py +++ b/docs/examples/basic_meter/observer.py @@ -21,13 +21,10 @@ from opentelemetry import metrics from opentelemetry.sdk.metrics import MeterProvider, ValueObserver from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter -from opentelemetry.sdk.metrics.export.batcher import UngroupedBatcher -from opentelemetry.sdk.metrics.export.controller import PushController metrics.set_meter_provider(MeterProvider()) meter = metrics.get_meter(__name__) -exporter = ConsoleMetricsExporter() -controller = PushController(meter=meter, exporter=exporter, interval=2) +metrics.get_meter_provider().start_pipeline(meter, ConsoleMetricsExporter(), 5) # Callback to gather cpu usage diff --git a/docs/examples/cloud_monitoring/basic_metrics.py b/docs/examples/cloud_monitoring/basic_metrics.py index e0ceb420b62..fa00fc068b6 100644 --- a/docs/examples/cloud_monitoring/basic_metrics.py +++ b/docs/examples/cloud_monitoring/basic_metrics.py @@ -20,13 +20,11 @@ CloudMonitoringMetricsExporter, ) from opentelemetry.sdk.metrics import Counter, MeterProvider -from opentelemetry.sdk.metrics.export.controller import PushController -meter = metrics.get_meter(__name__, True) - -# Gather and export metrics every 5 seconds -controller = PushController( - meter=meter, exporter=CloudMonitoringMetricsExporter(), interval=5 +metrics.set_meter_provider(MeterProvider()) +meter = metrics.get_meter(__name__) +metrics.get_meter_provider().start_pipeline( + meter, CloudMonitoringMetricsExporter(), 5 ) requests_counter = meter.create_metric( diff --git a/docs/examples/opencensus-exporter-metrics/collector.py b/docs/examples/opencensus-exporter-metrics/collector.py index 89dabd12eab..725f07b77ab 100644 --- a/docs/examples/opencensus-exporter-metrics/collector.py +++ b/docs/examples/opencensus-exporter-metrics/collector.py @@ -21,7 +21,6 @@ OpenCensusMetricsExporter, ) from opentelemetry.sdk.metrics import Counter, MeterProvider -from opentelemetry.sdk.metrics.export.controller import PushController exporter = OpenCensusMetricsExporter( service_name="basic-service", endpoint="localhost:55678" @@ -29,7 +28,7 @@ metrics.set_meter_provider(MeterProvider()) meter = metrics.get_meter(__name__) -controller = PushController(meter, exporter, 5) +metrics.get_meter_provider().start_pipeline(meter, exporter, 5) requests_counter = meter.create_metric( name="requests", diff --git a/ext/opentelemetry-ext-prometheus/src/opentelemetry/ext/prometheus/__init__.py b/ext/opentelemetry-ext-prometheus/src/opentelemetry/ext/prometheus/__init__.py index 59ef3f1708a..da22042dcc5 100644 --- a/ext/opentelemetry-ext-prometheus/src/opentelemetry/ext/prometheus/__init__.py +++ b/ext/opentelemetry-ext-prometheus/src/opentelemetry/ext/prometheus/__init__.py @@ -29,7 +29,6 @@ from opentelemetry import metrics from opentelemetry.ext.prometheus import PrometheusMetricsExporter from opentelemetry.sdk.metrics import Counter, Meter - from opentelemetry.sdk.metrics.export.controller import PushController from prometheus_client import start_http_server # Start Prometheus client @@ -37,13 +36,12 @@ # Meter is responsible for creating and recording metrics metrics.set_meter_provider(MeterProvider()) - meter = metrics.meter() + meter = metrics.get_meter(__name__) # exporter to export metrics to Prometheus prefix = "MyAppPrefix" exporter = PrometheusMetricsExporter(prefix) - # controller collects metrics created from meter and exports it via the - # exporter every interval - controller = PushController(meter, exporter, 5) + # Starts the collect/export pipeline for metrics + metrics.get_meter_provider().start_pipeline(meter, exporter, 5) counter = meter.create_metric( "requests", diff --git a/ext/opentelemetry-ext-system-metrics/src/opentelemetry/ext/system_metrics/__init__.py b/ext/opentelemetry-ext-system-metrics/src/opentelemetry/ext/system_metrics/__init__.py index 09c633f14b4..10b09795571 100644 --- a/ext/opentelemetry-ext-system-metrics/src/opentelemetry/ext/system_metrics/__init__.py +++ b/ext/opentelemetry-ext-system-metrics/src/opentelemetry/ext/system_metrics/__init__.py @@ -28,9 +28,12 @@ .. code:: python + from opentelemetry import metrics from opentelemetry.ext.system_metrics import SystemMetrics + from opentelemetry.sdk.metrics import MeterProvider, from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter + metrics.set_meter_provider(MeterProvider()) exporter = ConsoleMetricsExporter() SystemMetrics(exporter) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 23007dde5a8..746f1ab4a50 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -14,6 +14,8 @@ ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) - Add SumObserver, UpDownSumObserver and LastValueAggregator in metrics ([#789](https://github.com/open-telemetry/opentelemetry-python/pull/789)) +- Add start_pipeline to MeterProvider + ([#791](https://github.com/open-telemetry/opentelemetry-python/pull/791)) ## 0.8b0 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 7156f68c165..e19d33580b5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -12,13 +12,19 @@ # See the License for the specific language governing permissions and # limitations under the License. +import atexit import logging import threading from typing import Dict, Sequence, Tuple, Type from opentelemetry import metrics as metrics_api +from opentelemetry.sdk.metrics.export import ( + ConsoleMetricsExporter, + MetricsExporter, +) from opentelemetry.sdk.metrics.export.aggregate import Aggregator from opentelemetry.sdk.metrics.export.batcher import UngroupedBatcher +from opentelemetry.sdk.metrics.export.controller import PushController from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.util.instrumentation import InstrumentationInfo @@ -449,24 +455,64 @@ class MeterProvider(metrics_api.MeterProvider): Args: stateful: Indicates whether meters created are going to be stateful resource: Resource for this MeterProvider + shutdown_on_exit: Register an atexit hook to shut down when the + application exists """ def __init__( - self, stateful=True, resource: Resource = Resource.create_empty(), + self, + stateful=True, + resource: Resource = Resource.create_empty(), + shutdown_on_exit: bool = True, ): self.stateful = stateful self.resource = resource + self._controllers = [] + self._exporters = set() + self._atexit_handler = None + if shutdown_on_exit: + self._atexit_handler = atexit.register(self.shutdown) def get_meter( self, instrumenting_module_name: str, instrumenting_library_version: str = "", ) -> "metrics_api.Meter": + """See `opentelemetry.metrics.MeterProvider`.get_meter.""" if not instrumenting_module_name: # Reject empty strings too. - raise ValueError("get_meter called with missing module name.") + instrumenting_module_name = "ERROR:MISSING MODULE NAME" + logger.error("get_meter called with missing module name.") return Meter( self, InstrumentationInfo( - instrumenting_module_name, instrumenting_library_version + instrumenting_module_name, instrumenting_library_version, ), ) + + def start_pipeline( + self, + meter: metrics_api.Meter, + exporter: MetricsExporter = None, + interval: float = 15.0, + ) -> None: + """Method to begin the collect/export pipeline. + + Args: + meter: The meter to collect metrics from. + exporter: The exporter to export metrics to. + interval: The collect/export interval in seconds. + """ + if not exporter: + exporter = ConsoleMetricsExporter() + self._exporters.add(exporter) + # TODO: Controller type configurable? + self._controllers.append(PushController(meter, exporter, interval)) + + def shutdown(self) -> None: + for controller in self._controllers: + controller.shutdown() + for exporter in self._exporters: + exporter.shutdown() + if self._atexit_handler is not None: + atexit.unregister(self._atexit_handler) + self._atexit_handler = None diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py index 88abed410a1..7448f353c45 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py @@ -12,30 +12,35 @@ # See the License for the specific language governing permissions and # limitations under the License. -import atexit import threading from opentelemetry.context import attach, detach, set_value +from opentelemetry.metrics import Meter +from opentelemetry.sdk.metrics.export import MetricsExporter class PushController(threading.Thread): - """A push based controller, used for exporting. + """A push based controller, used for collecting and exporting. Uses a worker thread that periodically collects metrics for exporting, exports them and performs some post-processing. + + Args: + meter: The meter used to collect metrics. + exporter: The exporter used to export metrics. + interval: The collect/export interval in seconds. """ daemon = True - def __init__(self, meter, exporter, interval, shutdown_on_exit=True): + def __init__( + self, meter: Meter, exporter: MetricsExporter, interval: float + ): super().__init__() self.meter = meter self.exporter = exporter self.interval = interval self.finished = threading.Event() - self._atexit_handler = None - if shutdown_on_exit: - self._atexit_handler = atexit.register(self.shutdown) self.start() def run(self): @@ -46,17 +51,13 @@ def shutdown(self): self.finished.set() # Run one more collection pass to flush metrics batched in the meter self.tick() - self.exporter.shutdown() - if self._atexit_handler is not None: - atexit.unregister(self._atexit_handler) - self._atexit_handler = None def tick(self): # Collect all of the meter's metrics to be exported self.meter.collect() + # Export the collected metrics token = attach(set_value("suppress_instrumentation", True)) - # Export the given metrics in the batcher self.exporter.export(self.meter.batcher.checkpoint_set()) detach(token) - # Perform post-exporting logic based on batcher configuration + # Perform post-exporting logic self.meter.batcher.finished_collection() diff --git a/opentelemetry-sdk/tests/metrics/export/test_export.py b/opentelemetry-sdk/tests/metrics/export/test_export.py index 178e41b2134..901d5a94046 100644 --- a/opentelemetry-sdk/tests/metrics/export/test_export.py +++ b/opentelemetry-sdk/tests/metrics/export/test_export.py @@ -614,7 +614,6 @@ def test_push_controller(self): controller.shutdown() self.assertTrue(controller.finished.isSet()) - exporter.shutdown.assert_any_call() # shutdown should flush the meter self.assertEqual(meter.collect.call_count, 1) diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 02cf2c93548..a92af748ac6 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -38,6 +38,30 @@ def test_resource_empty(self): # pylint: disable=protected-access self.assertIs(meter.resource, resources._EMPTY_RESOURCE) + def test_start_pipeline(self): + exporter = mock.Mock() + meter_provider = metrics.MeterProvider() + meter = meter_provider.get_meter(__name__) + # pylint: disable=protected-access + meter_provider.start_pipeline(meter, exporter, 6) + try: + self.assertEqual(len(meter_provider._exporters), 1) + self.assertEqual(len(meter_provider._controllers), 1) + finally: + meter_provider.shutdown() + + def test_shutdown(self): + controller = mock.Mock() + exporter = mock.Mock() + meter_provider = metrics.MeterProvider() + # pylint: disable=protected-access + meter_provider._controllers = [controller] + meter_provider._exporters = [exporter] + meter_provider.shutdown() + self.assertEqual(controller.shutdown.call_count, 1) + self.assertEqual(exporter.shutdown.call_count, 1) + self.assertIsNone(meter_provider._atexit_handler) + class TestMeter(unittest.TestCase): def test_extends_api(self): From 37af971fa620b23edaa2dc64b6f05976daf8dbb6 Mon Sep 17 00:00:00 2001 From: alrex Date: Wed, 10 Jun 2020 10:12:41 -0700 Subject: [PATCH 11/20] chore: Add majorgreys to approvers (#802) --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 63cba3eb3b9..6f1be8fbce3 100644 --- a/README.md +++ b/README.md @@ -103,7 +103,8 @@ Meeting notes are available as a public [Google doc](https://docs.google.com/doc Approvers ([@open-telemetry/python-approvers](https://github.com/orgs/open-telemetry/teams/python-approvers)): -- [Carlos Alberto Cortez](https://github.com/carlosalberto), LightStep +- [Carlos Alberto Cortez](https://github.com/carlosalberto), Lightstep +- [Tahir H. Butt](https://github.com/majorgreys) DataDog - [Chris Kleinknecht](https://github.com/c24t), Google - [Diego Hurtado](https://github.com/ocelotl) - [Hector Hernandez](https://github.com/hectorhdzg), Microsoft @@ -114,7 +115,7 @@ Approvers ([@open-telemetry/python-approvers](https://github.com/orgs/open-telem Maintainers ([@open-telemetry/python-maintainers](https://github.com/orgs/open-telemetry/teams/python-maintainers)): -- [Alex Boten](https://github.com/codeboten), LightStep +- [Alex Boten](https://github.com/codeboten), Lightstep - [Leighton Chen](https://github.com/lzchen), Microsoft - [Yusuke Tsutsumi](https://github.com/toumorokoshi), Zillow Group From fea75c7e68d1d1b1eb9b844757f71c289beebbe1 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Thu, 11 Jun 2020 10:16:16 -0700 Subject: [PATCH 12/20] some refactoring to use opentelemetry-instrumentation --- ext/opentelemetry-ext-asgi/setup.cfg | 1 + .../src/opentelemetry/ext/asgi/__init__.py | 65 +++++++------------ .../instrumentation/starlette/__init__.py | 11 ++-- 3 files changed, 32 insertions(+), 45 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index 4ce67e02d7a..7fb93455613 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -39,6 +39,7 @@ package_dir= packages=find_namespace: install_requires = opentelemetry-api == 0.9.dev0 + opentelemetry-instrumentation ==0.9.dev0 asgiref ~= 3.0 [options.extras_require] 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 69c30848da3..b1235d46de8 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -22,11 +22,13 @@ import typing import urllib from functools import wraps +from typing import Tuple from asgiref.compatibility import guarantee_single_callable from opentelemetry import context, propagators, trace from opentelemetry.ext.asgi.version import __version__ # noqa +from opentelemetry.instrumentation.utils import http_status_to_canonical_code from opentelemetry.trace.status import Status, StatusCanonicalCode @@ -44,37 +46,6 @@ def get_header_from_scope(scope: dict, header_name: str) -> typing.List[str]: ] -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(scope): """Collects HTTP request attributes from the ASGI scope and returns a dictionary to be used as span creation attributes.""" @@ -134,11 +105,19 @@ def set_status_code(span, status_code): span.set_status(Status(http_status_to_canonical_code(status_code))) -def get_default_span_name(scope): - """Default implementation for name_callback""" +def get_default_span_details(scope: dict) -> Tuple[str, dict]: + """Default implementation for span_details_callback + + Args: + scope: the asgi scope dictionary + + Returns: + a tuple of the span, and any attributes to attach to the + span. + """ method_or_path = scope.get("method") or scope.get("path") - return method_or_path + return method_or_path, {} class OpenTelemetryMiddleware: @@ -149,15 +128,17 @@ 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. + span_details_callbackj: 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, name_callback=None): + def __init__(self, app, span_details_callback=None): self.app = guarantee_single_callable(app) self.tracer = trace.get_tracer(__name__, __version__) - self.name_callback = name_callback or get_default_span_name + self.span_details_callback = ( + span_details_callback or get_default_span_details + ) async def __call__(self, scope, receive, send): """The ASGI application @@ -173,13 +154,15 @@ async def __call__(self, scope, receive, send): token = context.attach( propagators.extract(get_header_from_scope, scope) ) - span_name = self.name_callback(scope) + span_name, additional_attributes = self.span_details_callback(scope) + attributes = collect_request_attributes(scope) + attributes.update(additional_attributes) try: with self.tracer.start_as_current_span( span_name + " asgi", kind=trace.SpanKind.SERVER, - attributes=collect_request_attributes(scope), + attributes=attributes, ): @wraps(receive) diff --git a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index f198233fca4..21179aa13d8 100644 --- a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -37,7 +37,7 @@ def instrument_app(app: applications.Starlette): """ if not getattr(app, "_is_instrumented_by_opentelemetry", False): app.add_middleware( - OpenTelemetryMiddleware, name_callback=_get_route_name + OpenTelemetryMiddleware, span_details_callback=_get_route_name ) app._is_instrumented_by_opentelemetry = True @@ -53,18 +53,21 @@ class _InstrumentedStarlette(applications.Starlette): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.add_middleware( - OpenTelemetryMiddleware, name_callback=_get_route_name + OpenTelemetryMiddleware, span_details_callback=_get_route_name ) def _get_route_name(scope): """Callback to retrieve the starlette route being served. """ + import pdb + + pdb.set_trace() app = scope["app"] for route in app.routes: match, _ = route.matches(scope) if match == Match.FULL: - return route.path + return route.path, {} # method only exists for http, if websocket # leave it blank. - return scope.get("method", "") + return scope.get("method", ""), {} From 8c129a391b72362e028244e725fa7fdca5904c15 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Thu, 11 Jun 2020 12:04:20 -0700 Subject: [PATCH 13/20] Adding documentation, more unit tests --- docs-requirements.txt | 1 + docs/ext/starlette/starlette.rst | 9 +++ .../README.rst | 51 +++++--------- .../instrumentation/starlette/__init__.py | 38 +++++----- .../tests/test_starlette_instrumentation.py | 70 ++++++++++++++++--- 5 files changed, 110 insertions(+), 59 deletions(-) create mode 100644 docs/ext/starlette/starlette.rst diff --git a/docs-requirements.txt b/docs-requirements.txt index 60f8cdc2cf1..a5b3fe17ae1 100644 --- a/docs-requirements.txt +++ b/docs-requirements.txt @@ -24,3 +24,4 @@ boto~=2.0 google-cloud-trace >=0.23.0 google-cloud-monitoring>=0.36.0 botocore~=1.0 +starlette~=0.13 diff --git a/docs/ext/starlette/starlette.rst b/docs/ext/starlette/starlette.rst new file mode 100644 index 00000000000..8e2d1d7bc83 --- /dev/null +++ b/docs/ext/starlette/starlette.rst @@ -0,0 +1,9 @@ +.. include:: ../../../ext/opentelemetry-instrumentation-starlette/README.rst + +API +--- + +.. automodule:: opentelemetry.instrumentation.starlette + :members: + :undoc-members: + :show-inheritance: \ No newline at end of file diff --git a/ext/opentelemetry-instrumentation-starlette/README.rst b/ext/opentelemetry-instrumentation-starlette/README.rst index c5da75441cf..1d05c0b717f 100644 --- a/ext/opentelemetry-instrumentation-starlette/README.rst +++ b/ext/opentelemetry-instrumentation-starlette/README.rst @@ -1,5 +1,5 @@ -OpenTelemetry ASGI Middleware -============================= +OpenTelemetry Starlette Instrumentation +======================================= |pypi| @@ -7,8 +7,10 @@ OpenTelemetry ASGI Middleware :target: https://pypi.org/project/opentelemetry-instrumentation-starlette/ -This library provides a ASGI middleware that can be used on any ASGI framework -(such as Django, Starlette, FastAPI or Quart) to track requests timing through OpenTelemetry. +This library provides automatic and manual instrumentation of Starlette web frameworks, +instrumenting http requests served by applications utilizing the framework. + +auto-instrumentation using the opentelemetry-instrumentation package is also supported. Installation ------------ @@ -18,40 +20,23 @@ Installation pip install opentelemetry-instrumentation-starlette -Usage (Quart) -------------- - -.. code-block:: python - - from quart import Quart - from opentelemetry.instrumentation.starlette import OpenTelemetryMiddleware - - app = Quart(__name__) - app.starlette_app = OpenTelemetryMiddleware(app.starlette_app) - - @app.route("/") - async def hello(): - return "Hello!" - - if __name__ == "__main__": - app.run(debug=True) - - -Usage (Django 3.0) ------------------- - -Modify the application's ``starlette.py`` file as shown below. +Usage +----- .. code-block:: python - import os - from django.core.starlette import get_starlette_application - from opentelemetry.instrumentation.starlette import OpenTelemetryMiddleware + from opentelemetry.instrumentation.starlette import StarletteInstrumentor + from starlette import applications + from starlette.responses import PlainTextResponse + from starlette.routing import Route - os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'starlette_example.settings') + def home(request): + return PlainTextResponse("hi") - application = get_starlette_application() - application = OpenTelemetryMiddleware(application) + app = applications.Starlette( + routes=[Route("/foobar", home)] + ) + StarletteInstrumentor.instrument_app(app) References diff --git a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index 21179aa13d8..e8e63169942 100644 --- a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -11,13 +11,6 @@ # 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-instrumentation-starlette package provides an ASGI middleware that can be used -on any ASGI framework (such as Django-channels / Quart) to track requests -timing through OpenTelemetry. -""" - from opentelemetry.ext.asgi import OpenTelemetryMiddleware from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.starlette.version import __version__ # noqa @@ -37,7 +30,8 @@ def instrument_app(app: applications.Starlette): """ if not getattr(app, "_is_instrumented_by_opentelemetry", False): app.add_middleware( - OpenTelemetryMiddleware, span_details_callback=_get_route_name + OpenTelemetryMiddleware, + span_details_callback=_get_route_details, ) app._is_instrumented_by_opentelemetry = True @@ -53,21 +47,31 @@ class _InstrumentedStarlette(applications.Starlette): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.add_middleware( - OpenTelemetryMiddleware, span_details_callback=_get_route_name + OpenTelemetryMiddleware, span_details_callback=_get_route_details ) -def _get_route_name(scope): +def _get_route_details(scope): """Callback to retrieve the starlette route being served. - """ - import pdb - pdb.set_trace() + TODO: there is currently no way to retrieve the path from + a starlette application from scope. + + See: https://github.com/encode/starlette/pull/804 + """ app = scope["app"] - for route in app.routes: - match, _ = route.matches(scope) + route = None + for starlette_route in app.routes: + match, _ = starlette_route.matches(scope) if match == Match.FULL: - return route.path, {} + route = starlette_route.path + break + elif match == Match.PARTIAL: + route = starlette_route.path # method only exists for http, if websocket # leave it blank. - return scope.get("method", ""), {} + span_name = route or scope.get("method", "") + attributes = {} + if route: + attributes["http.route"] = route + return span_name, attributes diff --git a/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py b/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py index 441242b86b6..d0bdf5a4c59 100644 --- a/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py +++ b/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py @@ -18,31 +18,83 @@ import opentelemetry.instrumentation.starlette as otel_starlette from opentelemetry.test.test_base import TestBase +from starlette import applications from starlette.responses import PlainTextResponse from starlette.routing import Route from starlette.testclient import TestClient -class TestStarletteApplication(TestBase): +class TestStarletteManualInstrumentation(TestBase): + def _create_app(self): + app = self._create_starlette_app() + self._instrumentor.instrument_app(app) + return app + def setUp(self): super().setUp() - self._app = self._create_instrumented_app() + self._instrumentor = otel_starlette.StarletteInstrumentor() + self._app = self._create_app() self._client = TestClient(self._app) def test_basic_starlette_call(self): self._client.get("/foobar") spans = self.memory_exporter.get_finished_spans() - self.assertEquals(len(spans), 3) + self.assertEqual(len(spans), 3) for span in spans: - self.assertIn("foobar", span.name) + self.assertIn("/foobar", span.name) - @staticmethod - def _create_instrumented_app(): - """Create an instrumented starlette application""" + def test_starlette_route_attribute_added(self): + """Ensure that starlette routes are used as the span name.""" + self._client.get("/user/123") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + for span in spans: + self.assertIn("/user/{username}", span.name) + self.assertEqual( + spans[-1].attributes["http.route"], "/user/{username}" + ) + @staticmethod + def _create_starlette_app(): def home(request): return PlainTextResponse("hi") - return otel_starlette._InstrumentedStarlette( - routes=[Route("/foobar", home)] + app = applications.Starlette( + routes=[Route("/foobar", home), Route("/user/{username}", home)] ) + return app + + +class TestAutoInstrumentation(TestStarletteManualInstrumentation): + """Test the auto-instrumented variant + + Extending the manual instrumentation as most test cases apply + to both. + """ + + def _create_app(self): + # instrumentation is handled by the instrument call + self._instrumentor.instrument() + return self._create_starlette_app() + + def tearDown(self): + self._instrumentor.uninstrument() + super().tearDown() + + def test_uninstrument(self): + """ verify uninstrumented un-monkeypatches.""" + + +class TestAutoInstrumentationLogic(unittest.TestCase): + def test_instrumentation(self): + instrumentor = otel_starlette.StarletteInstrumentor() + original = applications.Starlette + instrumentor.instrument() + try: + instrumented = applications.Starlette + self.assertIsNot(original, instrumented) + finally: + instrumentor.uninstrument() + + should_be_original = applications.Starlette + self.assertIs(original, should_be_original) From 0a6222ac052546fc02f56fd4d93d99d4660e9e75 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Thu, 11 Jun 2020 13:43:11 -0700 Subject: [PATCH 14/20] Fixing linting --- .../src/opentelemetry_example_app/version.py | 2 +- .../tests/test_asgi_middleware.py | 6 +++--- .../tests/test_boto_instrumentation.py | 2 +- .../tests/test_botocore_instrumentation.py | 2 +- .../setup.cfg | 6 +++--- .../instrumentation/starlette/__init__.py | 15 ++++++++++----- .../instrumentation/starlette/version.py | 2 +- .../tests/test_starlette_instrumentation.py | 9 ++++----- opentelemetry-instrumentation/README.rst | 2 +- scripts/check_for_valid_readme.py | 2 +- 10 files changed, 26 insertions(+), 22 deletions(-) diff --git a/docs/examples/opentelemetry-example-app/src/opentelemetry_example_app/version.py b/docs/examples/opentelemetry-example-app/src/opentelemetry_example_app/version.py index 603bf0b7e5f..6d4fefa599e 100644 --- a/docs/examples/opentelemetry-example-app/src/opentelemetry_example_app/version.py +++ b/docs/examples/opentelemetry-example-app/src/opentelemetry_example_app/version.py @@ -12,4 +12,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -__version__ = "0.9.dev0" +__version__ = "0.10.dev0" diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index edc90444a7b..084425fa859 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -177,8 +177,8 @@ def test_override_span_name(self): span_name = "Dymaxion" # pylint:disable=unused-argument - def get_predefined_span_name(scope): - return span_name + def get_predefined_span_details(scope): + return span_name, {} def update_expected_span_name(expected): for entry in expected: @@ -188,7 +188,7 @@ def update_expected_span_name(expected): return expected app = otel_asgi.OpenTelemetryMiddleware( - simple_asgi, name_callback=get_predefined_span_name + simple_asgi, span_details_callback=get_predefined_span_details ) self.seed_app(app) self.send_default_request() diff --git a/ext/opentelemetry-ext-boto/tests/test_boto_instrumentation.py b/ext/opentelemetry-ext-boto/tests/test_boto_instrumentation.py index 492fac5a883..a629b108705 100644 --- a/ext/opentelemetry-ext-boto/tests/test_boto_instrumentation.py +++ b/ext/opentelemetry-ext-boto/tests/test_boto_instrumentation.py @@ -19,13 +19,13 @@ import boto.elasticache import boto.s3 import boto.sts - from moto import ( # pylint: disable=import-error mock_ec2_deprecated, mock_lambda_deprecated, mock_s3_deprecated, mock_sts_deprecated, ) + from opentelemetry.ext.boto import BotoInstrumentor from opentelemetry.test.test_base import TestBase diff --git a/ext/opentelemetry-ext-botocore/tests/test_botocore_instrumentation.py b/ext/opentelemetry-ext-botocore/tests/test_botocore_instrumentation.py index 56b136ea290..64d0c3d7b7c 100644 --- a/ext/opentelemetry-ext-botocore/tests/test_botocore_instrumentation.py +++ b/ext/opentelemetry-ext-botocore/tests/test_botocore_instrumentation.py @@ -1,6 +1,5 @@ import botocore.session from botocore.exceptions import ParamValidationError - from moto import ( # pylint: disable=import-error mock_ec2, mock_kinesis, @@ -9,6 +8,7 @@ mock_s3, mock_sqs, ) + from opentelemetry.ext.botocore import BotocoreInstrumentor from opentelemetry.test.test_base import TestBase diff --git a/ext/opentelemetry-instrumentation-starlette/setup.cfg b/ext/opentelemetry-instrumentation-starlette/setup.cfg index 59d63c6f165..9f5097d2537 100644 --- a/ext/opentelemetry-instrumentation-starlette/setup.cfg +++ b/ext/opentelemetry-instrumentation-starlette/setup.cfg @@ -39,8 +39,8 @@ package_dir= =src packages=find_namespace: install_requires = - opentelemetry-api == 0.9.dev0 - opentelemetry-ext-asgi == 0.9.dev0 + opentelemetry-api == 0.10.dev0 + opentelemetry-ext-asgi == 0.10.dev0 [options.entry_points] opentelemetry_instrumentor = @@ -48,7 +48,7 @@ opentelemetry_instrumentor = [options.extras_require] test = - opentelemetry-test == 0.9.dev0 + opentelemetry-test == 0.10.dev0 starlette requests # needed for testclient diff --git a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index e8e63169942..17959d83959 100644 --- a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -11,11 +11,14 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from typing import Optional + +from starlette import applications +from starlette.routing import Match + from opentelemetry.ext.asgi import OpenTelemetryMiddleware from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.starlette.version import __version__ # noqa -from starlette import applications -from starlette.routing import Match class StarletteInstrumentor(BaseInstrumentor): @@ -24,16 +27,18 @@ class StarletteInstrumentor(BaseInstrumentor): See `BaseInstrumentor` """ + _original_starlette = None + @staticmethod def instrument_app(app: applications.Starlette): """Instrument a previously instrumented Starlette application. """ - if not getattr(app, "_is_instrumented_by_opentelemetry", False): + if not getattr(app, "is_instrumented_by_opentelemetry", False): app.add_middleware( OpenTelemetryMiddleware, span_details_callback=_get_route_details, ) - app._is_instrumented_by_opentelemetry = True + app.is_instrumented_by_opentelemetry = True def _instrument(self, **kwargs): self._original_starlette = applications.Starlette @@ -66,7 +71,7 @@ def _get_route_details(scope): if match == Match.FULL: route = starlette_route.path break - elif match == Match.PARTIAL: + if match == Match.PARTIAL: route = starlette_route.path # method only exists for http, if websocket # leave it blank. diff --git a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/version.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/version.py index 603bf0b7e5f..6d4fefa599e 100644 --- a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/version.py +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/version.py @@ -12,4 +12,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -__version__ = "0.9.dev0" +__version__ = "0.10.dev0" diff --git a/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py b/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py index d0bdf5a4c59..47b17c0b91d 100644 --- a/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py +++ b/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py @@ -12,17 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -import sys import unittest -import unittest.mock as mock -import opentelemetry.instrumentation.starlette as otel_starlette -from opentelemetry.test.test_base import TestBase from starlette import applications from starlette.responses import PlainTextResponse from starlette.routing import Route from starlette.testclient import TestClient +import opentelemetry.instrumentation.starlette as otel_starlette +from opentelemetry.test.test_base import TestBase + class TestStarletteManualInstrumentation(TestBase): def _create_app(self): @@ -56,7 +55,7 @@ def test_starlette_route_attribute_added(self): @staticmethod def _create_starlette_app(): - def home(request): + def home(_): return PlainTextResponse("hi") app = applications.Starlette( diff --git a/opentelemetry-instrumentation/README.rst b/opentelemetry-instrumentation/README.rst index f01b689b7ea..6be744251b2 100644 --- a/opentelemetry-instrumentation/README.rst +++ b/opentelemetry-instrumentation/README.rst @@ -25,7 +25,7 @@ opentelemetry-instrument The code in ``program.py`` needs to use one of the packages for which there is an OpenTelemetry integration. For a list of the available integrations please -check :doc:`here <../../index>`. +check `here `_ opentelemetry-bootstrap diff --git a/scripts/check_for_valid_readme.py b/scripts/check_for_valid_readme.py index edf94d9c3e1..d555b4fa2f9 100644 --- a/scripts/check_for_valid_readme.py +++ b/scripts/check_for_valid_readme.py @@ -10,7 +10,7 @@ def is_valid_rst(path): """Checks if RST can be rendered on PyPI.""" with open(path) as readme_file: markup = readme_file.read() - return readme_renderer.rst.render(markup) is not None + return readme_renderer.rst.render(markup, stream=sys.stderr) is not None def parse_args(): From 83aff3e5db5c3fe4e86faeda4a8519699c07f49f Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Thu, 11 Jun 2020 15:17:55 -0700 Subject: [PATCH 15/20] removing invalid python envs, fixing tox docker-tests --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 7136da44653..b565acc7e2e 100644 --- a/tox.ini +++ b/tox.ini @@ -373,7 +373,7 @@ commands_pre = -e {toxinidir}/ext/opentelemetry-ext-sqlalchemy \ -e {toxinidir}/ext/opentelemetry-ext-redis \ -e {toxinidir}/ext/opentelemetry-ext-system-metrics \ - -e {toxinidir}/ext/opentelemetry-ext-opencensusexporter \ + -e {toxinidir}/ext/opentelemetry-ext-opencensusexporter docker-compose up -d python check_availability.py commands = From d5f498d85273d692ae09c09b35a36c03d1274a14 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Thu, 11 Jun 2020 15:59:42 -0700 Subject: [PATCH 16/20] removing starlette invalid python versions. --- tox.ini | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index b565acc7e2e..02f02ecda95 100644 --- a/tox.ini +++ b/tox.ini @@ -56,8 +56,9 @@ envlist = py3{4,5,6,7,8}-test-ext-requests pypy3-test-ext-requests - ; opentelemetry-instrumentation-starlette - py3{4,5,6,7,8}-test-instrumentation-starlette + ; opentelemetry-instrumentation-starlette. + ; starlette only supports 3.6 and above. + py3{6,7,8}-test-instrumentation-starlette pypy3-test-instrumentation-starlette ; opentelemetry-ext-jinja2 From 871ee6ee51e9881f63f109d535d694c91de427c7 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Fri, 12 Jun 2020 09:07:09 -0700 Subject: [PATCH 17/20] addressing feedback removing remaining asgi references. --- .../src/opentelemetry/ext/asgi/__init__.py | 2 +- .../src/opentelemetry/ext/datadog/exporter.py | 1 + ext/opentelemetry-instrumentation-starlette/CHANGELOG.md | 6 +----- ext/opentelemetry-instrumentation-starlette/setup.cfg | 8 ++++---- .../tests/test_starlette_instrumentation.py | 9 ++++++--- 5 files changed, 13 insertions(+), 13 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 b1235d46de8..ef384111e7c 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -128,7 +128,7 @@ class OpenTelemetryMiddleware: Args: app: The ASGI application callable to forward requests to. - span_details_callbackj: Callback which calculates a generic span + span_details_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. """ diff --git a/ext/opentelemetry-ext-datadog/src/opentelemetry/ext/datadog/exporter.py b/ext/opentelemetry-ext-datadog/src/opentelemetry/ext/datadog/exporter.py index 35d0f98203c..81a31e43372 100644 --- a/ext/opentelemetry-ext-datadog/src/opentelemetry/ext/datadog/exporter.py +++ b/ext/opentelemetry-ext-datadog/src/opentelemetry/ext/datadog/exporter.py @@ -33,6 +33,7 @@ DEFAULT_AGENT_URL = "http://localhost:8126" _INSTRUMENTATION_SPAN_TYPES = { "opentelemetry.ext.aiohttp-client": DatadogSpanTypes.HTTP, + "opentelemetry.ext.asgi": DatadogSpanTypes.WEB, "opentelemetry.ext.dbapi": DatadogSpanTypes.SQL, "opentelemetry.ext.django": DatadogSpanTypes.WEB, "opentelemetry.ext.flask": DatadogSpanTypes.WEB, diff --git a/ext/opentelemetry-instrumentation-starlette/CHANGELOG.md b/ext/opentelemetry-instrumentation-starlette/CHANGELOG.md index 886cbff8f19..f7132ca8306 100644 --- a/ext/opentelemetry-instrumentation-starlette/CHANGELOG.md +++ b/ext/opentelemetry-instrumentation-starlette/CHANGELOG.md @@ -2,8 +2,4 @@ ## Unreleased -## 0.8b0 - -Released 2020-05-27 - -- Add ASGI middleware ([#716](https://github.com/open-telemetry/opentelemetry-python/pull/716)) +- Initial release ([#777](https://github.com/open-telemetry/opentelemetry-python/pull/777)) \ No newline at end of file diff --git a/ext/opentelemetry-instrumentation-starlette/setup.cfg b/ext/opentelemetry-instrumentation-starlette/setup.cfg index 9f5097d2537..efb06c10bed 100644 --- a/ext/opentelemetry-instrumentation-starlette/setup.cfg +++ b/ext/opentelemetry-instrumentation-starlette/setup.cfg @@ -14,12 +14,12 @@ # [metadata] name = opentelemetry-instrumentation-starlette -description = ASGI Middleware for OpenTelemetry +description = OpenTelemetry Starlette Instrumentation long_description = file: README.rst -long_description_content_type = tinstrumentation/x-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/instrumentation/opentelemetry-instrumentation-starlette +url = https://github.com/open-telemetry/opentelemetry-python/ext/opentelemetry-instrumentation-starlette platforms = any license = Apache-2.0 classifiers = @@ -53,4 +53,4 @@ test = requests # needed for testclient [options.packages.find] -where = src +where = src \ No newline at end of file diff --git a/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py b/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py index 47b17c0b91d..a49db07c9a7 100644 --- a/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py +++ b/ext/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py @@ -52,6 +52,9 @@ def test_starlette_route_attribute_added(self): self.assertEqual( spans[-1].attributes["http.route"], "/user/{username}" ) + # ensure that at least one attribute that is populated by + # the asgi instrumentation is successfully feeding though. + self.assertEqual(spans[-1].attributes["http.flavor"], "1.1") @staticmethod def _create_starlette_app(): @@ -80,12 +83,12 @@ def tearDown(self): self._instrumentor.uninstrument() super().tearDown() - def test_uninstrument(self): - """ verify uninstrumented un-monkeypatches.""" - class TestAutoInstrumentationLogic(unittest.TestCase): def test_instrumentation(self): + """Verify that instrumentation methods are instrumenting and + removing as expected. + """ instrumentor = otel_starlette.StarletteInstrumentor() original = applications.Starlette instrumentor.instrument() From 1b85bd4061fa9fc225048ff1fffff9b00d573d85 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Fri, 12 Jun 2020 09:39:48 -0700 Subject: [PATCH 18/20] More fixes --- ext/opentelemetry-ext-asgi/setup.cfg | 3 ++- .../src/opentelemetry/ext/asgi/__init__.py | 7 ++++--- ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py | 3 +-- ext/opentelemetry-instrumentation-starlette/setup.cfg | 9 ++++----- .../opentelemetry/instrumentation/starlette/__init__.py | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index 820c2728b08..ad0cde065f4 100644 --- a/ext/opentelemetry-ext-asgi/setup.cfg +++ b/ext/opentelemetry-ext-asgi/setup.cfg @@ -5,7 +5,8 @@ # 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 +# +# 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 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 ef384111e7c..43b8804c245 100644 --- a/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py +++ b/ext/opentelemetry-ext-asgi/src/opentelemetry/ext/asgi/__init__.py @@ -128,9 +128,10 @@ class OpenTelemetryMiddleware: Args: app: The ASGI application callable to forward requests to. - span_details_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. + span_details_callback: Callback which should return a string + and a tuple, representing the desired span name and a + dictionary with any additional span attributes to set. + Optional: Defaults to get_default_span_details. """ def __init__(self, app, span_details_callback=None): diff --git a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py index 084425fa859..05aa84b3c41 100644 --- a/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py +++ b/ext/opentelemetry-ext-asgi/tests/test_asgi_middleware.py @@ -176,8 +176,7 @@ 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_details(scope): + def get_predefined_span_details(_): return span_name, {} def update_expected_span_name(expected): diff --git a/ext/opentelemetry-instrumentation-starlette/setup.cfg b/ext/opentelemetry-instrumentation-starlette/setup.cfg index efb06c10bed..0db9b6c026f 100644 --- a/ext/opentelemetry-instrumentation-starlette/setup.cfg +++ b/ext/opentelemetry-instrumentation-starlette/setup.cfg @@ -28,13 +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 Programming Language :: Python :: 3.8 [options] -python_requires = >=3.5 +python_requires = >=3.6 package_dir= =src packages=find_namespace: @@ -49,8 +48,8 @@ opentelemetry_instrumentor = [options.extras_require] test = opentelemetry-test == 0.10.dev0 - starlette - requests # needed for testclient + starlette ~=0.13.0 + requests ~=2.23.0 # needed for testclient [options.packages.find] -where = src \ No newline at end of file +where = src diff --git a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index 17959d83959..197a38d7591 100644 --- a/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/ext/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -59,7 +59,7 @@ def __init__(self, *args, **kwargs): def _get_route_details(scope): """Callback to retrieve the starlette route being served. - TODO: there is currently no way to retrieve the path from + TODO: there is currently no way to retrieve http.route from a starlette application from scope. See: https://github.com/encode/starlette/pull/804 From a9dbb2980e5757a81fbc55e7324764e5276c89a9 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Mon, 15 Jun 2020 09:41:09 -0700 Subject: [PATCH 19/20] Apply suggestions from code review Co-authored-by: alrex Co-authored-by: Leighton Chen --- ext/opentelemetry-ext-asgi/setup.cfg | 2 +- ext/opentelemetry-instrumentation-starlette/setup.cfg | 4 ++-- tox.ini | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/opentelemetry-ext-asgi/setup.cfg b/ext/opentelemetry-ext-asgi/setup.cfg index ad0cde065f4..ab0e3e7f47c 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.10.dev0 - opentelemetry-instrumentation ==0.10.dev0 + opentelemetry-instrumentation == 0.10.dev0 asgiref ~= 3.0 [options.extras_require] diff --git a/ext/opentelemetry-instrumentation-starlette/setup.cfg b/ext/opentelemetry-instrumentation-starlette/setup.cfg index 0db9b6c026f..7659c9fadb6 100644 --- a/ext/opentelemetry-instrumentation-starlette/setup.cfg +++ b/ext/opentelemetry-instrumentation-starlette/setup.cfg @@ -48,8 +48,8 @@ opentelemetry_instrumentor = [options.extras_require] test = opentelemetry-test == 0.10.dev0 - starlette ~=0.13.0 - requests ~=2.23.0 # needed for testclient + starlette ~= 0.13.0 + requests ~= 2.23.0 # needed for testclient [options.packages.find] where = src diff --git a/tox.ini b/tox.ini index 02f02ecda95..bbc16370457 100644 --- a/tox.ini +++ b/tox.ini @@ -168,7 +168,7 @@ changedir = test-api: opentelemetry-api/tests test-sdk: opentelemetry-sdk/tests instrumentation-base: opentelemetry-instrumentation/tests - starlette: ext/opentelemetry-instrumentation-starlette/tests +test-instrumentation-starlette: ext/opentelemetry-instrumentation-starlette/tests test-proto: opentelemetry-proto/tests test-ext-grpc: ext/opentelemetry-ext-grpc/tests test-ext-aiohttp-client: ext/opentelemetry-ext-aiohttp-client/tests @@ -217,7 +217,7 @@ commands_pre = grpc: pip install {toxinidir}/ext/opentelemetry-ext-grpc[test] - wsgi,flask,django,asgi,pyrmaid,starlette: pip install {toxinidir}/tests/util + wsgi,flask,django,asgi,pyramid,starlette: pip install {toxinidir}/tests/util wsgi,flask,django,pyramid: pip install {toxinidir}/ext/opentelemetry-ext-wsgi asgi,starlette: pip install {toxinidir}/ext/opentelemetry-ext-asgi From 40b8941814fcf4c4c305e2b77f4c8a1e3f9070ee Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Mon, 15 Jun 2020 10:48:49 -0700 Subject: [PATCH 20/20] fixing tox syntax was breaking all instrumentation tests --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index bbc16370457..2be97b937db 100644 --- a/tox.ini +++ b/tox.ini @@ -168,7 +168,7 @@ changedir = test-api: opentelemetry-api/tests test-sdk: opentelemetry-sdk/tests instrumentation-base: opentelemetry-instrumentation/tests -test-instrumentation-starlette: ext/opentelemetry-instrumentation-starlette/tests + test-instrumentation-starlette: ext/opentelemetry-instrumentation-starlette/tests test-proto: opentelemetry-proto/tests test-ext-grpc: ext/opentelemetry-ext-grpc/tests test-ext-aiohttp-client: ext/opentelemetry-ext-aiohttp-client/tests