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

Micrometer route gets sub router attached at the end #249

Closed
basalt79 opened this issue Dec 16, 2024 · 9 comments
Closed

Micrometer route gets sub router attached at the end #249

basalt79 opened this issue Dec 16, 2024 · 9 comments
Assignees
Labels
invalid This doesn't seem right

Comments

@basalt79
Copy link

Version

vertx: 4.5.7 - 4.5.11

Context

Define a main route and add a sub route.
Calling sub route api.
on 200 -> route string in metrics looks ok
on fail() -> route string in metrics gets the sub route attached at the end of the url.

Reproducer

see: https://github.com/basalt79/vertx-web-metrics

Steps to reproduce

  1. Launch MainVerticleTest
  2. see failing tests

OR

  1. Launch MainVerticle
  2. navigate to
http://localhost:7777/good
http://localhost:7777/bad
http://localhost:7777/notfound
http://localhost:7777/first/good
http://localhost:7777/first/bad
http://localhost:7777/first/notfound
http://localhost:7777/first/second/good
http://localhost:7777/first/second/bad
http://localhost:7777/first/second/notfound

and check afterwards the metrics result at http://localhost:7777/metrics

Extra

Since the effect of this hits metrics, i put this issue here, but maybe the root cause is located at https://github.com/vert-x3/vertx-web/

@basalt79 basalt79 added the bug Something isn't working label Dec 16, 2024
@tsegismont tsegismont self-assigned this Jan 2, 2025
@tsegismont tsegismont added this to the 4.5.12 milestone Jan 2, 2025
@tsegismont
Copy link
Contributor

Thanks for your report @basalt79

Does the issue affect 4.5.6 as well?

@basalt79
Copy link
Author

basalt79 commented Jan 2, 2025

Thanks for taking care @tsegismont .
Yes, with 4.5.6, 4.5.5, 4.5.3 its the same behavior. I created a branch for this version, see https://github.com/basalt79/vertx-web-metrics/tree/4.5.6

@basalt79
Copy link
Author

basalt79 commented Jan 2, 2025

I have added a reproducer with 4.0.0, this was the first version where the route was added to the metrics.
https://github.com/basalt79/vertx-web-metrics/tree/4.0.0

this happens only if I use subRouter like https://github.com/basalt79/vertx-web-metrics/blob/4.0.0/src/main/java/com/noenv/georg/MainVerticle.java#L104

@tsegismont
If I define each route dedicated, its working https://github.com/basalt79/vertx-web-metrics/blob/4.0.0/src/main/java/com/noenv/georg/MainVerticle.java#L81

i have not adapted the unit tests, because if i dont use subRouter, the />/ part is missing anyway.

since we use subRouter for the main API routes from Swagger, the behavior is on every service.

@tsegismont
Copy link
Contributor

Thank you for all these details, I'll look into your reproducer

@vietj vietj modified the milestones: 4.5.12, 4.5.13 Jan 23, 2025
@vietj vietj modified the milestones: 4.5.13, 4.5.14 Feb 10, 2025
@basalt79
Copy link
Author

Hi @tsegismont , have you already got the chance to take a look at the reproducer?

@tsegismont
Copy link
Contributor

@basalt79 I was able to write a unit test for vertx-micrometer-metrics thanks to your reproduce. I'll share my findings when I understand the cause.

In case you haven't figured it out yourself, you could write a MeterFilter to change the bad metrics name

@tsegismont
Copy link
Contributor

When the route label is enabled, the Vert.x Web Router can notify the metrics SPI implementation that a route handler has been invoked.

For example, if you set two routes with same path:

router.route("/bad").handler(RoutingContext::next);
router.route("/bad").handler(ctx -> ctx.fail(500));

You will get a metric with route tag that contains:

/bad>/bad

Similarly, if you invoke reroute:

router.route("/good").handler(ctx -> ctx.reroute("/first/second/bad"));

The metric route tag contains:

/first/>/second/>/good>/first/>/second/>/bad

Back to your reproducer, when ctx.fail is invoked, the router is traversed again from the root level to find a failure handler. If the request uri starts with /first/second, the subrouter bound to /first and /second are traversed, in this order, this is why the metric route tag contains /bad>/first/>/second/.

So this works as designed. Have you tried to create a MeterFilter to modify the route tag that are not suitable or perhaps you could disable the route label, and use matchers with the path label?

@basalt79
Copy link
Author

Hi @tsegismont I will check the MeterFilter and also have a look at the Matchers thx for the hit!
currently we work around with wildcards in Prometheus queries.

@tsegismont tsegismont added invalid This doesn't seem right and removed bug Something isn't working labels Mar 18, 2025
@tsegismont tsegismont removed this from the 4.5.14 milestone Mar 18, 2025
@tsegismont
Copy link
Contributor

Thanks for your feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Development

No branches or pull requests

3 participants