OLTP exporter: Fix exponential histogram zero count (#4343)

open-telemetry · Mar 29, 2023 · 63de729 · 63de729
1 parent 410532a
commit 63de729
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 7 deletions.
diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs
@@ -335,6 +335,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric)
 
                             var exponentialHistogramData = metricPoint.GetExponentialHistogramData();
                             dataPoint.Scale = exponentialHistogramData.Scale;
+                            dataPoint.ZeroCount = (ulong)exponentialHistogramData.ZeroCount;
 
                             dataPoint.Positive = new OtlpMetrics.ExponentialHistogramDataPoint.Types.Buckets();
                             dataPoint.Positive.Offset = exponentialHistogramData.PositiveBuckets.Offset;

diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs
@@ -383,6 +383,8 @@ public void TestCounterToOtlpMetric(string name, string description, string unit
         [Theory]
         [InlineData("test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Cumulative)]
         [InlineData("test_counter", null, null, null, 123.45, MetricReaderTemporalityPreference.Cumulative)]
+        [InlineData("test_counter", null, null, -123L, null, MetricReaderTemporalityPreference.Cumulative)]
+        [InlineData("test_counter", null, null, null, -123.45, MetricReaderTemporalityPreference.Cumulative)]
         [InlineData("test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta)]
         [InlineData("test_counter", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)]
         [InlineData("test_counter", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, "key1", "value1", "key2", 123)]
@@ -472,6 +474,8 @@ public void TestUpDownCounterToOtlpMetric(string name, string description, strin
         [Theory]
         [InlineData("test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Cumulative)]
         [InlineData("test_histogram", null, null, null, 123.45, MetricReaderTemporalityPreference.Cumulative)]
+        [InlineData("test_histogram", null, null, -123L, null, MetricReaderTemporalityPreference.Cumulative)]
+        [InlineData("test_histogram", null, null, null, -123.45, MetricReaderTemporalityPreference.Cumulative)]
         [InlineData("test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta)]
         [InlineData("test_histogram", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)]
         [InlineData("test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, "key1", "value1", "key2", 123)]
@@ -497,11 +501,13 @@ public void TestExponentialHistogramToOtlpMetric(string name, string description
             {
                 var histogram = meter.CreateHistogram<long>(name, unit, description);
                 histogram.Record(longValue.Value, attributes);
+                histogram.Record(0, attributes);
             }
             else
             {
                 var histogram = meter.CreateHistogram<double>(name, unit, description);
                 histogram.Record(doubleValue.Value, attributes);
+                histogram.Record(0, attributes);
             }
 
             provider.ForceFlush();
@@ -537,23 +543,49 @@ public void TestExponentialHistogramToOtlpMetric(string name, string description
             Assert.True(dataPoint.StartTimeUnixNano > 0);
             Assert.True(dataPoint.TimeUnixNano > 0);
 
-            Assert.Equal(1UL, dataPoint.Count);
+            Assert.Equal(20, dataPoint.Scale);
+            Assert.Equal(2UL, dataPoint.Count);
+            Assert.Equal(1UL, dataPoint.ZeroCount);
 
             if (longValue.HasValue)
             {
+                // Known issue: Negative measurements affect the Sum. Per the spec, they should not.
                 Assert.Equal((double)longValue, dataPoint.Sum);
+                if (longValue > 0)
+                {
+                    Assert.True(dataPoint.Positive.Offset > 0);
+                    Assert.Equal(1UL, dataPoint.Positive.BucketCounts[0]);
+                    Assert.True(dataPoint.Negative.Offset == 0);
+                    Assert.Empty(dataPoint.Negative.BucketCounts);
+                }
+                else
+                {
+                    Assert.True(dataPoint.Negative.Offset > 0);
+                    Assert.Equal(1UL, dataPoint.Negative.BucketCounts[0]);
+                    Assert.True(dataPoint.Positive.Offset == 0);
+                    Assert.Empty(dataPoint.Positive.BucketCounts);
+                }
             }
             else
             {
+                // Known issue: Negative measurements affect the Sum. Per the spec, they should not.
                 Assert.Equal(doubleValue, dataPoint.Sum);
+                if (doubleValue > 0)
+                {
+                    Assert.True(dataPoint.Positive.Offset > 0);
+                    Assert.Equal(1UL, dataPoint.Positive.BucketCounts[0]);
+                    Assert.True(dataPoint.Negative.Offset == 0);
+                    Assert.Empty(dataPoint.Negative.BucketCounts);
+                }
+                else
+                {
+                    Assert.True(dataPoint.Negative.Offset > 0);
+                    Assert.Equal(1UL, dataPoint.Negative.BucketCounts[0]);
+                    Assert.True(dataPoint.Positive.Offset == 0);
+                    Assert.Empty(dataPoint.Positive.BucketCounts);
+                }
             }
 
-            Assert.Equal(0UL, dataPoint.ZeroCount);
-            Assert.Equal(20, dataPoint.Scale);
-            Assert.True(dataPoint.Positive.Offset > 0);
-            Assert.Equal(1UL, dataPoint.Positive.BucketCounts[0]);
-            Assert.True(dataPoint.Negative.Offset <= 0);
-
             if (attributes.Length > 0)
             {
                 OtlpTestHelpers.AssertOtlpAttributes(attributes, dataPoint.Attributes);
@@ -569,6 +601,8 @@ public void TestExponentialHistogramToOtlpMetric(string name, string description
         [Theory]
         [InlineData("test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Cumulative)]
         [InlineData("test_histogram", null, null, null, 123.45, MetricReaderTemporalityPreference.Cumulative)]
+        [InlineData("test_histogram", null, null, -123L, null, MetricReaderTemporalityPreference.Cumulative)]
+        [InlineData("test_histogram", null, null, null, -123.45, MetricReaderTemporalityPreference.Cumulative)]
         [InlineData("test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta)]
         [InlineData("test_histogram", "description", "unit", 123L, null, MetricReaderTemporalityPreference.Cumulative)]
         [InlineData("test_histogram", null, null, 123L, null, MetricReaderTemporalityPreference.Delta, "key1", "value1", "key2", 123)]
@@ -632,6 +666,7 @@ public void TestHistogramToOtlpMetric(string name, string description, string un
 
             Assert.Equal(1UL, dataPoint.Count);
 
+            // Known issue: Negative measurements affect the Sum. Per the spec, they should not.
             if (longValue.HasValue)
             {
                 Assert.Equal((double)longValue, dataPoint.Sum);