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 for http requests: URI templating not working with interfaces #21034

Closed
baiglin opened this issue Oct 27, 2021 · 15 comments
Closed

Micrometer for http requests: URI templating not working with interfaces #21034

baiglin opened this issue Oct 27, 2021 · 15 comments
Assignees
Labels
area/metrics kind/bug Something isn't working triage/needs-feedback We are waiting for feedback.

Comments

@baiglin
Copy link

baiglin commented Oct 27, 2021

Describe the bug

Hello,

While integrating Micrometer into our application, checking the http metrics and more specifically the URI, I could see that in some case it was not templatized.
It seems to come from the fact that we used in some case an API instead of annotating the class directly and in this case it seems the @path annotation is not used to resolve the template URI.

Best Regards

Expected behavior

Have a tag with the template URI instead of the whole URI

Actual behavior

Full URI is set as metric tag.

How to Reproduce?

micrometer_reproducer.zip

This is a small reproducer that contains the two versions of a dummy resource, one usin an API, the other no to highlight the difference.

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

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

No response

Additional information

No response

@baiglin baiglin added the kind/bug Something isn't working label Oct 27, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 27, 2021

/cc @ebullient

@ebullient
Copy link
Member

Thanks for opening this issue. In the meanwhile, you can use matchPattern to force usage of the templated URI.

@ebullient ebullient self-assigned this Oct 27, 2021
@baiglin
Copy link
Author

baiglin commented Oct 27, 2021

Thanks for opening this issue. In the meanwhile, you can use matchPattern to force usage of the templated URI.

Exactly, there is still a way out of it, but was really surprise it was not always working especially with the work done in #17676, unfortunately I did not have to dig more to find where the missing part comes from exactly.

@ebullient
Copy link
Member

I may not be chasing interfaces correctly for path annotation detection. Thanks for the reproducer, that helps a lot. ;)

@imjuoy
Copy link

imjuoy commented Apr 24, 2022

What is the current status of resolving this issue? This issue is affecting us for now, since we use micrometer for metrics and are getting WARNING's regarding "number of tags for http.server.requests".
Should we be moving ahead with setting "match-pattern" properties as mentioned below:
https://github.com/tomekl007/nessie/blob/be4e348f4b1b4e12453912cb594a4781dc73a5d6/servers/quarkus-server/src/main/resources/application.properties#L173

@ebullient
Copy link
Member

You should set the match pattern as that gets you out of trouble right now, there are some things I'm waiting for to fix this issue, but the match pattern will continue to work regardless (at some future point, you may not need it, but it will work, so.. )

@imjuoy
Copy link

imjuoy commented Apr 25, 2022

@ebullient Thanks for the confirmation.
I have a question regarding having multiple path parameters.
Everywhere I refer to for examples of configurations, it's mostly just one path param. For example:

In our case, the application has three path parameters. Let's say:
/v1/api/param1/param2/param3

Param2 has limited set of values and will never lead to high cardinality situations.
Is it possible to just record param2 and keep param1 and param3 templatized?

@ebullient
Copy link
Member

At the moment, no. I was making some tradeoffs re: cost because this comparison is done with every request measurement.

I could add an option to use replaceAll rather than flat assignment...

@imjuoy
Copy link

imjuoy commented Apr 25, 2022

Can you please elaborate what you meant by :
"I could add an option to use replaceAll rather than flat assignment..."

@ebullient
Copy link
Member

ebullient commented Apr 25, 2022

static String applyMatchPatterns(String path, Map<Pattern, String> matchPatterns) {

From there. I just assign the match pattern value at the moment, but the pattern is a proper regex, I could allow use of replaceAll as well (which would allow you to use matching groups)

@imjuoy
Copy link

imjuoy commented Apr 26, 2022

I want to make sure, that I have explained the problem correctly.
So given that I have three path params, param1, param2, param3,
I want to write a regex such that :
/v1/api/regex=/v1/api/{param1}/value of param2/{param3}

@ebullient
Copy link
Member

Right.
If I allowed use of replaceAll, you could specify the match string like this:
/v1/api/[^/]+/([^/]+)/.*=/v1/api/{param1}/$1/{param3}
(single group)

@rkorver
Copy link

rkorver commented Dec 21, 2023

When will there be a proper solution to the problem that micrometer doesn't properly recognize http request uri's? Thanks in advance!

@geoand
Copy link
Contributor

geoand commented Sep 20, 2024

Is this still an issue? If it is, can someone attach a reproduce that uses the latest version of Quarkus so I can look into it?

Thanks

@geoand geoand added the triage/needs-feedback We are waiting for feedback. label Sep 20, 2024
@geoand
Copy link
Contributor

geoand commented Oct 2, 2024

Closing for lack of feedback

@geoand geoand closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics kind/bug Something isn't working triage/needs-feedback We are waiting for feedback.
Projects
None yet
Development

No branches or pull requests

5 participants