Separate InstrumentationLibrary from metric.Descriptor

[jmacd]

Please excuse a relatively large PR. I've made efforts to keep it easy-to-review as described below.

The problem being addressed is that the metric.Descriptor contains the instrumentation library name and version, which force consumers of the metric data to re-group metrics by instrumentation library.

One goal is to move the Descriptor type into the new metric/sdkapi sub-package, however that move will only create a larger PR so this PR leaves it in place and a future, simple PR will apply the move. We had to refactor this because the InstrumentOption and MeterOption types do not belong in sdkapi. Thus, NewDescriptor needed to lose those options.

The metric.NewDescriptor() function was problematic because it took both InstrumentOption and MeterOption, and our goal is to disentangle these. A few valid non-test uses of NewDescriptor are modified to manage the InstrumentOption values themselves, i.e., compute an InstrumentConfig and extract the two options (description and unit) manually. All the test-only uses of metric.NewDescriptor() are replaced by metrictest.NewDescriptor() with the old behavior (i.e., accepting varargs options for description and unit). This minimizes the size of this PR.

The metric Controller now maintains one Accumulator per instrumentation library, as opposed to one Accumulator per SDK. This changes the export path, giving it a two-level structure where the exporter sees one instrumentation library at a time, then can visit each export record.

The name CheckpointSet became problematic and was never a good name. "Checkpoint" is the write-oriented behavior that is carried out by the metric Processor; "Reader" is the read-oriented behavior carried out by the exporter. The existing CheckpointSet has been renamed Reader. A new type named InstrumentationLibraryReader is introduced to read one library at a time, which gives a two-level ForEach() style of iteration.

The Controller now takes a factory for building processors instead of a single processor, thus many constructors change from controller.New(processor.New(...), ...) to controller.New(processor.NewFactory(...), ...).

The real, functional changes in this PR are:

1. in metric/registry a new *Provider type maintains a unique-map of Meter implementations, separate from the unique-map of instrument/kind that it already kept. The new *Provider wraps around an implementation of MeterProvider that is not required to maintain name-to-Meter uniqueness, whereas before this was encapsulated in a single unique map.
2. in internal/metric/global a new *Provider type defers MeterProvider initialization explicitly, whereas before the uniqueness was implicitly managed by the registry
3. in sdk/metric/controller/basic, now using the registry Provider, which exposes a List() method to access the registered instrumentation libraries, to export two-level information through the new InstrumentationLibraryReader. there is a slight change of lock semantics; whereas before the Controller used its one Accumulator's state as a lock to determine when refresh was needed, it now uses the same lock used to protect against multiple Start()/Stop() calls; no significant change of behavior here
4. in exporters/otlp/otlpmetric/... the grouping by instrumentation library is removed; calls to source() and sink() are carried out once per instrumentation library.
5. the metric SDK properly implements the instrumentation library SchemaURL field (and is tested)

A bit of test cleanup has been applied. There were a number of test-only implementations of the former CheckpointSet interface that were consolidated into helpers in sdk/metric/processor/processortest.

Part of #2090.
Fixes #2086.
Part of #2119.

File-by-file review notes (non-test files): #2197 (comment)

[codecov]

Codecov Report

Merging #2197 (6e984a5) into main (92551d3) will decrease coverage by 0.0%.
The diff coverage is 78.4%.

@@           Coverage Diff           @@
##            main   #2197     +/-   ##
=======================================
- Coverage   72.5%   72.5%   -0.1%     
=======================================
  Files        168     168             
  Lines      11764   11872    +108     
=======================================
+ Hits        8535    8609     +74     
- Misses      2995    3029     +34     
  Partials     234     234             

Impacted Files	Coverage Δ
...rs/otlp/otlpmetric/internal/otlpmetrictest/data.go	0.0% <0.0%> (ø)
...tlp/otlpmetric/internal/otlpmetrictest/otlptest.go	0.0% <0.0%> (ø)
sdk/export/metric/metric.go	44.6% <ø> (ø)
sdk/metric/processor/basic/config.go	100.0% <ø> (ø)
metric/config.go	67.5% <16.6%> (-20.6%)	⬇️
exporters/prometheus/prometheus.go	65.2% <46.1%> (+<0.1%)	⬆️
sdk/metric/processor/processortest/test.go	85.5% <65.0%> (-6.0%)	⬇️
exporters/stdout/stdoutmetric/metric.go	76.2% <72.8%> (-1.9%)	⬇️
metric/metric.go	54.8% <77.7%> (-1.7%)	⬇️
internal/metric/registry/registry.go	86.8% <84.6%> (ø)
... and 12 more

[jmacd]

@evantorrie please take a look?

[codeboten]

Still working through the PR, just wanted to share the feedback so far

[evantorrie]

LGTM.

[MadVikingGod]

I'm happy with where it is, if you find you can remove the functions discussed above then 👍 to that too.

[jmacd]

Please take a look at the sync.Map now used as the MeterProvider in sdk/metric/controller/basic/controllergo.

[jmacd]

This is ready to merge, I think. Thanks all for your patience.

[jmacd]

@MadVikingGod I reverted two commits that changed the controller to use a sync.Map behind its MeterProvider implementation. This can be addressed in a future PR. I've left a TODO about it, the spec will require this sort of improvement. I propose any further changes to this PR be left as TODOs, but I think it's ready to merge as-is.

[MadVikingGod]

I have talked with @jmacd offline about this, and agree that the sync.Map code is better off as an improvement in a later PR. Count this as reapproving this from me.

[jmacd]

Thank you, approvers & maintainers. This was a tough one, I appreciate your patience. 🚀