Change exclude lists to just exclude_urls, add tests for flask and django (#872)

Author: cnnradams

Diff for: ext/opentelemetry-ext-django/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Use one general exclude list instead of two ([#872](https://github.com/open-telemetry/opentelemetry-python/pull/872))
+
 ## 0.8b0
 
 Released 2020-05-27

Diff for: ext/opentelemetry-ext-django/README.rst

@@ -20,11 +20,15 @@ Configuration
 
 Exclude lists
 *************
-Excludes certain hosts and paths from being tracked. Pass in comma delimited string into environment variables.
-Host refers to the entire url and path refers to the part of the url after the domain. Host matches the exact string that is given, where as path matches if the url starts with the given excluded path.
+To exclude certain URLs from being tracked, set the environment variable ``OPENTELEMETRY_PYTHON_DJANGO_EXCLUDED_URLS`` with comma delimited regexes representing which URLs to exclude.
 
-Excluded hosts: OPENTELEMETRY_PYTHON_DJANGO_EXCLUDED_HOSTS
-Excluded paths: OPENTELEMETRY_PYTHON_DJANGO_EXCLUDED_PATHS
+For example,
+
+::
+
+    export OPENTELEMETRY_PYTHON_DJANGO_EXCLUDED_URLS="client/.*/info,healthcheck"
+
+will exclude requests such as ``https://site/client/123/info`` and ``https://site/xyz/healthcheck``.
 
 References
 ----------

Diff for: ext/opentelemetry-ext-django/src/opentelemetry/ext/django/middleware.py

@@ -24,7 +24,7 @@
 )
 from opentelemetry.propagators import extract
 from opentelemetry.trace import SpanKind, get_tracer
-from opentelemetry.util import disable_trace
+from opentelemetry.util import ExcludeList
 
 try:
     from django.utils.deprecation import MiddlewareMixin
@@ -44,12 +44,11 @@ class _DjangoMiddleware(MiddlewareMixin):
     _environ_token = "opentelemetry-instrumentor-django.token"
     _environ_span_key = "opentelemetry-instrumentor-django.span_key"
 
-    _excluded_hosts = Configuration().DJANGO_EXCLUDED_HOSTS or []
-    _excluded_paths = Configuration().DJANGO_EXCLUDED_PATHS or []
-    if _excluded_hosts:
-        _excluded_hosts = str.split(_excluded_hosts, ",")
-    if _excluded_paths:
-        _excluded_paths = str.split(_excluded_paths, ",")
+    _excluded_urls = Configuration().DJANGO_EXCLUDED_URLS or []
+    if _excluded_urls:
+        _excluded_urls = ExcludeList(str.split(_excluded_urls, ","))
+    else:
+        _excluded_urls = ExcludeList(_excluded_urls)
 
     def process_view(
         self, request, view_func, view_args, view_kwargs
@@ -62,11 +61,7 @@ def process_view(
         #     key.lower().replace('_', '-').replace("http-", "", 1): value
         #     for key, value in request.META.items()
         # }
-        if disable_trace(
-            request.build_absolute_uri("?"),
-            self._excluded_hosts,
-            self._excluded_paths,
-        ):
+        if self._excluded_urls.url_disabled(request.build_absolute_uri("?")):
             return
 
         environ = request.META
@@ -97,11 +92,7 @@ def process_exception(self, request, exception):
         # Django can call this method and process_response later. In order
         # to avoid __exit__ and detach from being called twice then, the
         # respective keys are being removed here.
-        if disable_trace(
-            request.build_absolute_uri("?"),
-            self._excluded_hosts,
-            self._excluded_paths,
-        ):
+        if self._excluded_urls.url_disabled(request.build_absolute_uri("?")):
             return
 
         if self._environ_activation_key in request.META.keys():
@@ -116,11 +107,7 @@ def process_exception(self, request, exception):
             request.META.pop(self._environ_token, None)
 
     def process_response(self, request, response):
-        if disable_trace(
-            request.build_absolute_uri("?"),
-            self._excluded_hosts,
-            self._excluded_paths,
-        ):
+        if self._excluded_urls.url_disabled(request.build_absolute_uri("?")):
             return response
 
         if (

Diff for: ext/opentelemetry-ext-django/tests/test_middleware.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 from sys import modules
+from unittest.mock import patch
 
 from django.conf import settings
 from django.conf.urls import url
@@ -24,15 +25,17 @@
 from opentelemetry.test.wsgitestutil import WsgiTestBase
 from opentelemetry.trace import SpanKind
 from opentelemetry.trace.status import StatusCanonicalCode
+from opentelemetry.util import ExcludeList
 
 # pylint: disable=import-error
-from .views import error, excluded, excluded2, traced
+from .views import error, excluded, excluded_noarg, excluded_noarg2, traced
 
 urlpatterns = [
     url(r"^traced/", traced),
     url(r"^error/", error),
-    url(r"^excluded/", excluded),
-    url(r"^excluded2/", excluded2),
+    url(r"^excluded_arg/", excluded),
+    url(r"^excluded_noarg/", excluded_noarg),
+    url(r"^excluded_noarg2/", excluded_noarg2),
 ]
 _django_instrumentor = DjangoInstrumentor()
 
@@ -111,3 +114,25 @@ def test_error(self):
             span.attributes["http.url"], "http://testserver/error/"
         )
         self.assertEqual(span.attributes["http.scheme"], "http")
+
+    @patch(
+        "opentelemetry.ext.django.middleware._DjangoMiddleware._excluded_urls",
+        ExcludeList(["http://testserver/excluded_arg/123", "excluded_noarg"]),
+    )
+    def test_exclude_lists(self):
+        client = Client()
+        client.get("/excluded_arg/123")
+        span_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 0)
+
+        client.get("/excluded_arg/125")
+        span_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 1)
+
+        client.get("/excluded_noarg/")
+        span_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 1)
+
+        client.get("/excluded_noarg2/")
+        span_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 1)

