Always remit at least one histogram bucket

open-telemetry · Jul 17, 2023 · e2b9bb9 · e2b9bb9
1 parent 9b6aa2f
commit e2b9bb9
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 40 deletions.
diff --git a/receiver/prometheusreceiver/internal/metricfamily.go b/receiver/prometheusreceiver/internal/metricfamily.go
@@ -88,55 +88,48 @@ func (mg *metricGroup) sortPoints() {
 
 func (mg *metricGroup) toDistributionPoint(dest pmetric.HistogramDataPointSlice) {
 	if !mg.hasCount {
-		// Otel histograms are required to have a count
-		// TODO: Decide if we also want to drop data if the sum isn't defined
 		return
 	}
 
-	point := pmetric.NewHistogramDataPoint()
-	pointIsStale := value.IsStaleNaN(mg.sum) || value.IsStaleNaN(mg.count)
+	mg.sortPoints()
 
-	if len(mg.complexValue) > 0 { // copy over the buckets if we have any
-		mg.sortPoints()
+	bucketCount := len(mg.complexValue) + 1
+	// if the final bucket is +Inf, we ignore it
+	if bucketCount > 1 && mg.complexValue[bucketCount-2].boundary == math.Inf(1) {
+		bucketCount--
+	}
 
-		bucketCount := len(mg.complexValue) + 1
-		// if the final bucket is +Inf, we ignore it
-		if mg.complexValue[len(mg.complexValue)-1].boundary == math.Inf(1) {
-			bucketCount--
-		}
+	// for OTLP the bounds won't include +inf
+	bounds := make([]float64, bucketCount-1)
+	bucketCounts := make([]uint64, bucketCount)
+	var adjustedCount float64
 
-		// for OTLP the bounds won't include +inf
-		bounds := make([]float64, bucketCount-1)
-		bucketCounts := make([]uint64, bucketCount)
-		var adjustedCount float64
-
-		for i := 0; i < bucketCount-1; i++ {
-			bounds[i] = mg.complexValue[i].boundary
-			adjustedCount = mg.complexValue[i].value
-
-			// Buckets still need to be sent to know to set them as stale,
-			// but a staleness NaN converted to uint64 would be an extremely large number.
-			// Setting to 0 instead.
-			if pointIsStale {
-				adjustedCount = 0
-			} else if i != 0 {
-				adjustedCount -= mg.complexValue[i-1].value
-			}
-			bucketCounts[i] = uint64(adjustedCount)
-		}
+	pointIsStale := value.IsStaleNaN(mg.sum) || value.IsStaleNaN(mg.count)
+	for i := 0; i < bucketCount-1; i++ {
+		bounds[i] = mg.complexValue[i].boundary
+		adjustedCount = mg.complexValue[i].value
 
-		// Add the final bucket based on the total count
-		adjustedCount = mg.count
+		// Buckets still need to be sent to know to set them as stale,
+		// but a staleness NaN converted to uint64 would be an extremely large number.
+		// Setting to 0 instead.
 		if pointIsStale {
 			adjustedCount = 0
-		} else if bucketCount > 1 {
-			adjustedCount -= mg.complexValue[bucketCount-2].value
+		} else if i != 0 {
+			adjustedCount -= mg.complexValue[i-1].value
 		}
-		bucketCounts[bucketCount-1] = uint64(adjustedCount)
+		bucketCounts[i] = uint64(adjustedCount)
+	}
 
-		point.ExplicitBounds().FromRaw(bounds)
-		point.BucketCounts().FromRaw(bucketCounts)
+	// Add the final bucket based on the total count
+	adjustedCount = mg.count
+	if pointIsStale {
+		adjustedCount = 0
+	} else if bucketCount > 1 {
+		adjustedCount -= mg.complexValue[bucketCount-2].value
 	}
+	bucketCounts[bucketCount-1] = uint64(adjustedCount)
+
+	point := dest.AppendEmpty()
 
 	if pointIsStale {
 		point.SetFlags(pmetric.DefaultDataPointFlags.WithNoRecordedValue(true))
@@ -147,6 +140,9 @@ func (mg *metricGroup) toDistributionPoint(dest pmetric.HistogramDataPointSlice)
 		}
 	}
 
+	point.ExplicitBounds().FromRaw(bounds)
+	point.BucketCounts().FromRaw(bucketCounts)
+
 	// The timestamp MUST be in retrieved from milliseconds and converted to nanoseconds.
 	tsNanos := timestampFromMs(mg.ts)
 	if mg.created != 0 {
@@ -158,7 +154,6 @@ func (mg *metricGroup) toDistributionPoint(dest pmetric.HistogramDataPointSlice)
 	point.SetTimestamp(tsNanos)
 	populateAttributes(pmetric.MetricTypeHistogram, mg.ls, point.Attributes())
 	mg.setExemplars(point.Exemplars())
-	point.MoveTo(dest.AppendEmpty())
 }
 
 func (mg *metricGroup) setExemplars(exemplars pmetric.ExemplarSlice) {

diff --git a/receiver/prometheusreceiver/internal/metricfamily_test.go b/receiver/prometheusreceiver/internal/metricfamily_test.go
@@ -233,6 +233,7 @@ func TestMetricGroupData_toDistributionUnitTest(t *testing.T) {
 				point.SetSum(1004.78)
 				point.SetTimestamp(pcommon.Timestamp(11 * time.Millisecond))      // the time in milliseconds -> nanoseconds.
 				point.SetStartTimestamp(pcommon.Timestamp(11 * time.Millisecond)) // the time in milliseconds -> nanoseconds.
+				point.BucketCounts().FromRaw([]uint64{66})
 				attributes := point.Attributes()
 				attributes.PutStr("a", "A")
 				attributes.PutStr("b", "B")

diff --git a/receiver/prometheusreceiver/internal/transaction_test.go b/receiver/prometheusreceiver/internal/transaction_test.go
@@ -1206,6 +1206,7 @@ func TestMetricBuilderHistogram(t *testing.T) {
 				pt0 := hist0.DataPoints().AppendEmpty()
 				pt0.SetCount(10)
 				pt0.SetSum(99)
+				pt0.BucketCounts().FromRaw([]uint64{10})
 				pt0.SetTimestamp(tsNanos)
 				pt0.SetStartTimestamp(startTimestamp)
 				pt0.Attributes().PutStr("foo", "bar")

diff --git a/receiver/prometheusreceiver/metrics_receiver_test.go b/receiver/prometheusreceiver/metrics_receiver_test.go
@@ -1123,7 +1123,7 @@ func verifyTarget3(t *testing.T, td *testData, resourceMetrics []pmetric.Resourc
 					histogramPointComparator: []histogramPointComparator{
 						compareHistogramStartTimestamp(ts1),
 						compareHistogramTimestamp(ts1),
-						compareHistogram(10, 100, nil),
+						compareHistogram(10, 100, []uint64{10}),
 					},
 				},
 			}),
@@ -1185,7 +1185,7 @@ func verifyTarget3(t *testing.T, td *testData, resourceMetrics []pmetric.Resourc
 					histogramPointComparator: []histogramPointComparator{
 						compareHistogramStartTimestamp(ts1),
 						compareHistogramTimestamp(ts2),
-						compareHistogram(15, 101, nil),
+						compareHistogram(15, 101, []uint64{15}),
 					},
 				},
 			}),