Micrometer Vertx JAX-RS path template is not properly recovered for metric uri labels

[hacst]

Describe the bug
To reduce label cardinality Micrometer default http metrics have to use the URL template string instead of raw request URL (e.g.
/items/123 must be converted back to /items/{id}). The implementation for this is in VertxMeterBinderContainerFilterUtil. Unfortunately the approach taken there is not reliable as it does a simple search and replace of the passed value with the parameter which produces wrong behavior in many common cases.

Expected behavior
/{foo}/{bar} handling /123/1 should result in http metric label {foo}/{bar}

Actual behavior
Due to the search and replace approach /{foo}/{bar} handling /123/1 instead can result in a http metric label like {bar}23/{bar}.

To Reproduce
Using a basic JAX-RS example with quarkus micrometer enabled:

@Path("/test")
public class ExampleResource {
    @Path("/hello/{foo}/{bar}")
    @GET
    @Produces(MediaType.TEXT_PLAIN)
    public String hello(@PathParam("foo") int foo, @PathParam("bar") int bar) {
        return "hello";
    }
}

Steps to reproduce the behavior:

1. Start a quarkus dev server
2. Request http://localhost:8080/test/hello/123/1
3. Inspect http://localhost:8080/q/metrics
   Notice the replacement failing for the uri label:

http_server_requests_seconds_count{method="GET",outcome="SUCCESS",status="200",uri="/test/hello/{bar}23/{bar}",} 1.0

I'm not sure if the ordering is guaranteed so you might have to swap 123/1 with 1/123.

Environment (please complete the following information):

• Quarkus 1.11.1-Final but other previous versions share the behavior

[ghost]

/cc @ebullient

[ebullient]

Thanks for the reproducer. This kind of parameter matching is .. difficult.

Can you try using a match pattern? I added this specifically to get around accuracy limitations in understanding rest paths:

quarkus.micrometer.binder.vertx.match-pattern=/hello/\\d+/\\d+=/hello/{foo}/{bar}

[hacst]

I tried the option as described but it did not seem to work. I played with the pattern a bit but didn't investigate deeper.

Even if it worked I fear this will be very hard to get right in changing services with many developers. Do you see a way of making this work out of the box? As we have the parameters there must have already been some kind of awareness of the template at that layer of request processing. Or maybe the patterns can be pre-created or something.

[ebullient]

The maps we have in hand are completely unordered. I could make strings and sort the parameterized values by length to make sure the match was more accurate -- but that is more expensive than a regex match, and could still be wrong some of the time.

There may be a few more tricks down the line with Vert.x 4, but there isn't a clear way to grab the parameterized endpoint used in the original annotation. Maybe I'll chew on that a little more for more Quarkus magic.

Can you share the patterns you've tried?

[hacst]

I tried your original and
quarkus.micrometer.binder.vertx.match-pattern=/test/hello/\\d+/\\d+=/test/hello/{foo}/{bar}
to try to fix the reproducer but it had no effect on the metrics (can't remember the other attempts).

I dug a bit deeper into the code and noticed that this filter isn't actually executed as part of any of any normal vertx filter chains. From what I can tell it is explicitly run as a ContainerRequestFilter (or ServerRequestFilter).

E.g. the ContainerRequestFilter:

public class VertxMeterBinderRestEasyContainerFilter implements ContainerRequestFilter {

    @Override
    public void filter(final ContainerRequestContext requestContext) {
        VertxMeterBinderContainerFilterUtil.doFilter(CDI.current().select(CurrentVertxRequest.class).get().getCurrent(),
                requestContext.getUriInfo());
    }
}

I think in both cases as UriInfo exists they must be pretty late and the information must've at least been available at some point in the callstack. I checked for the ContainerRequestFilter and something like:

@Provider
public class MyContainerVertxFilter implements ContainerRequestFilter {
    @Override
    public void filter(final ContainerRequestContext requestContext) {
        System.out.println("ContainerVertxFilter");
        var resourceMethodInvoker = ((PostMatchContainerRequestContext) requestContext).getResourceMethod();
        var resourceMethod = resourceMethodInvoker.getMethod();
        var resourceClass = resourceMethodInvoker.getResourceClass();
        UriBuilder resourceUriBuilder= UriBuilder.fromResource(resourceClass)
                .path(resourceMethod);
        String resourceUri= resourceUriBuilder.toTemplate();
        System.out.println(resourceUri);
   }
}

Gives me the template string. This relies on knowing the ContainerRequestFilter being a PostMatchContainerRequestContext in RestEasy but I think that should be the case here too. Now to get this robust and performant might require some additional though.

But investigating in a direction like that seems to be the best way to me. Otherwise I fear a large amount of all quarkus applications with micrometer metrics out there will have broken uri labels on their metrics.

[ebullient]

Well, I agree with you, which is why the matchPattern is there (and should work). (regardless of what other path might be taken, as we have lots of ways to handle http traffic).

I'll update the current tests to chase why the pattern isn't matching, I must have made a mistake somewhere.
I think the best way is to anchor the annotation information somewhere at bean processing time (as we know it then, it is right in the annotation). You make a good suggestion, I'll play with it.

[ebullient]

Ok. The PR does sort out the behavior of matchPattern to make sure the fallback case works reliably.

I'll need to take another gander at the more ambitious solution, which may need to wait until Vert.x 4 (see #14905)

[hacst]

Awesome. Hoping for pre Vert.x 4 (Quarkus 2.0) obviously ;) Please let me know you decide to go that way though because we'll probably want to investigate a workaround in the direction of what I described above on our end in that case. Otherwise the only way I see would be to tell developers to add matchPatterns for every endpoint with parameters in all services which would not go over well ;)

[ebullient]

Using JAX-RS is the most common? Or native/naked Vert.x web? Or servlets with undertow...

[hacst]

Pretty exclusively JAX-RS right now for us. I am aware that a general solution for Quarkus is much harder. That's why I would investigate workarounds on our end if it turns out to be infeasible for a while.

[ebullient]

I tried your original and
quarkus.micrometer.binder.vertx.match-pattern=/test/hello/\\d+/\\d+=/test/hello/{foo}/{bar}
to try to fix the reproducer but it had no effect on the metrics (can't remember the other attempts).

Needs to be extra escaped, e.g.:

quarkus.micrometer.binder.vertx.match-pattern=/test/hello/\\\\d+/\\\\d+=/test/hello/{foo}/{bar}
quarkus.micrometer.binder.vertx.match-pattern=/test/hello/[0-9]+/[0-9]+=/test/hello/{foo}/{bar}

But there was also some other wonkiness going on that is resolved in the linked PR.

I dug a bit deeper into the code and noticed that this filter isn't actually executed as part of any of any normal vertx filter chains.

Correct. The Vert.x filter does a hand-off at the container boundary (Vert.x http/web filter chains don't see it, for the most part). I use ContainerFilters w/ Resteasy & Resteasy reactive to deal w/ JAX-RS details. Undertow servlets & websockets are on a totally different path (that isn't well instrumented, either).