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

Client Metrics does not use template URI variables #15231

Closed
mmaggioni-wise opened this issue Feb 22, 2021 · 6 comments · Fixed by #17676
Closed

Client Metrics does not use template URI variables #15231

mmaggioni-wise opened this issue Feb 22, 2021 · 6 comments · Fixed by #17676
Assignees
Labels
area/metrics kind/enhancement New feature or request
Milestone

Comments

@mmaggioni-wise
Copy link

Description

Client Metrics does not use template URI variable for the called api.

instead of having one metric:
http_client_requests_seconds_count{.... status="200",uri="/api/v1/surveys/{surveyId}",} 5.0

there are multiple metrics:
http_client_requests_seconds_count{.... status="200",uri="/api/v1/surveys/387",} 3.0
http_client_requests_seconds_count{.... status="200",uri="/api/v1/surveys/388",} 2.0

I don't think it's the correct behavior, it should probably works like the server metrics.

I did a very little debugging (i m not very familiar with the code), and the problem seems in the HttpRequestMetric, the routingContext variable is null, so he ll just return the complete path.

It may be a problem with my dependencies?

Thanks

@mmaggioni-wise mmaggioni-wise added the kind/enhancement New feature or request label Feb 22, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 22, 2021

/cc @jmartisk

@jmartisk
Copy link
Contributor

@ebullient

@ebullient
Copy link
Contributor

ebullient commented Feb 22, 2021

I have an existing PR in flight for improved URI templating, #15047
Can you confirm the version of Quarkus and extensions you're using?

To handle this in the meanwhile, you can set a match pattern:
quarkus.micrometer.binder.http-client.match-pattern=/api/v1/surveys/[0-9]+=/api/v1/surveys/{surveyId}

@mmaggioni-wise
Copy link
Author

Thank you!

Quarkus version is the 1.12.0.Final, the extensions are this ones:

cdi, jaeger, micrometer, mutiny, rest-client, resteasy, resteasy-jsonb, resteasy-multipart, security, security-properties-file, smallrye-context-propagation, smallrye-fault-tolerance, smallrye-health, smallrye-jwt, smallrye-openapi, smallrye-opentracing, swagger-ui, vertx, vertx-web

I took them from the "Installed features" log at the start, is it ok?

The implementation of the Client is the class ClientRequestContextImpl from resteasy-client.4.5.9-Final.

I saw your PR but didn t thought was for rest clients too tbh

@ebullient
Copy link
Contributor

It isn't, yet. ;)
And that list looks fine, just wanted to confirm what you're running. Thanks!

@ebullient
Copy link
Contributor

I broke #15047 apart in the end. Will use this to track rest client support after all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics kind/enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

4 participants