fix: Flask incorrect resource naming when error thrown in middleware (#3274) (#3284)

Before this change, if an error was thrown in the middleware, we would just get <method> <status_code> as the resource name. With this change, we get <method> <path> which means the error will be grouped with the rest of the endpoint's hits/errors.

Co-authored-by: Brett Langdon <[email protected]>
Co-authored-by: Kyle Verhoog <[email protected]>
(cherry picked from commit 706a1d3)

Co-authored-by: Zachary Groves <[email protected]>
Co-authored-by: Brett Langdon <[email protected]>

Author: 3 people

ddtrace/contrib/flask/patch.py

@@ -262,6 +262,8 @@ def unpatch():
 def _wrap_start_response(func, span, request):
     def traced_start_response(status_code, headers):
         code, _, _ = status_code.partition(" ")
+        # If values are accessible, set the resource as `<method> <path>` and add other request tags
+        _set_request_tags(span)
 
         # Override root span resource name to be `<method> 404` for 404 requests
         # DEV: We do this because we want to make it easier to see all unknown requests together
@@ -467,25 +469,9 @@ def _traced_request(pin, wrapped, instance, args, kwargs):
         if not pin.enabled or not span:
             return wrapped(*args, **kwargs)
 
-        try:
-            request = flask._request_ctx_stack.top.request
-
-            # DEV: This name will include the blueprint name as well (e.g. `bp.index`)
-            if not span.get_tag(FLASK_ENDPOINT) and request.endpoint:
-                span.resource = u" ".join((request.method, request.endpoint))
-                span._set_str_tag(FLASK_ENDPOINT, request.endpoint)
-
-            if not span.get_tag(FLASK_URL_RULE) and request.url_rule and request.url_rule.rule:
-                span.resource = u" ".join((request.method, request.url_rule.rule))
-                span._set_str_tag(FLASK_URL_RULE, request.url_rule.rule)
-
-            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():
-                    # 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)
-        except Exception:
-            log.debug('failed to set tags for "flask.request" span', exc_info=True)
+        # This call may be unnecessary since we try to add the tags earlier
+        # We just haven't been able to confirm this yet
+        _set_request_tags(span)
 
         with pin.tracer.trace(
             ".".join(("flask", name)), service=trace_utils.int_service(pin, config.flask, pin)
@@ -518,3 +504,25 @@ def traced_jsonify(wrapped, instance, args, kwargs):
 
     with pin.tracer.trace("flask.jsonify"):
         return wrapped(*args, **kwargs)
+
+
+def _set_request_tags(span):
+    try:
+        request = flask._request_ctx_stack.top.request
+
+        # DEV: This name will include the blueprint name as well (e.g. `bp.index`)
+        if not span.get_tag(FLASK_ENDPOINT) and request.endpoint:
+            span.resource = u" ".join((request.method, request.endpoint))
+            span._set_str_tag(FLASK_ENDPOINT, request.endpoint)
+
+        if not span.get_tag(FLASK_URL_RULE) and request.url_rule and request.url_rule.rule:
+            span.resource = u" ".join((request.method, request.url_rule.rule))
+            span._set_str_tag(FLASK_URL_RULE, request.url_rule.rule)
+
+        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():
+                # 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)
+    except Exception:
+        log.debug('failed to set tags for "flask.request" span', exc_info=True)

releasenotes/notes/flask_improve_resource_naming_when_error_occurs-ea094145551e4ec7.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    flask: fix resource naming of request span when errors occur in middleware.

tests/contrib/flask/test_request.py

@@ -887,6 +887,19 @@ def test_http_request_header_tracing(self):
         assert span.get_tag("http.request.headers.my-header") == "my_value"
         assert span.get_tag("http.request.headers.host") == "localhost"
 
+    def test_correct_resource_when_middleware_error(self):
+        @self.app.route("/helloworld")
+        @self.app.before_first_request
+        def error():
+            raise Exception()
+
+        self.client.get("/helloworld")
+
+        spans = self.get_spans()
+        req_span = spans[0]
+        assert req_span.resource == "GET /helloworld"
+        assert req_span.get_tag("http.status_code") == "500"
+
     def test_http_response_header_tracing(self):
         @self.app.route("/response_headers")
         def response_headers():