Diff for: ext/opentelemetry-ext-django/tests/views.py

@@ -13,5 +13,9 @@ def excluded(request):  # pylint: disable=unused-argument
     return HttpResponse()
 
 
-def excluded2(request):  # pylint: disable=unused-argument
+def excluded_noarg(request):  # pylint: disable=unused-argument
+    return HttpResponse()
+
+
+def excluded_noarg2(request):  # pylint: disable=unused-argument
     return HttpResponse()

Diff for: ext/opentelemetry-ext-flask/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Use one general exclude list instead of two ([#872](https://github.com/open-telemetry/opentelemetry-python/pull/872))
+
 ## 0.7b1
 
 Released 2020-05-12

Diff for: ext/opentelemetry-ext-flask/README.rst

@@ -21,12 +21,15 @@ Configuration
 
 Exclude lists
 *************
-Excludes certain hosts and paths from being tracked. Pass in comma delimited string into environment variables.
-Host refers to the entire url and path refers to the part of the url after the domain. Host matches the exact string that is given, where as path matches if the url starts with the given excluded path.
+To exclude certain URLs from being tracked, set the environment variable ``OPENTELEMETRY_PYTHON_FLASK_EXCLUDED_URLS`` with comma delimited regexes representing which URLs to exclude.
 
-Excluded hosts: OPENTELEMETRY_PYTHON_FLASK_EXCLUDED_HOSTS
-Excluded paths: OPENTELEMETRY_PYTHON_FLASK_EXCLUDED_PATHS
+For example,
 
+::
+
+    export OPENTELEMETRY_PYTHON_FLASK_EXCLUDED_URLS="client/.*/info,healthcheck"
+
+will exclude requests such as ``https://site/client/123/info`` and ``https://site/xyz/healthcheck``.
 
 References
 ----------

Diff for: ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py

@@ -55,7 +55,7 @@ def hello():
 from opentelemetry import configuration, context, propagators, trace
 from opentelemetry.ext.flask.version import __version__
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
-from opentelemetry.util import disable_trace, time_ns
+from opentelemetry.util import ExcludeList, time_ns
 
 _logger = getLogger(__name__)
 
@@ -65,22 +65,14 @@ def hello():
 _ENVIRON_TOKEN = "opentelemetry-flask.token"
 
 
-def get_excluded_hosts():
-    hosts = configuration.Configuration().FLASK_EXCLUDED_HOSTS or []
-    if hosts:
-        hosts = str.split(hosts, ",")
-    return hosts
+def get_excluded_urls():
+    urls = configuration.Configuration().FLASK_EXCLUDED_URLS or []
+    if urls:
+        urls = str.split(urls, ",")
+    return ExcludeList(urls)
 
 
-def get_excluded_paths():
-    paths = configuration.Configuration().FLASK_EXCLUDED_PATHS or []
-    if paths:
-        paths = str.split(paths, ",")
-    return paths
-
-
-_excluded_hosts = get_excluded_hosts()
-_excluded_paths = get_excluded_paths()
+_excluded_urls = get_excluded_urls()
 
 
 def _rewrapped_app(wsgi_app):
@@ -92,9 +84,7 @@ def _wrapped_app(environ, start_response):
         environ[_ENVIRON_STARTTIME_KEY] = time_ns()
 
         def _start_response(status, response_headers, *args, **kwargs):
-            if not disable_trace(
-                flask.request.url, _excluded_hosts, _excluded_paths
-            ):
+            if not _excluded_urls.url_disabled(flask.request.url):
                 span = flask.request.environ.get(_ENVIRON_SPAN_KEY)
 
                 if span:
@@ -116,7 +106,7 @@ def _start_response(status, response_headers, *args, **kwargs):
 
 
 def _before_request():
-    if disable_trace(flask.request.url, _excluded_hosts, _excluded_paths):
+    if _excluded_urls.url_disabled(flask.request.url):
         return
 
     environ = flask.request.environ
@@ -148,7 +138,7 @@ def _before_request():
 
 
 def _teardown_request(exc):
-    if disable_trace(flask.request.url, _excluded_hosts, _excluded_paths):
+    if _excluded_urls.url_disabled(flask.request.url):
         return
 
     activation = flask.request.environ.get(_ENVIRON_ACTIVATION_KEY)

Diff for: ext/opentelemetry-ext-flask/tests/test_programmatic.py

@@ -12,12 +12,15 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from unittest.mock import patch
+
 from flask import Flask, request
 
 from opentelemetry import trace
 from opentelemetry.ext.flask import FlaskInstrumentor
 from opentelemetry.test.test_base import TestBase
 from opentelemetry.test.wsgitestutil import WsgiTestBase
+from opentelemetry.util import ExcludeList
 
 # pylint: disable=import-error
 from .base_test import InstrumentationTest
@@ -139,3 +142,24 @@ def test_internal_error(self):
         self.assertEqual(span_list[0].name, "_hello_endpoint")
         self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER)
         self.assertEqual(span_list[0].attributes, expected_attrs)
+
+    @patch(
+        "opentelemetry.ext.flask._excluded_urls",
+        ExcludeList(["http://localhost/excluded_arg/123", "excluded_noarg"]),
+    )
+    def test_exclude_lists(self):
+        self.client.get("/excluded_arg/123")
+        span_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 0)
+
+        self.client.get("/excluded_arg/125")
+        span_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 1)
+
+        self.client.get("/excluded_noarg")
+        span_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 1)
+
+        self.client.get("/excluded_noarg2")
+        span_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 1)

