Metrics: Use ReadOnlyTagCollection to encapsulate key/values on MetricPoint

[CodeBlanch]

Closes #2062

Relates to #2062 (comment)

Changes

Add ReadOnlyTagCollection and expose Tags on MetricPoint instead of string[] Keys & object[] Values

Public API Changes

public struct MetricPoint
{
-        public string[] Keys { get; internal set; }
-        public object[] Values { get; internal set; }
+        public ReadOnlyTagCollection Tags { get; }
}

New struct:

    public readonly struct ReadOnlyTagCollection
    {
        public int Count { get; }
        public Enumerator GetEnumerator() {}

        public struct Enumerator
        {
            public KeyValuePair<string, object> Current { get; private set; }
            public bool MoveNext() {}
        }
    }

TODOs

• CHANGELOG.md updated for non-trivial changes
• Changes in public API reviewed

[cijothomas]

The API use the word 'Tags' instead of Attributes, so we can stick with same name here too.

[CodeBlanch]

I renamed the property "Tags" and then I renamed the structure ReadOnlyTagCollection. There isn't anything really MetricPoint-specific about it, so I thought it would be good to keep it general in case we wanted to use it in other spots in the future?

[cijothomas]

@CodeBlanch This looks good. It do increase our public API than using a simple IEnumerable, but its worth it in my opinion.
(Don't know if we could piggyback on Batch?....It might cause confusion..)

[codecov]

Codecov Report

Merging #2642 (bd81f6b) into main (31a2d67) will increase coverage by 0.02%.
The diff coverage is 92.45%.

@@            Coverage Diff             @@
##             main    #2642      +/-   ##
==========================================
+ Coverage   81.31%   81.34%   +0.02%     
==========================================
  Files         245      246       +1     
  Lines        8645     8658      +13     
==========================================
+ Hits         7030     7043      +13     
  Misses       1615     1615              

Impacted Files	Coverage Δ
...tryProtocol/Implementation/MetricItemExtensions.cs	50.00% <42.85%> (-0.71%)	⬇️
...ometheus/Implementation/PrometheusSerializerExt.cs	100.00% <100.00%> (ø)
src/OpenTelemetry/Metrics/MetricPoint.cs	90.51% <100.00%> (-0.24%)	⬇️
src/OpenTelemetry/ReadOnlyTagCollection.cs	100.00% <100.00%> (ø)

[cijothomas]

We might be able to leverage this for Activity.TagLists, by doing the alloc-free thingie internally and exposing this ReadOnlyTagCollection for exporters, so each exporters doesn't have to repeat the alloc-free foreach.

[cijothomas]

LGTM.
(I know the PR is in draft, but this looks good)

[CodeBlanch]

I ran a benchmark to make sure struct ReadOnlyTagCollection doesn't have any strange perf as opposed to class ReadOnlyTagCollection, seems good:

Struct

Method	Mean	Error	StdDev	Allocated
MetricPointBenchmark	17.89 ns	0.380 ns	0.437 ns	-

Class

Method	Mean	Error	StdDev	Allocated
MetricPointBenchmark	17.66 ns	0.292 ns	0.273 ns	-

Benchmarks

    [MemoryDiagnoser]
    public class MetricPointBenchmarks
    {
        private readonly string[] keys = new string[] { "key1", "key2" };
        private readonly object[] values = new object[] { 1, 2 };
        private MetricPoint metricPoint;

        public MetricPointBenchmarks()
        {
            this.metricPoint = new MetricPoint(
                AggregationType.LongSumIncomingDelta,
                DateTimeOffset.UtcNow,
                this.keys,
                this.values,
                null);
        }

        [Benchmark]
        public int MetricPointBenchmark()
        {
            ref var metricPoint = ref this.metricPoint;

            var tags = metricPoint.Tags;

            int tagCount = 0;
            foreach (var tag in tags)
            {
                if (!string.IsNullOrEmpty(tag.Key))
                {
                    tagCount++;
                }
            }

            return tagCount;
        }
    }