Add reference to Instrument on IMetric

[alanwest]

We need a handle to the instrument that generates a metric so that we can include description, unit, and meter name/version upon export.

@victlu @cijothomas

[codecov]

Codecov Report

Merging #2124 (fc92730) into metrics (8577221) will decrease coverage by 0.23%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           metrics    #2124      +/-   ##
===========================================
- Coverage    78.20%   77.96%   -0.24%     
===========================================
  Files          212      212              
  Lines         6634     6658      +24     
===========================================
+ Hits          5188     5191       +3     
- Misses        1446     1467      +21     

Impacted Files	Coverage Δ
src/OpenTelemetry/Metrics/AggregatorStore.cs	0.00% <0.00%> (ø)
...Metrics/MetricAggregators/GaugeMetricAggregator.cs	0.00% <0.00%> (ø)
...ics/MetricAggregators/HistogramMetricAggregator.cs	0.00% <0.00%> (ø)
...y/Metrics/MetricAggregators/SumMetricAggregator.cs	0.00% <0.00%> (ø)
...trics/MetricAggregators/SummaryMetricAggregator.cs	0.00% <0.00%> (ø)
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs	82.35% <0.00%> (+8.82%)	⬆️

[victlu]

I don't think this is is correct.

Logically, Instrument is not 1:1 with metric (aka an Aggregator). It is possible for multiple instruments to contribute values to one metric. (Although, recent view spec has limited View configuration to matching only 1 instrument.)

The proposed View API will allow user to configure description. So, if anything, we need metrics to have it's own copy of description, rather than refer to the instrument.

My suggestion would be to expand IMetric to include Description (and maybe Unit) field. And pass in description (from the source Instrument or future View configuration) when we create the MetricAggregator in the AggregatorStore::MapToMetrics().

[victlu]

My suggestion would be to expand IMetric to include Description (and maybe Unit) field. And pass in description (from the source Instrument or future View configuration) when we create the MetricAggregator in the AggregatorStore::MapToMetrics().

Given we also need to populate InstrumentLibrary info for OTLP protocol, then, we will need a reference to the Meter. (Although we may run into issues (in future) where multiple Instrument may map to one MetricAggregator. But, I say we ignore that and just use the Meter from the Instrument when we create the MetricAggregator.)

[cijothomas]

The proposed View API will allow user to configure description. So, if anything, we need metrics to have it's own copy of description, rather than refer to the instrument.

My suggestion would be to expand IMetric to include Description (and maybe Unit) field. And pass in description (from the source Instrument or future View configuration) when we create the MetricAggregator in the AggregatorStore::MapToMetrics().

+1

[cijothomas]

LGTM.
If you can modify the Console Exporter to print the meter name,version, unit, description, that'll be a good validation!

[alanwest]

If you can modify the Console Exporter to print the meter name,version, unit, description, that'll be a good validation!

Yep, adding that back right now... will be just a moment

Requested changes are made. Merging to make progress. Let us know if any issues.