From c074853a7cf9177847017c7656a2ccce461b191a Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 5 May 2020 10:59:29 -0600 Subject: [PATCH 1/3] flask: Add Flask Instrumentation fixes (#601) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reffactoring the Flask instrumentation to better enable auto-instrumentation. Co-authored-by: Mauricio Vásquez --- .../flask_example.py | 4 +- docs/getting-started.rst | 7 +- .../src/opentelemetry/ext/flask/__init__.py | 225 ++++++++++-------- ...test_flask_integration.py => base_test.py} | 57 ++--- ext/opentelemetry-ext-flask/tests/conftest.py | 25 -- .../tests/test_automatic.py | 64 +++++ .../tests/test_programmatic.py | 56 +++++ .../tests/test_instrumentor.py | 1 + 8 files changed, 279 insertions(+), 160 deletions(-) rename ext/opentelemetry-ext-flask/tests/{test_flask_integration.py => base_test.py} (73%) delete mode 100644 ext/opentelemetry-ext-flask/tests/conftest.py create mode 100644 ext/opentelemetry-ext-flask/tests/test_automatic.py create mode 100644 ext/opentelemetry-ext-flask/tests/test_programmatic.py diff --git a/docs/examples/opentelemetry-example-app/src/opentelemetry_example_app/flask_example.py b/docs/examples/opentelemetry-example-app/src/opentelemetry_example_app/flask_example.py index 863d6f3389..8f44273b6e 100644 --- a/docs/examples/opentelemetry-example-app/src/opentelemetry_example_app/flask_example.py +++ b/docs/examples/opentelemetry-example-app/src/opentelemetry_example_app/flask_example.py @@ -34,15 +34,15 @@ trace.set_tracer_provider(TracerProvider()) opentelemetry.ext.requests.RequestsInstrumentor().instrument() -FlaskInstrumentor().instrument() trace.get_tracer_provider().add_span_processor( SimpleExportSpanProcessor(ConsoleSpanExporter()) ) - app = flask.Flask(__name__) +FlaskInstrumentor().instrument_app(app) + @app.route("/") def hello(): diff --git a/docs/getting-started.rst b/docs/getting-started.rst index f25cf79b77..5d20fbe2c0 100644 --- a/docs/getting-started.rst +++ b/docs/getting-started.rst @@ -184,9 +184,6 @@ And let's write a small Flask application that sends an HTTP request, activating .. code-block:: python # flask_example.py - from opentelemetry.ext.flask import FlaskInstrumentor - FlaskInstrumentor().instrument() # This needs to be executed before importing Flask - import flask import requests @@ -195,6 +192,7 @@ And let's write a small Flask application that sends an HTTP request, activating from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.export import ConsoleSpanExporter from opentelemetry.sdk.trace.export import SimpleExportSpanProcessor + from opentelemetry.ext.flask import FlaskInstrumentor trace.set_tracer_provider(TracerProvider()) trace.get_tracer_provider().add_span_processor( @@ -202,7 +200,8 @@ And let's write a small Flask application that sends an HTTP request, activating ) app = flask.Flask(__name__) - opentelemetry.ext.requests.RequestsInstrumentor().instrument() + FlaskInstrumentor().instrument_app(app) + opentelemetry.ext.http_requests.RequestsInstrumentor().instrument() @app.route("/") def hello(): diff --git a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py index 1e936da115..040c8770c6 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -29,12 +29,13 @@ .. code-block:: python - from opentelemetry.ext.flask import FlaskInstrumentor - FlaskInstrumentor().instrument() # This needs to be executed before importing Flask from flask import Flask + from opentelemetry.ext.flask import FlaskInstrumentor app = Flask(__name__) + FlaskInstrumentor().instrument_app(app) + @app.route("/") def hello(): return "Hello!" @@ -46,7 +47,7 @@ def hello(): --- """ -import logging +from logging import getLogger import flask @@ -60,7 +61,7 @@ def hello(): time_ns, ) -logger = logging.getLogger(__name__) +_logger = getLogger(__name__) _ENVIRON_STARTTIME_KEY = "opentelemetry-flask.starttime_key" _ENVIRON_SPAN_KEY = "opentelemetry-flask.span_key" @@ -68,102 +69,104 @@ def hello(): _ENVIRON_TOKEN = "opentelemetry-flask.token" +def _rewrapped_app(wsgi_app): + def _wrapped_app(environ, start_response): + # We want to measure the time for route matching, etc. + # In theory, we could start the span here and use + # update_name later but that API is "highly discouraged" so + # we better avoid it. + environ[_ENVIRON_STARTTIME_KEY] = time_ns() + + def _start_response(status, response_headers, *args, **kwargs): + + if not _disable_trace(flask.request.url): + + span = flask.request.environ.get(_ENVIRON_SPAN_KEY) + + if span: + otel_wsgi.add_response_attributes( + span, status, response_headers + ) + else: + _logger.warning( + "Flask environ's OpenTelemetry span " + "missing at _start_response(%s)", + status, + ) + + return start_response(status, response_headers, *args, **kwargs) + + return wsgi_app(environ, _start_response) + + return _wrapped_app + + +def _before_request(): + if _disable_trace(flask.request.url): + return + + environ = flask.request.environ + span_name = flask.request.endpoint or otel_wsgi.get_default_span_name( + environ + ) + token = context.attach( + propagators.extract(otel_wsgi.get_header_from_environ, environ) + ) + + tracer = trace.get_tracer(__name__, __version__) + + attributes = otel_wsgi.collect_request_attributes(environ) + if flask.request.url_rule: + # For 404 that result from no route found, etc, we + # don't have a url_rule. + attributes["http.route"] = flask.request.url_rule.rule + span = tracer.start_span( + span_name, + kind=trace.SpanKind.SERVER, + attributes=attributes, + start_time=environ.get(_ENVIRON_STARTTIME_KEY), + ) + activation = tracer.use_span(span, end_on_exit=True) + activation.__enter__() + environ[_ENVIRON_ACTIVATION_KEY] = activation + environ[_ENVIRON_SPAN_KEY] = span + environ[_ENVIRON_TOKEN] = token + + +def _teardown_request(exc): + activation = flask.request.environ.get(_ENVIRON_ACTIVATION_KEY) + if not activation: + _logger.warning( + "Flask environ's OpenTelemetry activation missing" + "at _teardown_flask_request(%s)", + exc, + ) + return + + if exc is None: + activation.__exit__(None, None, None) + else: + activation.__exit__( + type(exc), exc, getattr(exc, "__traceback__", None) + ) + context.detach(flask.request.environ.get(_ENVIRON_TOKEN)) + + class _InstrumentedFlask(flask.Flask): def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - # Single use variable here to avoid recursion issues. - wsgi = self.wsgi_app - - def wrapped_app(environ, start_response): - # We want to measure the time for route matching, etc. - # In theory, we could start the span here and use - # update_name later but that API is "highly discouraged" so - # we better avoid it. - environ[_ENVIRON_STARTTIME_KEY] = time_ns() - - def _start_response(status, response_headers, *args, **kwargs): - if not _disable_trace(flask.request.url): - span = flask.request.environ.get(_ENVIRON_SPAN_KEY) - if span: - otel_wsgi.add_response_attributes( - span, status, response_headers - ) - else: - logger.warning( - "Flask environ's OpenTelemetry span " - "missing at _start_response(%s)", - status, - ) - - return start_response( - status, response_headers, *args, **kwargs - ) - - return wsgi(environ, _start_response) - - self.wsgi_app = wrapped_app - - @self.before_request - def _before_flask_request(): - # Do not trace if the url is excluded - if _disable_trace(flask.request.url): - return - environ = flask.request.environ - span_name = ( - flask.request.endpoint - or otel_wsgi.get_default_span_name(environ) - ) - token = context.attach( - propagators.extract(otel_wsgi.get_header_from_environ, environ) - ) + self._original_wsgi_ = self.wsgi_app + self.wsgi_app = _rewrapped_app(self.wsgi_app) - tracer = trace.get_tracer(__name__, __version__) - - attributes = otel_wsgi.collect_request_attributes(environ) - if flask.request.url_rule: - # For 404 that result from no route found, etc, we - # don't have a url_rule. - attributes["http.route"] = flask.request.url_rule.rule - span = tracer.start_span( - span_name, - kind=trace.SpanKind.SERVER, - attributes=attributes, - start_time=environ.get(_ENVIRON_STARTTIME_KEY), - ) - activation = tracer.use_span(span, end_on_exit=True) - activation.__enter__() - environ[_ENVIRON_ACTIVATION_KEY] = activation - environ[_ENVIRON_SPAN_KEY] = span - environ[_ENVIRON_TOKEN] = token - - @self.teardown_request - def _teardown_flask_request(exc): - # Not traced if the url is excluded - if _disable_trace(flask.request.url): - return - activation = flask.request.environ.get(_ENVIRON_ACTIVATION_KEY) - if not activation: - logger.warning( - "Flask environ's OpenTelemetry activation missing" - "at _teardown_flask_request(%s)", - exc, - ) - return - - if exc is None: - activation.__exit__(None, None, None) - else: - activation.__exit__( - type(exc), exc, getattr(exc, "__traceback__", None) - ) - context.detach(flask.request.environ.get(_ENVIRON_TOKEN)) + self.before_request(_before_request) + self.teardown_request(_teardown_request) def _disable_trace(url): excluded_hosts = configuration.Configuration().FLASK_EXCLUDED_HOSTS excluded_paths = configuration.Configuration().FLASK_EXCLUDED_PATHS + if excluded_hosts: excluded_hosts = str.split(excluded_hosts, ",") if disable_tracing_hostname(url, excluded_hosts): @@ -176,18 +179,50 @@ def _disable_trace(url): class FlaskInstrumentor(BaseInstrumentor): - """A instrumentor for flask.Flask + # pylint: disable=protected-access,attribute-defined-outside-init + """An instrumentor for flask.Flask See `BaseInstrumentor` """ - def __init__(self): - super().__init__() - self._original_flask = None - def _instrument(self, **kwargs): self._original_flask = flask.Flask flask.Flask = _InstrumentedFlask + def instrument_app(self, app): # pylint: disable=no-self-use + if not hasattr(app, "_is_instrumented"): + app._is_instrumented = False + + if not app._is_instrumented: + app._original_wsgi_app = app.wsgi_app + app.wsgi_app = _rewrapped_app(app.wsgi_app) + + app.before_request(_before_request) + app.teardown_request(_teardown_request) + app._is_instrumented = True + else: + _logger.warning( + "Attempting to instrument Flask app while already instrumented" + ) + def _uninstrument(self, **kwargs): flask.Flask = self._original_flask + + def uninstrument_app(self, app): # pylint: disable=no-self-use + if not hasattr(app, "_is_instrumented"): + app._is_instrumented = False + + if app._is_instrumented: + app.wsgi_app = app._original_wsgi_app + + # FIXME add support for other Flask blueprints that are not None + app.before_request_funcs[None].remove(_before_request) + app.teardown_request_funcs[None].remove(_teardown_request) + del app._original_wsgi_app + + app._is_instrumented = False + else: + _logger.warning( + "Attempting to uninstrument Flask " + "app while already uninstrumented" + ) diff --git a/ext/opentelemetry-ext-flask/tests/test_flask_integration.py b/ext/opentelemetry-ext-flask/tests/base_test.py similarity index 73% rename from ext/opentelemetry-ext-flask/tests/test_flask_integration.py rename to ext/opentelemetry-ext-flask/tests/base_test.py index 1babfff2f5..7147afd719 100644 --- a/ext/opentelemetry-ext-flask/tests/test_flask_integration.py +++ b/ext/opentelemetry-ext-flask/tests/base_test.py @@ -12,16 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -import unittest from unittest.mock import patch -from flask import Flask, request +from flask import request from werkzeug.test import Client from werkzeug.wrappers import BaseResponse -from opentelemetry import trace as trace_api -from opentelemetry.configuration import Configuration -from opentelemetry.test.wsgitestutil import WsgiTestBase +from opentelemetry import trace def expected_attributes(override_attributes): @@ -42,36 +39,30 @@ def expected_attributes(override_attributes): return default_attributes -class TestFlaskIntegration(WsgiTestBase): - def setUp(self): - # No instrumentation code is here because it is present in the - # conftest.py file next to this file. - super().setUp() - Configuration._instance = None # pylint:disable=protected-access - Configuration.__slots__ = [] - self.app = Flask(__name__) - - def hello_endpoint(helloid): - if helloid == 500: - raise ValueError(":-(") - return "Hello: " + str(helloid) +class InstrumentationTest: + @staticmethod + def _hello_endpoint(helloid): + if helloid == 500: + raise ValueError(":-(") + return "Hello: " + str(helloid) + def _common_initialization(self): def excluded_endpoint(): return "excluded" def excluded2_endpoint(): return "excluded2" - self.app.route("/hello/")(hello_endpoint) + # pylint: disable=no-member + self.app.route("/hello/")(self._hello_endpoint) + self.app.route("/excluded/")(self._hello_endpoint) self.app.route("/excluded")(excluded_endpoint) self.app.route("/excluded2")(excluded2_endpoint) + # pylint: disable=attribute-defined-outside-init self.client = Client(self.app, BaseResponse) - def tearDown(self): - Configuration._instance = None # pylint:disable=protected-access - Configuration.__slots__ = [] - + # pylint: disable=no-member def test_only_strings_in_environ(self): """ Some WSGI servers (such as Gunicorn) expect keys in the environ object @@ -99,8 +90,8 @@ def test_simple(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "hello_endpoint") - self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) + self.assertEqual(span_list[0].name, "_hello_endpoint") + self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) def test_404(self): @@ -119,7 +110,7 @@ def test_404(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) self.assertEqual(span_list[0].name, "/bye") - self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) + self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) def test_internal_error(self): @@ -136,14 +127,16 @@ def test_internal_error(self): resp.close() span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "hello_endpoint") - self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) + self.assertEqual(span_list[0].name, "_hello_endpoint") + self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) @patch.dict( "os.environ", # type: ignore { - "OPENTELEMETRY_PYTHON_FLASK_EXCLUDED_HOSTS": "http://localhost/excluded", + "OPENTELEMETRY_PYTHON_FLASK_EXCLUDED_HOSTS": ( + "http://localhost/excluded" + ), "OPENTELEMETRY_PYTHON_FLASK_EXCLUDED_PATHS": "excluded2", }, ) @@ -153,8 +146,4 @@ def test_excluded_path(self): self.client.get("/excluded2") span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "hello_endpoint") - - -if __name__ == "__main__": - unittest.main() + self.assertEqual(span_list[0].name, "_hello_endpoint") diff --git a/ext/opentelemetry-ext-flask/tests/conftest.py b/ext/opentelemetry-ext-flask/tests/conftest.py deleted file mode 100644 index 8c0754f2c6..0000000000 --- a/ext/opentelemetry-ext-flask/tests/conftest.py +++ /dev/null @@ -1,25 +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. - -from opentelemetry.ext.flask import FlaskInstrumentor - -_FLASK_INSTRUMENTOR = FlaskInstrumentor() - - -def pytest_sessionstart(session): # pylint: disable=unused-argument - _FLASK_INSTRUMENTOR.instrument() - - -def pytest_sessionfinish(session): # pylint: disable=unused-argument - _FLASK_INSTRUMENTOR.uninstrument() diff --git a/ext/opentelemetry-ext-flask/tests/test_automatic.py b/ext/opentelemetry-ext-flask/tests/test_automatic.py new file mode 100644 index 0000000000..04f43d6e64 --- /dev/null +++ b/ext/opentelemetry-ext-flask/tests/test_automatic.py @@ -0,0 +1,64 @@ +# 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 flask +from werkzeug.test import Client +from werkzeug.wrappers import BaseResponse + +from opentelemetry.configuration import Configuration +from opentelemetry.ext.flask import FlaskInstrumentor +from opentelemetry.test.test_base import TestBase +from opentelemetry.test.wsgitestutil import WsgiTestBase + +# pylint: disable=import-error +from .base_test import InstrumentationTest + + +class TestAutomatic(InstrumentationTest, TestBase, WsgiTestBase): + def setUp(self): + super().setUp() + + Configuration._instance = None # pylint: disable=protected-access + Configuration.__slots__ = [] # pylint: disable=protected-access + FlaskInstrumentor().instrument() + + self.app = flask.Flask(__name__) + + self._common_initialization() + + def tearDown(self): + super().tearDown() + with self.disable_logging(): + FlaskInstrumentor().uninstrument() + + def test_uninstrument(self): + # pylint: disable=access-member-before-definition + resp = self.client.get("/hello/123") + self.assertEqual(200, resp.status_code) + self.assertEqual([b"Hello: 123"], list(resp.response)) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + FlaskInstrumentor().uninstrument() + self.app = flask.Flask(__name__) + + self.app.route("/hello/")(self._hello_endpoint) + # pylint: disable=attribute-defined-outside-init + self.client = Client(self.app, BaseResponse) + + resp = self.client.get("/hello/123") + self.assertEqual(200, resp.status_code) + self.assertEqual([b"Hello: 123"], list(resp.response)) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py new file mode 100644 index 0000000000..1075f808cb --- /dev/null +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -0,0 +1,56 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from flask import Flask + +from opentelemetry.configuration import Configuration +from opentelemetry.ext.flask import FlaskInstrumentor +from opentelemetry.test.test_base import TestBase +from opentelemetry.test.wsgitestutil import WsgiTestBase + +# pylint: disable=import-error +from .base_test import InstrumentationTest + + +class TestProgrammatic(InstrumentationTest, TestBase, WsgiTestBase): + def setUp(self): + super().setUp() + + Configuration._instance = None # pylint: disable=protected-access + Configuration.__slots__ = [] # pylint: disable=protected-access + self.app = Flask(__name__) + + FlaskInstrumentor().instrument_app(self.app) + + self._common_initialization() + + def tearDown(self): + super().tearDown() + with self.disable_logging(): + FlaskInstrumentor().uninstrument_app(self.app) + + def test_uninstrument(self): + resp = self.client.get("/hello/123") + self.assertEqual(200, resp.status_code) + self.assertEqual([b"Hello: 123"], list(resp.response)) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + FlaskInstrumentor().uninstrument_app(self.app) + + resp = self.client.get("/hello/123") + self.assertEqual(200, resp.status_code) + self.assertEqual([b"Hello: 123"], list(resp.response)) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) diff --git a/opentelemetry-auto-instrumentation/tests/test_instrumentor.py b/opentelemetry-auto-instrumentation/tests/test_instrumentor.py index 40e762230a..a768a40eb4 100644 --- a/opentelemetry-auto-instrumentation/tests/test_instrumentor.py +++ b/opentelemetry-auto-instrumentation/tests/test_instrumentor.py @@ -34,6 +34,7 @@ def test_protect(self): self.assertIs(instrumentor.uninstrument(), None) self.assertEqual(instrumentor.instrument(), "instrumented") + with self.assertLogs(level=WARNING): self.assertIs(instrumentor.instrument(), None) From 48ad6ad9ab2c31d7acbe5eeaf3cd5c337cecb5df Mon Sep 17 00:00:00 2001 From: alrex Date: Tue, 5 May 2020 11:07:59 -0700 Subject: [PATCH 2/3] docs: updating readme (#639) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is making the readme consistent with some of the other SIG repos: - [js](https://github.com/open-telemetry/opentelemetry-js/blob/master/README.md) - [collector](https://github.com/open-telemetry/opentelemetry-collector) Co-authored-by: Mauricio Vásquez Co-authored-by: Yusuke Tsutsumi --- README.md | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0333853eec..6e47958fe1 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,42 @@ -# OpenTelemetry Python -[![Gitter chat](https://img.shields.io/gitter/room/opentelemetry/opentelemetry-python)](https://gitter.im/open-telemetry/opentelemetry-python) -[![Build status](https://travis-ci.org/open-telemetry/opentelemetry-python.svg?branch=master)](https://travis-ci.org/open-telemetry/opentelemetry-python) +--- +

+ + Getting Started +   •   + API Documentation +   •   + Getting In Touch (Gitter) + +

+ +

+ + GitHub release (latest by date including pre-releases) + + + Codecov Status + + + license + +
+ + Build Status + + Beta +

+ +

+ + Contributing +   •   + Examples + +

+ +--- + +## About this project The Python [OpenTelemetry](https://opentelemetry.io/) client. From b5b297c7606fdf851c32f413968d842aa7e23b60 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 5 May 2020 13:50:55 -0600 Subject: [PATCH 3/3] api: Add reset for configuration testing (#636) It is a common occurrence in tests that the global Configuration object needs to be "reset" between tests. This means that its attributes need to be set back to their original values. Since the Configuration object is immutable by design, some additional, non-production available mechanism is needed to perform this action. The need for this feature was mentioned in a conversation in #630. --- .../tests/base_test.py | 5 ++++ .../tests/test_automatic.py | 3 -- .../tests/test_programmatic.py | 3 -- .../opentelemetry/configuration/__init__.py | 17 +++++++++++ .../tests/configuration/test_configuration.py | 30 ++++++++++++++----- 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/ext/opentelemetry-ext-flask/tests/base_test.py b/ext/opentelemetry-ext-flask/tests/base_test.py index 7147afd719..42341826df 100644 --- a/ext/opentelemetry-ext-flask/tests/base_test.py +++ b/ext/opentelemetry-ext-flask/tests/base_test.py @@ -19,6 +19,7 @@ from werkzeug.wrappers import BaseResponse from opentelemetry import trace +from opentelemetry.configuration import Configuration def expected_attributes(override_attributes): @@ -40,6 +41,10 @@ def expected_attributes(override_attributes): class InstrumentationTest: + def setUp(self): # pylint: disable=invalid-name + super().setUp() # pylint: disable=no-member + Configuration._reset() # pylint: disable=protected-access + @staticmethod def _hello_endpoint(helloid): if helloid == 500: diff --git a/ext/opentelemetry-ext-flask/tests/test_automatic.py b/ext/opentelemetry-ext-flask/tests/test_automatic.py index 04f43d6e64..b94c7b33d6 100644 --- a/ext/opentelemetry-ext-flask/tests/test_automatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_automatic.py @@ -16,7 +16,6 @@ from werkzeug.test import Client from werkzeug.wrappers import BaseResponse -from opentelemetry.configuration import Configuration from opentelemetry.ext.flask import FlaskInstrumentor from opentelemetry.test.test_base import TestBase from opentelemetry.test.wsgitestutil import WsgiTestBase @@ -29,8 +28,6 @@ class TestAutomatic(InstrumentationTest, TestBase, WsgiTestBase): def setUp(self): super().setUp() - Configuration._instance = None # pylint: disable=protected-access - Configuration.__slots__ = [] # pylint: disable=protected-access FlaskInstrumentor().instrument() self.app = flask.Flask(__name__) diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index 1075f808cb..4e17f25fdc 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -14,7 +14,6 @@ from flask import Flask -from opentelemetry.configuration import Configuration from opentelemetry.ext.flask import FlaskInstrumentor from opentelemetry.test.test_base import TestBase from opentelemetry.test.wsgitestutil import WsgiTestBase @@ -27,8 +26,6 @@ class TestProgrammatic(InstrumentationTest, TestBase, WsgiTestBase): def setUp(self): super().setUp() - Configuration._instance = None # pylint: disable=protected-access - Configuration.__slots__ = [] # pylint: disable=protected-access self.app = Flask(__name__) FlaskInstrumentor().instrument_app(self.app) diff --git a/opentelemetry-api/src/opentelemetry/configuration/__init__.py b/opentelemetry-api/src/opentelemetry/configuration/__init__.py index 57b1c324c6..ad546b0b86 100644 --- a/opentelemetry-api/src/opentelemetry/configuration/__init__.py +++ b/opentelemetry-api/src/opentelemetry/configuration/__init__.py @@ -122,3 +122,20 @@ def __new__(cls) -> "Configuration": def __getattr__(self, name): return None + + @classmethod + def _reset(cls): + """ + This method "resets" the global configuration attributes + + It is not intended to be used by production code but by testing code + only. + """ + + for slot in cls.__slots__: + if slot in cls.__dict__.keys(): + delattr(cls, slot) + delattr(cls, "_{}".format(slot)) + + cls.__slots__ = [] + cls._instance = None diff --git a/opentelemetry-api/tests/configuration/test_configuration.py b/opentelemetry-api/tests/configuration/test_configuration.py index 9688ec28b6..c736c97262 100644 --- a/opentelemetry-api/tests/configuration/test_configuration.py +++ b/opentelemetry-api/tests/configuration/test_configuration.py @@ -20,14 +20,10 @@ class TestConfiguration(TestCase): - def setUp(self): - # This is added here to force a reload of the whole Configuration - # class, resetting its internal attributes so that each tests starts - # with a clean class. - from opentelemetry.configuration import Configuration # type: ignore - def tearDown(self): - from opentelemetry.configuration import Configuration # type: ignore + # This call resets the attributes of the Configuration class so that + # each test is executed in the same conditions. + Configuration._reset() def test_singleton(self): self.assertIsInstance(Configuration(), Configuration) @@ -72,3 +68,23 @@ def test_slots(self): def test_getattr(self): self.assertIsNone(Configuration().XYZ) + + def test_reset(self): + environ_patcher = patch.dict( + "os.environ", # type: ignore + {"OPENTELEMETRY_PYTHON_TRACER_PROVIDER": "tracer_provider"}, + ) + + environ_patcher.start() + + self.assertEqual( + Configuration().TRACER_PROVIDER, "tracer_provider" + ) # pylint: disable=no-member + + environ_patcher.stop() + + Configuration._reset() + + self.assertIsNone( + Configuration().TRACER_PROVIDER + ) # pylint: disable=no-member