Skip to content
Merged
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
4 changes: 3 additions & 1 deletion ddtrace/contrib/flask/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,9 @@ def _traced_request(pin, wrapped, instance, args, kwargs):

if not span.get_tag(FLASK_VIEW_ARGS) and request.view_args and config.flask.get("collect_view_args"):
for k, v in request.view_args.items():
span._set_str_tag(u".".join((FLASK_VIEW_ARGS, k)), v)
# DEV: Do not use `_set_str_tag` here since view args can be string/int/float/path/uuid/etc
# https://flask.palletsprojects.com/en/1.1.x/api/#url-route-registrations
span.set_tag(u".".join((FLASK_VIEW_ARGS, k)), v)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to cache the result of these string concats?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it might. we can handle that in a different PR

except Exception:
log.debug('failed to set tags for "flask.request" span', exc_info=True)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
fixes:
- |
Fixes error with tagging non-string Flask view args.
47 changes: 47 additions & 0 deletions tests/contrib/flask/test_request.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# -*- coding: utf-8 -*-
import json

import flask
from flask import abort
from flask import jsonify
from flask import make_response

from ddtrace.constants import ANALYTICS_SAMPLE_RATE_KEY
Expand Down Expand Up @@ -82,6 +85,50 @@ def index():
self.assertEqual(handler_span.resource, "/")
self.assertEqual(req_span.error, 0)

def test_route_params_request(self):
"""
When making a request to an endpoint with non-string url params
We create the expected spans
"""

@self.app.route("/route_params/<first>/<int:second>/<float:third>/<path:fourth>")
def route_params(first, second, third, fourth):
return jsonify(
{
"first": first,
"second": second,
"third": third,
"fourth": fourth,
}
)

res = self.client.get("/route_params/test/100/5.5/some/sub/path")
self.assertEqual(res.status_code, 200)
if isinstance(res.data, bytes):
data = json.loads(res.data.decode())
else:
data = json.loads(res.data)

assert data == {
"first": "test",
"second": 100,
"third": 5.5,
"fourth": "some/sub/path",
}

spans = self.get_spans()
self.assertEqual(len(spans), 8)

root = spans[0]
assert root.name == "flask.request"
assert root.get_tag(http.URL) == "http://localhost/route_params/test/100/5.5/some/sub/path"
assert root.get_tag("flask.endpoint") == "route_params"
assert root.get_tag("flask.url_rule") == "/route_params/<first>/<int:second>/<float:third>/<path:fourth>"
assert root.get_tag("flask.view_args.first") == "test"
assert root.get_metric("flask.view_args.second") == 100.0
assert root.get_metric("flask.view_args.third") == 5.5
assert root.get_tag("flask.view_args.fourth") == "some/sub/path"

def test_request_query_string_trace(self):
"""Make sure when making a request that we create the expected spans and capture the query string."""

Expand Down