Adding MinMax to Histograms (#2735)

open-telemetry · Oct 14, 2022 · 8db45a3 · 8db45a3
1 parent af7ab89
commit 8db45a3
Show file tree

Hide file tree

Showing 17 changed files with 379 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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<OpenTelemetry.Resources.ResourceBuilder!>! 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<OpenTelemetry.Metrics.MeterProviderBuilder!>! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
 static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddInstrumentation<T>(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
@@ -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<OpenTelemetry.Resources.ResourceBuilder!>! 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<OpenTelemetry.Metrics.MeterProviderBuilder!>! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
 static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddInstrumentation<T>(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
@@ -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<OpenTelemetry.Resources.ResourceBuilder!>! 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<OpenTelemetry.Metrics.MeterProviderBuilder!>! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
 static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddInstrumentation<T>(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
@@ -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<OpenTelemetry.Resources.ResourceBuilder!>! 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<OpenTelemetry.Metrics.MeterProviderBuilder!>! configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
 static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddInstrumentation<T>(this OpenTelemetry.Metrics.MeterProviderBuilder! meterProviderBuilder) -> OpenTelemetry.Metrics.MeterProviderBuilder!

diff --git 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
@@ -54,13 +54,23 @@ internal enum AggregationType
         DoubleGauge = 5,
 
         /// <summary>
-        /// Histogram.
+        /// Histogram with sum, count, buckets.
         /// </summary>
         Histogram = 6,
 
         /// <summary>
-        /// Histogram with sum, count only.
+        /// Histogram with sum, count, min, max, buckets.
         /// </summary>
-        HistogramSumCount = 7,
+        HistogramMinMax = 7,
+
+        /// <summary>
+        /// Histogram with sum, count.
+        /// </summary>
+        HistogramSumCount = 8,
+
+        /// <summary>
+        /// Histogram with sum, count, min, max.
+        /// </summary>
+        HistogramSumCountMinMax = 9,
     }
 }
diff --git a/src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs b/src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs
@@ -21,7 +21,7 @@ namespace OpenTelemetry.Metrics
     /// <summary>
     /// Stores configuration for a histogram metric stream with explicit bucket boundaries.
     /// </summary>
-    public class ExplicitBucketHistogramConfiguration : MetricStreamConfiguration
+    public class ExplicitBucketHistogramConfiguration : HistogramConfiguration
     {
         /// <summary>
         /// 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
@@ -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
@@ -0,0 +1,26 @@
+// <copyright file="HistogramConfiguration.cs" company="OpenTelemetry Authors">
+// 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.
+// </copyright>
+
+namespace OpenTelemetry.Metrics;
+
+public class HistogramConfiguration : MetricStreamConfiguration
+{
+    /// <summary>
+    /// Gets or sets a value indicating whether Min, Max
+    /// should be collected.
+    /// </summary>
+    public bool RecordMinMax { get; set; } = true;
+}
diff --git 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
             {