Repository: scoutapp/scout_apm_python
Pull request #388 (2019-11-23 17:26:08)

Tag Starlette 5XX responses as errors
Ref #383.


Patch:
diff --git a/src/scout_apm/async_/starlette.py b/src/scout_apm/async_/starlette.py
index 5d1d36b..a004d28 100644
--- a/src/scout_apm/async_/starlette.py
+++ b/src/scout_apm/async_/starlette.py
@@ -75,11 +75,12 @@ class ScoutMiddleware:
                 tracked_request.tag("username", username)
 
         async def wrapped_send(data):
-            # Finish HTTP span when body finishes sending, not later (e.g.
-            # after background tasks)
-            if data.get("type", None) == "http.response.body" and not data.get(
-                "more_body", False
-            ):
+            type_ = data.get("type", None)
+            if type_ == "http.response.start" and 500 <= data.get("status", 200) <= 599:
+                tracked_request.tag("error", "true")
+            elif type_ == "http.response.body" and not data.get("more_body", False):
+                # Finish HTTP span when body finishes sending, not later (e.g.
+                # after background tasks)
                 grab_extra_data()
                 tracked_request.stop_span()
             return await send(data)
diff --git a/src/scout_apm/pyramid.py b/src/scout_apm/pyramid.py
index ad765bb..218447d 100644
--- a/src/scout_apm/pyramid.py
+++ b/src/scout_apm/pyramid.py
@@ -66,8 +66,8 @@ def instruments(handler, registry):
                 try:
                     response = handler(request)
                 finally:
-                    # Routing further down the call chain. So time it starting
-                    # above, but only name it if it gets a name
+                    # Routing lives further down the call chain. So time it
+                    # starting above, but only set the name if it gets a name
                     if request.matched_route is not None:
                         tracked_request.is_real_request = True
                         span.operation = "Controller/" + request.matched_route.name
@@ -75,6 +75,8 @@ def instruments(handler, registry):
                 tracked_request.tag("error", "true")
                 raise
 
+            if 500 <= response.status_code <= 599:
+                tracked_request.tag("error", "true")
         finally:
             tracked_request.stop_span()
 


Test patch:
diff --git a/tests/integration/test_pyramid.py b/tests/integration/test_pyramid.py
index 012dde4..3c28b76 100644
--- a/tests/integration/test_pyramid.py
+++ b/tests/integration/test_pyramid.py
@@ -40,6 +40,9 @@ def app_with_scout(config=None):
     def crash(request):
         raise ValueError("BØØM!")  # non-ASCII
 
+    def return_error(request):
+        return Response("Something went wrong", status=503)
+
     with Configurator() as configurator:
         configurator.add_route("home", "/")
         configurator.add_view(home, route_name="home", request_method="GET")
@@ -47,6 +50,8 @@ def app_with_scout(config=None):
         configurator.add_view(hello, route_name="hello")
         configurator.add_route("crash", "/crash/")
         configurator.add_view(crash, route_name="crash")
+        configurator.add_route("return_error", "/return-error/")
+        configurator.add_view(return_error, route_name="return_error")
 
         # Setup according to https://docs.scoutapm.com/#pyramid
         configurator.add_settings(**config)
@@ -180,6 +185,21 @@ def test_server_error(tracked_requests):
     assert span.operation == "Controller/crash"
 
 
+def test_return_error(tracked_requests):
+    with app_with_scout() as app:
+        response = TestApp(app).get("/return-error/", expect_errors=True)
+
+    assert response.status_int == 503
+    assert response.text == "Something went wrong"
+    assert len(tracked_requests) == 1
+    tracked_request = tracked_requests[0]
+    assert tracked_request.tags["path"] == "/return-error/"
+    assert tracked_request.tags["error"] == "true"
+    assert len(tracked_request.complete_spans) == 1
+    span = tracked_request.complete_spans[0]
+    assert span.operation == "Controller/return_error"
+
+
 def test_no_monitor(tracked_requests):
     # With an empty config, "SCOUT_MONITOR" defaults to False.
     with app_with_scout({}) as app:
diff --git a/tests/integration/test_starlette_py36plus.py b/tests/integration/test_starlette_py36plus.py
index 9d3f8bc..471671f 100644
--- a/tests/integration/test_starlette_py36plus.py
+++ b/tests/integration/test_starlette_py36plus.py
@@ -58,6 +58,10 @@ def app_with_scout(*, app=None, scout_config=None):
     async def crash(request):
         raise ValueError("BØØM!")  # non-ASCII
 
+    @app.route("/return-error/")
+    async def return_error(request):
+        return PlainTextResponse("Something went wrong", status_code=503)
+
     @app.route("/background-jobs/")
     async def background_jobs(request):
         def sync_noop():
@@ -279,6 +283,22 @@ async def test_server_error(tracked_requests):
     )
 
 
+@async_test
+async def test_return_error(tracked_requests):
+    with app_with_scout() as app:
+        communicator = ApplicationCommunicator(app, get_scope(path="/return-error/"))
+        await communicator.send_input({"type": "http.request"})
+        response_start = await communicator.receive_output()
+        await communicator.receive_output()
+
+    assert response_start["type"] == "http.response.start"
+    assert response_start["status"] == 503
+    tracked_request = tracked_requests[0]
+    assert len(tracked_request.complete_spans) == 1
+    assert tracked_request.tags["path"] == "/return-error/"
+    assert tracked_request.tags["error"] == "true"
+
+
 @async_test
 async def test_no_monitor(tracked_requests):
     with app_with_scout(scout_config={"monitor": False}) as app: