From bcbd21f6c76452d332e42b83779527149faddb1f Mon Sep 17 00:00:00 2001 From: Matt Oberle Date: Wed, 14 Jul 2021 14:59:40 -0400 Subject: [PATCH 1/4] Provide excluded_urls argument to Flask instrumentation The FastAPI instrumentation accepts an 'explicit_urls' argument on initialization. This commit translates that concept to the Flask instrumentation. --- CHANGELOG.md | 4 + .../README.rst | 6 ++ .../instrumentation/flask/__init__.py | 86 +++++++++++++------ .../tests/test_programmatic.py | 48 ++++++++--- 4 files changed, 105 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66b2b6705d..8b7de813e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.4.0-0.23b0...HEAD) +### Changed +- Enable explicit excluded\_urls `opentelemetry-instrumentation-flask` + ([#604](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/604)) + ## [1.4.0-0.23b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.4.0-0.23b0) - 2021-07-21 ### Removed diff --git a/instrumentation/opentelemetry-instrumentation-flask/README.rst b/instrumentation/opentelemetry-instrumentation-flask/README.rst index 919684e083..a139ab2683 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/README.rst +++ b/instrumentation/opentelemetry-instrumentation-flask/README.rst @@ -31,6 +31,12 @@ For example, will exclude requests such as ``https://site/client/123/info`` and ``https://site/xyz/healthcheck``. +You can also pass the comma delimited regexes to the ``instrument_app`` method directly: + +.. code-block:: python + + FlaskInstrumentor().instrument_app(app, excluded_urls="client/.*/info,healthcheck") + References ---------- diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index c27207d196..2399935cf8 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -63,7 +63,7 @@ def hello(): from opentelemetry.propagate import extract from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.util._time import _time_ns -from opentelemetry.util.http import get_excluded_urls +from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls _logger = getLogger(__name__) @@ -73,7 +73,7 @@ def hello(): _ENVIRON_TOKEN = "opentelemetry-flask.token" -_excluded_urls = get_excluded_urls("FLASK") +_excluded_urls_from_env = get_excluded_urls("FLASK") def get_default_span_name(): @@ -85,7 +85,7 @@ def get_default_span_name(): return span_name -def _rewrapped_app(wsgi_app, response_hook=None): +def _rewrapped_app(wsgi_app, response_hook=None, excluded_urls=None): def _wrapped_app(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 @@ -94,7 +94,9 @@ def _wrapped_app(wrapped_app_environ, start_response): wrapped_app_environ[_ENVIRON_STARTTIME_KEY] = _time_ns() def _start_response(status, response_headers, *args, **kwargs): - if not _excluded_urls.url_disabled(flask.request.url): + if excluded_urls is None or not excluded_urls.url_disabled( + flask.request.url + ): span = flask.request.environ.get(_ENVIRON_SPAN_KEY) propagator = get_global_response_propagator() @@ -123,9 +125,11 @@ def _start_response(status, response_headers, *args, **kwargs): return _wrapped_app -def _wrapped_before_request(request_hook=None, tracer=None): +def _wrapped_before_request( + request_hook=None, tracer=None, excluded_urls=None +): def _before_request(): - if _excluded_urls.url_disabled(flask.request.url): + if excluded_urls and excluded_urls.url_disabled(flask.request.url): return flask_request_environ = flask.request.environ span_name = get_default_span_name() @@ -163,29 +167,33 @@ def _before_request(): return _before_request -def _teardown_request(exc): - # pylint: disable=E1101 - if _excluded_urls.url_disabled(flask.request.url): - return +def _wrapped_teardown_request(excluded_urls=None): + def _teardown_request(exc): + # pylint: disable=E1101 + if excluded_urls and excluded_urls.url_disabled(flask.request.url): + return - activation = flask.request.environ.get(_ENVIRON_ACTIVATION_KEY) - if not activation: - # This request didn't start a span, maybe because it was created in a - # way that doesn't run `before_request`, like when it is created with - # `app.test_request_context`. - return + activation = flask.request.environ.get(_ENVIRON_ACTIVATION_KEY) + if not activation: + # This request didn't start a span, maybe because it was created in + # a way that doesn't run `before_request`, like when it is created + # with `app.test_request_context`. + 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)) + 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)) + + return _teardown_request class _InstrumentedFlask(flask.Flask): + _excluded_urls = None _tracer_provider = None _request_hook = None _response_hook = None @@ -209,6 +217,10 @@ def __init__(self, *args, **kwargs): ) self._before_request = _before_request self.before_request(_before_request) + + _teardown_request = _wrapped_teardown_request( + excluded_urls=_InstrumentedFlask._excluded_urls, + ) self.teardown_request(_teardown_request) @@ -232,6 +244,12 @@ def _instrument(self, **kwargs): _InstrumentedFlask._response_hook = response_hook tracer_provider = kwargs.get("tracer_provider") _InstrumentedFlask._tracer_provider = tracer_provider + excluded_urls = kwargs.get("excluded_urls") + _InstrumentedFlask._excluded_urls = ( + _excluded_urls_from_env + if excluded_urls is None + else parse_excluded_urls(excluded_urls) + ) flask.Flask = _InstrumentedFlask def _uninstrument(self, **kwargs): @@ -239,20 +257,36 @@ def _uninstrument(self, **kwargs): @staticmethod def instrument_app( - app, request_hook=None, response_hook=None, tracer_provider=None + app, + request_hook=None, + response_hook=None, + tracer_provider=None, + excluded_urls=None, ): if not hasattr(app, "_is_instrumented_by_opentelemetry"): app._is_instrumented_by_opentelemetry = False if not app._is_instrumented_by_opentelemetry: + excluded_urls = ( + parse_excluded_urls(excluded_urls) + if excluded_urls is not None + else _excluded_urls_from_env + ) app._original_wsgi_app = app.wsgi_app app.wsgi_app = _rewrapped_app(app.wsgi_app, response_hook) tracer = trace.get_tracer(__name__, __version__, tracer_provider) - _before_request = _wrapped_before_request(request_hook, tracer) + _before_request = _wrapped_before_request( + request_hook, tracer, excluded_urls=excluded_urls, + ) app._before_request = _before_request app.before_request(_before_request) + + _teardown_request = _wrapped_teardown_request( + excluded_urls=excluded_urls, + ) + app._teardown_request = _teardown_request app.teardown_request(_teardown_request) app._is_instrumented_by_opentelemetry = True else: @@ -267,7 +301,7 @@ def uninstrument_app(app): # FIXME add support for other Flask blueprints that are not None app.before_request_funcs[None].remove(app._before_request) - app.teardown_request_funcs[None].remove(_teardown_request) + app.teardown_request_funcs[None].remove(app._teardown_request) del app._original_wsgi_app app._is_instrumented_by_opentelemetry = False else: diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 4e3e313945..e377950835 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -53,25 +53,25 @@ class TestProgrammatic(InstrumentationTest, TestBase, WsgiTestBase): def setUp(self): super().setUp() - self.app = Flask(__name__) - - FlaskInstrumentor().instrument_app(self.app) - - self._common_initialization() - self.env_patch = patch.dict( "os.environ", { - "OTEL_PYTHON_FLASK_EXCLUDED_URLS": "http://localhost/excluded_arg/123,excluded_noarg" + "OTEL_PYTHON_FLASK_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg" }, ) self.env_patch.start() + self.exclude_patch = patch( - "opentelemetry.instrumentation.flask._excluded_urls", + "opentelemetry.instrumentation.flask._excluded_urls_from_env", get_excluded_urls("FLASK"), ) self.exclude_patch.start() + self.app = Flask(__name__) + FlaskInstrumentor().instrument_app(self.app) + + self._common_initialization() + def tearDown(self): super().tearDown() self.env_patch.stop() @@ -221,20 +221,42 @@ def test_internal_error(self): self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) - def test_exclude_lists(self): - self.client.get("/excluded_arg/123") + def test_exclude_lists_from_env(self): + self.client.get("/env_excluded_arg/123") + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 0) + + self.client.get("/env_excluded_arg/125") + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + self.client.get("/env_excluded_noarg") + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + self.client.get("/env_excluded_noarg2") + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + def test_exclude_lists_from_explicit(self): + excluded_urls = "http://localhost/explicit_excluded_arg/123,explicit_excluded_noarg" + app = Flask(__name__) + FlaskInstrumentor().instrument_app(app, excluded_urls=excluded_urls) + client = app.test_client() + + client.get("/explicit_excluded_arg/123") span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 0) - self.client.get("/excluded_arg/125") + client.get("/explicit_excluded_arg/125") span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.client.get("/excluded_noarg") + client.get("/explicit_excluded_noarg") span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.client.get("/excluded_noarg2") + client.get("/explicit_excluded_noarg2") span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) From ab3d177f4c573a6ac13776833b761a76808b7afe Mon Sep 17 00:00:00 2001 From: Matt Oberle Date: Thu, 22 Jul 2021 13:39:55 -0400 Subject: [PATCH 2/4] Update CHANGELOG with better description --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b7de813e6..c843ee80b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.4.0-0.23b0...HEAD) ### Changed -- Enable explicit excluded\_urls `opentelemetry-instrumentation-flask` +- Enable explicit excluded\_urls argument in `opentelemetry-instrumentation-flask` ([#604](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/604)) ## [1.4.0-0.23b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.4.0-0.23b0) - 2021-07-21 From 0e62052c139fb95a681bb2c33dbea80270242a6e Mon Sep 17 00:00:00 2001 From: Matt Oberle Date: Fri, 23 Jul 2021 16:11:21 -0400 Subject: [PATCH 3/4] Update CHANGELOG.md Co-authored-by: Leighton Chen --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c843ee80b7..302b36d05b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.4.0-0.23b0...HEAD) ### Changed -- Enable explicit excluded\_urls argument in `opentelemetry-instrumentation-flask` +- Enable explicit `excluded_urls` argument in `opentelemetry-instrumentation-flask` ([#604](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/604)) ## [1.4.0-0.23b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.4.0-0.23b0) - 2021-07-21 From 5996b7e2849b97aa29e4d5d90c95e2b85698feee Mon Sep 17 00:00:00 2001 From: Matt Oberle Date: Mon, 26 Jul 2021 13:57:01 -0400 Subject: [PATCH 4/4] Pass excluded_urls to Flask auto-instrumentation The Flask instrumentation can be applied on two levels: * The `module` level (auto-instrumentation), which patches the `Flask` class. * The `app` level, which patches a `Flask` instance. This commit applies a few missing `excluded_urls` keyword arguments to ensure the auto-instrumentation provides the same configurability as the app-level instrumentation. --- .../instrumentation/flask/__init__.py | 12 ++++++++--- .../tests/test_automatic.py | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 2399935cf8..d2210eb539 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -205,7 +205,9 @@ def __init__(self, *args, **kwargs): self._is_instrumented_by_opentelemetry = True self.wsgi_app = _rewrapped_app( - self.wsgi_app, _InstrumentedFlask._response_hook + self.wsgi_app, + _InstrumentedFlask._response_hook, + excluded_urls=_InstrumentedFlask._excluded_urls, ) tracer = trace.get_tracer( @@ -213,7 +215,9 @@ def __init__(self, *args, **kwargs): ) _before_request = _wrapped_before_request( - _InstrumentedFlask._request_hook, tracer, + _InstrumentedFlask._request_hook, + tracer, + excluded_urls=_InstrumentedFlask._excluded_urls, ) self._before_request = _before_request self.before_request(_before_request) @@ -273,7 +277,9 @@ def instrument_app( else _excluded_urls_from_env ) app._original_wsgi_app = app.wsgi_app - app.wsgi_app = _rewrapped_app(app.wsgi_app, response_hook) + app.wsgi_app = _rewrapped_app( + app.wsgi_app, response_hook, excluded_urls=excluded_urls + ) tracer = trace.get_tracer(__name__, __version__, tracer_provider) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_automatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_automatic.py index d081bed9ee..aeff470947 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_automatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_automatic.py @@ -59,3 +59,23 @@ 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) + + def test_exluded_urls_explicit(self): + FlaskInstrumentor().uninstrument() + FlaskInstrumentor().instrument(excluded_urls="/hello/456") + + self.app = flask.Flask(__name__) + self.app.route("/hello/")(self._hello_endpoint) + client = Client(self.app, BaseResponse) + + resp = 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) + + resp = client.get("/hello/456") + self.assertEqual(200, resp.status_code) + self.assertEqual([b"Hello: 456"], list(resp.response)) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1)