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

Submounted routes use incorrect path in labels #40

Open
ter0 opened this issue Jun 24, 2022 · 2 comments
Open

Submounted routes use incorrect path in labels #40

ter0 opened this issue Jun 24, 2022 · 2 comments

Comments

@ter0
Copy link

ter0 commented Jun 24, 2022

When submounting routes, rather than the full path template being used, only the mount prefix is used.

Running the following app:

from starlette.applications import Starlette
from starlette.middleware import Middleware
from starlette.responses import Response
from starlette.routing import Mount, Route
from starlette_prometheus import PrometheusMiddleware, metrics


async def foo(request):
    return Response()


async def bar_baz(request):
    return Response()


routes = [
    Route("/foo", foo),
    Mount("/bar", Route("/baz", bar_baz)),
    Route("/metrics", metrics),
]
middleware = [Middleware(PrometheusMiddleware)]
app = Starlette(routes=routes, middleware=middleware)

Then making the following requests:

$ curl localhost:8000/foo
$ curl localhost:8000/bar/baz
$ curl localhost:8000/metrics

Gives the following output (I only included one metric as an example, but it's the same for all of them). Note the label for the request to localhost:8000/bar/baz has a path label of /bar.

starlette_requests_total{method="GET",path_template="/foo"} 1.0
starlette_requests_total{method="GET",path_template="/bar"} 1.0
starlette_requests_total{method="GET",path_template="/metrics"} 1.0
@Dimaqa
Copy link

Dimaqa commented Oct 10, 2022

Hello! I've faced the same issue.
That could be fixed by patching get_path_template function in PrometheusMiddleware class

    @staticmethod
    def get_path_template(request: Request) -> Tuple[str, bool]:
        for route in request.app.routes:
                match, _ = route.matches(request.scope)
                if match == Match.FULL:
                    return route.path, True
        return request.url.path, False

to something like

    @staticmethod
    def get_path_template(request: Request) -> Tuple[str, bool]:
        for route in request.app.routes:
            if isinstance(route, Mount):
                for inner_route in route.routes:
                    match, _ = inner_route.matches(request.scope)
                    if match == Match.FULL:
                        return route.path, True
            else:
                match, _ = route.matches(request.scope)
                if match == Match.FULL:
                    return route.path, True

        return request.url.path, False

@alex-oleshkevich
Copy link

This snippet handles more cases (incl. inner mount objects):

import dataclasses, copy

@dataclasses.dataclass
class RouteMatch:
    name: str = ''
    pattern: str = ''


def find_matched_route(scope: Scope, routes: typing.Iterable[BaseRoute]) -> RouteMatch | None:
    for route in routes:
        if isinstance(route, (Mount, Host)):
            mount_match, child_scope = route.matches(scope)
            if mount_match == Match.FULL:
                scope.update(child_scope)
                if match := find_matched_route(scope, route.routes):
                    return match
        elif isinstance(route, Route):
            match, child_scope = route.matches(scope)
            if match == Match.FULL:
                return RouteMatch(name=route.name, pattern=route.path_format)

  scope_copy = copy.copy(scope) # scope will be mutated so make a copy!
  route_match = find_matched_route(scope_copy, scope['app'].routes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants