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

Delete prometheus-client-bridge #1136

Open
jack-berg opened this issue Dec 18, 2023 · 5 comments
Open

Delete prometheus-client-bridge #1136

jack-berg opened this issue Dec 18, 2023 · 5 comments

Comments

@jack-berg
Copy link
Member

Component(s)

prometheus-client-bridge

What happened?

The functionality is implemented in opentelemetry-java in open-telemetry/opentelemetry-java#6015, which allows you to associate your own PrometheusRegistry with PrometheusHttpServer.

Component version

latest

Log output

No response

Additional context

No response

@jkwatson
Copy link
Contributor

I'm not sure that PR is a full replacement. I'm using the PrometheusServlet to serve up prometheus data. That uses the default registry, and the bridge lets me just let the default registry collect OTel metrics. I don't see how that PR will replace that usage, but I might be missing something.

@jack-berg
Copy link
Member Author

I'm using the PrometheusServlet to serve up prometheus data.

You should be able to do this via PrometheusHttpServerBuilder#setPrometheusRegistry(PrometheusRegistry.defaultRegistry), no? The code registers an OTEL implementation of MultiCollector with whatever prometheus registry you configure: https://github.com/open-telemetry/opentelemetry-java/pull/6015/files#diff-6c5e18c7125e4889e56b4ba2225ca3a3d36e61d4a19a44b2c7cb06a4c9f43af8R62

In doing so, I think you should be able to define your own handler which reads from all the collectors associated with the registry.

@jkwatson
Copy link
Contributor

I'm using the PrometheusServlet to serve up prometheus data.

You should be able to do this via PrometheusHttpServerBuilder#setPrometheusRegistry(PrometheusRegistry.defaultRegistry), no? The code registers an OTEL implementation of MultiCollector with whatever prometheus registry you configure: https://github.com/open-telemetry/opentelemetry-java/pull/6015/files#diff-6c5e18c7125e4889e56b4ba2225ca3a3d36e61d4a19a44b2c7cb06a4c9f43af8R62

In doing so, I think you should be able to define your own handler which reads from all the collectors associated with the registry.

But, I'm not using a Prometheus Http server...I'm using the built-in tomcat from spring-boot, with the prometheus servlet serving up the metrics.

@jack-berg
Copy link
Member Author

Ah I think if I understand correctly, you would need to use PrometheusMetricReader and register it with both SdkMeterProvider and PrometheusRegistry.defaultRegistry.

I was going to say that we would have to make the new PrometheusMetricReader class public, but it looks like that's already the case. So open-telemetry/opentelemetry-java#6015 actually defines two MetricReader implementations:

  • PrometheusMetricReader, which implements MultiCollector and doesn't do anything until you do something with it which calls MetricSnapshots collect()
  • PrometheusHttpServer, which uses PrometheusMetricReader internally, starts up an HTTP server, and calls PrometheusMetricReader#collect() to serve up responses to metric requests

Side note - if we go with this approach, we'll need to be careful to document that most users will want PrometheusHttpServer, NOT PrometheusMetricReader.

@jkwatson
Copy link
Contributor

Oh! 🤔 I'd be happy to try that out, once it's merged and released; if it works, then I'm 👍🏽 on this proposal.

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

No branches or pull requests

2 participants