From 88c1864cb79ba6bf9623db6a77f4383c38799197 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Wed, 26 Oct 2022 12:35:07 -0400 Subject: [PATCH] Histogram MinMax API fixes (#3822) --- .../ConsoleMetricExporter.cs | 4 +- .../Implementation/MetricItemExtensions.cs | 6 +-- .../.publicApi/net462/PublicAPI.Unshipped.txt | 4 +- .../.publicApi/net6.0/PublicAPI.Unshipped.txt | 4 +- .../netstandard2.0/PublicAPI.Unshipped.txt | 4 +- .../netstandard2.1/PublicAPI.Unshipped.txt | 4 +- src/OpenTelemetry/Metrics/MetricPoint.cs | 49 ++++++------------- .../Metrics/MetricViewTests.cs | 20 ++++---- 8 files changed, 35 insertions(+), 60 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs index 3f4076b5f78..cdeefcbe457 100644 --- a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs +++ b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs @@ -98,9 +98,9 @@ public override ExportResult Export(in Batch batch) var sum = metricPoint.GetHistogramSum(); var count = metricPoint.GetHistogramCount(); bucketsBuilder.Append($"Sum: {sum} Count: {count} "); - if (metricPoint.HasMinMax()) + if (metricPoint.TryGetHistogramMinMaxValues(out double min, out double max)) { - bucketsBuilder.Append($"Min: {metricPoint.GetHistogramMin()} Max: {metricPoint.GetHistogramMax()} "); + bucketsBuilder.Append($"Min: {min} Max: {max} "); } bucketsBuilder.AppendLine(); diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs index 50ad352ddfc..f3f5f679914 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs @@ -257,10 +257,10 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) dataPoint.Count = (ulong)metricPoint.GetHistogramCount(); dataPoint.Sum = metricPoint.GetHistogramSum(); - if (metricPoint.HasMinMax()) + if (metricPoint.TryGetHistogramMinMaxValues(out double min, out double max)) { - dataPoint.Min = metricPoint.GetHistogramMin(); - dataPoint.Max = metricPoint.GetHistogramMax(); + dataPoint.Min = min; + dataPoint.Max = max; } foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets()) diff --git a/src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt index dc2cac75890..206a7f2aa98 100644 --- a/src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt @@ -14,9 +14,7 @@ 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 +OpenTelemetry.Metrics.MetricPoint.TryGetHistogramMinMaxValues(out double min, out double max) -> bool OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder! OpenTelemetry.Trace.BatchExportActivityProcessorOptions.BatchExportActivityProcessorOptions(Microsoft.Extensions.Configuration.IConfiguration! configuration) -> void static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! diff --git a/src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt index dc2cac75890..206a7f2aa98 100644 --- a/src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt @@ -14,9 +14,7 @@ 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 +OpenTelemetry.Metrics.MetricPoint.TryGetHistogramMinMaxValues(out double min, out double max) -> bool OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder! OpenTelemetry.Trace.BatchExportActivityProcessorOptions.BatchExportActivityProcessorOptions(Microsoft.Extensions.Configuration.IConfiguration! configuration) -> void static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! diff --git a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index dc2cac75890..206a7f2aa98 100644 --- a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -14,9 +14,7 @@ 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 +OpenTelemetry.Metrics.MetricPoint.TryGetHistogramMinMaxValues(out double min, out double max) -> bool OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder! OpenTelemetry.Trace.BatchExportActivityProcessorOptions.BatchExportActivityProcessorOptions(Microsoft.Extensions.Configuration.IConfiguration! configuration) -> void static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! diff --git a/src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt index dc2cac75890..206a7f2aa98 100644 --- a/src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt @@ -14,9 +14,7 @@ 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 +OpenTelemetry.Metrics.MetricPoint.TryGetHistogramMinMaxValues(out double min, out double max) -> bool OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder! OpenTelemetry.Trace.BatchExportActivityProcessorOptions.BatchExportActivityProcessorOptions(Microsoft.Extensions.Configuration.IConfiguration! configuration) -> void static Microsoft.Extensions.DependencyInjection.MeterProviderBuilderServiceCollectionExtensions.ConfigureOpenTelemetryMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index 4ea8fd08711..99adab1af6e 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -243,44 +243,27 @@ public readonly HistogramBuckets GetHistogramBuckets() } /// - /// Gets the minimum value of the histogram associated with the metric - /// point if present. Check by using . + /// Gets the Histogram Min and Max values. /// - /// - /// Applies to metric type. - /// - /// A histogram's maximum value. + /// The histogram minimum value. + /// The histogram maximum value. + /// True if minimum and maximum value exist, false otherwise. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public double GetHistogramMin() + public bool TryGetHistogramMinMaxValues(out double min, out double max) { - return this.histogramBuckets.SnapshotMin; - } + if (this.aggType == AggregationType.HistogramWithMinMax || + this.aggType == AggregationType.HistogramWithMinMaxBuckets) + { + Debug.Assert(this.histogramBuckets != null, "histogramBuckets was null"); - /// - /// 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; - } + min = this.histogramBuckets.SnapshotMin; + max = this.histogramBuckets.SnapshotMax; + return true; + } - /// - /// 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.HistogramWithMinMaxBuckets || - this.aggType == AggregationType.HistogramWithMinMax; + min = 0; + max = 0; + return false; } internal readonly MetricPoint Copy() diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 1dda7eb6fa5..b710e6733b6 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -599,14 +599,15 @@ public void HistogramMinMax(double[] values, HistogramConfiguration histogramCon } 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); + if (histogramPoint.TryGetHistogramMinMaxValues(out double min, out double max)) + { + Assert.Equal(expectedMin, min); + Assert.Equal(expectedMax, max); + } + else + { + Assert.Fail("MinMax expected"); + } } [Theory] @@ -636,8 +637,7 @@ public void HistogramMinMaxNotPresent(double[] values, HistogramConfiguration hi } var histogramPoint = metricPoints[0]; - - Assert.False(histogramPoint.HasMinMax()); + Assert.False(histogramPoint.TryGetHistogramMinMaxValues(out double _, out double _)); } [Fact]