Skip to content

Commit 5fd6662

Browse files
committed
contrib/starlette: fix outcome for 500 responses without exception (elastic#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.
1 parent 8a4e9c9 commit 5fd6662

File tree

5 files changed

+49
-0
lines changed

5 files changed

+49
-0
lines changed

elasticapm/contrib/asgi.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ async def wrapped_send(message) -> None:
5050
await set_context(lambda: middleware.get_data_from_response(message, constants.TRANSACTION), "response")
5151
result = "HTTP {}xx".format(message["status"] // 100)
5252
elasticapm.set_transaction_result(result, override=False)
53+
elasticapm.set_transaction_outcome(http_status_code=message["status"], override=False)
5354
await send(message)
5455

5556
return wrapped_send

elasticapm/contrib/starlette/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ async def wrapped_send(message) -> None:
147147
)
148148
result = "HTTP {}xx".format(message["status"] // 100)
149149
elasticapm.set_transaction_result(result, override=False)
150+
elasticapm.set_transaction_outcome(http_status_code=message["status"], override=False)
150151
await send(message)
151152

152153
_mocked_receive = None

tests/contrib/asgi/app.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,8 @@ async def boom() -> None:
5959
@app.route("/body")
6060
async def json():
6161
return jsonify({"hello": "world"})
62+
63+
64+
@app.route("/500", methods=["GET"])
65+
async def error():
66+
return "KO", 500

tests/contrib/asgi/asgi_tests.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,23 @@ async def test_transaction_span(instrumented_app, elasticapm_client):
6666
assert span["sync"] == False
6767

6868

69+
@pytest.mark.asyncio
70+
async def test_transaction_span(instrumented_app, elasticapm_client):
71+
async with async_asgi_testclient.TestClient(instrumented_app) as client:
72+
resp = await client.get("/500")
73+
assert resp.status_code == 500
74+
assert resp.text == "KO"
75+
76+
assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
77+
assert len(elasticapm_client.events[constants.SPAN]) == 0
78+
transaction = elasticapm_client.events[constants.TRANSACTION][0]
79+
assert transaction["name"] == "GET unknown route"
80+
assert transaction["result"] == "HTTP 5xx"
81+
assert transaction["outcome"] == "failure"
82+
assert transaction["context"]["request"]["url"]["full"] == "/500"
83+
assert transaction["context"]["response"]["status_code"] == 500
84+
85+
6986
@pytest.mark.asyncio
7087
async def test_transaction_ignore_url(instrumented_app, elasticapm_client):
7188
elasticapm_client.config.update("1", transaction_ignore_urls="/foo*")

tests/contrib/asyncio/starlette_tests.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ async def with_slash(request):
110110
async def without_slash(request):
111111
return PlainTextResponse("Hi {}".format(request.path_params["name"]))
112112

113+
@app.route("/500/", methods=["GET"])
114+
async def with_500_status_code(request):
115+
return PlainTextResponse("Oops", status_code=500)
116+
113117
@sub.route("/hi")
114118
async def hi_from_sub(request):
115119
return PlainTextResponse("sub")
@@ -236,6 +240,27 @@ def test_exception(app, elasticapm_client):
236240
assert error["context"]["request"] == transaction["context"]["request"]
237241

238242

243+
def test_failure_outcome_with_500_status_code(app, elasticapm_client):
244+
client = TestClient(app)
245+
246+
client.get("/500/")
247+
248+
assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
249+
transaction = elasticapm_client.events[constants.TRANSACTION][0]
250+
spans = elasticapm_client.spans_for_transaction(transaction)
251+
assert len(spans) == 0
252+
253+
assert transaction["name"] == "GET /500/"
254+
assert transaction["result"] == "HTTP 5xx"
255+
assert transaction["outcome"] == "failure"
256+
assert transaction["type"] == "request"
257+
request = transaction["context"]["request"]
258+
assert request["method"] == "GET"
259+
assert transaction["context"]["response"]["status_code"] == 500
260+
261+
assert len(elasticapm_client.events[constants.ERROR]) == 0
262+
263+
239264
@pytest.mark.parametrize("header_name", [constants.TRACEPARENT_HEADER_NAME, constants.TRACEPARENT_LEGACY_HEADER_NAME])
240265
def test_traceparent_handling(app, elasticapm_client, header_name):
241266
client = TestClient(app)

0 commit comments

Comments
 (0)