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

Metrics: create independent MeterProvider #2086

Closed
jmacd opened this issue Jul 15, 2021 · 2 comments · Fixed by #2197
Closed

Metrics: create independent MeterProvider #2086

jmacd opened this issue Jul 15, 2021 · 2 comments · Fixed by #2197
Labels
area:metrics Part of OpenTelemetry Metrics enhancement New feature or request

Comments

@jmacd
Copy link
Contributor

jmacd commented Jul 15, 2021

The existing Metrics SDK supports a MeterProvider API but the underlying components treat the Resource as part of the export Record and do not formally recognize a division between Meters. This issue outlines a proposal to solve this. The outcome will be that each:

  1. metric.Descriptor no longer includes instrumentation name and version, nor Resource
  2. metric/registry supports a UniqueMeterProvider that maintains a map of named MeterImpls.
  3. metrics Export() includes additional SourceData (instrumentation name and version, schema URL, Resource)
  4. sdk/metric/controller maintains once accumulator/checkpointer per Meter, exports them separately
  5. OTLP exporter for metrics no longer needs to group data, is much simpler.

This is too much change for a single PR, but I have prototyped it in #2085. This issue will cover breaking up the draft PR into smaller pieces.

Another goal is met in the draft PR above, that metric.Descriptor could be moved out of the top-level metric package into a sub-package (possibly named "sdkapi"), as demonstrated in #2044.

@jmacd jmacd added the bug Something isn't working label Jul 15, 2021
@jmacd
Copy link
Contributor Author

jmacd commented Jul 15, 2021

Discussed in the Go SIG meeting. The main objection to the draft PR #2085 is that it splits Metrics exports into one call per Meter. Instead, follow what's been done for Trace, export a slice of InstrumentationLibraryMetrics.

@jmacd
Copy link
Contributor Author

jmacd commented Jul 15, 2021

Next steps: I will continue with the refactoring proposed here in smaller PRs.

  1. Remove instrumentation library and version from metric.Descriptor. Pass this information through a MeterProvider and store it in the metric Controller. Add registry.UniqueMeterProvider.
  2. Create a metric/sdkapi subpackage and move Descriptor, InstrumentImpl, MeterImpl, SyncImpl, AsyncImpl, and a few other implementation-specific types into this package. These are the types that will survive regardless of which API design is chosen, as they support the current SDK's low-level interface shared by any API.
  3. Remove Resource from the metric export Record.
  4. Remove grouping from OTLP exporter, receive a slice of InstrumetationLibraryMetric recods.

@MadVikingGod MadVikingGod added area:metrics Part of OpenTelemetry Metrics enhancement New feature or request and removed bug Something isn't working labels Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics enhancement New feature or request
Projects
None yet
2 participants