Merge pull request prometheus#1314 from prometheus/beorn7/histogram

histogram: Enable detection of a native histogram without observations

prometheus/histogram.go

@@ -428,12 +428,12 @@ type HistogramOpts struct {
 	// a major version bump.
 	NativeHistogramBucketFactor float64
 	// All observations with an absolute value of less or equal
-	// NativeHistogramZeroThreshold are accumulated into a “zero”
-	// bucket. For best results, this should be close to a bucket
-	// boundary. This is usually the case if picking a power of two. If
+	// NativeHistogramZeroThreshold are accumulated into a “zero” bucket.
+	// For best results, this should be close to a bucket boundary. This is
+	// usually the case if picking a power of two. If
 	// NativeHistogramZeroThreshold is left at zero,
-	// DefNativeHistogramZeroThreshold is used as the threshold. To configure
-	// a zero bucket with an actual threshold of zero (i.e. only
+	// DefNativeHistogramZeroThreshold is used as the threshold. To
+	// configure a zero bucket with an actual threshold of zero (i.e. only
 	// observations of precisely zero will go into the zero bucket), set
 	// NativeHistogramZeroThreshold to the NativeHistogramZeroThresholdZero
 	// constant (or any negative float value).
@@ -446,23 +446,28 @@ type HistogramOpts struct {
 	// Histogram are sufficiently wide-spread. In particular, this could be
 	// used as a DoS attack vector. Where the observed values depend on
 	// external inputs, it is highly recommended to set a
-	// NativeHistogramMaxBucketNumber.)  Once the set
+	// NativeHistogramMaxBucketNumber.) Once the set
 	// NativeHistogramMaxBucketNumber is exceeded, the following strategy is
-	// enacted: First, if the last reset (or the creation) of the histogram
-	// is at least NativeHistogramMinResetDuration ago, then the whole
-	// histogram is reset to its initial state (including regular
-	// buckets). If less time has passed, or if
-	// NativeHistogramMinResetDuration is zero, no reset is
-	// performed. Instead, the zero threshold is increased sufficiently to
-	// reduce the number of buckets to or below
-	// NativeHistogramMaxBucketNumber, but not to more than
-	// NativeHistogramMaxZeroThreshold. Thus, if
-	// NativeHistogramMaxZeroThreshold is already at or below the current
-	// zero threshold, nothing happens at this step. After that, if the
-	// number of buckets still exceeds NativeHistogramMaxBucketNumber, the
-	// resolution of the histogram is reduced by doubling the width of the
-	// sparse buckets (up to a growth factor between one bucket to the next
-	// of 2^(2^4) = 65536, see above).
+	// enacted:
+	//  - First, if the last reset (or the creation) of the histogram is at
+	//    least NativeHistogramMinResetDuration ago, then the whole
+	//    histogram is reset to its initial state (including regular
+	//    buckets).
+	//  - If less time has passed, or if NativeHistogramMinResetDuration is
+	//    zero, no reset is performed. Instead, the zero threshold is
+	//    increased sufficiently to reduce the number of buckets to or below
+	//    NativeHistogramMaxBucketNumber, but not to more than
+	//    NativeHistogramMaxZeroThreshold. Thus, if
+	//    NativeHistogramMaxZeroThreshold is already at or below the current
+	//    zero threshold, nothing happens at this step.
+	//  - After that, if the number of buckets still exceeds
+	//    NativeHistogramMaxBucketNumber, the resolution of the histogram is
+	//    reduced by doubling the width of the sparse buckets (up to a
+	//    growth factor between one bucket to the next of 2^(2^4) = 65536,
+	//    see above).
+	//  - Any increased zero threshold or reduced resolution is reset back
+	//    to their original values once NativeHistogramMinResetDuration has
+	//    passed (since the last reset or the creation of the histogram).
 	NativeHistogramMaxBucketNumber  uint32
 	NativeHistogramMinResetDuration time.Duration
 	NativeHistogramMaxZeroThreshold float64
@@ -782,6 +787,16 @@ func (h *histogram) Write(out *dto.Metric) error {
 		his.ZeroCount = proto.Uint64(zeroBucket)
 		his.NegativeSpan, his.NegativeDelta = makeBuckets(&coldCounts.nativeHistogramBucketsNegative)
 		his.PositiveSpan, his.PositiveDelta = makeBuckets(&coldCounts.nativeHistogramBucketsPositive)
+
+		// Add a no-op span to a histogram without observations and with
+		// a zero threshold of zero. Otherwise, a native histogram would
+		// look like a classic histogram to scrapers.
+		if *his.ZeroThreshold == 0 && *his.ZeroCount == 0 && len(his.PositiveSpan) == 0 && len(his.NegativeSpan) == 0 {
+			his.PositiveSpan = []*dto.BucketSpan{{
+				Offset: proto.Int32(0),
+				Length: proto.Uint32(0),
+			}}
+		}
 	}
 	addAndResetCounts(hotCounts, coldCounts)
 	return nil
@@ -854,13 +869,16 @@ func (h *histogram) limitBuckets(counts *histogramCounts, value float64, bucket
 	h.doubleBucketWidth(hotCounts, coldCounts)
 }
 
-// maybeReset resests the whole histogram if at least h.nativeHistogramMinResetDuration
+// maybeReset resets the whole histogram if at least h.nativeHistogramMinResetDuration
 // has been passed. It returns true if the histogram has been reset. The caller
 // must have locked h.mtx.
-func (h *histogram) maybeReset(hot, cold *histogramCounts, coldIdx uint64, value float64, bucket int) bool {
+func (h *histogram) maybeReset(
+	hot, cold *histogramCounts, coldIdx uint64, value float64, bucket int,
+) bool {
 	// We are using the possibly mocked h.now() rather than
 	// time.Since(h.lastResetTime) to enable testing.
-	if h.nativeHistogramMinResetDuration == 0 || h.now().Sub(h.lastResetTime) < h.nativeHistogramMinResetDuration {
+	if h.nativeHistogramMinResetDuration == 0 ||
+		h.now().Sub(h.lastResetTime) < h.nativeHistogramMinResetDuration {
 		return false
 	}
 	// Completely reset coldCounts.

prometheus/histogram_test.go

@@ -485,6 +485,17 @@ func TestNativeHistogram(t *testing.T) {
 			factor:       1,
 			want:         `sample_count:3 sample_sum:6 bucket:<cumulative_count:0 upper_bound:0.005 > bucket:<cumulative_count:0 upper_bound:0.01 > bucket:<cumulative_count:0 upper_bound:0.025 > bucket:<cumulative_count:0 upper_bound:0.05 > bucket:<cumulative_count:0 upper_bound:0.1 > bucket:<cumulative_count:0 upper_bound:0.25 > bucket:<cumulative_count:0 upper_bound:0.5 > bucket:<cumulative_count:1 upper_bound:1 > bucket:<cumulative_count:2 upper_bound:2.5 > bucket:<cumulative_count:3 upper_bound:5 > bucket:<cumulative_count:3 upper_bound:10 > `, // Has conventional buckets because there are no sparse buckets.
 		},
+		{
+			name:   "no observations",
+			factor: 1.1,
+			want:   `sample_count:0 sample_sum:0 schema:3 zero_threshold:2.938735877055719e-39 zero_count:0 `,
+		},
+		{
+			name:          "no observations and zero threshold of zero resulting in no-op span",
+			factor:        1.1,
+			zeroThreshold: NativeHistogramZeroThresholdZero,
+			want:          `sample_count:0 sample_sum:0 schema:3 zero_threshold:0 zero_count:0 positive_span:<offset:0 length:0 > `,
+		},
 		{
 			name:         "factor 1.1 results in schema 3",
 			observations: []float64{0, 1, 2, 3},