Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork218
issues/1506: ensure Prometheus /metrics endpoint streams correctly wi…#1536
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hi chris-wood-mo! 👋
Welcome, and thank you for opening your first PR in the repo!
Please wait for triaging by our maintainers.
Please take a look at ourcontributing guide.
| ).getBody(); | ||
| varresponse =exchange.getResponse(); | ||
| response.getHeaders().add("Content-Type","text/plain; version=0.0.4"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We could pass it within ResponseEntity, no sense to hardcode it here.
contract-typespec/api/prometheus.tsp Outdated
| @summary("exposeClusterMetrics") | ||
| @operationId("exposeClusterMetrics") | ||
| exposeClusterMetrics(@pathclusterName:string):string; | ||
| exposeClusterMetrics(@pathclusterName:string):void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this one is incorrect, response should be string, but with required content type
contract-typespec/api/prometheus.tsp Outdated
| @summary("exposeAllMetrics") | ||
| @operationId("exposeAllMetrics") | ||
| exposeAllMetrics():string; | ||
| exposeAllMetrics():void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please see comment below
… and updated response of exposeClusterMetrics and exposeAllMetrics to be string
chris-wood-mo commentedNov 28, 2025
Hi@germanosin, Thanks for your comments! I have reviewed them and updated the code. I have also made sure to test the changes again and make sure they work both locally and from a Kubernetes cluster using a service monitor to scrape the metrics into Prometheus. Both have worked with the changes made. The screenshot below shows this working locally: Thanks, Chris |
Uh oh!
There was an error while loading.Please reload this page.
What changes did you make? (Give an overview)
Is there anything you'd like reviewers to focus on?
How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)
As you can see from the evidence attached, this now works locally and returns the format expected by Prometheus.
Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)
Check outContributing andCode of Conduct
A picture of a cute animal (not mandatory but encouraged)
