-
Notifications
You must be signed in to change notification settings - Fork 635
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
Changes from 13 commits
44e8a9d
a08a4c4
97b57de
3d25b53
d2ab9f7
0c913e1
1c97d72
f199d74
043fe16
4c1fc68
67a1cd2
472b86c
d7045b5
8197d2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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_path, | ||
time_ns, | ||
) | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
@@ -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 | ||
|
@@ -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 excluded | ||
if _disable_trace(flask.request.url): | ||
return | ||
environ = flask.request.environ | ||
span_name = ( | ||
flask.request.endpoint | ||
|
@@ -132,6 +140,9 @@ def _before_flask_request(): | |
|
||
@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( | ||
|
@@ -150,6 +161,20 @@ def _teardown_flask_request(exc): | |
context.detach(flask.request.environ.get(_ENVIRON_TOKEN)) | ||
|
||
|
||
def _disable_trace(url): | ||
excluded_hosts = configuration.Configuration().FLASK_EXCLUDED_HOSTS | ||
excluded_paths = configuration.Configuration().FLASK_EXCLUDED_PATHS | ||
Comment on lines
+165
to
+166
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this separation necessary? I think we can just define a sequence of regular expressions in one environment variable and if the hostname or path matches any regular expression it is rejected. This will be useful for the end user who would not need to repeat a lot of related paths in the environment variable, but use a single regex that matches them all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I worry a little bit about that, as it makes it hard if you happen to have a host or path that share the same name. Horrible example, but what about the following: host: localhost and you only want to filter for the localhost host. |
||
if excluded_hosts: | ||
excluded_hosts = str.split(excluded_hosts, ",") | ||
if disable_tracing_hostname(url, excluded_hosts): | ||
return True | ||
if excluded_paths: | ||
excluded_paths = str.split(excluded_paths, ",") | ||
if disable_tracing_path(url, excluded_paths): | ||
return True | ||
return False | ||
|
||
|
||
class FlaskInstrumentor(BaseInstrumentor): | ||
"""A instrumentor for flask.Flask | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to be explained here that the values of these environment variables are regexes that should match with everything but the
r"(https?|ftp)://.*?/"
(are there any other protocols besideshttp
,https
andftp
that we want to take into consideration?) part of the URL the user wants excluded from tracing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've included an explanation of what hosts and paths means. I don't think we need to inform the user of how the implementation is actually doing this. As for other protocols, these are the ones being excluded in OpenCensus, any new protocols I feel can be added later on.