From a3d9ba7fe6bac5196c5224698be50e2673231be5 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 18 Nov 2021 23:06:51 -0800 Subject: [PATCH 1/4] Added MetricPointAttributes to encapsulate keys + values on MetricPoint struct. --- src/OpenTelemetry/Metrics/MetricPoint.cs | 56 +++++++++++++++++++++--- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index b4eb717eaf4..a0af5e98d5a 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -15,6 +15,7 @@ // using System; +using System.Collections.Generic; using System.Threading; namespace OpenTelemetry.Metrics @@ -37,8 +38,7 @@ internal MetricPoint( { this.AggType = aggType; this.StartTime = startTime; - this.Keys = keys; - this.Values = values; + this.Attributes = new MetricPointAttributes(keys, values); this.EndTime = default; this.LongValue = default; this.longVal = default; @@ -70,9 +70,7 @@ internal MetricPoint( } } - public string[] Keys { get; internal set; } - - public object[] Values { get; internal set; } + public MetricPointAttributes Attributes { get; } public DateTimeOffset StartTime { get; internal set; } @@ -290,4 +288,52 @@ internal void TakeSnapShot(bool outputDelta) } } } + + public readonly struct MetricPointAttributes + { + private readonly string[] keys; + private readonly object[] values; + + internal MetricPointAttributes(string[] keys, object[] values) + { + this.keys = keys; + this.values = values; + } + + public int Count => keys?.Length ?? 0; + + public Enumerator GetEnumerator() => new Enumerator(this); + + public struct Enumerator + { + private readonly MetricPointAttributes metricPointAttributes; + private int index; + + internal Enumerator(MetricPointAttributes metricPointAttributes) + { + this.metricPointAttributes = metricPointAttributes; + this.index = 0; + this.Current = default; + } + + public KeyValuePair Current { get; private set; } + + public bool MoveNext() + { + int index = this.index; + + if (index < this.metricPointAttributes.Count) + { + this.Current = new KeyValuePair( + this.metricPointAttributes.keys[index], + this.metricPointAttributes.values[index]); + + this.index++; + return true; + } + + return false; + } + } + } } From f1fced1e6a2d56c3d6929cb3320abb2c9a2c899f Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 19 Nov 2021 22:08:53 -0800 Subject: [PATCH 2/4] Code review and code fixes. --- .../ConsoleMetricExporter.cs | 13 +-- .../Implementation/MetricItemExtensions.cs | 20 ++-- .../Implementation/PrometheusSerializerExt.cs | 42 ++++---- .../.publicApi/net461/PublicAPI.Unshipped.txt | 11 ++- .../netstandard2.0/PublicAPI.Unshipped.txt | 11 ++- src/OpenTelemetry/Metrics/MetricPoint.cs | 75 +++------------ src/OpenTelemetry/ReadOnlyTagCollection.cs | 95 +++++++++++++++++++ .../MetricTests.cs | 7 +- .../HttpClientTests.netcore31.cs | 7 +- .../Metrics/MetricAPITest.cs | 6 +- 10 files changed, 170 insertions(+), 117 deletions(-) create mode 100644 src/OpenTelemetry/ReadOnlyTagCollection.cs diff --git a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs index 06e416fdcb5..e1ec1e3ca2d 100644 --- a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs +++ b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs @@ -80,15 +80,12 @@ public override ExportResult Export(in Batch batch) { string valueDisplay = string.Empty; StringBuilder tagsBuilder = new StringBuilder(); - if (metricPoint.Keys != null) + foreach (var tag in metricPoint.Tags) { - for (int i = 0; i < metricPoint.Keys.Length; i++) - { - tagsBuilder.Append(metricPoint.Keys[i]); - tagsBuilder.Append(':'); - tagsBuilder.Append(metricPoint.Values[i]); - tagsBuilder.Append(' '); - } + tagsBuilder.Append(tag.Key); + tagsBuilder.Append(':'); + tagsBuilder.Append(tag.Value); + tagsBuilder.Append(' '); } var tags = tagsBuilder.ToString().TrimEnd(); diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs index ef23b5883c3..942c01f20ec 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs @@ -156,7 +156,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) TimeUnixNano = (ulong)metricPoint.EndTime.ToUnixTimeNanoseconds(), }; - AddAttributes(metricPoint.Keys, metricPoint.Values, dataPoint.Attributes); + AddAttributes(metricPoint.Tags, dataPoint.Attributes); dataPoint.AsInt = metricPoint.LongValue; sum.DataPoints.Add(dataPoint); @@ -180,7 +180,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) TimeUnixNano = (ulong)metricPoint.EndTime.ToUnixTimeNanoseconds(), }; - AddAttributes(metricPoint.Keys, metricPoint.Values, dataPoint.Attributes); + AddAttributes(metricPoint.Tags, dataPoint.Attributes); dataPoint.AsDouble = metricPoint.DoubleValue; sum.DataPoints.Add(dataPoint); @@ -201,7 +201,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) TimeUnixNano = (ulong)metricPoint.EndTime.ToUnixTimeNanoseconds(), }; - AddAttributes(metricPoint.Keys, metricPoint.Values, dataPoint.Attributes); + AddAttributes(metricPoint.Tags, dataPoint.Attributes); dataPoint.AsInt = metricPoint.LongValue; gauge.DataPoints.Add(dataPoint); @@ -222,7 +222,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) TimeUnixNano = (ulong)metricPoint.EndTime.ToUnixTimeNanoseconds(), }; - AddAttributes(metricPoint.Keys, metricPoint.Values, dataPoint.Attributes); + AddAttributes(metricPoint.Tags, dataPoint.Attributes); dataPoint.AsDouble = metricPoint.DoubleValue; gauge.DataPoints.Add(dataPoint); @@ -245,7 +245,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) TimeUnixNano = (ulong)metricPoint.EndTime.ToUnixTimeNanoseconds(), }; - AddAttributes(metricPoint.Keys, metricPoint.Values, dataPoint.Attributes); + AddAttributes(metricPoint.Tags, dataPoint.Attributes); dataPoint.Count = (ulong)metricPoint.LongValue; dataPoint.Sum = metricPoint.DoubleValue; @@ -272,15 +272,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) return otlpMetric; } - private static void AddAttributes(string[] keys, object[] values, RepeatedField attributes) + private static void AddAttributes(ReadOnlyTagCollection tags, RepeatedField attributes) { - if (keys != null) + foreach (var tag in tags) { - for (int i = 0; i < keys.Length; i++) - { - KeyValuePair tag = new KeyValuePair(keys[i], values[i]); - attributes.Add(tag.ToOtlpAttribute()); - } + attributes.Add(tag.ToOtlpAttribute()); } } diff --git a/src/OpenTelemetry.Exporter.Prometheus/Implementation/PrometheusSerializerExt.cs b/src/OpenTelemetry.Exporter.Prometheus/Implementation/PrometheusSerializerExt.cs index b197a4195db..6ddced4e2ce 100644 --- a/src/OpenTelemetry.Exporter.Prometheus/Implementation/PrometheusSerializerExt.cs +++ b/src/OpenTelemetry.Exporter.Prometheus/Implementation/PrometheusSerializerExt.cs @@ -39,25 +39,25 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric) { foreach (ref var metricPoint in metric.GetMetricPoints()) { - var keys = metricPoint.Keys; - var values = metricPoint.Values; + var tags = metricPoint.Tags; var timestamp = metricPoint.EndTime.ToUnixTimeMilliseconds(); // Counter and Gauge cursor = WriteMetricName(buffer, cursor, metric.Name, metric.Unit); - if (keys != null && keys.Length > 0) + if (tags.Count > 0) { buffer[cursor++] = unchecked((byte)'{'); - for (var i = 0; i < keys.Length; i++) + int i = 0; + foreach (var tag in tags) { - if (i > 0) + if (i++ > 0) { buffer[cursor++] = unchecked((byte)','); } - cursor = WriteLabel(buffer, cursor, keys[i], values[i]); + cursor = WriteLabel(buffer, cursor, tag.Key, tag.Value); } buffer[cursor++] = unchecked((byte)'}'); @@ -85,8 +85,7 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric) { foreach (ref var metricPoint in metric.GetMetricPoints()) { - var keys = metricPoint.Keys; - var values = metricPoint.Values; + var tags = metricPoint.Tags; var timestamp = metricPoint.EndTime.ToUnixTimeMilliseconds(); // Histogram buckets @@ -100,13 +99,10 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric) cursor = WriteMetricName(buffer, cursor, metric.Name, metric.Unit); cursor = WriteAsciiStringNoEscape(buffer, cursor, "_bucket{"); - if (keys != null) + foreach (var tag in tags) { - for (var i = 0; i < keys.Length; i++) - { - cursor = WriteLabel(buffer, cursor, keys[i], values[i]); - buffer[cursor++] = unchecked((byte)','); - } + cursor = WriteLabel(buffer, cursor, tag.Key, tag.Value); + buffer[cursor++] = unchecked((byte)','); } cursor = WriteAsciiStringNoEscape(buffer, cursor, "le=\""); @@ -134,18 +130,19 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric) cursor = WriteMetricName(buffer, cursor, metric.Name, metric.Unit); cursor = WriteAsciiStringNoEscape(buffer, cursor, "_sum"); - if (keys != null && keys.Length > 0) + if (tags.Count > 0) { buffer[cursor++] = unchecked((byte)'{'); - for (var i = 0; i < keys.Length; i++) + int i = 0; + foreach (var tag in tags) { - if (i > 0) + if (i++ > 0) { buffer[cursor++] = unchecked((byte)','); } - cursor = WriteLabel(buffer, cursor, keys[i], values[i]); + cursor = WriteLabel(buffer, cursor, tag.Key, tag.Value); } buffer[cursor++] = unchecked((byte)'}'); @@ -164,18 +161,19 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric) cursor = WriteMetricName(buffer, cursor, metric.Name, metric.Unit); cursor = WriteAsciiStringNoEscape(buffer, cursor, "_count"); - if (keys != null && keys.Length > 0) + if (tags.Count > 0) { buffer[cursor++] = unchecked((byte)'{'); - for (var i = 0; i < keys.Length; i++) + int i = 0; + foreach (var tag in tags) { - if (i > 0) + if (i++ > 0) { buffer[cursor++] = unchecked((byte)','); } - cursor = WriteLabel(buffer, cursor, keys[i], values[i]); + cursor = WriteLabel(buffer, cursor, tag.Key, tag.Value); } buffer[cursor++] = unchecked((byte)'}'); diff --git a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt index fa81310cf4f..7fda23de593 100644 --- a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt @@ -60,11 +60,10 @@ OpenTelemetry.Metrics.MetricPoint.BucketCounts.get -> long[] OpenTelemetry.Metrics.MetricPoint.DoubleValue.get -> double OpenTelemetry.Metrics.MetricPoint.EndTime.get -> System.DateTimeOffset OpenTelemetry.Metrics.MetricPoint.ExplicitBounds.get -> double[] -OpenTelemetry.Metrics.MetricPoint.Keys.get -> string[] OpenTelemetry.Metrics.MetricPoint.LongValue.get -> long OpenTelemetry.Metrics.MetricPoint.MetricPoint() -> void OpenTelemetry.Metrics.MetricPoint.StartTime.get -> System.DateTimeOffset -OpenTelemetry.Metrics.MetricPoint.Values.get -> object[] +OpenTelemetry.Metrics.MetricPoint.Tags.get -> OpenTelemetry.ReadOnlyTagCollection OpenTelemetry.Metrics.MetricReader OpenTelemetry.Metrics.MetricReader.Collect(int timeoutMilliseconds = -1) -> bool OpenTelemetry.Metrics.MetricReader.Dispose() -> void @@ -92,6 +91,14 @@ OpenTelemetry.Metrics.MetricType.LongSum = 26 -> OpenTelemetry.Metrics.MetricTyp OpenTelemetry.Metrics.MetricTypeExtensions OpenTelemetry.Metrics.PeriodicExportingMetricReader OpenTelemetry.Metrics.PeriodicExportingMetricReader.PeriodicExportingMetricReader(OpenTelemetry.BaseExporter exporter, int exportIntervalMilliseconds = 60000, int exportTimeoutMilliseconds = 30000) -> void +OpenTelemetry.ReadOnlyTagCollection +OpenTelemetry.ReadOnlyTagCollection.Count.get -> int +OpenTelemetry.ReadOnlyTagCollection.Enumerator +OpenTelemetry.ReadOnlyTagCollection.Enumerator.Current.get -> System.Collections.Generic.KeyValuePair +OpenTelemetry.ReadOnlyTagCollection.Enumerator.Enumerator() -> void +OpenTelemetry.ReadOnlyTagCollection.Enumerator.MoveNext() -> bool +OpenTelemetry.ReadOnlyTagCollection.GetEnumerator() -> OpenTelemetry.ReadOnlyTagCollection.Enumerator +OpenTelemetry.ReadOnlyTagCollection.ReadOnlyTagCollection() -> void OpenTelemetry.Trace.BatchExportActivityProcessorOptions OpenTelemetry.Trace.BatchExportActivityProcessorOptions.BatchExportActivityProcessorOptions() -> void override OpenTelemetry.BaseExportProcessor.OnForceFlush(int timeoutMilliseconds) -> bool diff --git a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index fa81310cf4f..7fda23de593 100644 --- a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -60,11 +60,10 @@ OpenTelemetry.Metrics.MetricPoint.BucketCounts.get -> long[] OpenTelemetry.Metrics.MetricPoint.DoubleValue.get -> double OpenTelemetry.Metrics.MetricPoint.EndTime.get -> System.DateTimeOffset OpenTelemetry.Metrics.MetricPoint.ExplicitBounds.get -> double[] -OpenTelemetry.Metrics.MetricPoint.Keys.get -> string[] OpenTelemetry.Metrics.MetricPoint.LongValue.get -> long OpenTelemetry.Metrics.MetricPoint.MetricPoint() -> void OpenTelemetry.Metrics.MetricPoint.StartTime.get -> System.DateTimeOffset -OpenTelemetry.Metrics.MetricPoint.Values.get -> object[] +OpenTelemetry.Metrics.MetricPoint.Tags.get -> OpenTelemetry.ReadOnlyTagCollection OpenTelemetry.Metrics.MetricReader OpenTelemetry.Metrics.MetricReader.Collect(int timeoutMilliseconds = -1) -> bool OpenTelemetry.Metrics.MetricReader.Dispose() -> void @@ -92,6 +91,14 @@ OpenTelemetry.Metrics.MetricType.LongSum = 26 -> OpenTelemetry.Metrics.MetricTyp OpenTelemetry.Metrics.MetricTypeExtensions OpenTelemetry.Metrics.PeriodicExportingMetricReader OpenTelemetry.Metrics.PeriodicExportingMetricReader.PeriodicExportingMetricReader(OpenTelemetry.BaseExporter exporter, int exportIntervalMilliseconds = 60000, int exportTimeoutMilliseconds = 30000) -> void +OpenTelemetry.ReadOnlyTagCollection +OpenTelemetry.ReadOnlyTagCollection.Count.get -> int +OpenTelemetry.ReadOnlyTagCollection.Enumerator +OpenTelemetry.ReadOnlyTagCollection.Enumerator.Current.get -> System.Collections.Generic.KeyValuePair +OpenTelemetry.ReadOnlyTagCollection.Enumerator.Enumerator() -> void +OpenTelemetry.ReadOnlyTagCollection.Enumerator.MoveNext() -> bool +OpenTelemetry.ReadOnlyTagCollection.GetEnumerator() -> OpenTelemetry.ReadOnlyTagCollection.Enumerator +OpenTelemetry.ReadOnlyTagCollection.ReadOnlyTagCollection() -> void OpenTelemetry.Trace.BatchExportActivityProcessorOptions OpenTelemetry.Trace.BatchExportActivityProcessorOptions.BatchExportActivityProcessorOptions() -> void override OpenTelemetry.BaseExportProcessor.OnForceFlush(int timeoutMilliseconds) -> bool diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index 100a58d98cd..4dd20d74791 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -15,19 +15,19 @@ // using System; -using System.Collections.Generic; using System.Threading; namespace OpenTelemetry.Metrics { public struct MetricPoint { + private readonly AggregationType aggType; + private readonly object lockObject; + private readonly long[] bucketCounts; private long longVal; private long lastLongSum; private double doubleVal; private double lastDoubleSum; - private object lockObject; - private long[] bucketCounts; internal MetricPoint( AggregationType aggType, @@ -36,9 +36,9 @@ internal MetricPoint( object[] values, double[] histogramBounds) { - this.AggType = aggType; + this.aggType = aggType; this.StartTime = startTime; - this.Attributes = new MetricPointAttributes(keys, values); + this.Tags = new ReadOnlyTagCollection(keys, values); this.EndTime = default; this.LongValue = default; this.longVal = default; @@ -48,14 +48,14 @@ internal MetricPoint( this.lastDoubleSum = default; this.MetricPointStatus = MetricPointStatus.NoCollectPending; - if (this.AggType == AggregationType.Histogram) + if (this.aggType == AggregationType.Histogram) { this.ExplicitBounds = histogramBounds; this.BucketCounts = new long[this.ExplicitBounds.Length + 1]; this.bucketCounts = new long[this.ExplicitBounds.Length + 1]; this.lockObject = new object(); } - else if (this.AggType == AggregationType.HistogramSumCount) + else if (this.aggType == AggregationType.HistogramSumCount) { this.ExplicitBounds = null; this.BucketCounts = null; @@ -71,7 +71,10 @@ internal MetricPoint( } } - public MetricPointAttributes Attributes { get; } + /// + /// Gets the tags associated with the metric point. + /// + public ReadOnlyTagCollection Tags { get; } public DateTimeOffset StartTime { get; internal set; } @@ -87,11 +90,9 @@ internal MetricPoint( internal MetricPointStatus MetricPointStatus { get; private set; } - private readonly AggregationType AggType { get; } - internal void Update(long number) { - switch (this.AggType) + switch (this.aggType) { case AggregationType.LongSumIncomingDelta: { @@ -135,7 +136,7 @@ internal void Update(long number) internal void Update(double number) { - switch (this.AggType) + switch (this.aggType) { case AggregationType.DoubleSumIncomingDelta: { @@ -211,7 +212,7 @@ internal void Update(double number) internal void TakeSnapshot(bool outputDelta) { - switch (this.AggType) + switch (this.aggType) { case AggregationType.LongSumIncomingDelta: case AggregationType.LongSumIncomingCumulative: @@ -355,52 +356,4 @@ internal void TakeSnapshot(bool outputDelta) } } } - - public readonly struct MetricPointAttributes - { - private readonly string[] keys; - private readonly object[] values; - - internal MetricPointAttributes(string[] keys, object[] values) - { - this.keys = keys; - this.values = values; - } - - public int Count => keys?.Length ?? 0; - - public Enumerator GetEnumerator() => new Enumerator(this); - - public struct Enumerator - { - private readonly MetricPointAttributes metricPointAttributes; - private int index; - - internal Enumerator(MetricPointAttributes metricPointAttributes) - { - this.metricPointAttributes = metricPointAttributes; - this.index = 0; - this.Current = default; - } - - public KeyValuePair Current { get; private set; } - - public bool MoveNext() - { - int index = this.index; - - if (index < this.metricPointAttributes.Count) - { - this.Current = new KeyValuePair( - this.metricPointAttributes.keys[index], - this.metricPointAttributes.values[index]); - - this.index++; - return true; - } - - return false; - } - } - } } diff --git a/src/OpenTelemetry/ReadOnlyTagCollection.cs b/src/OpenTelemetry/ReadOnlyTagCollection.cs new file mode 100644 index 00000000000..fdbfeeaa7de --- /dev/null +++ b/src/OpenTelemetry/ReadOnlyTagCollection.cs @@ -0,0 +1,95 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Collections.Generic; + +namespace OpenTelemetry +{ + /// + /// A read-only collection of tag key/value pairs. + /// + // Note: Does not implement IReadOnlyCollection<> or IEnumerable<> to + // prevent accidental boxing. + public readonly struct ReadOnlyTagCollection + { + private readonly string[] keys; + private readonly object[] values; + + internal ReadOnlyTagCollection(string[] keys, object[] values) + { + this.keys = keys; + this.values = values; + } + + /// + /// Gets the number of tags in the collection. + /// + public int Count => this.keys?.Length ?? 0; + + /// + /// Returns an enumerator that iterates through the tags. + /// + /// . + public Enumerator GetEnumerator() => new(this); + + /// + /// Enumerates the elements of a . + /// + // Note: Does not implement IEnumerator<> to prevent accidental boxing. + public struct Enumerator + { + private readonly ReadOnlyTagCollection source; + private int index; + + internal Enumerator(ReadOnlyTagCollection source) + { + this.source = source; + this.index = 0; + this.Current = default; + } + + /// + /// Gets the tag at the current position of the enumerator. + /// + public KeyValuePair Current { get; private set; } + + /// + /// Advances the enumerator to the next element of the . + /// + /// if the enumerator was + /// successfully advanced to the next element; if the enumerator has passed the end of the + /// collection. + public bool MoveNext() + { + int index = this.index; + + if (index < this.source.Count) + { + this.Current = new KeyValuePair( + this.source.keys[index], + this.source.values[index]); + + this.index++; + return true; + } + + return false; + } + } + } +} diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index c52fdb2fc20..9e22ff0ad00 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -113,10 +113,11 @@ public async Task RequestMetricIsCaptured() Assert.Equal(1, bucket.Count); */ - var attributes = new KeyValuePair[metricPoint.Keys.Length]; - for (int i = 0; i < attributes.Length; i++) + var attributes = new KeyValuePair[metricPoint.Tags.Count]; + int i = 0; + foreach (var tag in metricPoint.Tags) { - attributes[i] = new KeyValuePair(metricPoint.Keys[i], metricPoint.Values[i]); + attributes[i++] = tag; } var method = new KeyValuePair(SemanticConventions.AttributeHttpMethod, "GET"); diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs index 0172a9eb8f8..e668b953bc6 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs @@ -162,10 +162,11 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut Assert.Equal(1L, metricPoint.LongValue); Assert.Equal(activity.Duration.TotalMilliseconds, metricPoint.DoubleValue); - var attributes = new KeyValuePair[metricPoint.Keys.Length]; - for (int i = 0; i < attributes.Length; i++) + var attributes = new KeyValuePair[metricPoint.Tags.Count]; + int i = 0; + foreach (var tag in metricPoint.Tags) { - attributes[i] = new KeyValuePair(metricPoint.Keys[i], metricPoint.Values[i]); + attributes[i++] = tag; } var method = new KeyValuePair(SemanticConventions.AttributeHttpMethod, tc.Method); diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 2ab58a86ce5..d87c66bea86 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -66,8 +66,7 @@ public void ObserverCallbackTest() Assert.Single(metricPoints); var metricPoint = metricPoints[0]; Assert.Equal(100, metricPoint.LongValue); - Assert.NotNull(metricPoint.Keys); - Assert.NotNull(metricPoint.Values); + Assert.True(metricPoint.Tags.Count > 0); } [Fact] @@ -97,8 +96,7 @@ public void ObserverCallbackExceptionTest() Assert.Single(metricPoints); var metricPoint = metricPoints[0]; Assert.Equal(100, metricPoint.LongValue); - Assert.NotNull(metricPoint.Keys); - Assert.NotNull(metricPoint.Values); + Assert.True(metricPoint.Tags.Count > 0); } [Theory] From fb0e70e10a2bc8d5e85ddadc107d7b7becf50197 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Sat, 20 Nov 2021 09:54:16 -0800 Subject: [PATCH 3/4] CHANGELOG update. --- src/OpenTelemetry/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 95402c66ab0..4f94318c16c 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Added `ReadOnlyTagCollection` and expose `Tags` on `MetricPoint` instead of + `Keys`+`Values` + ([#2642](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2642)) + ## 1.2.0-beta2 Released 2021-Nov-19 From 9ae9fbfc4dc7203c5a6341aa0d4abf24427e3ca6 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Sat, 20 Nov 2021 10:06:13 -0800 Subject: [PATCH 4/4] Added an assert for keys & values in MetricPoint ctor. --- src/OpenTelemetry/Metrics/MetricPoint.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index 4dd20d74791..ee45e369a60 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -15,6 +15,7 @@ // using System; +using System.Diagnostics; using System.Threading; namespace OpenTelemetry.Metrics @@ -36,6 +37,8 @@ internal MetricPoint( object[] values, double[] histogramBounds) { + Debug.Assert((keys?.Length ?? 0) == (values?.Length ?? 0), "Key and value array lengths did not match."); + this.aggType = aggType; this.StartTime = startTime; this.Tags = new ReadOnlyTagCollection(keys, values);