Diff for: ext/opentelemetry-ext-pyramid/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Use one general exclude list instead of two ([#872](https://github.com/open-telemetry/opentelemetry-python/pull/872))
+
 ## 0.9b0
 
 Released 2020-06-10

Diff for: ext/opentelemetry-ext-pyramid/README.rst

@@ -13,6 +13,17 @@ Installation
 
     pip install opentelemetry-ext-pyramid
 
+Exclude lists
+*************
+To exclude certain URLs from being tracked, set the environment variable ``OPENTELEMETRY_PYTHON_PYRAMID_EXCLUDED_URLS`` with comma delimited regexes representing which URLs to exclude.
+
+For example, 
+
+::
+
+    export OPENTELEMETRY_PYTHON_PYRAMID_EXCLUDED_URLS="client/.*/info,healthcheck"
+
+will exclude requests such as ``https://site/client/123/info`` and ``https://site/xyz/healthcheck``.
 
 References
 ----------

Diff for: ext/opentelemetry-ext-pyramid/src/opentelemetry/ext/pyramid/callbacks.py

@@ -8,7 +8,7 @@
 import opentelemetry.ext.wsgi as otel_wsgi
 from opentelemetry import configuration, context, propagators, trace
 from opentelemetry.ext.pyramid.version import __version__
-from opentelemetry.util import disable_trace, time_ns
+from opentelemetry.util import ExcludeList, time_ns
 
 TWEEN_NAME = "opentelemetry.ext.pyramid.trace_tween_factory"
 SETTING_TRACE_ENABLED = "opentelemetry-pyramid.trace_enabled"
@@ -22,22 +22,14 @@
 _logger = getLogger(__name__)
 
 
-def get_excluded_hosts():
-    hosts = configuration.Configuration().PYRAMID_EXCLUDED_HOSTS or []
-    if hosts:
-        hosts = str.split(hosts, ",")
-    return hosts
+def get_excluded_urls():
+    urls = configuration.Configuration().PYRAMID_EXCLUDED_URLS or []
+    if urls:
+        urls = str.split(urls, ",")
+    return ExcludeList(urls)
 
 
-def get_excluded_paths():
-    paths = configuration.Configuration().PYRAMID_EXCLUDED_PATHS or []
-    if paths:
-        paths = str.split(paths, ",")
-    return paths
-
-
-_excluded_hosts = get_excluded_hosts()
-_excluded_paths = get_excluded_paths()
+_excluded_urls = get_excluded_urls()
 
 
 def includeme(config):
@@ -119,7 +111,7 @@ def disabled_tween(request):
 
     # make a request tracing function
     def trace_tween(request):
-        if disable_trace(request.url, _excluded_hosts, _excluded_paths):
+        if _excluded_urls.url_disabled(request.url):
             request.environ[_ENVIRON_ENABLED_KEY] = False
             # short-circuit when we don't want to trace anything
             return handler(request)

Diff for: ext/opentelemetry-ext-pyramid/tests/test_programmatic.py

@@ -20,6 +20,7 @@
 from opentelemetry.ext.pyramid import PyramidInstrumentor
 from opentelemetry.test.test_base import TestBase
 from opentelemetry.test.wsgitestutil import WsgiTestBase
+from opentelemetry.util import ExcludeList
 
 # pylint: disable=import-error
 from .pyramid_base_test import InstrumentationTest
@@ -169,12 +170,8 @@ def test_warnings(self, mock_logger):
         self.assertEqual(mock_logger.warning.called, True)
 
     @patch(
-        "opentelemetry.ext.pyramid.callbacks._excluded_hosts",
-        ["http://localhost/excluded_arg/123"],
-    )
-    @patch(
-        "opentelemetry.ext.pyramid.callbacks._excluded_paths",
-        ["excluded_noarg"],
+        "opentelemetry.ext.pyramid.callbacks._excluded_urls",
+        ExcludeList(["http://localhost/excluded_arg/123", "excluded_noarg"]),
     )
     def test_exclude_lists(self):
         self.client.get("/excluded_arg/123")