From e410b837dc4bb4a814652741289c1e6a9bc8ca69 Mon Sep 17 00:00:00 2001
From: Jan Chaloupka <jchaloup@redhat.com>
Date: Tue, 31 Mar 2020 18:41:42 +0200
Subject: [PATCH] quantile: if the last upper bound is +Inf, return the
 previous upper bound

In case the last upper bound is +Inf, computed quantile is +Inf as well.
Given there's no restriction on how far individual upper bounds are from each other,
cut the last interval and consider the second last upper bound as the final one.
---
 .../metrics/testutil/metrics.go               |  4 ++
 .../metrics/testutil/metrics_test.go          | 54 +++++++++++--------
 2 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/staging/src/k8s.io/component-base/metrics/testutil/metrics.go b/staging/src/k8s.io/component-base/metrics/testutil/metrics.go
index 2af9c01058e..a3ae816b41a 100644
--- a/staging/src/k8s.io/component-base/metrics/testutil/metrics.go
+++ b/staging/src/k8s.io/component-base/metrics/testutil/metrics.go
@@ -262,6 +262,10 @@ func bucketQuantile(q float64, buckets []bucket) float64 {
 		return buckets[0].upperBound * (rank / buckets[0].count)
 	}
 
+	if b == len(buckets)-1 && math.IsInf(buckets[b].upperBound, 1) {
+		return buckets[len(buckets)-2].upperBound
+	}
+
 	// linear approximation of b-th bucket
 	brank := rank - buckets[b-1].count
 	bSize := buckets[b].upperBound - buckets[b-1].upperBound
diff --git a/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go b/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go
index f44f559abbb..51fb49ffee6 100644
--- a/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go
+++ b/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go
@@ -18,6 +18,7 @@ package testutil
 
 import (
 	"fmt"
+	"math"
 	"testing"
 
 	"k8s.io/utils/pointer"
@@ -54,6 +55,7 @@ func samples2Histogram(samples []float64, upperBounds []float64) Histogram {
 
 func TestHistogramQuantile(t *testing.T) {
 	tests := []struct {
+		name    string
 		samples []float64
 		bounds  []float64
 		q50     float64
@@ -61,7 +63,7 @@ func TestHistogramQuantile(t *testing.T) {
 		q99     float64
 	}{
 		{
-			// repeating numbers
+			name:    "Repeating numbers",
 			samples: []float64{0.5, 0.5, 0.5, 0.5, 1.5, 1.5, 1.5, 1.5, 3, 3, 3, 3, 6, 6, 6, 6},
 			bounds:  []float64{1, 2, 4, 8},
 			q50:     2,
@@ -69,7 +71,7 @@ func TestHistogramQuantile(t *testing.T) {
 			q99:     7.84,
 		},
 		{
-			// random numbers
+			name:    "Random numbers",
 			samples: []float64{11, 67, 61, 21, 40, 36, 52, 63, 8, 3, 67, 35, 61, 1, 36, 58},
 			bounds:  []float64{10, 20, 40, 80},
 			q50:     40,
@@ -77,35 +79,45 @@ func TestHistogramQuantile(t *testing.T) {
 			q99:     79.2,
 		},
 		{
-			// the last bucket is empty
+			name:    "The last bucket is empty",
 			samples: []float64{6, 34, 30, 10, 20, 18, 26, 31, 4, 2, 33, 17, 30, 1, 18, 29},
 			bounds:  []float64{10, 20, 40, 80},
 			q50:     20,
 			q90:     36,
 			q99:     39.6,
 		},
+		{
+			name:    "The last bucket has positive infinity upper bound",
+			samples: []float64{5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 500},
+			bounds:  []float64{10, 20, 40, math.Inf(1)},
+			q50:     5.3125,
+			q90:     9.5625,
+			q99:     40,
+		},
 	}
 
 	for _, test := range tests {
-		h := samples2Histogram(test.samples, test.bounds)
-		q50 := h.Quantile(0.5)
-		q90 := h.Quantile(0.9)
-		q99 := h.Quantile(0.99)
-		q999999 := h.Quantile(0.999999)
+		t.Run(test.name, func(t *testing.T) {
+			h := samples2Histogram(test.samples, test.bounds)
+			q50 := h.Quantile(0.5)
+			q90 := h.Quantile(0.9)
+			q99 := h.Quantile(0.99)
+			q999999 := h.Quantile(0.999999)
 
-		if q50 != test.q50 {
-			t.Errorf("Expected q50 to be %v, got %v instead", test.q50, q50)
-		}
-		if q90 != test.q90 {
-			t.Errorf("Expected q90 to be %v, got %v instead", test.q90, q90)
-		}
-		if q99 != test.q99 {
-			t.Errorf("Expected q99 to be %v, got %v instead", test.q99, q99)
-		}
-		lastUpperBound := test.bounds[len(test.bounds)-1]
-		if !(q999999 < lastUpperBound) {
-			t.Errorf("Expected q999999 to be less than %v, got %v instead", lastUpperBound, q999999)
-		}
+			if q50 != test.q50 {
+				t.Errorf("Expected q50 to be %v, got %v instead", test.q50, q50)
+			}
+			if q90 != test.q90 {
+				t.Errorf("Expected q90 to be %v, got %v instead", test.q90, q90)
+			}
+			if q99 != test.q99 {
+				t.Errorf("Expected q99 to be %v, got %v instead", test.q99, q99)
+			}
+			lastUpperBound := test.bounds[len(test.bounds)-1]
+			if !(q999999 < lastUpperBound) {
+				t.Errorf("Expected q999999 to be less than %v, got %v instead", lastUpperBound, q999999)
+			}
+		})
 	}
 }