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

Documentation improvement on composable HttpRoutes with metrics #113

Open
vjroby opened this issue Jul 17, 2023 · 3 comments
Open

Documentation improvement on composable HttpRoutes with metrics #113

vjroby opened this issue Jul 17, 2023 · 3 comments

Comments

@vjroby
Copy link

vjroby commented Jul 17, 2023

Hi team,

I discovered a weird behaviour if metrics are enabled on an HttpRoutes value that is composed with some other value.

For example let's say there are apiRoutes and adminRoutes defined and compose like this:

val metrics: MetricsOps[F] = ???
val apiRoutes: HttpRoutes[F] = ???
val adminRoutes: HttpRoutes[F] = ???

val apiRoutesWithMetrics = Metrics[IO](metrics)(apiRoutes)

httpApp = (
        apiRoutesWithMetrics <+> adminRoutes.  //  <+> SemigroupK Cats
      ).mapF(_.getOrElseF(NotFound()))

return httpApp // <- this is used to create the http4s webserver

Whenever there is a request meant for the adminRoutes the metrics defined for apiRoutesWithMetrics gets incremented for 4xx requests although the request is send back to the client with 200 OK status.

If the routes are composed the other way around adminRoutes <+> apiRoutesWithMetrics this behaviour does not happen.

I'm guessing is because the request goes through the first routes and exists with NotFound (400) and the metrics are incremented and then the request go to the admin routes and there is a route defined but the metrics are already incremented.

I'm not saying it's a bug in the library but maybe there can be added to the documentation.
I'm happy to add it.

What is your opinion on this?

Regards,
Robert

@hamnis
Copy link
Contributor

hamnis commented Jul 27, 2023

your guess is correct.

You could try to use the
Router and mount the routes separately there.

Example:

Router(
  "/admin" -> adminRoutes,
  "/" -> apiRoutesWithMetrics
)

that way you will not have that behaviour.

@vjroby
Copy link
Author

vjroby commented Jul 31, 2023

Thanks for replying.

If I'm going that way I will have incremented metrics also for /admin routes, right?
I'm asking because this is something I want to avoid.

@hamnis
Copy link
Contributor

hamnis commented Aug 1, 2023

No that does not mean that.

Since you have only installed the metrics middleware on the api routes the /admin routes is not part of the routing.

I would strongly suggest using something like

Router(
  "/admin" -> adminRoutes,
  "/api" -> apiRoutesWithMetrics
)

to make this destinction even more clear.

This means however, a route defined in /api would start matching on Root, since /api is now the new Root node.

For more information see the Router documentation.

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

2 participants