From 8d339bb7408bd71d743d290a5a59a8024d17af27 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai Date: Tue, 23 Nov 2021 13:00:39 -0800 Subject: [PATCH] Update MetricPoint HistogramMeasurements (#2664) --- .../ConsoleMetricExporter.cs | 52 ++++++------ .../Implementation/MetricItemExtensions.cs | 13 +-- .../Implementation/PrometheusSerializerExt.cs | 61 ++++++-------- .../.publicApi/net461/PublicAPI.Unshipped.txt | 13 ++- .../netstandard2.0/PublicAPI.Unshipped.txt | 13 ++- src/OpenTelemetry/CHANGELOG.md | 11 ++- src/OpenTelemetry/Metrics/HistogramBucket.cs | 31 +++++++ src/OpenTelemetry/Metrics/HistogramBuckets.cs | 82 ++++++++++++++++++ .../Metrics/HistogramMeasurements.cs | 45 ---------- src/OpenTelemetry/Metrics/MetricPoint.cs | 84 ++++++++----------- .../Metrics/AggregatorTest.cs | 30 ++++--- .../Metrics/MetricViewTests.cs | 39 +++++---- 12 files changed, 279 insertions(+), 195 deletions(-) create mode 100644 src/OpenTelemetry/Metrics/HistogramBucket.cs create mode 100644 src/OpenTelemetry/Metrics/HistogramBuckets.cs delete mode 100644 src/OpenTelemetry/Metrics/HistogramMeasurements.cs diff --git a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs index 3465f413143..064c1dad14a 100644 --- a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs +++ b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs @@ -99,42 +99,40 @@ public override ExportResult Export(in Batch batch) var count = metricPoint.GetHistogramCount(); bucketsBuilder.Append($"Sum: {sum} Count: {count} \n"); - var explicitBounds = metricPoint.GetExplicitBounds(); - if (explicitBounds != null) + bool isFirstIteration = true; + double previousExplicitBound = default; + foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets()) { - var bucketCounts = metricPoint.GetBucketCounts(); - for (int i = 0; i < explicitBounds.Length + 1; i++) + if (isFirstIteration) { - if (i == 0) - { - bucketsBuilder.Append("(-Infinity,"); - bucketsBuilder.Append(explicitBounds[i]); - bucketsBuilder.Append(']'); - bucketsBuilder.Append(':'); - bucketsBuilder.Append(bucketCounts[i]); - } - else if (i == explicitBounds.Length) + bucketsBuilder.Append("(-Infinity,"); + bucketsBuilder.Append(histogramMeasurement.ExplicitBound); + bucketsBuilder.Append(']'); + bucketsBuilder.Append(':'); + bucketsBuilder.Append(histogramMeasurement.BucketCount); + previousExplicitBound = histogramMeasurement.ExplicitBound; + isFirstIteration = false; + } + else + { + bucketsBuilder.Append('('); + bucketsBuilder.Append(previousExplicitBound); + bucketsBuilder.Append(','); + if (histogramMeasurement.ExplicitBound != double.PositiveInfinity) { - bucketsBuilder.Append('('); - bucketsBuilder.Append(explicitBounds[i - 1]); - bucketsBuilder.Append(','); - bucketsBuilder.Append("+Infinity]"); - bucketsBuilder.Append(':'); - bucketsBuilder.Append(bucketCounts[i]); + bucketsBuilder.Append(histogramMeasurement.ExplicitBound); } else { - bucketsBuilder.Append('('); - bucketsBuilder.Append(explicitBounds[i - 1]); - bucketsBuilder.Append(','); - bucketsBuilder.Append(explicitBounds[i]); - bucketsBuilder.Append(']'); - bucketsBuilder.Append(':'); - bucketsBuilder.Append(bucketCounts[i]); + bucketsBuilder.Append("+Infinity"); } - bucketsBuilder.AppendLine(); + bucketsBuilder.Append(']'); + bucketsBuilder.Append(':'); + bucketsBuilder.Append(histogramMeasurement.BucketCount); } + + bucketsBuilder.AppendLine(); } valueDisplay = bucketsBuilder.ToString(); diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs index 1857c899ddb..e85cdcda231 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs @@ -249,17 +249,12 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) dataPoint.Count = (ulong)metricPoint.LongValue; dataPoint.Sum = metricPoint.DoubleValue; - var bucketCounts = metricPoint.GetBucketCounts(); - if (bucketCounts != null) + foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets()) { - var explicitBounds = metricPoint.GetExplicitBounds(); - for (int i = 0; i < bucketCounts.Length; i++) + dataPoint.BucketCounts.Add((ulong)histogramMeasurement.BucketCount); + if (histogramMeasurement.ExplicitBound != double.PositiveInfinity) { - dataPoint.BucketCounts.Add((ulong)bucketCounts[i]); - if (i < bucketCounts.Length - 1) - { - dataPoint.ExplicitBounds.Add(explicitBounds[i]); - } + dataPoint.ExplicitBounds.Add(histogramMeasurement.ExplicitBound); } } diff --git a/src/OpenTelemetry.Exporter.Prometheus/Implementation/PrometheusSerializerExt.cs b/src/OpenTelemetry.Exporter.Prometheus/Implementation/PrometheusSerializerExt.cs index 59b727a86eb..ea554caa82b 100644 --- a/src/OpenTelemetry.Exporter.Prometheus/Implementation/PrometheusSerializerExt.cs +++ b/src/OpenTelemetry.Exporter.Prometheus/Implementation/PrometheusSerializerExt.cs @@ -91,51 +91,42 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric) var tags = metricPoint.Tags; var timestamp = metricPoint.EndTime.ToUnixTimeMilliseconds(); - var bucketCounts = metricPoint.GetBucketCounts(); - if (bucketCounts != null) + long totalCount = 0; + foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets()) { - // Histogram buckets - var explicitBounds = metricPoint.GetExplicitBounds(); - long totalCount = 0; - for (int idxBound = 0; idxBound < explicitBounds.Length + 1; idxBound++) - { - totalCount += bucketCounts[idxBound]; + totalCount += histogramMeasurement.BucketCount; - cursor = WriteMetricName(buffer, cursor, metric.Name, metric.Unit); - cursor = WriteAsciiStringNoEscape(buffer, cursor, "_bucket{"); + cursor = WriteMetricName(buffer, cursor, metric.Name, metric.Unit); + cursor = WriteAsciiStringNoEscape(buffer, cursor, "_bucket{"); - foreach (var tag in tags) - { - cursor = WriteLabel(buffer, cursor, tag.Key, tag.Value); - buffer[cursor++] = unchecked((byte)','); - } + foreach (var tag in tags) + { + cursor = WriteLabel(buffer, cursor, tag.Key, tag.Value); + buffer[cursor++] = unchecked((byte)','); + } - cursor = WriteAsciiStringNoEscape(buffer, cursor, "le=\""); + cursor = WriteAsciiStringNoEscape(buffer, cursor, "le=\""); - if (idxBound < explicitBounds.Length) - { - cursor = WriteDouble(buffer, cursor, explicitBounds[idxBound]); - } - else - { - cursor = WriteAsciiStringNoEscape(buffer, cursor, "+Inf"); - } + if (histogramMeasurement.ExplicitBound != double.PositiveInfinity) + { + cursor = WriteDouble(buffer, cursor, histogramMeasurement.ExplicitBound); + } + else + { + cursor = WriteAsciiStringNoEscape(buffer, cursor, "+Inf"); + } - cursor = WriteAsciiStringNoEscape(buffer, cursor, "\"} "); + cursor = WriteAsciiStringNoEscape(buffer, cursor, "\"} "); - cursor = WriteLong(buffer, cursor, totalCount); - buffer[cursor++] = unchecked((byte)' '); + cursor = WriteLong(buffer, cursor, totalCount); + buffer[cursor++] = unchecked((byte)' '); - cursor = WriteLong(buffer, cursor, timestamp); + cursor = WriteLong(buffer, cursor, timestamp); - buffer[cursor++] = ASCII_LINEFEED; - } + buffer[cursor++] = ASCII_LINEFEED; } // Histogram sum - var count = metricPoint.GetHistogramCount(); - var sum = metricPoint.GetHistogramSum(); - cursor = WriteMetricName(buffer, cursor, metric.Name, metric.Unit); cursor = WriteAsciiStringNoEscape(buffer, cursor, "_sum"); @@ -159,7 +150,7 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric) buffer[cursor++] = unchecked((byte)' '); - cursor = WriteDouble(buffer, cursor, sum); + cursor = WriteDouble(buffer, cursor, metricPoint.GetHistogramSum()); buffer[cursor++] = unchecked((byte)' '); cursor = WriteLong(buffer, cursor, timestamp); @@ -190,7 +181,7 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric) buffer[cursor++] = unchecked((byte)' '); - cursor = WriteLong(buffer, cursor, count); + cursor = WriteLong(buffer, cursor, metricPoint.GetHistogramCount()); buffer[cursor++] = unchecked((byte)' '); cursor = WriteLong(buffer, cursor, timestamp); diff --git a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt index 4d61f50b233..abde8e7515a 100644 --- a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt @@ -13,6 +13,17 @@ OpenTelemetry.Metrics.AggregationTemporalityAttribute.Supported.get -> OpenTelem OpenTelemetry.Metrics.BaseExportingMetricReader OpenTelemetry.Metrics.BaseExportingMetricReader.BaseExportingMetricReader(OpenTelemetry.BaseExporter exporter) -> void OpenTelemetry.Metrics.BaseExportingMetricReader.SupportedExportModes.get -> OpenTelemetry.Metrics.ExportModes +OpenTelemetry.Metrics.HistogramBucket +OpenTelemetry.Metrics.HistogramBucket.BucketCount.get -> long +OpenTelemetry.Metrics.HistogramBucket.ExplicitBound.get -> double +OpenTelemetry.Metrics.HistogramBucket.HistogramBucket() -> void +OpenTelemetry.Metrics.HistogramBuckets +OpenTelemetry.Metrics.HistogramBuckets.Enumerator +OpenTelemetry.Metrics.HistogramBuckets.Enumerator.Current.get -> OpenTelemetry.Metrics.HistogramBucket +OpenTelemetry.Metrics.HistogramBuckets.Enumerator.Enumerator() -> void +OpenTelemetry.Metrics.HistogramBuckets.Enumerator.MoveNext() -> bool +OpenTelemetry.Metrics.HistogramBuckets.GetEnumerator() -> OpenTelemetry.Metrics.HistogramBuckets.Enumerator +OpenTelemetry.Metrics.MetricPoint.GetHistogramBuckets() -> OpenTelemetry.Metrics.HistogramBuckets OpenTelemetry.Metrics.MetricPointsAccessor OpenTelemetry.Metrics.MetricPointsAccessor.MetricPointsAccessor() -> void OpenTelemetry.Metrics.MetricPointsAccessor.Dispose() -> void @@ -52,8 +63,6 @@ OpenTelemetry.Metrics.Metric.Unit.get -> string OpenTelemetry.Metrics.MetricPoint OpenTelemetry.Metrics.MetricPoint.DoubleValue.get -> double OpenTelemetry.Metrics.MetricPoint.EndTime.get -> System.DateTimeOffset -OpenTelemetry.Metrics.MetricPoint.GetBucketCounts() -> long[] -OpenTelemetry.Metrics.MetricPoint.GetExplicitBounds() -> double[] OpenTelemetry.Metrics.MetricPoint.GetHistogramCount() -> long OpenTelemetry.Metrics.MetricPoint.GetHistogramSum() -> double OpenTelemetry.Metrics.MetricPoint.LongValue.get -> long diff --git a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index 4d61f50b233..abde8e7515a 100644 --- a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -13,6 +13,17 @@ OpenTelemetry.Metrics.AggregationTemporalityAttribute.Supported.get -> OpenTelem OpenTelemetry.Metrics.BaseExportingMetricReader OpenTelemetry.Metrics.BaseExportingMetricReader.BaseExportingMetricReader(OpenTelemetry.BaseExporter exporter) -> void OpenTelemetry.Metrics.BaseExportingMetricReader.SupportedExportModes.get -> OpenTelemetry.Metrics.ExportModes +OpenTelemetry.Metrics.HistogramBucket +OpenTelemetry.Metrics.HistogramBucket.BucketCount.get -> long +OpenTelemetry.Metrics.HistogramBucket.ExplicitBound.get -> double +OpenTelemetry.Metrics.HistogramBucket.HistogramBucket() -> void +OpenTelemetry.Metrics.HistogramBuckets +OpenTelemetry.Metrics.HistogramBuckets.Enumerator +OpenTelemetry.Metrics.HistogramBuckets.Enumerator.Current.get -> OpenTelemetry.Metrics.HistogramBucket +OpenTelemetry.Metrics.HistogramBuckets.Enumerator.Enumerator() -> void +OpenTelemetry.Metrics.HistogramBuckets.Enumerator.MoveNext() -> bool +OpenTelemetry.Metrics.HistogramBuckets.GetEnumerator() -> OpenTelemetry.Metrics.HistogramBuckets.Enumerator +OpenTelemetry.Metrics.MetricPoint.GetHistogramBuckets() -> OpenTelemetry.Metrics.HistogramBuckets OpenTelemetry.Metrics.MetricPointsAccessor OpenTelemetry.Metrics.MetricPointsAccessor.MetricPointsAccessor() -> void OpenTelemetry.Metrics.MetricPointsAccessor.Dispose() -> void @@ -52,8 +63,6 @@ OpenTelemetry.Metrics.Metric.Unit.get -> string OpenTelemetry.Metrics.MetricPoint OpenTelemetry.Metrics.MetricPoint.DoubleValue.get -> double OpenTelemetry.Metrics.MetricPoint.EndTime.get -> System.DateTimeOffset -OpenTelemetry.Metrics.MetricPoint.GetBucketCounts() -> long[] -OpenTelemetry.Metrics.MetricPoint.GetExplicitBounds() -> double[] OpenTelemetry.Metrics.MetricPoint.GetHistogramCount() -> long OpenTelemetry.Metrics.MetricPoint.GetHistogramSum() -> double OpenTelemetry.Metrics.MetricPoint.LongValue.get -> long diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 77e87c2a07e..e9cace8ad15 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -11,8 +11,15 @@ ([#2657](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2657)) * Remove MetricStreamConfiguration.Aggregation, as the feature to customize -aggregation is not implemented yet. -([#2660](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2660)) + aggregation is not implemented yet. + ([#2660](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2660)) + +* Removed the public property `HistogramMeasurements` and added a public method + `GetHistogramBuckets` instead. Renamed the class `HistogramMeasurements` to + `HistogramBuckets` and added an enumerator of type `HistogramBucket` for + enumerating `BucketCounts` and `ExplicitBounds`. Removed `GetBucketCounts` and + `GetExplicitBounds` methods from `MetricPoint`. + ([#2664](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2664)) ## 1.2.0-beta2 diff --git a/src/OpenTelemetry/Metrics/HistogramBucket.cs b/src/OpenTelemetry/Metrics/HistogramBucket.cs new file mode 100644 index 00000000000..646d3a2964b --- /dev/null +++ b/src/OpenTelemetry/Metrics/HistogramBucket.cs @@ -0,0 +1,31 @@ +// +// 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. +// + +namespace OpenTelemetry.Metrics +{ + public readonly struct HistogramBucket + { + internal HistogramBucket(double explicitBound, long bucketCount) + { + this.ExplicitBound = explicitBound; + this.BucketCount = bucketCount; + } + + public double ExplicitBound { get; } + + public long BucketCount { get; } + } +} diff --git a/src/OpenTelemetry/Metrics/HistogramBuckets.cs b/src/OpenTelemetry/Metrics/HistogramBuckets.cs new file mode 100644 index 00000000000..2ef2e43f07c --- /dev/null +++ b/src/OpenTelemetry/Metrics/HistogramBuckets.cs @@ -0,0 +1,82 @@ +// +// 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. +// + +namespace OpenTelemetry.Metrics +{ + public class HistogramBuckets + { + internal readonly long[] BucketCounts; + + internal readonly long[] AggregatedBucketCounts; + + internal readonly double[] ExplicitBounds; + + internal readonly object LockObject; + + internal long CountVal; + + internal long Count; + + internal double SumVal; + + internal double Sum; + + internal HistogramBuckets(double[] histogramBounds) + { + this.ExplicitBounds = histogramBounds; + this.BucketCounts = histogramBounds != null ? new long[histogramBounds.Length + 1] : null; + this.AggregatedBucketCounts = histogramBounds != null ? new long[histogramBounds.Length + 1] : null; + this.LockObject = new object(); + } + + public Enumerator GetEnumerator() => new(this); + + public struct Enumerator + { + private readonly int numberOfBuckets; + private readonly int numberofExplicitBounds; + private readonly HistogramBuckets histogramMeasurements; + private int index; + + internal Enumerator(HistogramBuckets histogramMeasurements) + { + this.histogramMeasurements = histogramMeasurements; + this.index = 0; + this.Current = default; + this.numberOfBuckets = histogramMeasurements.ExplicitBounds == null ? 0 : histogramMeasurements.BucketCounts.Length; + this.numberofExplicitBounds = histogramMeasurements.ExplicitBounds == null ? 0 : histogramMeasurements.ExplicitBounds.Length; + } + + public HistogramBucket Current { get; private set; } + + public bool MoveNext() + { + if (this.index < this.numberOfBuckets) + { + double explicitBound = this.index < this.numberofExplicitBounds + ? this.histogramMeasurements.ExplicitBounds[this.index] + : double.PositiveInfinity; + long bucketCount = this.histogramMeasurements.AggregatedBucketCounts[this.index]; + this.Current = new HistogramBucket(explicitBound, bucketCount); + this.index++; + return true; + } + + return false; + } + } + } +} diff --git a/src/OpenTelemetry/Metrics/HistogramMeasurements.cs b/src/OpenTelemetry/Metrics/HistogramMeasurements.cs deleted file mode 100644 index a59b325a5ca..00000000000 --- a/src/OpenTelemetry/Metrics/HistogramMeasurements.cs +++ /dev/null @@ -1,45 +0,0 @@ -// -// 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. -// - -namespace OpenTelemetry.Metrics -{ - internal class HistogramMeasurements - { - internal readonly long[] BucketCounts; - - internal readonly long[] AggregatedBucketCounts; - - internal readonly double[] ExplicitBounds; - - internal readonly object LockObject; - - internal long CountVal; - - internal long Count; - - internal double SumVal; - - internal double Sum; - - internal HistogramMeasurements(double[] histogramBounds) - { - this.ExplicitBounds = histogramBounds; - this.BucketCounts = histogramBounds != null ? new long[histogramBounds.Length + 1] : null; - this.AggregatedBucketCounts = histogramBounds != null ? new long[histogramBounds.Length + 1] : null; - this.LockObject = new object(); - } - } -} diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index e90c8b77972..2e696a3bb8e 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -23,7 +23,7 @@ namespace OpenTelemetry.Metrics public struct MetricPoint { private readonly AggregationType aggType; - private readonly HistogramMeasurements histogramMeasurements; + private readonly HistogramBuckets histogramBuckets; private long longVal; private long lastLongSum; private double doubleVal; @@ -52,15 +52,15 @@ internal MetricPoint( if (this.aggType == AggregationType.Histogram) { - this.histogramMeasurements = new HistogramMeasurements(histogramBounds); + this.histogramBuckets = new HistogramBuckets(histogramBounds); } else if (this.aggType == AggregationType.HistogramSumCount) { - this.histogramMeasurements = new HistogramMeasurements(null); + this.histogramBuckets = new HistogramBuckets(null); } else { - this.histogramMeasurements = null; + this.histogramBuckets = null; } } @@ -79,51 +79,39 @@ internal MetricPoint( internal MetricPointStatus MetricPointStatus { get; private set; } - public long[] GetBucketCounts() - { - if (this.aggType == AggregationType.Histogram) - { - return this.histogramMeasurements.AggregatedBucketCounts; - } - else - { - throw new NotSupportedException($"{nameof(this.GetBucketCounts)} is not supported for this metric type."); - } - } - - public double[] GetExplicitBounds() + public long GetHistogramCount() { - if (this.aggType == AggregationType.Histogram) + if (this.aggType == AggregationType.Histogram || this.aggType == AggregationType.HistogramSumCount) { - return this.histogramMeasurements.ExplicitBounds; + return this.histogramBuckets.Count; } else { - throw new NotSupportedException($"{nameof(this.GetExplicitBounds)} is not supported for this metric type."); + throw new NotSupportedException($"{nameof(this.GetHistogramCount)} is not supported for this metric type."); } } - public long GetHistogramCount() + public double GetHistogramSum() { if (this.aggType == AggregationType.Histogram || this.aggType == AggregationType.HistogramSumCount) { - return this.histogramMeasurements.Count; + return this.histogramBuckets.Sum; } else { - throw new NotSupportedException($"{nameof(this.GetHistogramCount)} is not supported for this metric type."); + throw new NotSupportedException($"{nameof(this.GetHistogramSum)} is not supported for this metric type."); } } - public double GetHistogramSum() + public HistogramBuckets GetHistogramBuckets() { if (this.aggType == AggregationType.Histogram || this.aggType == AggregationType.HistogramSumCount) { - return this.histogramMeasurements.Sum; + return this.histogramBuckets; } else { - throw new NotSupportedException($"{nameof(this.GetHistogramSum)} is not supported for this metric type."); + throw new NotSupportedException($"{nameof(this.GetHistogramBuckets)} is not supported for this metric type."); } } @@ -202,20 +190,20 @@ internal void Update(double number) case AggregationType.Histogram: { int i; - for (i = 0; i < this.histogramMeasurements.ExplicitBounds.Length; i++) + for (i = 0; i < this.histogramBuckets.ExplicitBounds.Length; i++) { // Upper bound is inclusive - if (number <= this.histogramMeasurements.ExplicitBounds[i]) + if (number <= this.histogramBuckets.ExplicitBounds[i]) { break; } } - lock (this.histogramMeasurements.LockObject) + lock (this.histogramBuckets.LockObject) { - this.histogramMeasurements.CountVal++; - this.histogramMeasurements.SumVal += number; - this.histogramMeasurements.BucketCounts[i]++; + this.histogramBuckets.CountVal++; + this.histogramBuckets.SumVal += number; + this.histogramBuckets.BucketCounts[i]++; } break; @@ -223,10 +211,10 @@ internal void Update(double number) case AggregationType.HistogramSumCount: { - lock (this.histogramMeasurements.LockObject) + lock (this.histogramBuckets.LockObject) { - this.histogramMeasurements.CountVal++; - this.histogramMeasurements.SumVal += number; + this.histogramBuckets.CountVal++; + this.histogramBuckets.SumVal += number; } break; @@ -348,22 +336,22 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.Histogram: { - lock (this.histogramMeasurements.LockObject) + lock (this.histogramBuckets.LockObject) { - this.histogramMeasurements.Count = this.histogramMeasurements.CountVal; - this.histogramMeasurements.Sum = this.histogramMeasurements.SumVal; + this.histogramBuckets.Count = this.histogramBuckets.CountVal; + this.histogramBuckets.Sum = this.histogramBuckets.SumVal; if (outputDelta) { - this.histogramMeasurements.CountVal = 0; - this.histogramMeasurements.SumVal = 0; + this.histogramBuckets.CountVal = 0; + this.histogramBuckets.SumVal = 0; } - for (int i = 0; i < this.histogramMeasurements.BucketCounts.Length; i++) + for (int i = 0; i < this.histogramBuckets.BucketCounts.Length; i++) { - this.histogramMeasurements.AggregatedBucketCounts[i] = this.histogramMeasurements.BucketCounts[i]; + this.histogramBuckets.AggregatedBucketCounts[i] = this.histogramBuckets.BucketCounts[i]; if (outputDelta) { - this.histogramMeasurements.BucketCounts[i] = 0; + this.histogramBuckets.BucketCounts[i] = 0; } } @@ -375,14 +363,14 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.HistogramSumCount: { - lock (this.histogramMeasurements.LockObject) + lock (this.histogramBuckets.LockObject) { - this.histogramMeasurements.Count = this.histogramMeasurements.CountVal; - this.histogramMeasurements.Sum = this.histogramMeasurements.SumVal; + this.histogramBuckets.Count = this.histogramBuckets.CountVal; + this.histogramBuckets.Sum = this.histogramBuckets.SumVal; if (outputDelta) { - this.histogramMeasurements.CountVal = 0; - this.histogramMeasurements.SumVal = 0; + this.histogramBuckets.CountVal = 0; + this.histogramBuckets.SumVal = 0; } this.MetricPointStatus = MetricPointStatus.NoCollectPending; diff --git a/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs b/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs index fc6f3aed88a..8a276f5d618 100644 --- a/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs +++ b/test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs @@ -50,12 +50,14 @@ public void HistogramDistributeToAllBucketsDefault() histogramPoint.TakeSnapshot(true); var count = histogramPoint.GetHistogramCount(); - var bucketCounts = histogramPoint.GetBucketCounts(); Assert.Equal(22, count); - for (int i = 0; i < bucketCounts.Length; i++) + + int actualCount = 0; + foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets()) { - Assert.Equal(2, bucketCounts[i]); + Assert.Equal(2, histogramMeasurement.BucketCount); + actualCount++; } } @@ -80,17 +82,24 @@ public void HistogramDistributeToAllBucketsCustom() var count = histogramPoint.GetHistogramCount(); var sum = histogramPoint.GetHistogramSum(); - var bucketCounts = histogramPoint.GetBucketCounts(); // Sum of all recordings Assert.Equal(40, sum); // Count = # of recordings Assert.Equal(7, count); - Assert.Equal(boundaries.Length + 1, bucketCounts.Length); - Assert.Equal(5, bucketCounts[0]); - Assert.Equal(2, bucketCounts[1]); - Assert.Equal(0, bucketCounts[2]); + + int index = 0; + int actualCount = 0; + var expectedBucketCounts = new long[] { 5, 2, 0 }; + foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets()) + { + Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount); + index++; + actualCount++; + } + + Assert.Equal(boundaries.Length + 1, actualCount); } [Fact] @@ -118,8 +127,9 @@ public void HistogramWithOnlySumCount() // Count = # of recordings Assert.Equal(7, count); - Assert.Throws(() => histogramPoint.GetBucketCounts()); - Assert.Throws(() => histogramPoint.GetExplicitBounds()); + // There should be no enumeration of BucketCounts and ExplicitBounds for HistogramSumCount + var enumerator = histogramPoint.GetHistogramBuckets().GetEnumerator(); + Assert.False(enumerator.MoveNext()); } } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 1daf90a7b52..d3e356072bc 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -390,18 +390,21 @@ public void ViewToProduceCustomHistogramBound() var count = histogramPoint.GetHistogramCount(); var sum = histogramPoint.GetHistogramSum(); - var bucketCounts = histogramPoint.GetBucketCounts(); - var explicitBounds = histogramPoint.GetExplicitBounds(); Assert.Equal(40, sum); Assert.Equal(7, count); - Assert.Equal(Metric.DefaultHistogramBounds.Length + 1, bucketCounts.Length); - Assert.Equal(2, bucketCounts[0]); - Assert.Equal(1, bucketCounts[1]); - Assert.Equal(2, bucketCounts[2]); - Assert.Equal(2, bucketCounts[3]); - Assert.Equal(0, bucketCounts[4]); - Assert.Equal(0, bucketCounts[5]); + + int index = 0; + int actualCount = 0; + var expectedBucketCounts = new long[] { 2, 1, 2, 2, 0, 0, 0, 0, 0, 0, 0 }; + foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets()) + { + Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount); + index++; + actualCount++; + } + + Assert.Equal(Metric.DefaultHistogramBounds.Length + 1, actualCount); List metricPointsCustom = new List(); foreach (ref var mp in metricCustom.GetMetricPoints()) @@ -414,15 +417,21 @@ public void ViewToProduceCustomHistogramBound() count = histogramPoint.GetHistogramCount(); sum = histogramPoint.GetHistogramSum(); - bucketCounts = histogramPoint.GetBucketCounts(); - explicitBounds = histogramPoint.GetExplicitBounds(); Assert.Equal(40, sum); Assert.Equal(7, count); - Assert.Equal(boundaries.Length + 1, bucketCounts.Length); - Assert.Equal(5, bucketCounts[0]); - Assert.Equal(2, bucketCounts[1]); - Assert.Equal(0, bucketCounts[2]); + + index = 0; + actualCount = 0; + expectedBucketCounts = new long[] { 5, 2, 0 }; + foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets()) + { + Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount); + index++; + actualCount++; + } + + Assert.Equal(boundaries.Length + 1, actualCount); } [Fact]