From 8db45a3af6ae256f200f971d3d87edabf9a6c781 Mon Sep 17 00:00:00 2001 From: Michael Maxwell Date: Fri, 14 Oct 2022 14:00:07 -0700 Subject: [PATCH] Adding MinMax to Histograms (#2735) --- .../CHANGELOG.md | 4 + .../Implementation/MetricItemExtensions.cs | 6 + .../.publicApi/net462/PublicAPI.Unshipped.txt | 7 + .../.publicApi/net6.0/PublicAPI.Unshipped.txt | 7 + .../netstandard2.0/PublicAPI.Unshipped.txt | 7 + .../netstandard2.1/PublicAPI.Unshipped.txt | 7 + src/OpenTelemetry/CHANGELOG.md | 4 + src/OpenTelemetry/Metrics/AggregationType.cs | 16 +- .../ExplicitBucketHistogramConfiguration.cs | 2 +- src/OpenTelemetry/Metrics/HistogramBuckets.cs | 8 +- .../Metrics/HistogramConfiguration.cs | 26 +++ src/OpenTelemetry/Metrics/Metric.cs | 15 +- src/OpenTelemetry/Metrics/MetricPoint.cs | 200 +++++++++++++++++- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 3 +- .../Metrics/MetricStreamIdentity.cs | 5 + .../Metrics/MetricTestData.cs | 17 ++ .../Metrics/MetricViewTests.cs | 68 ++++++ 17 files changed, 379 insertions(+), 23 deletions(-) create mode 100644 src/OpenTelemetry/Metrics/HistogramConfiguration.cs diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md index d291db93422..585c05c2e63 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* OTLP histogram data points will now include `Min` and `Max` values when + they are present. + ([#2735](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2735)) + * Adds support for limiting the length and count of attributes exported from the OTLP log exporter. These [Attribute Limits](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/sdk-environment-variables.md#attribute-limits) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs index 4933871e7b2..50ad352ddfc 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs @@ -257,6 +257,12 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) dataPoint.Count = (ulong)metricPoint.GetHistogramCount(); dataPoint.Sum = metricPoint.GetHistogramSum(); + if (metricPoint.HasMinMax()) + { + dataPoint.Min = metricPoint.GetHistogramMin(); + dataPoint.Max = metricPoint.GetHistogramMax(); + } + foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets()) { dataPoint.BucketCounts.Add((ulong)histogramMeasurement.BucketCount); diff --git a/src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt index bf3ab6eedd6..eedd81fc086 100644 --- a/src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt @@ -10,6 +10,13 @@ OpenTelemetry.Logs.LogRecord.TraceFlags.set -> void OpenTelemetry.Logs.LogRecord.TraceId.set -> void OpenTelemetry.Logs.LogRecord.TraceState.set -> void OpenTelemetry.Logs.OpenTelemetryLoggerOptions.ConfigureResource(System.Action! configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! +OpenTelemetry.Metrics.HistogramConfiguration +OpenTelemetry.Metrics.HistogramConfiguration.HistogramConfiguration() -> void +OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.get -> bool +OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.set -> void +OpenTelemetry.Metrics.MetricPoint.GetHistogramMax() -> double +OpenTelemetry.Metrics.MetricPoint.GetHistogramMin() -> double +OpenTelemetry.Metrics.MetricPoint.HasMinMax() -> bool static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder! meterProviderBuilder) -> OpenTelemetry.Metrics.MeterProviderBuilder! diff --git a/src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt index bf3ab6eedd6..eedd81fc086 100644 --- a/src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt @@ -10,6 +10,13 @@ OpenTelemetry.Logs.LogRecord.TraceFlags.set -> void OpenTelemetry.Logs.LogRecord.TraceId.set -> void OpenTelemetry.Logs.LogRecord.TraceState.set -> void OpenTelemetry.Logs.OpenTelemetryLoggerOptions.ConfigureResource(System.Action! configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! +OpenTelemetry.Metrics.HistogramConfiguration +OpenTelemetry.Metrics.HistogramConfiguration.HistogramConfiguration() -> void +OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.get -> bool +OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.set -> void +OpenTelemetry.Metrics.MetricPoint.GetHistogramMax() -> double +OpenTelemetry.Metrics.MetricPoint.GetHistogramMin() -> double +OpenTelemetry.Metrics.MetricPoint.HasMinMax() -> bool static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder! meterProviderBuilder) -> OpenTelemetry.Metrics.MeterProviderBuilder! diff --git a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index bf3ab6eedd6..eedd81fc086 100644 --- a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -10,6 +10,13 @@ OpenTelemetry.Logs.LogRecord.TraceFlags.set -> void OpenTelemetry.Logs.LogRecord.TraceId.set -> void OpenTelemetry.Logs.LogRecord.TraceState.set -> void OpenTelemetry.Logs.OpenTelemetryLoggerOptions.ConfigureResource(System.Action! configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! +OpenTelemetry.Metrics.HistogramConfiguration +OpenTelemetry.Metrics.HistogramConfiguration.HistogramConfiguration() -> void +OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.get -> bool +OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.set -> void +OpenTelemetry.Metrics.MetricPoint.GetHistogramMax() -> double +OpenTelemetry.Metrics.MetricPoint.GetHistogramMin() -> double +OpenTelemetry.Metrics.MetricPoint.HasMinMax() -> bool static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder! meterProviderBuilder) -> OpenTelemetry.Metrics.MeterProviderBuilder! diff --git a/src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt index bf3ab6eedd6..eedd81fc086 100644 --- a/src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt @@ -10,6 +10,13 @@ OpenTelemetry.Logs.LogRecord.TraceFlags.set -> void OpenTelemetry.Logs.LogRecord.TraceId.set -> void OpenTelemetry.Logs.LogRecord.TraceState.set -> void OpenTelemetry.Logs.OpenTelemetryLoggerOptions.ConfigureResource(System.Action! configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! +OpenTelemetry.Metrics.HistogramConfiguration +OpenTelemetry.Metrics.HistogramConfiguration.HistogramConfiguration() -> void +OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.get -> bool +OpenTelemetry.Metrics.HistogramConfiguration.RecordMinMax.set -> void +OpenTelemetry.Metrics.MetricPoint.GetHistogramMax() -> double +OpenTelemetry.Metrics.MetricPoint.GetHistogramMin() -> double +OpenTelemetry.Metrics.MetricPoint.HasMinMax() -> bool static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services, System.Action! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder! meterProviderBuilder) -> OpenTelemetry.Metrics.MeterProviderBuilder! diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 5ea2486a319..2c6f3092d3a 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Make recording of `Min` and `Max` for histograms configurable, enabled by + default. + ([#2735](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2735)) + * Changed default bucket boundaries for Explicit Bucket Histogram from [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000] to [0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000]. diff --git a/src/OpenTelemetry/Metrics/AggregationType.cs b/src/OpenTelemetry/Metrics/AggregationType.cs index bd87c9e5b4f..eff60b2d91d 100644 --- a/src/OpenTelemetry/Metrics/AggregationType.cs +++ b/src/OpenTelemetry/Metrics/AggregationType.cs @@ -54,13 +54,23 @@ internal enum AggregationType DoubleGauge = 5, /// - /// Histogram. + /// Histogram with sum, count, buckets. /// Histogram = 6, /// - /// Histogram with sum, count only. + /// Histogram with sum, count, min, max, buckets. /// - HistogramSumCount = 7, + HistogramMinMax = 7, + + /// + /// Histogram with sum, count. + /// + HistogramSumCount = 8, + + /// + /// Histogram with sum, count, min, max. + /// + HistogramSumCountMinMax = 9, } } diff --git a/src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs b/src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs index 58bfc245b10..8be618748fc 100644 --- a/src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs +++ b/src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs @@ -21,7 +21,7 @@ namespace OpenTelemetry.Metrics /// /// Stores configuration for a histogram metric stream with explicit bucket boundaries. /// - public class ExplicitBucketHistogramConfiguration : MetricStreamConfiguration + public class ExplicitBucketHistogramConfiguration : HistogramConfiguration { /// /// Gets or sets the optional boundaries of the histogram metric stream. diff --git a/src/OpenTelemetry/Metrics/HistogramBuckets.cs b/src/OpenTelemetry/Metrics/HistogramBuckets.cs index 2a2149b31b0..c0160e534ee 100644 --- a/src/OpenTelemetry/Metrics/HistogramBuckets.cs +++ b/src/OpenTelemetry/Metrics/HistogramBuckets.cs @@ -31,13 +31,17 @@ public class HistogramBuckets internal readonly double[] ExplicitBounds; internal readonly long[] RunningBucketCounts; - internal readonly long[] SnapshotBucketCounts; internal double RunningSum; - internal double SnapshotSum; + internal double RunningMin = double.PositiveInfinity; + internal double SnapshotMin; + + internal double RunningMax = double.NegativeInfinity; + internal double SnapshotMax; + internal int IsCriticalSectionOccupied = 0; private readonly BucketLookupNode bucketLookupTreeRoot; diff --git a/src/OpenTelemetry/Metrics/HistogramConfiguration.cs b/src/OpenTelemetry/Metrics/HistogramConfiguration.cs new file mode 100644 index 00000000000..37980098937 --- /dev/null +++ b/src/OpenTelemetry/Metrics/HistogramConfiguration.cs @@ -0,0 +1,26 @@ +// +// 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 HistogramConfiguration : MetricStreamConfiguration +{ + /// + /// Gets or sets a value indicating whether Min, Max + /// should be collected. + /// + public bool RecordMinMax { get; set; } = true; +} diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index c40e0f82ac7..6f5771121b7 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -34,7 +34,8 @@ internal Metric( AggregationTemporality temporality, int maxMetricPointsPerMetricStream, double[] histogramBounds = null, - string[] tagKeysInteresting = null) + string[] tagKeysInteresting = null, + bool histogramRecordMinMax = true) { this.InstrumentIdentity = instrumentIdentity; @@ -118,15 +119,9 @@ internal Metric( { this.MetricType = MetricType.Histogram; - if (histogramBounds != null - && histogramBounds.Length == 0) - { - aggType = AggregationType.HistogramSumCount; - } - else - { - aggType = AggregationType.Histogram; - } + aggType = histogramBounds != null && histogramBounds.Length == 0 + ? (histogramRecordMinMax ? AggregationType.HistogramSumCountMinMax : AggregationType.HistogramSumCount) + : (histogramRecordMinMax ? AggregationType.HistogramMinMax : AggregationType.Histogram); } else { diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index 55abb65b9c5..50dc46d1135 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -58,11 +58,13 @@ internal MetricPoint( this.deltaLastValue = default; this.MetricPointStatus = MetricPointStatus.NoCollectPending; - if (this.aggType == AggregationType.Histogram) + if (this.aggType == AggregationType.Histogram || + this.aggType == AggregationType.HistogramMinMax) { this.histogramBuckets = new HistogramBuckets(histogramExplicitBounds); } - else if (this.aggType == AggregationType.HistogramSumCount) + else if (this.aggType == AggregationType.HistogramSumCount || + this.aggType == AggregationType.HistogramSumCountMinMax) { this.histogramBuckets = new HistogramBuckets(null); } @@ -187,7 +189,10 @@ public readonly double GetGaugeLastValueDouble() [MethodImpl(MethodImplOptions.AggressiveInlining)] public readonly long GetHistogramCount() { - if (this.aggType != AggregationType.Histogram && this.aggType != AggregationType.HistogramSumCount) + if (this.aggType != AggregationType.Histogram && + this.aggType != AggregationType.HistogramSumCount && + this.aggType != AggregationType.HistogramMinMax && + this.aggType != AggregationType.HistogramSumCountMinMax) { this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramCount)); } @@ -205,7 +210,10 @@ public readonly long GetHistogramCount() [MethodImpl(MethodImplOptions.AggressiveInlining)] public readonly double GetHistogramSum() { - if (this.aggType != AggregationType.Histogram && this.aggType != AggregationType.HistogramSumCount) + if (this.aggType != AggregationType.Histogram && + this.aggType != AggregationType.HistogramSumCount && + this.aggType != AggregationType.HistogramMinMax && + this.aggType != AggregationType.HistogramSumCountMinMax) { this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramSum)); } @@ -223,7 +231,10 @@ public readonly double GetHistogramSum() [MethodImpl(MethodImplOptions.AggressiveInlining)] public readonly HistogramBuckets GetHistogramBuckets() { - if (this.aggType != AggregationType.Histogram && this.aggType != AggregationType.HistogramSumCount) + if (this.aggType != AggregationType.Histogram && + this.aggType != AggregationType.HistogramSumCount && + this.aggType != AggregationType.HistogramMinMax && + this.aggType != AggregationType.HistogramSumCountMinMax) { this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramBuckets)); } @@ -231,6 +242,47 @@ public readonly HistogramBuckets GetHistogramBuckets() return this.histogramBuckets; } + /// + /// Gets the minimum value of the histogram associated with the metric + /// point if present. Check by using . + /// + /// + /// Applies to metric type. + /// + /// A histogram's maximum value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public double GetHistogramMin() + { + return this.histogramBuckets.SnapshotMin; + } + + /// + /// Gets the maximum value of the histogram associated with the metric + /// point if present. Check by using . + /// + /// + /// Applies to metric type. + /// + /// A histogram's maximum value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public double GetHistogramMax() + { + return this.histogramBuckets.SnapshotMax; + } + + /// + /// Gets whether the histogram has a valid Min and Max value. + /// Should be used before + /// and to + /// ensure a valid value is returned. + /// + /// A minimum and maximum value exist. + public bool HasMinMax() + { + return this.aggType == AggregationType.HistogramMinMax || + this.aggType == AggregationType.HistogramSumCountMinMax; + } + internal readonly MetricPoint Copy() { MetricPoint copy = this; @@ -261,7 +313,9 @@ internal void Update(long number) } case AggregationType.Histogram: + case AggregationType.HistogramMinMax: case AggregationType.HistogramSumCount: + case AggregationType.HistogramSumCountMinMax: { this.Update((double)number); @@ -374,6 +428,63 @@ internal void Update(double number) sw.SpinOnce(); } + break; + } + + case AggregationType.HistogramMinMax: + { + int i = this.histogramBuckets.FindBucketIndex(number); + + var sw = default(SpinWait); + while (true) + { + if (Interlocked.Exchange(ref this.histogramBuckets.IsCriticalSectionOccupied, 1) == 0) + { + // Lock acquired + unchecked + { + this.runningValue.AsLong++; + this.histogramBuckets.RunningSum += number; + this.histogramBuckets.RunningBucketCounts[i]++; + this.histogramBuckets.RunningMin = Math.Min(this.histogramBuckets.RunningMin, number); + this.histogramBuckets.RunningMax = Math.Max(this.histogramBuckets.RunningMax, number); + } + + // Release lock + Interlocked.Exchange(ref this.histogramBuckets.IsCriticalSectionOccupied, 0); + break; + } + + sw.SpinOnce(); + } + + break; + } + + case AggregationType.HistogramSumCountMinMax: + { + var sw = default(SpinWait); + while (true) + { + if (Interlocked.Exchange(ref this.histogramBuckets.IsCriticalSectionOccupied, 1) == 0) + { + // Lock acquired + unchecked + { + this.runningValue.AsLong++; + this.histogramBuckets.RunningSum += number; + this.histogramBuckets.RunningMin = Math.Min(this.histogramBuckets.RunningMin, number); + this.histogramBuckets.RunningMax = Math.Max(this.histogramBuckets.RunningMax, number); + } + + // Release lock + Interlocked.Exchange(ref this.histogramBuckets.IsCriticalSectionOccupied, 0); + break; + } + + sw.SpinOnce(); + } + break; } } @@ -501,6 +612,7 @@ internal void TakeSnapshot(bool outputDelta) // Lock acquired this.snapshotValue.AsLong = this.runningValue.AsLong; this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; + if (outputDelta) { this.runningValue.AsLong = 0; @@ -539,10 +651,88 @@ internal void TakeSnapshot(bool outputDelta) // Lock acquired this.snapshotValue.AsLong = this.runningValue.AsLong; this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; + + if (outputDelta) + { + this.runningValue.AsLong = 0; + this.histogramBuckets.RunningSum = 0; + } + + this.MetricPointStatus = MetricPointStatus.NoCollectPending; + + // Release lock + Interlocked.Exchange(ref this.histogramBuckets.IsCriticalSectionOccupied, 0); + break; + } + + sw.SpinOnce(); + } + + break; + } + + case AggregationType.HistogramMinMax: + { + var sw = default(SpinWait); + while (true) + { + if (Interlocked.Exchange(ref this.histogramBuckets.IsCriticalSectionOccupied, 1) == 0) + { + // Lock acquired + this.snapshotValue.AsLong = this.runningValue.AsLong; + this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; + this.histogramBuckets.SnapshotMin = this.histogramBuckets.RunningMin; + this.histogramBuckets.SnapshotMax = this.histogramBuckets.RunningMax; + + if (outputDelta) + { + this.runningValue.AsLong = 0; + this.histogramBuckets.RunningSum = 0; + this.histogramBuckets.RunningMin = double.PositiveInfinity; + this.histogramBuckets.RunningMax = double.NegativeInfinity; + } + + for (int i = 0; i < this.histogramBuckets.RunningBucketCounts.Length; i++) + { + this.histogramBuckets.SnapshotBucketCounts[i] = this.histogramBuckets.RunningBucketCounts[i]; + if (outputDelta) + { + this.histogramBuckets.RunningBucketCounts[i] = 0; + } + } + + this.MetricPointStatus = MetricPointStatus.NoCollectPending; + + // Release lock + Interlocked.Exchange(ref this.histogramBuckets.IsCriticalSectionOccupied, 0); + break; + } + + sw.SpinOnce(); + } + + break; + } + + case AggregationType.HistogramSumCountMinMax: + { + var sw = default(SpinWait); + while (true) + { + if (Interlocked.Exchange(ref this.histogramBuckets.IsCriticalSectionOccupied, 1) == 0) + { + // Lock acquired + this.snapshotValue.AsLong = this.runningValue.AsLong; + this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; + this.histogramBuckets.SnapshotMin = this.histogramBuckets.RunningMin; + this.histogramBuckets.SnapshotMax = this.histogramBuckets.RunningMax; + if (outputDelta) { this.runningValue.AsLong = 0; this.histogramBuckets.RunningSum = 0; + this.histogramBuckets.RunningMin = double.PositiveInfinity; + this.histogramBuckets.RunningMax = double.NegativeInfinity; } this.MetricPointStatus = MetricPointStatus.NoCollectPending; diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index d2863d55d6e..4aa6769f346 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -156,8 +156,7 @@ internal List AddMetricsListWithViews(Instrument instrument, List metricIdentity1.Equals(metricIdentity2); public static bool operator !=(MetricStreamIdentity metricIdentity1, MetricStreamIdentity metricIdentity2) => !metricIdentity1.Equals(metricIdentity2); @@ -99,6 +103,7 @@ public bool Equals(MetricStreamIdentity other) && this.Unit == other.Unit && this.Description == other.Description && this.ViewId == other.ViewId + && this.HistogramRecordMinMax == other.HistogramRecordMinMax && StringArrayComparer.Equals(this.TagKeys, other.TagKeys) && HistogramBoundsEqual(this.HistogramBucketBounds, other.HistogramBucketBounds); } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs b/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs index 0664dc18c00..c4b8fdf944a 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs @@ -49,5 +49,22 @@ public static IEnumerable InvalidHistogramBoundaries new object[] { new double[] { 0, 1, 1, 2 } }, new object[] { new double[] { 0, 1, 2, -1 } }, }; + + public static IEnumerable ValidHistogramMinMax + => new List + { + new object[] { new double[] { -10, 0, 1, 9, 10, 11, 19 }, new HistogramConfiguration(), -10, 19 }, + new object[] { new double[] { double.NegativeInfinity }, new HistogramConfiguration(), double.NegativeInfinity, double.NegativeInfinity }, + new object[] { new double[] { double.NegativeInfinity, 0, double.PositiveInfinity }, new HistogramConfiguration(), double.NegativeInfinity, double.PositiveInfinity }, + new object[] { new double[] { 1 }, new HistogramConfiguration(), 1, 1 }, + new object[] { new double[] { 5, 100, 4, 101, -2, 97 }, new ExplicitBucketHistogramConfiguration() { Boundaries = new double[] { 10, 20 } }, -2, 101 }, + }; + + public static IEnumerable InvalidHistogramMinMax + => new List + { + new object[] { new double[] { 1 }, new HistogramConfiguration() { RecordMinMax = false } }, + new object[] { new double[] { 1 }, new ExplicitBucketHistogramConfiguration() { Boundaries = new double[] { 10, 20 }, RecordMinMax = false } }, + }; } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 125290c2b4c..1dda7eb6fa5 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -572,6 +572,74 @@ public void ViewToProduceCustomHistogramBound() Assert.Equal(boundaries.Length + 1, actualCount); } + [Theory] + [MemberData(nameof(MetricTestData.ValidHistogramMinMax), MemberType = typeof(MetricTestData))] + public void HistogramMinMax(double[] values, HistogramConfiguration histogramConfiguration, double expectedMin, double expectedMax) + { + using var meter = new Meter(Utils.GetCurrentMethodName()); + var histogram = meter.CreateHistogram("MyHistogram"); + var exportedItems = new List(); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView(histogram.Name, histogramConfiguration) + .AddInMemoryExporter(exportedItems) + .Build(); + + for (var i = 0; i < values.Length; i++) + { + histogram.Record(values[i]); + } + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + var metricPoints = new List(); + foreach (ref readonly var mp in exportedItems[0].GetMetricPoints()) + { + metricPoints.Add(mp); + } + + var histogramPoint = metricPoints[0]; + var hasMinMax = histogramPoint.HasMinMax(); + Assert.True(hasMinMax); + + var min = histogramPoint.GetHistogramMin(); + var max = histogramPoint.GetHistogramMax(); + + Assert.Equal(expectedMin, min); + Assert.Equal(expectedMax, max); + } + + [Theory] + [MemberData(nameof(MetricTestData.InvalidHistogramMinMax), MemberType = typeof(MetricTestData))] + public void HistogramMinMaxNotPresent(double[] values, HistogramConfiguration histogramConfiguration) + { + using var meter = new Meter(Utils.GetCurrentMethodName()); + var histogram = meter.CreateHistogram("MyHistogram"); + var exportedItems = new List(); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView(histogram.Name, histogramConfiguration) + .AddInMemoryExporter(exportedItems) + .Build(); + + for (var i = 0; i < values.Length; i++) + { + histogram.Record(values[i]); + } + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + var metricPoints = new List(); + foreach (ref readonly var mp in exportedItems[0].GetMetricPoints()) + { + metricPoints.Add(mp); + } + + var histogramPoint = metricPoints[0]; + + Assert.False(histogramPoint.HasMinMax()); + } + [Fact] public void ViewToSelectTagKeys() {