Provide excluded_urls argument to Flask instrumentation (#604)

Author: mattoberle

CHANGELOG.md

@@ -14,6 +14,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `opentelemetry-instrumentation-starlette`, `opentelemetry-instrumentation-urllib`, `opentelemetry-instrumentation-urllib3` Added `request_hook` and `response_hook` callbacks 
   ([#576](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/576))
 
+### Changed
+- 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
 
 ### Removed

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
 ----------
 

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
@@ -197,18 +205,26 @@ 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(
             __name__, __version__, _InstrumentedFlask._tracer_provider
         )
 
         _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)
+
+        _teardown_request = _wrapped_teardown_request(
+            excluded_urls=_InstrumentedFlask._excluded_urls,
+        )
         self.teardown_request(_teardown_request)
 
 
@@ -232,27 +248,51 @@ 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):
         flask.Flask = self._original_flask
 
     @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)
+            app.wsgi_app = _rewrapped_app(
+                app.wsgi_app, response_hook, excluded_urls=excluded_urls
+            )
 
             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 +307,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:

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/<int:helloid>")(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)

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)