Micrometer-Extension writes wrong URI-Tag when Path-Variables defined at Interface-Level

[lostiniceland]

Describe the bug

We are currently seing wrong metric-uri-tags for incoming requests when using the Micrometer-Prometheus extension.

Instead of placeholders for the path-variables, a metric for each path-variable-instance is created.
The issue is caused when placing the JAX-RS annotations on an interface, rather than the actual resource-class.

As a workaround, we have to duplicate @Path (and probably also @PathParam) to the implementing class, which is quite error-prone.

Unfortunatly using quarkus.micrometer.binder.http-server.match-patterns only work as workaround for the first appearance like /<id> but it fails for sub-queries like /<id>/do

Expected behavior

Metric is tagged with proper uri, replacing path-variables with a placeholder
http_server_requests_seconds_count{... uri="/{id}"} 1.0

Path annotation should be recognized regardless of location on interface or class

Actual behavior

A unique metric is written for each path-parameter accessing the endpoint

http_server_requests_seconds_count{... uri="/eae3806d-141c-46ed-a067-6f4b81f36062"} 2.0
http_server_requests_seconds_count{... uri="/eae3806d-141c-yyyy-xxxx-6f4b81f36062"} 1.0

How to Reproduce?

Enable dependency quarkus-micrometer-registry-prometheus

Create a endpoint consisting of interface and impl.

Path("/")
public interface SomeApi {
   @GET
   @Path("{id}")
   Response load(@PathParam("id) String id);
}

@RequestScoped
public class SomeResource implements SomeApi {
   public Response load(String id){
   ...do
   }
}

Access App with /someid to trigger metrics and check /q/metrics for http_server_requests_count

Output of uname -a or ver

No response

Output of java -version

17

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.15.3

Build tool (ie. output of mvnw --version or gradlew --version)

Maven 3.8.3

Additional information

Our main motivation for splitting interface and implementation is to avoid the annotation-hell due to swagger and jax-rs. The implementation in our case then has the meaningfull stuff like access-control, custom interceptors.
Furthermore the split is needed when doing contract first with OpenApi-Generators.

[quarkus-bot]

/cc @ebullient (micrometer)

[ebullient]

A reproducer will be helpful here (just to ensure we cover your use case correctly). Note that we're building the query based on what the runtime has resolved / provides us as the URL is processed, we aren't basing it on config or trying to figure this out as each REST/Servlet container does this differently.

These patterns are proper Java regex. They should apply to sub-patterns if you write them that way. The escaping required for backslashes gets a little bananas, but this one (for example) has subpaths in play:

quarkus/integration-tests/micrometer-prometheus/src/main/resources/application.properties

Line 21 in 3c920f7

quarkus.micrometer.binder.http-server.match-patterns=/message/match/\\\\d+/[0-9]+=/message/match/{id}/{sub},\

Can you show/share your configured match pattern?

[lostiniceland]

I will try to put something together over the weekend.

After a little debugging it looks like RestPathAnnotationProcessor is responsible for the issue, because it is not setting the runtime Annotation @QuarkusRestTemplatePath when @Path is only set on the interface.

This seems to be the culprit:

quarkus/extensions/resteasy-classic/resteasy/deployment/src/main/java/io/quarkus/resteasy/deployment/RestPathAnnotationProcessor.java

Line 100 in 0506e81

annotation = classInfo.classAnnotation(REST_PATH);

It is checking the classInfo. Not sure if there is something like interfaceInfo as well...

[lostiniceland]

quarkus-30843.zip

There's a working sample.

• Run Maven mvn quarkus:dev
• http://localhost:8080/hello
• http://localhost:8080/world
• http://localhost:8080/q/metrics
• See broken heaviour
• Uncomment @Path in SomeResource
• Execute Requests again
• Metrics now contain the proper uri-tag /{id}

I am currently working my way through the RestPathAnnotationProcessor and I think I've found a solution to fallback checking the interface when annotation is not present on the class. Though this only works for class-level annotations. The fallback needs to apply to method-level annotations on the interface as well and I am currently stuck navigating from the interface-types org.jboss.jandex.Type to the method (I was hoping to find something like declaredMethods there as well).
The idea was to iterate over the interface-types and find the matching method (by name) which is then checked for @Path. But I am not yet familiar enough with Jandex.

[ebullient]

Ok. Just as something else to attempt, I updated the API to have another method:

@Path("/")
public interface SomeApi {
    @GET
    @Path("{id}")
    Response get(@PathParam("id") String id);
    
    @GET
    @Path("{id}/boom")
    Response getBoom(@PathParam("id") String id);
}

and added a quick impl to SomeResource

    @Override
    public Response getBoom(String id) {
        return Response.ok().entity(new Greet("BOOM " + id)).build();
    }

With this match pattern:

quarkus.micrometer.binder.http-server.match-patterns=/[^/]+/boom=/{id}/boom

I get the following output:

http_server_requests_seconds_count{method="GET",outcome="SUCCESS",status="200",uri="/{id}/boom",} 3.0
http_server_requests_seconds_sum{method="GET",outcome="SUCCESS",status="200",uri="/{id}/boom",} 0.202644959

Not saying we shouldn't fix the detection parts, just trying to make sure that you can get the match pattern to work, because it should.

[lostiniceland]

I found a working draft though this is not yet fully fleshed out (missing tests). What do you think?

[lostiniceland]

@geoand how to backport this fix to 2.16 now?

[quarkus-bot]

@geoand triage/backport* labels may not be added to an issue. Please add them to the corresponding pull request.

This message is automatically generated by a bot.

[geoand]

We will handle it, you don't need to do anything