opentelemetry-go
opentelemetry-go copied to clipboard
Address exporter/prometheus package name collision
Originally posted by @dashpole in https://github.com/open-telemetry/opentelemetry-go/pull/3168#discussion_r970987617
Currently, the prometheus exporter package name is guaranteed to conflict with an import of "github.com/prometheus/client_golang/prometheus". This second import is required now because the prometheus exporter only returns a prometheus.Collector that needs to be registered with that package. This is not ideal for users as they will always need to override a package name when they setup the exporter.
Possible solutions
Rename the exporter
go.opentelemetry.io/otel/exporters/otelprometheus has been proposed.
Provide a registration method
The exporter could have a registration method that returns an http.Handler.
Add a Must function
The previous version of the prometheus exporter did not need this because of how it was structured. It was possible to pass a prometheus Registry (WithRegistry) and if it wasn't supplied it would create one, and then the exporter would expose the http.Handler (ServeHTTP()). This means clients would only need to provide a registry, and thus import prometheus, if they needed it for something other then initialization.
I think this is something that can be added to the new version.
The previous version of the prometheus exporter did not need this because of how it was structured. It was possible to pass a prometheus Registry (
WithRegistry) and if it wasn't supplied it would create one, and then the exporter would expose thehttp.Handler(ServeHTTP()). This means clients would only need to provide a registry, and thus import prometheus, if they needed it for something other then initialization.I think this is something that can be added to the new version.
It was pointed out in #3135 that this approach would require the New function to return an error as well as an Exporter.
Documenting a few experiments:
- Adding WithRegisterer seems about the same in terms of ease-of-use as what we have now.
- Making the exporter also an http handler does remove the need to import upstream prometheus (if I can get it to not panic?): https://github.com/open-telemetry/opentelemetry-go/compare/new_sdk/main...dashpole:opentelemetry-go:http_experiment. But we would still want to expose the underlying registry somehow, and allow configuration of the promhttp opts.