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 return 404 instead of 406 #15093

Closed
cortex93 opened this issue Feb 15, 2021 · 5 comments · Fixed by #19179
Closed

Micrometer return 404 instead of 406 #15093

cortex93 opened this issue Feb 15, 2021 · 5 comments · Fixed by #19179
Assignees
Labels
area/metrics kind/bug Something isn't working
Milestone

Comments

@cortex93
Copy link

Describe the bug
With

<quarkus.platform.version>1.11.3.Final</quarkus.platform.version>
<dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-micrometer-registry-prometheus</artifactId>
  </dependency>

doing a request like

GET http://localhost:8080/q/metrics
Accept: application/json

returns 404

Expected behavior
The endpoint actually exists but cannot fulfill the Accept header. The response should be 406 Not acceptable.

Actual behavior
Returns 404

To Reproduce

  1. Follow the micrometer metrics guide.
  2. instead of curl localhost:8080/q/metrics/, add the accept header with eg application/json

Environment (please complete the following information):

  • Output of uname -a or ver: Linux b5bdbb43a9f9 5.4.72-microsoft-standard-WSL2 Switch to the Maven distributed copy of the SubstrateVM annotations #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
  • Output of java -version: openjdk version "11.0.9.1" 2020-11-04 LTS
  • GraalVM version (if different from Java): N/A
  • Quarkus version or git rev: 1.11.3.Final
  • Build tool (ie. output of mvnw --version or gradlew --version): Apache Maven 3.6.2 (Red Hat 3.6.2-5)

Additional context
With MP metrics endpoint, the accept: application/json header is recognize and served appropriately. Having 406 instead of 404 can help diagnose what is wrong when migrating to micrometer.

@cortex93 cortex93 added the kind/bug Something isn't working label Feb 15, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 15, 2021

/cc @ebullient

@ebullient ebullient self-assigned this Feb 15, 2021
@ebullient
Copy link
Member

ebullient commented Feb 15, 2021

If you drop the accept header, it will work properly (prometheus w/ text/plain), which also would work fine with any microprofile metrics.

If you want the fallback of the json endpoint: quarkus.micrometer.export.json.enabled
Note that earlier versions of Quarkus had issues with the json exporter failing due to garbage collected guages (NaN values).

I'll have to see how/why the 404 is being returned, though I can understand the reasoning for why it is. This is being done w/ Vert.x Web (lower level), prometheus is registered first for only text/plain, and the json exporter (if enabled) is registered second for only application/json. The handler needed for that request is not found (404 is accurate), though 406 is the more nuanced answer. This may be a Vert.x ask..

@geoand
Copy link
Contributor

geoand commented Feb 16, 2021

@ebullient I am pretty sure that Reactive Routes (which is just a thin layer of Vert.x Web) properly handles this case.
I am mention it just in case you want to look at how it's done there

@ebullient
Copy link
Member

Y.. this is may be in the router that was added for /q endpoints...

@cortex93
Copy link
Author

If you drop the accept header, it will work properly (prometheus w/ text/plain), which also would work fine with any microprofile metrics.

If you want the fallback of the json endpoint: quarkus.micrometer.export.json.enabled
Note that earlier versions of Quarkus had issues with the json exporter failing due to garbage collected guages (NaN values).

I'll have to see how/why the 404 is being returned, though I can understand the reasoning for why it is. This is being done w/ Vert.x Web (lower level), prometheus is registered first for only text/plain, and the json exporter (if enabled) is registered second for only application/json. The handler needed for that request is not found (404 is accurate), though 406 is the more nuanced answer. This may be a Vert.x ask..

I'm not an advocate of 406 over 404. I'm re Just notice the behavior when doing some tests moving from the smallrye extension to micrometer. And my tests were using the accept json header. I struggle a little bit why the endpoint fails. And with the recent move to /q, it adds some confusion as the guide is not up to date regarding default value shown in the configuration section at the end (still refer to /metrics)
Thanks for pointing json support need to be enabled. I understand micrometer is the defacto, maybe some guidance to migrate from smallrye (different annotations for Counted, ...) would help.

@ebullient ebullient changed the title MicroMeter return 404 instead of 406 Micrometer return 404 instead of 406 Aug 2, 2021
@quarkus-bot quarkus-bot bot added this to the 2.2 - main milestone Aug 2, 2021
@gsmet gsmet modified the milestones: 2.2 - main, 2.1.1.Final Aug 3, 2021
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants