Proposal: BasicController to MeterProvider

[hdost]

Background

While looking into #961 I hadn't gone into the metrics side of things too much, and I noticed that the equivalent part is called the BasicController which isn't an obvious 1:1. Additionally, it seems to be missing some features. (I know that tracing is more the focus right now as I don't think we've stabilized tracing yet.

Proposal

• Move the code of opentelemetry-sdk::metrics::controllers::BasicController to opentelemetry-sdk::metrics::SdkMeterProvider
  ** New source file would be src/metrics/provider.rs
• Make the exporters accept a dyn MeterProvider (this is questionable though as the exporter shouldn't need to know about the MeterProvider the relationship should be inversely related. Additionally, the current mode of the MeterProvider doesn't allow for multiple exporters.

Reference
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#meterprovider

[jtescher]

Yeah the SDK needs considerable changes. I'm working on a branch to bring in the latest spec changes, adding support for views and other new constructs. Can post back here once I've got more thought through, but open to suggestions as well 👍

[hdost]

Yeah the SDK needs considerable changes. I'm working on a branch to bring in the latest spec changes, adding support for views and other new constructs. Can post back here once I've got more thought through, but open to suggestions as well 👍

Can you post as a separate branch? I could make some PRs to it that way.

[TommyCpp]

I actually also have a WIP for view support from #897. I can contribute to your branch if needed.

[hdost]

@jtescher I assume it's https://github.com/jtescher/opentelemetry-rust-1/tree/new-metrics ?

[jtescher]

Ah yeah, meant to include the link. Got most of the spec implemented, but needs cleanup / tests and to update the exporters outside of prometheus. @hdost saw you are updating proto, that'd be useful as the new metrics format lines up with that.

[jtescher]

Also still have to think through the most ergonomic way to register callbacks

[hdost]

Ah yeah, meant to include the link. Got most of the spec implemented, but needs cleanup / tests and to update the exporters outside of prometheus. @hdost saw you are updating proto, that'd be useful as the new metrics format lines up with that.

Hoping to have it ready to go before I go to bed tonight.

[hdost]

Update, it seems like the -proto project is not using the same version as I'm using locally. And the CI is Opaque 😢 it's not clear what the difference is. And I'm not really seeing docs on how I can specify the protoc or protobuf to use.
The CI seems to be using 3.20.3 which I've downloaded and attempted setting PROTOC and PROTOC_INCLUDE, but to no avail.

[TommyCpp]

The CI seems to be using 3.20.3 which I've downloaded and attempted setting PROTOC and PROTOC_INCLUDE, but to no avail.

Maybe add a step in GitHub action and run protoc --version? Although from the log I do see the setup steps says it using 3.20.3

[hdost]

The CI seems to be using 3.20.3 which I've downloaded and attempted setting PROTOC and PROTOC_INCLUDE, but to no avail.

Maybe add a step in GitHub action and run protoc --version? Although from the log I do see the setup steps says it using 3.20.3

The issue was my lock file 🤦🏼‍♂️

[DerekStrickland]

Totally selfish question alert. 🤓

I'm currently working on a Proof-Of-Concept that pushes metrics to an Otel Collector. I ran into the same issues as reported in #914 and was about to give his example a try, but it looks like the work on https://github.com/jtescher/opentelemetry-rust-1/tree/new-metrics is about to change the world 🤣 .

Any thoughts on how close this is to landing? Just wondering if I should wait, or keep going with my POC. It wouldn't be the end of the world if I had to refactor this in a couple of weeks, but on the other hand if it's days away, I could pivot and come back next week.

Another option, if it would be helpful and not a distraction, is that I could use my POC as a test case for the work on that branch. Figured I'd offer.

[hdost]

FYI #1000 is doing the large refactor, should come after 0.19.0 release

[TommyCpp]

#1000 has merged. Closing this issue