contrib/starlette: fix outcome for 500 responses without exception (#…

…2060)

* contrib/starlette: fix outcome for 500 responses without exception

Set the outcome based on the status code of the http.response.start
ASGI message instead of relying only on the raise of an exception.

* contrib/asgi: fix outcome for 500 responses without exception

Set the outcome based on the status code of the http.response.start
ASGI message instead of relying only on the raise of an exception.

elasticapm/contrib/asgi.py

@@ -50,6 +50,7 @@ async def wrapped_send(message) -> None:
             await set_context(lambda: middleware.get_data_from_response(message, constants.TRANSACTION), "response")
             result = "HTTP {}xx".format(message["status"] // 100)
             elasticapm.set_transaction_result(result, override=False)
+            elasticapm.set_transaction_outcome(http_status_code=message["status"], override=False)
         await send(message)
 
     return wrapped_send

elasticapm/contrib/starlette/__init__.py

@@ -147,6 +147,7 @@ async def wrapped_send(message) -> None:
                 )
                 result = "HTTP {}xx".format(message["status"] // 100)
                 elasticapm.set_transaction_result(result, override=False)
+                elasticapm.set_transaction_outcome(http_status_code=message["status"], override=False)
             await send(message)
 
         _mocked_receive = None

tests/contrib/asgi/app.py

@@ -59,3 +59,8 @@ async def boom() -> None:
 @app.route("/body")
 async def json():
     return jsonify({"hello": "world"})
+
+
+@app.route("/500", methods=["GET"])
+async def error():
+    return "KO", 500

tests/contrib/asgi/asgi_tests.py

@@ -66,6 +66,23 @@ async def test_transaction_span(instrumented_app, elasticapm_client):
     assert span["sync"] == False
 
 
+@pytest.mark.asyncio
+async def test_transaction_span(instrumented_app, elasticapm_client):
+    async with async_asgi_testclient.TestClient(instrumented_app) as client:
+        resp = await client.get("/500")
+        assert resp.status_code == 500
+        assert resp.text == "KO"
+
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
+    assert len(elasticapm_client.events[constants.SPAN]) == 0
+    transaction = elasticapm_client.events[constants.TRANSACTION][0]
+    assert transaction["name"] == "GET unknown route"
+    assert transaction["result"] == "HTTP 5xx"
+    assert transaction["outcome"] == "failure"
+    assert transaction["context"]["request"]["url"]["full"] == "/500"
+    assert transaction["context"]["response"]["status_code"] == 500
+
+
 @pytest.mark.asyncio
 async def test_transaction_ignore_url(instrumented_app, elasticapm_client):
     elasticapm_client.config.update("1", transaction_ignore_urls="/foo*")

tests/contrib/asyncio/starlette_tests.py

@@ -110,6 +110,10 @@ async def with_slash(request):
     async def without_slash(request):
         return PlainTextResponse("Hi {}".format(request.path_params["name"]))
 
+    @app.route("/500/", methods=["GET"])
+    async def with_500_status_code(request):
+        return PlainTextResponse("Oops", status_code=500)
+
     @sub.route("/hi")
     async def hi_from_sub(request):
         return PlainTextResponse("sub")
@@ -236,6 +240,27 @@ def test_exception(app, elasticapm_client):
     assert error["context"]["request"] == transaction["context"]["request"]
 
 
+def test_failure_outcome_with_500_status_code(app, elasticapm_client):
+    client = TestClient(app)
+
+    client.get("/500/")
+
+    assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
+    transaction = elasticapm_client.events[constants.TRANSACTION][0]
+    spans = elasticapm_client.spans_for_transaction(transaction)
+    assert len(spans) == 0
+
+    assert transaction["name"] == "GET /500/"
+    assert transaction["result"] == "HTTP 5xx"
+    assert transaction["outcome"] == "failure"
+    assert transaction["type"] == "request"
+    request = transaction["context"]["request"]
+    assert request["method"] == "GET"
+    assert transaction["context"]["response"]["status_code"] == 500
+
+    assert len(elasticapm_client.events[constants.ERROR]) == 0
+
+
 @pytest.mark.parametrize("header_name", [constants.TRACEPARENT_HEADER_NAME, constants.TRACEPARENT_LEGACY_HEADER_NAME])
 def test_traceparent_handling(app, elasticapm_client, header_name):
     client = TestClient(app)