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

OpenCensus metrics shim: update to new experimental specification #5011

Closed
3 tasks
dashpole opened this issue Dec 1, 2022 · 2 comments · Fixed by #5835
Closed
3 tasks

OpenCensus metrics shim: update to new experimental specification #5011

dashpole opened this issue Dec 1, 2022 · 2 comments · Fixed by #5835
Labels
Feature Request Suggest an idea for this project help wanted

Comments

@dashpole
Copy link
Contributor

dashpole commented Dec 1, 2022

Describe the solution you'd like

After open-telemetry/opentelemetry-specification#2951, and open-telemetry/opentelemetry-specification#2979, we should update our OpenCensus bridge implementation to comply with the new specification.

Note that the specifications are experimental.

Changes:

cc @jack-berg @jsuereth @psx95

@dashpole dashpole added the Feature Request Suggest an idea for this project label Dec 1, 2022
@psx95
Copy link
Contributor

psx95 commented Dec 1, 2022

I would like to take this up if its ok with everyone.

@jack-berg
Copy link
Member

Go for it! Some notes:

  • There's already a MetricProducer interface that mirrors the spec. Its in an internal package and has to stay internal until the spec stabilizes.
  • MetricProducer extends an empty interface called CollectionRegistration. CollectionRegistration shows up in MetricReader#register, which is akin to the proposed MetricReader#registerProducer method. The idea is that MetricProducers register are each registered with MetricReaders during SdkMeterProvider initialization.
  • CollectionRegistration is empty and we use a weird trick to cast to MetricProducer, which is actually useful.
  • I think we once the spec is stable, we could shake things up and achieve an API that is in line with the spirit of the spec, but with some minor changes we're locked into by the fact that the recently added spec details didn't exist when we had to write some of this code. I would propose adding a CollectionRegistration#getMetricProducer method. It would be ideal if we could rename CollectionRegistration to MetricProducer and include the collectAllMetrics() method, but that would be a breaking change.

Now that I've written that all out, I'm not sure how much progress can be made on this front with stabilizing the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Suggest an idea for this project help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants