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

* Starlette: use route name as transaction name if available

Unfortunately, we need to do some matching here that Starlette does
too, but currently isn't exposed. If/when encode/starlette#804
is merged, we can re-use the route from there.

closes #833

* detect trailing slash redirects from Starlette and give them a dedicated transaction name

* adapt original route for slash redirects

elasticapm/contrib/starlette/__init__.py

@@ -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
@@ -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.
@@ -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

tests/contrib/asyncio/starlette_tests.py

@@ -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"):
@@ -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
@@ -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