Skip to content

Conversation

augustas1
Copy link

@augustas1 augustas1 commented Jul 7, 2025

What does this PR do?

Spawns Prometheus exporter in the background. Currently metrics are not accessible via PROMETHEUS_PORT (default 9000), only via main port /metrics route.

Code addition matches the same what PrometheusBuilder::install does.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@alvarobartt @Narsil

@alvarobartt
Copy link
Member

Hey @augustas1, thanks for your contribution! Indeed this seems related to #729, but AFAIK the Prometheus port only applies for gRPC, as it requires to deploy the endpoint over HTTP; whereas on HTTP is not required as we can simply create the /metrics route within the default port.

Could you elaborate on why do you think this is required or why? I guess it can be misleading to expose a --prometheus-port argument and then not being "used" over HTTP for querying the /metrics endpoint but AFAIK the change should be on the argument description or on the docs rather than on the behaviour on whether Prometheus is spawned or not within the HTTP server, thoughts?

Thanks in advance 🤗

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants