OTel metrics API should accept metric.MeterProvider
rcrowe opened this issue · 2 comments
Currently, you are forced to provide a single metric.Reader to NewMetricsHook, which is then passed to metric.NewMeterProvider.
As a meter provider can support multiple exporters & inside the constructor you're creating a new provider, I'd love to expose an API for initialising the metrics hook without being forced to supply a single, specific reader but instead my owner provider. Our applications can only access the global meter & not a single exporter (because we have multiple supported backends) so am unable to use the API as is.
It's not clear to me why taking a metric.Reader was chosen over metric.MeterProvider. Being a pre-1.0 release is there an option to break the API to accept the meter provider, if there's not a specific reason for taking the reader?
@rcrowe thank you for bringing this up and sorry for not following up on this earlier.
There was no specific reason to accept the metric.Reader and I think I did this as I saw this somewhere else. But looking a little bit closely on the API contracts, it does make sense to accept a MeterProvider. For example, this is useful if someone is using otel.GetMeterProvider() to use at system level.
I will deprecate the current API and introduce this change. Given that there is no specific usage, change is easy and straightforward.
@rcrowe the newest OTel hook release contain the proposed constructor change [1]. Please upgrade the dependency and let us know if you see further improvements for the sdk
[1] - https://github.com/open-feature/go-sdk-contrib/releases/tag/hooks%2Fopen-telemetry%2Fv0.2.7