Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
chris-wood-mo wants to merge3 commits intokafbat:main
base:main
Choose a base branch
Loading
fromchris-wood-mo:issues/1506

Conversation

@chris-wood-mo
Copy link

@chris-wood-mochris-wood-mo commentedNov 28, 2025
edited
Loading

  • Breaking change? (if so, please describe the impact and migration path for existing application instances)

What changes did you make? (Give an overview)

  • Ensure Prometheus /metrics endpoint streams correctly with clean EOF

Is there anything you'd like reviewers to focus on?

How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)

  • No need to
  • Manually (please, describe, if necessary): Tested locally with docker-compose and within a Kubernetes cluster which included a service monitor which was able to scrape the metrics into Prometheus.
    As you can see from the evidence attached, this now works locally and returns the format expected by Prometheus.
curl -I http://localhost:8080/metricsHTTP/1.1 200 OKtransfer-encoding: chunkedAccess-Control-Allow-Credentials: trueAccess-Control-Allow-Methods: GET, PUT, POST, DELETE, OPTIONSAccess-Control-Max-Age: 3600Access-Control-Allow-Headers: Content-TypeContent-Type: text/plain; version=0.0.4
Screenshot 2025-11-28 at 15 29 44- [x] Unit checks- [x] Integration checks- [x] Covered by existing automation

Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g.ENVIRONMENT VARIABLES)
  • My changes generate no new warnings (e.g. Sonar is happy)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged

Check outContributing andCode of Conduct

A picture of a cute animal (not mandatory but encouraged)
cute_dog

@kapybrokapybrobot added status/triageIssues pending maintainers triage status/triage/manualManual triage in progress status/triage/completedAutomatic triage completed and removed status/triageIssues pending maintainers triage labelsNov 28, 2025
Copy link

@github-actionsgithub-actionsbot left a 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");
Copy link
Member

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.

@summary("exposeClusterMetrics")
@operationId("exposeClusterMetrics")
exposeClusterMetrics(@pathclusterName:string):string;
exposeClusterMetrics(@pathclusterName:string):void;
Copy link
Member

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

@summary("exposeAllMetrics")
@operationId("exposeAllMetrics")
exposeAllMetrics():string;
exposeAllMetrics():void;
Copy link
Member

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
Copy link
Author

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:
<img width="1512" height="827" alt="Screenshot 2025-11-28 at 21 26 49" src="https://github.com/user-attachments/assets/14244b23-a0c1-4adb-9a07-5317efcc9

Thanks,

Chris

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@github-actionsgithub-actions[bot]github-actions[bot] left review comments

@germanosingermanosingermanosin requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

status/triage/completedAutomatic triage completedstatus/triage/manualManual triage in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@chris-wood-mo@germanosin

[8]ページ先頭

©2009-2025 Movatter.jp