Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Starlette: use route name as transaction name if available #957

Merged
merged 7 commits into from
Nov 17, 2020
35 changes: 34 additions & 1 deletion elasticapm/contrib/starlette/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
from starlette.requests import Request
from starlette.responses import Response
from starlette.routing import Match
from starlette.types import ASGIApp

import elasticapm
Expand Down Expand Up @@ -181,7 +182,8 @@ async def _request_started(self, request: Request):
await set_context(
lambda: get_data_from_request(request, self.client.config, constants.TRANSACTION), "request"
)
elasticapm.set_transaction_name("{} {}".format(request.method, request.url.path), override=False)
transaction_name = self.get_route_name(request) or request.url.path
elasticapm.set_transaction_name("{} {}".format(request.method, transaction_name), override=False)

async def _request_finished(self, response: Response):
"""Captures the end of the request processing to APM.
Expand All @@ -195,3 +197,34 @@ async def _request_finished(self, response: Response):

result = "HTTP {}xx".format(response.status_code // 100)
elasticapm.set_transaction_result(result, override=False)

def get_route_name(self, request: Request) -> str:
route_name = None
app = request.app
scope = request.scope
routes = app.routes

for route in routes:
match, _ = route.matches(scope)
if match == Match.FULL:
route_name = route.path
break
elif match == Match.PARTIAL and route_name is None:
route_name = route.path
# Starlette magically redirects requests if the path matches a route name with a trailing slash
# appended or removed. To not spam the transaction names list, we do the same here and put these
# redirects all in the same "redirect trailing slashes" transaction name
if not route_name and app.router.redirect_slashes and scope["path"] != "/":
redirect_scope = dict(scope)
if scope["path"].endswith("/"):
redirect_scope["path"] = scope["path"][:-1]
trim = True
else:
redirect_scope["path"] = scope["path"] + "/"
trim = False
for route in routes:
match, _ = route.matches(redirect_scope)
if match != Match.NONE:
route_name = route.path + "/" if trim else route.path[:-1]
break
return route_name
42 changes: 42 additions & 0 deletions tests/contrib/asyncio/starlette_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ async def hi(request):
pass
return PlainTextResponse("ok")

@app.route("/hi/{name}", methods=["GET"])
async def hi_name(request):
name = request.path_params["name"]
return PlainTextResponse("Hi {}".format(name))

@app.route("/hello", methods=["GET", "POST"])
async def hello(request):
with async_capture_span("test"):
Expand All @@ -65,6 +70,14 @@ async def hello(request):
async def raise_exception(request):
raise ValueError()

@app.route("/hi/{name}/with/slash/", methods=["GET", "POST"])
async def with_slash(request):
return PlainTextResponse("Hi {}".format(request.path_params["name"]))

@app.route("/hi/{name}/without/slash", methods=["GET", "POST"])
async def without_slash(request):
return PlainTextResponse("Hi {}".format(request.path_params["name"]))

app.add_middleware(ElasticAPM, client=elasticapm_client)

return app
Expand Down Expand Up @@ -226,3 +239,32 @@ def test_starlette_transaction_ignore_urls(app, elasticapm_client):
elasticapm_client.config.update(1, transaction_ignore_urls="/*ello,/world")
response = client.get("/hello")
assert len(elasticapm_client.events[constants.TRANSACTION]) == 1


def test_transaction_name_is_route(app, elasticapm_client):
client = TestClient(app)

response = client.get("/hi/shay")

assert response.status_code == 200

assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
transaction = elasticapm_client.events[constants.TRANSACTION][0]
assert transaction["name"] == "GET /hi/{name}"
assert transaction["context"]["request"]["url"]["pathname"] == "/hi/shay"


@pytest.mark.parametrize(
"url,expected",
(
("/hi/shay/with/slash", "GET /hi/{name}/with/slash"),
("/hi/shay/without/slash/", "GET /hi/{name}/without/slash/"),
),
)
def test_trailing_slash_redirect_detection(app, elasticapm_client, url, expected):
client = TestClient(app)
response = client.get(url, allow_redirects=False)
assert response.status_code == 307
assert len(elasticapm_client.events[constants.TRANSACTION]) == 1
for transaction in elasticapm_client.events[constants.TRANSACTION]:
assert transaction["name"] == expected