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

Update Flask middleware to not use full URL with query params as a span name #725

Closed
Show file tree
Hide file tree
Changes from all 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
11 changes: 11 additions & 0 deletions contrib/opencensus-ext-flask/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# Changelog

## in development

- Don't use full URL with query parameters for the span name and span HTTP_URL
span attribute.

Query params can contain sensitive values which shouldn't be logged. Now just
the url without the query parameters is used.

Before: ``http://example.com/path/bar?foo=bar&bar=baz``, now:
``http://example.com/path/bar``.

## Unreleased
- Make ProbabilitySampler default

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def _before_request(self):
# Set the span name as the name of the current module name
span.name = '[{}]{}'.format(
flask.request.method,
flask.request.url)
flask.request.base_url)
tracer.add_attribute_to_current_span(
HTTP_METHOD, flask.request.method)
tracer.add_attribute_to_current_span(
Expand Down
30 changes: 30 additions & 0 deletions contrib/opencensus-ext-flask/tests/test_flask_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,36 @@ def test_header_is_none(self):
self.assertEqual(span.attributes, expected_attributes)
assert isinstance(span.parent_span, base.NullContextManager)

def test_query_params_are_not_included_as_part_of_span_name(self):
# query params can contain sensitive data so they are not included as
# part of span name
app = self.create_app()
flask_middleware.FlaskMiddleware(app=app,
sampler=samplers.AlwaysOnSampler())
context = app.test_request_context(
path='/path/value?foo=bar&bar=baz')

with context:
app.preprocess_request()
tracer = execution_context.get_opencensus_tracer()
self.assertIsNotNone(tracer)

span = tracer.current_span()

expected_attributes = {
# NOTE: Query params need to be include as per spec
# https://github.com/census-instrumentation/opencensus-specs
# TODO: Open feedback PR to spec and suggestion making query
# params optional since they can contain sensitive data
'http.url': u'http://localhost/path/value?foo=bar&bar=baz',
'http.method': 'GET',
}

self.assertEqual(span.name,
'[GET]http://localhost/path/value')
self.assertEqual(span.attributes, expected_attributes)
assert isinstance(span.parent_span, base.NullContextManager)

def test__after_request_not_sampled(self):
flask_trace_header = 'traceparent'
trace_id = '2dd43a1d6b2549c6bc2a1a54c2fc0b05'
Expand Down