From e5260e04fe0632e384206f17d429766033a60155 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 14 Apr 2020 13:40:32 -0600 Subject: [PATCH 01/25] Add support for programmatic instrumentation in Flask --- .../flask_example.py | 5 ++-- docs/getting-started.rst | 6 ++--- .../src/opentelemetry/ext/flask/__init__.py | 24 ++++++++++++------ ext/opentelemetry-ext-flask/tests/conftest.py | 25 ------------------- .../tests/test_flask_integration.py | 7 +++++- .../auto_instrumentation/instrumentor.py | 9 +++++-- .../tests/test_instrumentor.py | 1 + 7 files changed, 37 insertions(+), 40 deletions(-) delete mode 100644 ext/opentelemetry-ext-flask/tests/conftest.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 863d6f33890..33b197702ba 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 @@ -40,8 +40,9 @@ SimpleExportSpanProcessor(ConsoleSpanExporter()) ) - -app = flask.Flask(__name__) +Flask = FlaskInstrumentor().instrument(flask_class=flask.Flask) +app = Flask(__name__) +opentelemetry.ext.http_requests.enable(trace.get_tracer_provider()) @app.route("/") diff --git a/docs/getting-started.rst b/docs/getting-started.rst index f25cf79b77d..61f8a7758a8 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,9 @@ 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 + + Flask = FlaskInstrumentor().instrument(flask_class=flask.Flask) trace.set_tracer_provider(TracerProvider()) trace.get_tracer_provider().add_span_processor( 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 1e936da115a..eef2f1299b1 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -29,9 +29,10 @@ .. 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 + + Flask = FlaskInstrumentor().instrument(flask_class=Flask) app = Flask(__name__) @@ -183,11 +184,20 @@ class FlaskInstrumentor(BaseInstrumentor): def __init__(self): super().__init__() - self._original_flask = None + self._original_flask_class = None - def _instrument(self, **kwargs): - self._original_flask = flask.Flask + def _instrument( + self, flask_class=None + ): # pylint: disable=arguments-differ + if flask_class is not None: + self._original_flask_class = flask_class + return _InstrumentedFlask + + self._original_flask_class = flask.Flask flask.Flask = _InstrumentedFlask - def _uninstrument(self, **kwargs): - flask.Flask = self._original_flask + return None + + def _uninstrument(self): # pylint: disable=arguments-differ + flask.Flask = self._original_flask_class + return self._original_flask_class diff --git a/ext/opentelemetry-ext-flask/tests/conftest.py b/ext/opentelemetry-ext-flask/tests/conftest.py deleted file mode 100644 index 8c0754f2c62..00000000000 --- 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_flask_integration.py b/ext/opentelemetry-ext-flask/tests/test_flask_integration.py index 1babfff2f56..8b7a7774674 100644 --- a/ext/opentelemetry-ext-flask/tests/test_flask_integration.py +++ b/ext/opentelemetry-ext-flask/tests/test_flask_integration.py @@ -21,8 +21,11 @@ from opentelemetry import trace as trace_api from opentelemetry.configuration import Configuration +from opentelemetry.ext.flask import FlaskInstrumentor from opentelemetry.test.wsgitestutil import WsgiTestBase +Flask = FlaskInstrumentor().instrument(flask_class=Flask) + def expected_attributes(override_attributes): default_attributes = { @@ -143,7 +146,9 @@ def test_internal_error(self): @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", }, ) diff --git a/opentelemetry-auto-instrumentation/src/opentelemetry/auto_instrumentation/instrumentor.py b/opentelemetry-auto-instrumentation/src/opentelemetry/auto_instrumentation/instrumentor.py index f5d7cf7ddc2..d7b6ac4e177 100644 --- a/opentelemetry-auto-instrumentation/src/opentelemetry/auto_instrumentation/instrumentor.py +++ b/opentelemetry-auto-instrumentation/src/opentelemetry/auto_instrumentation/instrumentor.py @@ -52,7 +52,9 @@ def instrument(self, **kwargs): self._is_instrumented = True return result - _LOG.warning("Attempting to instrument while already instrumented") + _LOG.warning( + "Attempting to automatically instrument while already instrumented" + ) return None @@ -64,7 +66,10 @@ def uninstrument(self, **kwargs): self._is_instrumented = False return result - _LOG.warning("Attempting to uninstrument while already uninstrumented") + _LOG.warning( + "Attempting to automatically uninstrument while already" + " uninstrumented" + ) return None diff --git a/opentelemetry-auto-instrumentation/tests/test_instrumentor.py b/opentelemetry-auto-instrumentation/tests/test_instrumentor.py index 40e762230af..a768a40eb42 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 d9071446ceff0911f50b742450e1efb4821d7c63 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 28 Apr 2020 16:08:12 -0600 Subject: [PATCH 02/25] Refactor interface --- .../flask_example.py | 6 ++-- .../src/opentelemetry/ext/flask/__init__.py | 34 ++++++++++--------- 2 files changed, 22 insertions(+), 18 deletions(-) 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 33b197702ba..6ca34bae1f7 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 @@ -40,8 +40,10 @@ SimpleExportSpanProcessor(ConsoleSpanExporter()) ) -Flask = FlaskInstrumentor().instrument(flask_class=flask.Flask) -app = Flask(__name__) +app = flask.Flask(__name__) + +FlaskInstrumentor().instrument(app=app) + opentelemetry.ext.http_requests.enable(trace.get_tracer_provider()) 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 eef2f1299b1..9e558a04195 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -32,10 +32,10 @@ from flask import Flask from opentelemetry.ext.flask import FlaskInstrumentor - Flask = FlaskInstrumentor().instrument(flask_class=Flask) - app = Flask(__name__) + FlaskInstrumentor().instrument(app=app) + @app.route("/") def hello(): return "Hello!" @@ -182,22 +182,24 @@ class FlaskInstrumentor(BaseInstrumentor): See `BaseInstrumentor` """ - def __init__(self): - super().__init__() - self._original_flask_class = None + def _instrument(self, **kwargs): + app = kwargs.get("app") - def _instrument( - self, flask_class=None - ): # pylint: disable=arguments-differ - if flask_class is not None: - self._original_flask_class = flask_class - return _InstrumentedFlask + if app is not None: + pass - self._original_flask_class = flask.Flask - flask.Flask = _InstrumentedFlask + else: + self._original_flask_class = flask.Flask + flask.Flask = _InstrumentedFlask return None - def _uninstrument(self): # pylint: disable=arguments-differ - flask.Flask = self._original_flask_class - return self._original_flask_class + def _uninstrument(self, **kwargs): + app = kwargs.get("app") + + if app is not None: + pass + + else: + flask.Flask = self._original_flask_class + return self._original_flask_class From 1c29d9d40425f891219cc8db5905d3e929ee21ed Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 28 Apr 2020 17:01:44 -0600 Subject: [PATCH 03/25] WIP --- .../src/opentelemetry/ext/flask/__init__.py | 178 +++++++++--------- 1 file changed, 89 insertions(+), 89 deletions(-) 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 9e558a04195..720a62b01af 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -69,97 +69,94 @@ def hello(): _ENVIRON_TOKEN = "opentelemetry-flask.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 +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): + 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 wsgi(environ, _start_response) + return start_response( + status, response_headers, *args, **kwargs + ) - self.wsgi_app = wrapped_app + return wsgi_app(environ, _start_response) + return _wrapped_app + + +def _before_request(): + 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)) - @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) - ) - 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 +class _InstrumentedFlask(flask.Flask): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) - 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._original_wsgi_ = self.wsgi_app + self.wsgi_app = _rewrapped_app(self.wsgi_app) + + self.before_request(_before_request) + self.teardown_request(_teardown_request) def _disable_trace(url): @@ -186,10 +183,14 @@ def _instrument(self, **kwargs): app = kwargs.get("app") if app is not None: - pass else: - self._original_flask_class = flask.Flask + app.wsgi_app = _rewrapped_app(app.wsgi_app) + + app.before_request(_before_request) + app.teardown_request(_teardown_request) + + else: flask.Flask = _InstrumentedFlask return None @@ -201,5 +202,4 @@ def _uninstrument(self, **kwargs): pass else: - flask.Flask = self._original_flask_class - return self._original_flask_class + pass From 07d588250fb4ec4886ebed4be409a0e3e890b107 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 28 Apr 2020 18:54:20 -0600 Subject: [PATCH 04/25] More WIP --- .../src/opentelemetry/ext/flask/__init__.py | 13 ++++++------- ...integration.py => test_flask_instrumentation.py} | 9 ++++----- 2 files changed, 10 insertions(+), 12 deletions(-) rename ext/opentelemetry-ext-flask/tests/{test_flask_integration.py => test_flask_instrumentation.py} (96%) 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 720a62b01af..9c41acbaafb 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -100,6 +100,8 @@ def _start_response(status, response_headers, *args, **kwargs): def _before_request(): + from ipdb import set_trace + set_trace environ = flask.request.environ span_name = ( flask.request.endpoint @@ -182,23 +184,20 @@ class FlaskInstrumentor(BaseInstrumentor): def _instrument(self, **kwargs): app = kwargs.get("app") - if app is not None: + if app is None: + flask.Flask = _InstrumentedFlask else: + app.wsgi_app = _rewrapped_app(app.wsgi_app) app.before_request(_before_request) app.teardown_request(_teardown_request) - else: - flask.Flask = _InstrumentedFlask - - return None - def _uninstrument(self, **kwargs): app = kwargs.get("app") - if app is not None: + if app is None: pass else: diff --git a/ext/opentelemetry-ext-flask/tests/test_flask_integration.py b/ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py similarity index 96% rename from ext/opentelemetry-ext-flask/tests/test_flask_integration.py rename to ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py index 8b7a7774674..723a120e16b 100644 --- a/ext/opentelemetry-ext-flask/tests/test_flask_integration.py +++ b/ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py @@ -24,8 +24,6 @@ from opentelemetry.ext.flask import FlaskInstrumentor from opentelemetry.test.wsgitestutil import WsgiTestBase -Flask = FlaskInstrumentor().instrument(flask_class=Flask) - def expected_attributes(override_attributes): default_attributes = { @@ -45,7 +43,7 @@ def expected_attributes(override_attributes): return default_attributes -class TestFlaskIntegration(WsgiTestBase): +class TestFlaskInstrumentation(WsgiTestBase): def setUp(self): # No instrumentation code is here because it is present in the # conftest.py file next to this file. @@ -54,6 +52,8 @@ def setUp(self): Configuration.__slots__ = [] self.app = Flask(__name__) + FlaskInstrumentor().instrument(app=self.app) + def hello_endpoint(helloid): if helloid == 500: raise ValueError(":-(") @@ -72,8 +72,7 @@ def excluded2_endpoint(): self.client = Client(self.app, BaseResponse) def tearDown(self): - Configuration._instance = None # pylint:disable=protected-access - Configuration.__slots__ = [] + FlaskInstrumentor().uninstrument(app=self.app) def test_only_strings_in_environ(self): """ From 6dd12a4c49c515e550ec00ece090c95fe6200a9f Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 28 Apr 2020 22:31:06 -0600 Subject: [PATCH 05/25] Refactor test cases --- .../src/opentelemetry/ext/flask/__init__.py | 19 ++-- ..._flask_instrumentation.py => base_test.py} | 15 +++- .../tests/test_automatic.py | 87 +++++++++++++++++++ .../tests/test_programmatic.py | 82 +++++++++++++++++ 4 files changed, 195 insertions(+), 8 deletions(-) rename ext/opentelemetry-ext-flask/tests/{test_flask_instrumentation.py => base_test.py} (89%) 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/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py index 9c41acbaafb..077b3e92ac2 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -47,7 +47,7 @@ def hello(): --- """ -import logging +from logging import getLogger import flask @@ -61,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" @@ -100,8 +100,6 @@ def _start_response(status, response_headers, *args, **kwargs): def _before_request(): - from ipdb import set_trace - set_trace environ = flask.request.environ span_name = ( flask.request.endpoint @@ -185,10 +183,11 @@ def _instrument(self, **kwargs): app = kwargs.get("app") if app is None: + self._original_flask = flask.Flask flask.Flask = _InstrumentedFlask else: - + app._original_wsgi_app = app.wsgi_app app.wsgi_app = _rewrapped_app(app.wsgi_app) app.before_request(_before_request) @@ -198,7 +197,13 @@ def _uninstrument(self, **kwargs): app = kwargs.get("app") if app is None: - pass + flask.Flask = self._original_flask else: - pass + 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 diff --git a/ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py b/ext/opentelemetry-ext-flask/tests/base_test.py similarity index 89% rename from ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py rename to ext/opentelemetry-ext-flask/tests/base_test.py index 723a120e16b..71f825a52f9 100644 --- a/ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py +++ b/ext/opentelemetry-ext-flask/tests/base_test.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +<<<<<<< HEAD:ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py import unittest from unittest.mock import patch @@ -23,6 +24,11 @@ from opentelemetry.configuration import Configuration from opentelemetry.ext.flask import FlaskInstrumentor from opentelemetry.test.wsgitestutil import WsgiTestBase +======= +from flask import request + +from opentelemetry import trace as trace_api +>>>>>>> dd973af2... Refactor test cases:ext/opentelemetry-ext-flask/tests/base_test.py def expected_attributes(override_attributes): @@ -43,6 +49,7 @@ def expected_attributes(override_attributes): return default_attributes +<<<<<<< HEAD:ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py class TestFlaskInstrumentation(WsgiTestBase): def setUp(self): # No instrumentation code is here because it is present in the @@ -73,6 +80,9 @@ def excluded2_endpoint(): def tearDown(self): FlaskInstrumentor().uninstrument(app=self.app) +======= +class InstrumentationTest: +>>>>>>> dd973af2... Refactor test cases:ext/opentelemetry-ext-flask/tests/base_test.py def test_only_strings_in_environ(self): """ @@ -93,7 +103,7 @@ def assert_environ(): self.client.get("/assert_environ") self.assertEqual(nonstring_keys, set()) - def test_simple(self): + def test_simple_uninstrument(self): expected_attrs = expected_attributes( {"http.target": "/hello/123", "http.route": "/hello/"} ) @@ -141,6 +151,7 @@ def test_internal_error(self): self.assertEqual(span_list[0].name, "hello_endpoint") self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) +<<<<<<< HEAD:ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py @patch.dict( "os.environ", # type: ignore @@ -162,3 +173,5 @@ def test_excluded_path(self): if __name__ == "__main__": unittest.main() +======= +>>>>>>> dd973af2... Refactor test cases:ext/opentelemetry-ext-flask/tests/base_test.py 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 00000000000..264e9b7ef6b --- /dev/null +++ b/ext/opentelemetry-ext-flask/tests/test_automatic.py @@ -0,0 +1,87 @@ +# 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 logging import NOTSET, WARNING, disable +from unittest import main + +# This is used instead of from flask import Flask, request because if not then +# FlaskInstrumentor().instrument() would need to be called before importing +# Flask. This is just an intrinsic limitation due the fact that we are testing +# the instrumentor in a way that mimics how it would be called with the +# opentelemetry-auto-instrumentation command. This does not mean that the +# instrumentor should be used in this way in end user applications. For those +# cases, FlaskInstrumentor.instrument(app=app) should be used. +import flask +from werkzeug.test import Client +from werkzeug.wrappers import BaseResponse + +from opentelemetry import trace as trace_api +from opentelemetry.ext.flask import FlaskInstrumentor +from opentelemetry.test.wsgitestutil import WsgiTestBase + +from .base_test import InstrumentationTest, expected_attributes + + +class TestAutomatic(WsgiTestBase, InstrumentationTest): + def setUp(self): + super().setUp() + + FlaskInstrumentor().instrument() + + self.app = flask.Flask(__name__) + + def hello_endpoint(helloid): + if helloid == 500: + raise ValueError(":-(") + return "Hello: " + str(helloid) + + self.app.route("/hello/")(hello_endpoint) + + self.client = Client(self.app, BaseResponse) + + def tearDown(self): + disable(WARNING) + FlaskInstrumentor().uninstrument() + disable(NOTSET) + + def test_uninstrument(self): + expected_attrs = expected_attributes( + {"http.target": "/hello/123", "http.route": "/hello/"} + ) + 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) + self.assertEqual(span_list[0].name, "hello_endpoint") + self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) + self.assertEqual(span_list[0].attributes, expected_attrs) + + FlaskInstrumentor().uninstrument() + + expected_attrs = expected_attributes( + {"http.target": "/hello/123", "http.route": "/hello/"} + ) + 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) + self.assertEqual(span_list[0].name, "hello_endpoint") + self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) + self.assertEqual(span_list[0].attributes, expected_attrs) + + +if __name__ == "__main__": + main() 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 00000000000..ee950532f9d --- /dev/null +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -0,0 +1,82 @@ +# 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 logging import NOTSET, WARNING, disable +import unittest + +from flask import Flask +from werkzeug.test import Client +from werkzeug.wrappers import BaseResponse + +from opentelemetry import trace as trace_api +from opentelemetry.ext.flask import FlaskInstrumentor +from opentelemetry.test.wsgitestutil import WsgiTestBase + +from .base_test import InstrumentationTest, expected_attributes + + +class TestProgrammatic(WsgiTestBase, InstrumentationTest): + def setUp(self): + # No instrumentation code is here because it is present in the + # conftest.py file next to this file. + super().setUp() + + self.app = Flask(__name__) + + FlaskInstrumentor().instrument(app=self.app) + + def hello_endpoint(helloid): + if helloid == 500: + raise ValueError(":-(") + return "Hello: " + str(helloid) + + self.app.route("/hello/")(hello_endpoint) + + self.client = Client(self.app, BaseResponse) + + def tearDown(self): + disable(WARNING) + FlaskInstrumentor().uninstrument(app=self.app) + disable(NOTSET) + + def test_uninstrument(self): + expected_attrs = expected_attributes( + {"http.target": "/hello/123", "http.route": "/hello/"} + ) + 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) + self.assertEqual(span_list[0].name, "hello_endpoint") + self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) + self.assertEqual(span_list[0].attributes, expected_attrs) + + FlaskInstrumentor().uninstrument(app=self.app) + + expected_attrs = expected_attributes( + {"http.target": "/hello/123", "http.route": "/hello/"} + ) + 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) + self.assertEqual(span_list[0].name, "hello_endpoint") + self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) + self.assertEqual(span_list[0].attributes, expected_attrs) + + +if __name__ == "__main__": + unittest.main() From cf715555c828b25968827ac9fe2a9e3b70e872e3 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 28 Apr 2020 22:47:55 -0600 Subject: [PATCH 06/25] Add automatic test cases --- ext/opentelemetry-ext-flask/tests/test_automatic.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ext/opentelemetry-ext-flask/tests/test_automatic.py b/ext/opentelemetry-ext-flask/tests/test_automatic.py index 264e9b7ef6b..3bb44958c64 100644 --- a/ext/opentelemetry-ext-flask/tests/test_automatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_automatic.py @@ -69,6 +69,16 @@ def test_uninstrument(self): self.assertEqual(span_list[0].attributes, expected_attrs) FlaskInstrumentor().uninstrument() + self.app = flask.Flask(__name__) + + def hello_endpoint(helloid): + if helloid == 500: + raise ValueError(":-(") + return "Hello: " + str(helloid) + + self.app.route("/hello/")(hello_endpoint) + + self.client = Client(self.app, BaseResponse) expected_attrs = expected_attributes( {"http.target": "/hello/123", "http.route": "/hello/"} From a19849cb818623c2729a701468e4fb0e588d4f51 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 10:34:38 -0600 Subject: [PATCH 07/25] Fix lint --- .../src/opentelemetry/ext/flask/__init__.py | 10 ++- .../tests/base_test.py | 72 ------------------- .../tests/test_programmatic.py | 4 +- 3 files changed, 6 insertions(+), 80 deletions(-) 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 077b3e92ac2..bbd7ff5c0e6 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -91,19 +91,17 @@ def _start_response(status, response_headers, *args, **kwargs): status, ) - return start_response( - status, response_headers, *args, **kwargs - ) + return start_response(status, response_headers, *args, **kwargs) return wsgi_app(environ, _start_response) + return _wrapped_app def _before_request(): environ = flask.request.environ - span_name = ( - flask.request.endpoint - or otel_wsgi.get_default_span_name(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) diff --git a/ext/opentelemetry-ext-flask/tests/base_test.py b/ext/opentelemetry-ext-flask/tests/base_test.py index 71f825a52f9..e883c969edc 100644 --- a/ext/opentelemetry-ext-flask/tests/base_test.py +++ b/ext/opentelemetry-ext-flask/tests/base_test.py @@ -12,23 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -<<<<<<< HEAD:ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py -import unittest -from unittest.mock import patch - -from flask import Flask, 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.ext.flask import FlaskInstrumentor -from opentelemetry.test.wsgitestutil import WsgiTestBase -======= from flask import request from opentelemetry import trace as trace_api ->>>>>>> dd973af2... Refactor test cases:ext/opentelemetry-ext-flask/tests/base_test.py def expected_attributes(override_attributes): @@ -49,41 +35,7 @@ def expected_attributes(override_attributes): return default_attributes -<<<<<<< HEAD:ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py -class TestFlaskInstrumentation(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__) - - FlaskInstrumentor().instrument(app=self.app) - - def hello_endpoint(helloid): - if helloid == 500: - raise ValueError(":-(") - return "Hello: " + str(helloid) - - def excluded_endpoint(): - return "excluded" - - def excluded2_endpoint(): - return "excluded2" - - self.app.route("/hello/")(hello_endpoint) - self.app.route("/excluded")(excluded_endpoint) - self.app.route("/excluded2")(excluded2_endpoint) - - self.client = Client(self.app, BaseResponse) - - def tearDown(self): - FlaskInstrumentor().uninstrument(app=self.app) -======= class InstrumentationTest: ->>>>>>> dd973af2... Refactor test cases:ext/opentelemetry-ext-flask/tests/base_test.py - def test_only_strings_in_environ(self): """ Some WSGI servers (such as Gunicorn) expect keys in the environ object @@ -151,27 +103,3 @@ def test_internal_error(self): self.assertEqual(span_list[0].name, "hello_endpoint") self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) -<<<<<<< HEAD:ext/opentelemetry-ext-flask/tests/test_flask_instrumentation.py - - @patch.dict( - "os.environ", # type: ignore - { - "OPENTELEMETRY_PYTHON_FLASK_EXCLUDED_HOSTS": ( - "http://localhost/excluded" - ), - "OPENTELEMETRY_PYTHON_FLASK_EXCLUDED_PATHS": "excluded2", - }, - ) - def test_excluded_path(self): - self.client.get("/hello/123") - self.client.get("/excluded") - 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() -======= ->>>>>>> dd973af2... Refactor test cases:ext/opentelemetry-ext-flask/tests/base_test.py diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index ee950532f9d..d12b23e023a 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -13,7 +13,7 @@ # limitations under the License. from logging import NOTSET, WARNING, disable -import unittest +from unittest import main from flask import Flask from werkzeug.test import Client @@ -79,4 +79,4 @@ def test_uninstrument(self): if __name__ == "__main__": - unittest.main() + main() From d45c4f06adf78d3397c2bfb4576f63bdbbf14189 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 11:04:36 -0600 Subject: [PATCH 08/25] Lint fixes --- .../src/opentelemetry/ext/flask/__init__.py | 1 + ext/opentelemetry-ext-flask/tests/base_test.py | 1 + ext/opentelemetry-ext-flask/tests/test_automatic.py | 1 + ext/opentelemetry-ext-flask/tests/test_programmatic.py | 1 + 4 files changed, 4 insertions(+) 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 bbd7ff5c0e6..25002fed6ab 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -172,6 +172,7 @@ def _disable_trace(url): class FlaskInstrumentor(BaseInstrumentor): + # pylint: disable=protected-access,attribute-defined-outside-init """A instrumentor for flask.Flask See `BaseInstrumentor` diff --git a/ext/opentelemetry-ext-flask/tests/base_test.py b/ext/opentelemetry-ext-flask/tests/base_test.py index e883c969edc..2eb25c70b1b 100644 --- a/ext/opentelemetry-ext-flask/tests/base_test.py +++ b/ext/opentelemetry-ext-flask/tests/base_test.py @@ -36,6 +36,7 @@ def expected_attributes(override_attributes): class InstrumentationTest: + # pylint: disable=no-member def test_only_strings_in_environ(self): """ Some WSGI servers (such as Gunicorn) expect keys in the environ object diff --git a/ext/opentelemetry-ext-flask/tests/test_automatic.py b/ext/opentelemetry-ext-flask/tests/test_automatic.py index 3bb44958c64..81f2deb8d11 100644 --- a/ext/opentelemetry-ext-flask/tests/test_automatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_automatic.py @@ -30,6 +30,7 @@ from opentelemetry.ext.flask import FlaskInstrumentor from opentelemetry.test.wsgitestutil import WsgiTestBase +# pylint: disable=import-error from .base_test import InstrumentationTest, expected_attributes diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index d12b23e023a..e78044b5ef6 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -23,6 +23,7 @@ from opentelemetry.ext.flask import FlaskInstrumentor from opentelemetry.test.wsgitestutil import WsgiTestBase +# pylint: disable=import-error from .base_test import InstrumentationTest, expected_attributes From 139f1aa9a4703e2ca576a9aaf531a0eb450ff9fa Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 11:13:41 -0600 Subject: [PATCH 09/25] Fixing example app --- .../src/opentelemetry_example_app/flask_example.py | 2 -- 1 file changed, 2 deletions(-) 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 6ca34bae1f7..c9921a7faf3 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 @@ -44,8 +44,6 @@ FlaskInstrumentor().instrument(app=app) -opentelemetry.ext.http_requests.enable(trace.get_tracer_provider()) - @app.route("/") def hello(): From 820f519674d59d2b9610d68bac4ca5c8463527fc Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 12:44:19 -0600 Subject: [PATCH 10/25] Add docs entry --- docs/ext/django/django.rst | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 docs/ext/django/django.rst diff --git a/docs/ext/django/django.rst b/docs/ext/django/django.rst new file mode 100644 index 00000000000..1a2c844e28c --- /dev/null +++ b/docs/ext/django/django.rst @@ -0,0 +1,7 @@ +OpenTelemetry Django Instrumentation +==================================== + +.. automodule:: opentelemetry.ext.django + :members: + :undoc-members: + :show-inheritance: From 91c1b474cc6987069f437c7aa1ac70c141dc8249 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 17:43:38 -0600 Subject: [PATCH 11/25] Add instrument_app and uninstrument_app --- .../flask_example.py | 5 +-- docs/getting-started.rst | 6 +-- .../src/opentelemetry/ext/flask/__init__.py | 40 ++++++++++++------- .../tests/test_programmatic.py | 8 ++-- 4 files changed, 34 insertions(+), 25 deletions(-) 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 c9921a7faf3..d58063f6863 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 @@ -33,8 +33,7 @@ # It must be done before instrumenting any library trace.set_tracer_provider(TracerProvider()) -opentelemetry.ext.requests.RequestsInstrumentor().instrument() -FlaskInstrumentor().instrument() +opentelemetry.ext.http_requests.RequestsInstrumentor().instrument() trace.get_tracer_provider().add_span_processor( SimpleExportSpanProcessor(ConsoleSpanExporter()) @@ -42,7 +41,7 @@ app = flask.Flask(__name__) -FlaskInstrumentor().instrument(app=app) +FlaskInstrumentor().instrument_app(app) @app.route("/") diff --git a/docs/getting-started.rst b/docs/getting-started.rst index 61f8a7758a8..a8ce9bd23f4 100644 --- a/docs/getting-started.rst +++ b/docs/getting-started.rst @@ -104,6 +104,7 @@ Configure exporters to emit spans elsewhere The example above does emit information about all spans, but the output is a bit hard to read. In common cases, you would instead *export* this data to an application performance monitoring backend, to be visualized and queried. +oHEA It is also common to aggregate span and trace information from multiple services into a single database, so that actions that require multiple services can still all be visualized together. This concept is known as distributed tracing. One such distributed tracing backend is known as Jaeger. @@ -194,15 +195,14 @@ And let's write a small Flask application that sends an HTTP request, activating from opentelemetry.sdk.trace.export import SimpleExportSpanProcessor from opentelemetry.ext.flask import FlaskInstrumentor - Flask = FlaskInstrumentor().instrument(flask_class=flask.Flask) - trace.set_tracer_provider(TracerProvider()) trace.get_tracer_provider().add_span_processor( SimpleExportSpanProcessor(ConsoleSpanExporter()) ) 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 25002fed6ab..b9102c815d2 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -61,7 +61,7 @@ def hello(): time_ns, ) -logger = getLogger(__name__) +_logger = getLogger(__name__) _ENVIRON_STARTTIME_KEY = "opentelemetry-flask.starttime_key" _ENVIRON_SPAN_KEY = "opentelemetry-flask.span_key" @@ -85,7 +85,7 @@ def _start_response(status, response_headers, *args, **kwargs): span, status, response_headers ) else: - logger.warning( + _logger.warning( "Flask environ's OpenTelemetry span " "missing at _start_response(%s)", status, @@ -130,7 +130,7 @@ def _before_request(): def _teardown_request(exc): activation = flask.request.environ.get(_ENVIRON_ACTIVATION_KEY) if not activation: - logger.warning( + _logger.warning( "Flask environ's OpenTelemetry activation missing" "at _teardown_flask_request(%s)", exc, @@ -179,30 +179,40 @@ class FlaskInstrumentor(BaseInstrumentor): """ def _instrument(self, **kwargs): - app = kwargs.get("app") + self._original_flask = flask.Flask + flask.Flask = _InstrumentedFlask - if app is None: - self._original_flask = flask.Flask - flask.Flask = _InstrumentedFlask - - else: + def instrument_app(self, app): + # FIXME remove this protection once some mechanism in the + # BaseInstrumentor class is available for these methods. + if not self._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) + self._is_instrumented = True + else: + _logger.warning( + "Attempting to instrument while already instrumented" + ) def _uninstrument(self, **kwargs): - app = kwargs.get("app") - - if app is None: - flask.Flask = self._original_flask + flask.Flask = self._original_flask - else: + def uninstrument_app(self, app): + # FIXME remove this protection once some mechanism in the + # BaseInstrumentor class is available for these methods. + if self._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 + + self._is_instrumented = False + else: + _logger.warning( + "Attempting to uninstrument while already uninstrumented" + ) diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index e78044b5ef6..b560f483181 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -35,7 +35,7 @@ def setUp(self): self.app = Flask(__name__) - FlaskInstrumentor().instrument(app=self.app) + FlaskInstrumentor().instrument_app(self.app) def hello_endpoint(helloid): if helloid == 500: @@ -48,10 +48,10 @@ def hello_endpoint(helloid): def tearDown(self): disable(WARNING) - FlaskInstrumentor().uninstrument(app=self.app) + FlaskInstrumentor().uninstrument_app(self.app) disable(NOTSET) - def test_uninstrument(self): + def test_unonstrument(self): expected_attrs = expected_attributes( {"http.target": "/hello/123", "http.route": "/hello/"} ) @@ -64,7 +64,7 @@ def test_uninstrument(self): self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) - FlaskInstrumentor().uninstrument(app=self.app) + FlaskInstrumentor().uninstrument_app(self.app) expected_attrs = expected_attributes( {"http.target": "/hello/123", "http.route": "/hello/"} From 6e615570e01c361f5c7b7755090a4778721416b0 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 20:08:08 -0600 Subject: [PATCH 12/25] Update ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Mauricio Vásquez --- .../src/opentelemetry/ext/flask/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b9102c815d2..3bc2dff812e 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -34,7 +34,7 @@ app = Flask(__name__) - FlaskInstrumentor().instrument(app=app) + FlaskInstrumentor().instrument_app(app) @app.route("/") def hello(): From 108287d8b2c81145b8602d66c6f0088c5f61940c Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 20:10:50 -0600 Subject: [PATCH 13/25] Removed django file --- docs/ext/django/django.rst | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 docs/ext/django/django.rst diff --git a/docs/ext/django/django.rst b/docs/ext/django/django.rst deleted file mode 100644 index 1a2c844e28c..00000000000 --- a/docs/ext/django/django.rst +++ /dev/null @@ -1,7 +0,0 @@ -OpenTelemetry Django Instrumentation -==================================== - -.. automodule:: opentelemetry.ext.django - :members: - :undoc-members: - :show-inheritance: From 636edc2fd3dae9d2193c69e1d29dac9e121d707e Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 20:11:49 -0600 Subject: [PATCH 14/25] Update ext/opentelemetry-ext-flask/tests/test_programmatic.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Mauricio Vásquez --- ext/opentelemetry-ext-flask/tests/test_programmatic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index b560f483181..57347a2b280 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -51,7 +51,7 @@ def tearDown(self): FlaskInstrumentor().uninstrument_app(self.app) disable(NOTSET) - def test_unonstrument(self): + def test_uninstrument(self): expected_attrs = expected_attributes( {"http.target": "/hello/123", "http.route": "/hello/"} ) From d1c712716e24aef74822dcfc60f2201a7171a25a Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 20:22:07 -0600 Subject: [PATCH 15/25] Fix lint --- .../src/opentelemetry/ext/flask/__init__.py | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) 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 3bc2dff812e..48000a7a023 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -182,28 +182,30 @@ def _instrument(self, **kwargs): self._original_flask = flask.Flask flask.Flask = _InstrumentedFlask - def instrument_app(self, app): - # FIXME remove this protection once some mechanism in the - # BaseInstrumentor class is available for these methods. - if not self._is_instrumented: + 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) - self._is_instrumented = True + app._is_instrumented = True else: _logger.warning( - "Attempting to instrument while already instrumented" + "Attempting to instrument Flask app while already instrumented" ) def _uninstrument(self, **kwargs): flask.Flask = self._original_flask - def uninstrument_app(self, app): - # FIXME remove this protection once some mechanism in the - # BaseInstrumentor class is available for these methods. - if self._is_instrumented: + 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 @@ -211,8 +213,9 @@ def uninstrument_app(self, app): app.teardown_request_funcs[None].remove(_teardown_request) del app._original_wsgi_app - self._is_instrumented = False + app._is_instrumented = False else: _logger.warning( - "Attempting to uninstrument while already uninstrumented" + "Attempting to uninstrument Flask " + "app while already uninstrumented" ) From baa142d439a20b40d47c39d2ce62dc0c7f23b0d7 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 20:23:49 -0600 Subject: [PATCH 16/25] Removed unnecessary comment --- ext/opentelemetry-ext-flask/tests/test_programmatic.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index 57347a2b280..3a2a55fb9a5 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -29,8 +29,6 @@ class TestProgrammatic(WsgiTestBase, InstrumentationTest): def setUp(self): - # No instrumentation code is here because it is present in the - # conftest.py file next to this file. super().setUp() self.app = Flask(__name__) From 83c7a9736a798eb5e5166247766d7cb40861e64e Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 20:25:12 -0600 Subject: [PATCH 17/25] Remove unittest.main --- .../tests/test_automatic.py | 14 +------------- .../tests/test_programmatic.py | 5 ----- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/ext/opentelemetry-ext-flask/tests/test_automatic.py b/ext/opentelemetry-ext-flask/tests/test_automatic.py index 81f2deb8d11..00216198c7e 100644 --- a/ext/opentelemetry-ext-flask/tests/test_automatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_automatic.py @@ -13,15 +13,7 @@ # limitations under the License. from logging import NOTSET, WARNING, disable -from unittest import main - -# This is used instead of from flask import Flask, request because if not then -# FlaskInstrumentor().instrument() would need to be called before importing -# Flask. This is just an intrinsic limitation due the fact that we are testing -# the instrumentor in a way that mimics how it would be called with the -# opentelemetry-auto-instrumentation command. This does not mean that the -# instrumentor should be used in this way in end user applications. For those -# cases, FlaskInstrumentor.instrument(app=app) should be used. + import flask from werkzeug.test import Client from werkzeug.wrappers import BaseResponse @@ -92,7 +84,3 @@ def hello_endpoint(helloid): self.assertEqual(span_list[0].name, "hello_endpoint") self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) - - -if __name__ == "__main__": - main() diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index 3a2a55fb9a5..9bb97868f1c 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -13,7 +13,6 @@ # limitations under the License. from logging import NOTSET, WARNING, disable -from unittest import main from flask import Flask from werkzeug.test import Client @@ -75,7 +74,3 @@ def test_uninstrument(self): self.assertEqual(span_list[0].name, "hello_endpoint") self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) - - -if __name__ == "__main__": - main() From cda10f4ec8aa7031e42faaecc679595f6c87ce6f Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Apr 2020 20:28:10 -0600 Subject: [PATCH 18/25] Revert instrumentor log changes --- .../opentelemetry/auto_instrumentation/instrumentor.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/opentelemetry-auto-instrumentation/src/opentelemetry/auto_instrumentation/instrumentor.py b/opentelemetry-auto-instrumentation/src/opentelemetry/auto_instrumentation/instrumentor.py index d7b6ac4e177..f5d7cf7ddc2 100644 --- a/opentelemetry-auto-instrumentation/src/opentelemetry/auto_instrumentation/instrumentor.py +++ b/opentelemetry-auto-instrumentation/src/opentelemetry/auto_instrumentation/instrumentor.py @@ -52,9 +52,7 @@ def instrument(self, **kwargs): self._is_instrumented = True return result - _LOG.warning( - "Attempting to automatically instrument while already instrumented" - ) + _LOG.warning("Attempting to instrument while already instrumented") return None @@ -66,10 +64,7 @@ def uninstrument(self, **kwargs): self._is_instrumented = False return result - _LOG.warning( - "Attempting to automatically uninstrument while already" - " uninstrumented" - ) + _LOG.warning("Attempting to uninstrument while already uninstrumented") return None From 7316bcf4bdd40ab352d4fa751ddd43c8def571d0 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 4 May 2020 09:59:11 -0600 Subject: [PATCH 19/25] Update to include excluded paths --- .../flask_example.py | 2 +- .../src/opentelemetry/ext/flask/__init__.py | 31 ++++++++++++------- .../tests/base_test.py | 28 ++++++++++++++--- .../tests/test_automatic.py | 18 +++++++++-- .../tests/test_programmatic.py | 17 ++++++++-- 5 files changed, 73 insertions(+), 23 deletions(-) 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 d58063f6863..8f44273b6ed 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 @@ -33,7 +33,7 @@ # It must be done before instrumenting any library trace.set_tracer_provider(TracerProvider()) -opentelemetry.ext.http_requests.RequestsInstrumentor().instrument() +opentelemetry.ext.requests.RequestsInstrumentor().instrument() trace.get_tracer_provider().add_span_processor( SimpleExportSpanProcessor(ConsoleSpanExporter()) 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 48000a7a023..223c4adf856 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -78,18 +78,21 @@ def _wrapped_app(environ, start_response): environ[_ENVIRON_STARTTIME_KEY] = time_ns() def _start_response(status, response_headers, *args, **kwargs): - 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, - ) + + 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) @@ -99,6 +102,9 @@ def _start_response(status, response_headers, *args, **kwargs): 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 @@ -160,6 +166,7 @@ def __init__(self, *args, **kwargs): 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): diff --git a/ext/opentelemetry-ext-flask/tests/base_test.py b/ext/opentelemetry-ext-flask/tests/base_test.py index 2eb25c70b1b..ca9375f85af 100644 --- a/ext/opentelemetry-ext-flask/tests/base_test.py +++ b/ext/opentelemetry-ext-flask/tests/base_test.py @@ -12,9 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +from unittest.mock import patch + from flask import request -from opentelemetry import trace as trace_api +from opentelemetry import trace def expected_attributes(override_attributes): @@ -36,6 +38,7 @@ def expected_attributes(override_attributes): class InstrumentationTest: + # pylint: disable=no-member def test_only_strings_in_environ(self): """ @@ -65,7 +68,7 @@ def test_simple_uninstrument(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].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) def test_404(self): @@ -84,7 +87,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): @@ -102,5 +105,22 @@ def test_internal_error(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].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_PATHS": "excluded2", + }, + ) + def test_excluded_path(self): + self.client.get("/hello/123") + self.client.get("/excluded") + 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") diff --git a/ext/opentelemetry-ext-flask/tests/test_automatic.py b/ext/opentelemetry-ext-flask/tests/test_automatic.py index 00216198c7e..95b8c9551ca 100644 --- a/ext/opentelemetry-ext-flask/tests/test_automatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_automatic.py @@ -18,7 +18,8 @@ from werkzeug.test import Client from werkzeug.wrappers import BaseResponse -from opentelemetry import trace as trace_api +from opentelemetry import trace +from opentelemetry.configuration import Configuration from opentelemetry.ext.flask import FlaskInstrumentor from opentelemetry.test.wsgitestutil import WsgiTestBase @@ -30,6 +31,8 @@ class TestAutomatic(WsgiTestBase, InstrumentationTest): 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__) @@ -39,7 +42,16 @@ def hello_endpoint(helloid): raise ValueError(":-(") return "Hello: " + str(helloid) + def excluded_endpoint(): + return "excluded" + + def excluded2_endpoint(): + return "excluded2" + self.app.route("/hello/")(hello_endpoint) + self.app.route("/excluded/")(hello_endpoint) + self.app.route("/excluded")(excluded_endpoint) + self.app.route("/excluded2")(excluded2_endpoint) self.client = Client(self.app, BaseResponse) @@ -58,7 +70,7 @@ def test_uninstrument(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].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) FlaskInstrumentor().uninstrument() @@ -82,5 +94,5 @@ def hello_endpoint(helloid): 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].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index 9bb97868f1c..c453ae474ff 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -18,7 +18,8 @@ from werkzeug.test import Client from werkzeug.wrappers import BaseResponse -from opentelemetry import trace as trace_api +from opentelemetry import trace +from opentelemetry.configuration import Configuration from opentelemetry.ext.flask import FlaskInstrumentor from opentelemetry.test.wsgitestutil import WsgiTestBase @@ -30,6 +31,8 @@ class TestProgrammatic(WsgiTestBase, InstrumentationTest): 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) @@ -39,7 +42,15 @@ def hello_endpoint(helloid): raise ValueError(":-(") return "Hello: " + str(helloid) + def excluded_endpoint(): + return "excluded" + + def excluded2_endpoint(): + return "excluded2" + self.app.route("/hello/")(hello_endpoint) + self.app.route("/excluded")(excluded_endpoint) + self.app.route("/excluded2")(excluded2_endpoint) self.client = Client(self.app, BaseResponse) @@ -58,7 +69,7 @@ def test_uninstrument(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].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) FlaskInstrumentor().uninstrument_app(self.app) @@ -72,5 +83,5 @@ def test_uninstrument(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].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) From 60f9693b72929f4d81b9bf11fe3bb1b080a042e1 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 5 May 2020 08:07:25 -0600 Subject: [PATCH 20/25] Remove typo --- docs/getting-started.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/getting-started.rst b/docs/getting-started.rst index a8ce9bd23f4..5d20fbe2c0c 100644 --- a/docs/getting-started.rst +++ b/docs/getting-started.rst @@ -104,7 +104,6 @@ Configure exporters to emit spans elsewhere The example above does emit information about all spans, but the output is a bit hard to read. In common cases, you would instead *export* this data to an application performance monitoring backend, to be visualized and queried. -oHEA It is also common to aggregate span and trace information from multiple services into a single database, so that actions that require multiple services can still all be visualized together. This concept is known as distributed tracing. One such distributed tracing backend is known as Jaeger. From 9e8a5b47e4534641c4dc62e184b4330ddbf668ae Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 5 May 2020 08:08:03 -0600 Subject: [PATCH 21/25] Update ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mauricio Vásquez --- .../src/opentelemetry/ext/flask/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 223c4adf856..040c8770c6a 100644 --- a/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py +++ b/ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py @@ -180,7 +180,7 @@ def _disable_trace(url): class FlaskInstrumentor(BaseInstrumentor): # pylint: disable=protected-access,attribute-defined-outside-init - """A instrumentor for flask.Flask + """An instrumentor for flask.Flask See `BaseInstrumentor` """ From 2e56c53c38ed040b3b788563bc434bcc3e5e7743 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 5 May 2020 08:09:28 -0600 Subject: [PATCH 22/25] Rename test --- ext/opentelemetry-ext-flask/tests/base_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opentelemetry-ext-flask/tests/base_test.py b/ext/opentelemetry-ext-flask/tests/base_test.py index ca9375f85af..df1de01d311 100644 --- a/ext/opentelemetry-ext-flask/tests/base_test.py +++ b/ext/opentelemetry-ext-flask/tests/base_test.py @@ -59,7 +59,7 @@ def assert_environ(): self.client.get("/assert_environ") self.assertEqual(nonstring_keys, set()) - def test_simple_uninstrument(self): + def test_simple(self): expected_attrs = expected_attributes( {"http.target": "/hello/123", "http.route": "/hello/"} ) From 7d285c977339b8c7b36bb9214bccc7e7da9a1473 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 5 May 2020 08:24:45 -0600 Subject: [PATCH 23/25] More test fixes --- .../tests/base_test.py | 29 ++++++++++++++++-- .../tests/test_automatic.py | 30 ++++--------------- .../tests/test_programmatic.py | 24 +++------------ 3 files changed, 35 insertions(+), 48 deletions(-) diff --git a/ext/opentelemetry-ext-flask/tests/base_test.py b/ext/opentelemetry-ext-flask/tests/base_test.py index df1de01d311..ca79d887c9c 100644 --- a/ext/opentelemetry-ext-flask/tests/base_test.py +++ b/ext/opentelemetry-ext-flask/tests/base_test.py @@ -15,6 +15,8 @@ from unittest.mock import patch from flask import request +from werkzeug.test import Client +from werkzeug.wrappers import BaseResponse from opentelemetry import trace @@ -39,6 +41,27 @@ def expected_attributes(override_attributes): 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/")(self._hello_endpoint) + self.app.route("/excluded/")(self._hello_endpoint) + self.app.route("/excluded")(excluded_endpoint) + self.app.route("/excluded2")(excluded2_endpoint) + + self.client = Client(self.app, BaseResponse) + # pylint: disable=no-member def test_only_strings_in_environ(self): """ @@ -67,7 +90,7 @@ 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].name, "_hello_endpoint") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) @@ -104,7 +127,7 @@ 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].name, "_hello_endpoint") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) @@ -123,4 +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") + self.assertEqual(span_list[0].name, "_hello_endpoint") diff --git a/ext/opentelemetry-ext-flask/tests/test_automatic.py b/ext/opentelemetry-ext-flask/tests/test_automatic.py index 95b8c9551ca..a47f8c61ed2 100644 --- a/ext/opentelemetry-ext-flask/tests/test_automatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_automatic.py @@ -37,25 +37,10 @@ def setUp(self): self.app = flask.Flask(__name__) - def hello_endpoint(helloid): - if helloid == 500: - raise ValueError(":-(") - return "Hello: " + str(helloid) - - def excluded_endpoint(): - return "excluded" - - def excluded2_endpoint(): - return "excluded2" - - self.app.route("/hello/")(hello_endpoint) - self.app.route("/excluded/")(hello_endpoint) - self.app.route("/excluded")(excluded_endpoint) - self.app.route("/excluded2")(excluded2_endpoint) - - self.client = Client(self.app, BaseResponse) + self._common_initialization() def tearDown(self): + super().tearDown() disable(WARNING) FlaskInstrumentor().uninstrument() disable(NOTSET) @@ -69,19 +54,14 @@ def test_uninstrument(self): self.assertEqual([b"Hello: 123"], list(resp.response)) 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].name, "_hello_endpoint") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) FlaskInstrumentor().uninstrument() self.app = flask.Flask(__name__) - def hello_endpoint(helloid): - if helloid == 500: - raise ValueError(":-(") - return "Hello: " + str(helloid) - - self.app.route("/hello/")(hello_endpoint) + self.app.route("/hello/")(self._hello_endpoint) self.client = Client(self.app, BaseResponse) @@ -93,6 +73,6 @@ def hello_endpoint(helloid): self.assertEqual([b"Hello: 123"], list(resp.response)) 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].name, "_hello_endpoint") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index c453ae474ff..b7e8eeb3e0f 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -15,8 +15,6 @@ from logging import NOTSET, WARNING, disable from flask import Flask -from werkzeug.test import Client -from werkzeug.wrappers import BaseResponse from opentelemetry import trace from opentelemetry.configuration import Configuration @@ -37,24 +35,10 @@ def setUp(self): FlaskInstrumentor().instrument_app(self.app) - def hello_endpoint(helloid): - if helloid == 500: - raise ValueError(":-(") - return "Hello: " + str(helloid) - - def excluded_endpoint(): - return "excluded" - - def excluded2_endpoint(): - return "excluded2" - - self.app.route("/hello/")(hello_endpoint) - self.app.route("/excluded")(excluded_endpoint) - self.app.route("/excluded2")(excluded2_endpoint) - - self.client = Client(self.app, BaseResponse) + self._common_initialization() def tearDown(self): + super().tearDown() disable(WARNING) FlaskInstrumentor().uninstrument_app(self.app) disable(NOTSET) @@ -68,7 +52,7 @@ def test_uninstrument(self): self.assertEqual([b"Hello: 123"], list(resp.response)) 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].name, "_hello_endpoint") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) @@ -82,6 +66,6 @@ def test_uninstrument(self): self.assertEqual([b"Hello: 123"], list(resp.response)) 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].name, "_hello_endpoint") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) From d428dac9a4d0dd90312f5f40b31fd255fb8fb545 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 5 May 2020 09:05:35 -0600 Subject: [PATCH 24/25] More fixes --- ext/opentelemetry-ext-flask/tests/base_test.py | 4 ++-- ext/opentelemetry-ext-flask/tests/test_automatic.py | 13 ++++++------- .../tests/test_programmatic.py | 10 ++++------ 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/ext/opentelemetry-ext-flask/tests/base_test.py b/ext/opentelemetry-ext-flask/tests/base_test.py index ca79d887c9c..7147afd7193 100644 --- a/ext/opentelemetry-ext-flask/tests/base_test.py +++ b/ext/opentelemetry-ext-flask/tests/base_test.py @@ -40,7 +40,6 @@ def expected_attributes(override_attributes): class InstrumentationTest: - @staticmethod def _hello_endpoint(helloid): if helloid == 500: @@ -48,18 +47,19 @@ def _hello_endpoint(helloid): return "Hello: " + str(helloid) def _common_initialization(self): - def excluded_endpoint(): return "excluded" def excluded2_endpoint(): return "excluded2" + # 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) # pylint: disable=no-member diff --git a/ext/opentelemetry-ext-flask/tests/test_automatic.py b/ext/opentelemetry-ext-flask/tests/test_automatic.py index a47f8c61ed2..0e2ea090f19 100644 --- a/ext/opentelemetry-ext-flask/tests/test_automatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_automatic.py @@ -12,8 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from logging import NOTSET, WARNING, disable - import flask from werkzeug.test import Client from werkzeug.wrappers import BaseResponse @@ -21,13 +19,14 @@ from opentelemetry import trace 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, expected_attributes -class TestAutomatic(WsgiTestBase, InstrumentationTest): +class TestAutomatic(InstrumentationTest, TestBase, WsgiTestBase): def setUp(self): super().setUp() @@ -41,14 +40,14 @@ def setUp(self): def tearDown(self): super().tearDown() - disable(WARNING) - FlaskInstrumentor().uninstrument() - disable(NOTSET) + with self.disable_logging(): + FlaskInstrumentor().uninstrument() def test_uninstrument(self): expected_attrs = expected_attributes( {"http.target": "/hello/123", "http.route": "/hello/"} ) + # 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)) @@ -62,7 +61,7 @@ def test_uninstrument(self): self.app = flask.Flask(__name__) self.app.route("/hello/")(self._hello_endpoint) - + # pylint: disable=attribute-defined-outside-init self.client = Client(self.app, BaseResponse) expected_attrs = expected_attributes( diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index b7e8eeb3e0f..fd8aad8dc4f 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -12,20 +12,19 @@ # See the License for the specific language governing permissions and # limitations under the License. -from logging import NOTSET, WARNING, disable - from flask import Flask from opentelemetry import trace 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, expected_attributes -class TestProgrammatic(WsgiTestBase, InstrumentationTest): +class TestProgrammatic(InstrumentationTest, TestBase, WsgiTestBase): def setUp(self): super().setUp() @@ -39,9 +38,8 @@ def setUp(self): def tearDown(self): super().tearDown() - disable(WARNING) - FlaskInstrumentor().uninstrument_app(self.app) - disable(NOTSET) + with self.disable_logging(): + FlaskInstrumentor().uninstrument_app(self.app) def test_uninstrument(self): expected_attrs = expected_attributes( From aa9c92350c5fbdef217ed8c07993698e7213b495 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 5 May 2020 09:14:23 -0600 Subject: [PATCH 25/25] More fixes --- .../tests/test_automatic.py | 15 +-------------- .../tests/test_programmatic.py | 15 +-------------- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/ext/opentelemetry-ext-flask/tests/test_automatic.py b/ext/opentelemetry-ext-flask/tests/test_automatic.py index 0e2ea090f19..04f43d6e642 100644 --- a/ext/opentelemetry-ext-flask/tests/test_automatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_automatic.py @@ -16,14 +16,13 @@ from werkzeug.test import Client from werkzeug.wrappers import BaseResponse -from opentelemetry import trace 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, expected_attributes +from .base_test import InstrumentationTest class TestAutomatic(InstrumentationTest, TestBase, WsgiTestBase): @@ -44,18 +43,12 @@ def tearDown(self): FlaskInstrumentor().uninstrument() def test_uninstrument(self): - expected_attrs = expected_attributes( - {"http.target": "/hello/123", "http.route": "/hello/"} - ) # 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) - 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) FlaskInstrumentor().uninstrument() self.app = flask.Flask(__name__) @@ -64,14 +57,8 @@ def test_uninstrument(self): # pylint: disable=attribute-defined-outside-init self.client = Client(self.app, BaseResponse) - expected_attrs = expected_attributes( - {"http.target": "/hello/123", "http.route": "/hello/"} - ) 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) - 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) diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index fd8aad8dc4f..1075f808cb8 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -14,14 +14,13 @@ from flask import Flask -from opentelemetry import trace 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, expected_attributes +from .base_test import InstrumentationTest class TestProgrammatic(InstrumentationTest, TestBase, WsgiTestBase): @@ -42,28 +41,16 @@ def tearDown(self): FlaskInstrumentor().uninstrument_app(self.app) def test_uninstrument(self): - expected_attrs = expected_attributes( - {"http.target": "/hello/123", "http.route": "/hello/"} - ) 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) - 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) FlaskInstrumentor().uninstrument_app(self.app) - expected_attrs = expected_attributes( - {"http.target": "/hello/123", "http.route": "/hello/"} - ) 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) - 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)