Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exclude lists for flask integration #630

Merged
merged 14 commits into from
May 3, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ext/opentelemetry-ext-flask/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

- Add blacklisting for paths and hosts
lzchen marked this conversation as resolved.
Show resolved Hide resolved
([#6300](https://github.com/open-telemetry/opentelemetry-python/pull/630))
lzchen marked this conversation as resolved.
Show resolved Hide resolved

## 0.6b0

Released 2020-03-30
Expand Down
10 changes: 10 additions & 0 deletions ext/opentelemetry-ext-flask/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ Installation

pip install opentelemetry-ext-flask

Configuration
-------------

Blacklist
lzchen marked this conversation as resolved.
Show resolved Hide resolved
*********
Excludes certain hosts and paths from being tracked. Pass in comma delimited string into environment variables.

Blacklisted hosts: OPENTELEMETRY_PYTHON_FLASK_BLACKLIST_HOSTS
Blacklisted paths: OPENTELEMETRY_PYTHON_FLASK_BLACKLIST_PATHS
lzchen marked this conversation as resolved.
Show resolved Hide resolved


References
----------
Expand Down
51 changes: 38 additions & 13 deletions ext/opentelemetry-ext-flask/src/opentelemetry/ext/flask/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,14 @@ def hello():
import flask

import opentelemetry.ext.wsgi as otel_wsgi
from opentelemetry import context, propagators, trace
from opentelemetry import configuration, context, propagators, trace
from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.ext.flask.version import __version__
from opentelemetry.util import time_ns
from opentelemetry.util import (
disable_tracing_hostname,
disable_tracing_url,
time_ns
)

logger = logging.getLogger(__name__)

Expand All @@ -80,17 +84,18 @@ 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
Expand All @@ -102,6 +107,9 @@ def _start_response(status, response_headers, *args, **kwargs):

@self.before_request
def _before_flask_request():
# Do not trace if the url is blacklisted
lzchen marked this conversation as resolved.
Show resolved Hide resolved
if _disable_trace(flask.request.url):
return
environ = flask.request.environ
span_name = (
flask.request.endpoint
Expand Down Expand Up @@ -132,6 +140,9 @@ def _before_flask_request():

@self.teardown_request
def _teardown_flask_request(exc):
# Do not trace if the url is blacklisted
lzchen marked this conversation as resolved.
Show resolved Hide resolved
if _disable_trace(flask.request.url):
return
activation = flask.request.environ.get(_ENVIRON_ACTIVATION_KEY)
if not activation:
logger.warning(
Expand All @@ -150,6 +161,20 @@ def _teardown_flask_request(exc):
context.detach(flask.request.environ.get(_ENVIRON_TOKEN))


def _disable_trace(url):
blacklist_hosts = configuration.Configuration().FLASK_BLACKLIST_HOSTS
blacklist_paths = configuration.Configuration().FLASK_BLACKLIST_PATHS
if blacklist_hosts:
blacklist_hosts = str.split(blacklist_hosts, ',')
if disable_tracing_hostname(url, blacklist_hosts):
return True
if blacklist_paths:
blacklist_paths = str.split(blacklist_paths, ',')
if disable_tracing_url(url, blacklist_paths):
lzchen marked this conversation as resolved.
Show resolved Hide resolved
return True
return False


class FlaskInstrumentor(BaseInstrumentor):
"""A instrumentor for flask.Flask

Expand Down
35 changes: 32 additions & 3 deletions ext/opentelemetry-ext-flask/tests/test_flask_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@
# limitations under the License.

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.test.wsgitestutil import WsgiTestBase


Expand All @@ -45,18 +47,31 @@ 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
lzchen marked this conversation as resolved.
Show resolved Hide resolved
Configuration.__slots__ = []
self.app = Flask(__name__)

def hello_endpoint(helloid):
if helloid == 500:
raise ValueError(":-(")
return "Hello: " + str(helloid)

def blacklist_endpoint():
return "blacklist"

def blacklist2_endpoint():
return "blacklist2"
lzchen marked this conversation as resolved.
Show resolved Hide resolved

self.app.route("/hello/<int:helloid>")(hello_endpoint)
self.app.route("/blacklist")(blacklist_endpoint)
self.app.route("/blacklist2")(blacklist2_endpoint)
lzchen marked this conversation as resolved.
Show resolved Hide resolved

self.client = Client(self.app, BaseResponse)

def tearDown(self):
Configuration._instance = None # pylint:disable=protected-access
Configuration.__slots__ = []

def test_only_strings_in_environ(self):
"""
Some WSGI servers (such as Gunicorn) expect keys in the environ object
Expand All @@ -81,8 +96,7 @@ def test_simple(self):
{"http.target": "/hello/123", "http.route": "/hello/<int:helloid>"}
)
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")
Expand Down Expand Up @@ -126,6 +140,21 @@ def test_internal_error(self):
self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER)
self.assertEqual(span_list[0].attributes, expected_attrs)

@patch.dict(
"os.environ", # type: ignore
{
"OPENTELEMETRY_PYTHON_FLASK_BLACKLIST_HOSTS": "http://localhost/blacklist",
"OPENTELEMETRY_PYTHON_FLASK_BLACKLIST_PATHS": "blacklist2",
},
)
def test_blacklist_path(self):
self.client.get("/hello/123")
self.client.get("/blacklist")
self.client.get("/blacklist2")
lzchen marked this conversation as resolved.
Show resolved Hide resolved
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()
40 changes: 40 additions & 0 deletions opentelemetry-api/src/opentelemetry/util/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import re
import time
from logging import getLogger
from typing import Union
Expand Down Expand Up @@ -48,3 +49,42 @@ def _load_provider(provider: str) -> Union["TracerProvider", "MeterProvider"]:
"Failed to load configured provider %s", provider,
)
raise

# Pattern for matching the 'https://', 'http://', 'ftp://' part.
URL_PATTERN = '^(https?|ftp):\\/\\/'


def disable_tracing_url(url, blacklist_paths):
"""Disable tracing on the provided blacklist paths

If the url path starts with the blacklisted path, return True.

:type blacklist_paths: list
:param blacklist_paths: Paths to prevent from tracing

:rtype: bool
:returns: True if not tracing, False if tracing
"""
lzchen marked this conversation as resolved.
Show resolved Hide resolved
# Remove the 'https?|ftp://' if exists
url = re.sub(URL_PATTERN, '', url)

# Split the url by the first '/' and get the path part
url_path = url.split('/', 1)[1]

for path in blacklist_paths:
lzchen marked this conversation as resolved.
Show resolved Hide resolved
if url_path.startswith(path):
return True

return False


def disable_tracing_hostname(url, blacklist_hostnames):
"""Disable tracing for the provided blacklist URLs.

:type blacklist_hostnames: list
:param blacklist_hostnames: URLs to prevent tracing

:rtype: bool
:returns: True if not tracing, False if tracing
"""
return url in blacklist_hostnames
lzchen marked this conversation as resolved.
Show resolved Hide resolved