Merge pull request #99228 from Huang-Wei/hist-vec

sched: better support of HistogramVec in scheduler performance test

staging/src/k8s.io/component-base/metrics/testutil/metrics.go

@@ -176,13 +176,87 @@ type Histogram struct {
 	*dto.Histogram
 }
 
-// GetHistogramFromGatherer collects a metric from a gatherer implementing k8s.io/component-base/metrics.Gatherer interface.
+// HistogramVec wraps a slice of Histogram.
+// Note that each Histogram must have the same number of buckets.
+type HistogramVec []*Histogram
+
+// GetAggregatedSampleCount aggregates the sample count of each inner Histogram.
+func (vec HistogramVec) GetAggregatedSampleCount() uint64 {
+	var count uint64
+	for _, hist := range vec {
+		count += hist.GetSampleCount()
+	}
+	return count
+}
+
+// GetAggregatedSampleSum aggregates the sample sum of each inner Histogram.
+func (vec HistogramVec) GetAggregatedSampleSum() float64 {
+	var sum float64
+	for _, hist := range vec {
+		sum += hist.GetSampleSum()
+	}
+	return sum
+}
+
+// Quantile first aggregates inner buckets of each Histogram, and then
+// computes q-th quantile of a cumulative histogram.
+func (vec HistogramVec) Quantile(q float64) float64 {
+	var buckets []bucket
+
+	for i, hist := range vec {
+		for j, bckt := range hist.Bucket {
+			if i == 0 {
+				buckets = append(buckets, bucket{
+					count:      float64(bckt.GetCumulativeCount()),
+					upperBound: bckt.GetUpperBound(),
+				})
+			} else {
+				buckets[j].count += float64(bckt.GetCumulativeCount())
+			}
+		}
+	}
+
+	if len(buckets) == 0 || buckets[len(buckets)-1].upperBound != math.Inf(+1) {
+		// The list of buckets in dto.Histogram doesn't include the final +Inf bucket, so we
+		// add it here for the rest of the samples.
+		buckets = append(buckets, bucket{
+			count:      float64(vec.GetAggregatedSampleCount()),
+			upperBound: math.Inf(+1),
+		})
+	}
+
+	return bucketQuantile(q, buckets)
+}
+
+// Average computes wrapped histograms' average value.
+func (vec HistogramVec) Average() float64 {
+	return vec.GetAggregatedSampleSum() / float64(vec.GetAggregatedSampleCount())
+}
+
+// Validate makes sure the wrapped histograms have all necessary fields set and with valid values.
+func (vec HistogramVec) Validate() error {
+	bucketSize := 0
+	for i, hist := range vec {
+		if err := hist.Validate(); err != nil {
+			return err
+		}
+		if i == 0 {
+			bucketSize = len(hist.GetBucket())
+		} else if bucketSize != len(hist.GetBucket()) {
+			return fmt.Errorf("found different bucket size: expect %v, but got %v at index %v", bucketSize, len(hist.GetBucket()), i)
+		}
+	}
+	return nil
+}
+
+// GetHistogramVecFromGatherer collects a metric, that matches the input labelValue map,
+// from a gatherer implementing k8s.io/component-base/metrics.Gatherer interface.
 // Used only for testing purposes where we need to gather metrics directly from a running binary (without metrics endpoint).
-func GetHistogramFromGatherer(gatherer metrics.Gatherer, metricName string) (Histogram, error) {
+func GetHistogramVecFromGatherer(gatherer metrics.Gatherer, metricName string, lvMap map[string]string) (HistogramVec, error) {
 	var metricFamily *dto.MetricFamily
 	m, err := gatherer.Gather()
 	if err != nil {
-		return Histogram{}, err
+		return nil, err
 	}
 	for _, mFamily := range m {
 		if mFamily.GetName() == metricName {
@@ -192,23 +266,26 @@ func GetHistogramFromGatherer(gatherer metrics.Gatherer, metricName string) (His
 	}
 
 	if metricFamily == nil {
-		return Histogram{}, fmt.Errorf("metric %q not found", metricName)
+		return nil, fmt.Errorf("metric %q not found", metricName)
 	}
 
 	if metricFamily.GetMetric() == nil {
-		return Histogram{}, fmt.Errorf("metric %q is empty", metricName)
+		return nil, fmt.Errorf("metric %q is empty", metricName)
 	}
 
 	if len(metricFamily.GetMetric()) == 0 {
-		return Histogram{}, fmt.Errorf("metric %q is empty", metricName)
+		return nil, fmt.Errorf("metric %q is empty", metricName)
 	}
 
-	return Histogram{
+	vec := make(HistogramVec, 0)
-		// Histograms are stored under the first index (based on observation).
+	for _, metric := range metricFamily.GetMetric() {
-		// Given there's only one histogram registered per each metric name, accessing
+		if LabelsMatch(metric, lvMap) {
-		// the first index is sufficient.
+			if hist := metric.GetHistogram(); hist != nil {
-		metricFamily.GetMetric()[0].GetHistogram(),
+				vec = append(vec, &Histogram{hist})
-	}, nil
+			}
+		}
+	}
+	return vec, nil
 }
 
 func uint64Ptr(u uint64) *uint64 {
@@ -266,7 +343,7 @@ func (hist *Histogram) Quantile(q float64) float64 {
 
 	if len(buckets) == 0 || buckets[len(buckets)-1].upperBound != math.Inf(+1) {
 		// The list of buckets in dto.Histogram doesn't include the final +Inf bucket, so we
-		// add it here for the reset of the samples.
+		// add it here for the rest of the samples.
 		buckets = append(buckets, bucket{
 			count:      float64(hist.GetSampleCount()),
 			upperBound: math.Inf(+1),

staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go

@@ -19,6 +19,7 @@ package testutil
 import (
 	"fmt"
 	"math"
+	"reflect"
 	"testing"
 
 	"k8s.io/utils/pointer"
@@ -311,3 +312,203 @@ func TestLabelsMatch(t *testing.T) {
 		})
 	}
 }
+
+func TestHistogramVec_GetAggregatedSampleCount(t *testing.T) {
+	tests := []struct {
+		name string
+		vec  HistogramVec
+		want uint64
+	}{
+		{
+			name: "nil case",
+			want: 0,
+		},
+		{
+			name: "zero case",
+			vec: HistogramVec{
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(0), SampleSum: pointer.Float64Ptr(0.0)}},
+			},
+			want: 0,
+		},
+		{
+			name: "standard case",
+			vec: HistogramVec{
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(1), SampleSum: pointer.Float64Ptr(2.0)}},
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(2), SampleSum: pointer.Float64Ptr(4.0)}},
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(4), SampleSum: pointer.Float64Ptr(8.0)}},
+			},
+			want: 7,
+		},
+		{
+			name: "mixed case",
+			vec: HistogramVec{
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(1), SampleSum: pointer.Float64Ptr(2.0)}},
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(0), SampleSum: pointer.Float64Ptr(0.0)}},
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(2), SampleSum: pointer.Float64Ptr(4.0)}},
+			},
+			want: 3,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.vec.GetAggregatedSampleCount(); got != tt.want {
+				t.Errorf("GetAggregatedSampleCount() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestHistogramVec_GetAggregatedSampleSum(t *testing.T) {
+	tests := []struct {
+		name string
+		vec  HistogramVec
+		want float64
+	}{
+		{
+			name: "nil case",
+			want: 0.0,
+		},
+		{
+			name: "zero case",
+			vec: HistogramVec{
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(0), SampleSum: pointer.Float64Ptr(0.0)}},
+			},
+			want: 0.0,
+		},
+		{
+			name: "standard case",
+			vec: HistogramVec{
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(1), SampleSum: pointer.Float64Ptr(2.0)}},
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(2), SampleSum: pointer.Float64Ptr(4.0)}},
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(4), SampleSum: pointer.Float64Ptr(8.0)}},
+			},
+			want: 14.0,
+		},
+		{
+			name: "mixed case",
+			vec: HistogramVec{
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(1), SampleSum: pointer.Float64Ptr(2.0)}},
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(0), SampleSum: pointer.Float64Ptr(0.0)}},
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(2), SampleSum: pointer.Float64Ptr(4.0)}},
+			},
+			want: 6.0,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.vec.GetAggregatedSampleSum(); got != tt.want {
+				t.Errorf("GetAggregatedSampleSum() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestHistogramVec_Quantile(t *testing.T) {
+	tests := []struct {
+		name     string
+		samples  [][]float64
+		bounds   []float64
+		quantile float64
+		want     []float64
+	}{
+		{
+			name: "duplicated histograms",
+			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},
+				{0.5, 0.5, 0.5, 0.5, 1.5, 1.5, 1.5, 1.5, 3, 3, 3, 3, 6, 6, 6, 6},
+				{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},
+			want:   []float64{2, 6.4, 7.2, 7.84},
+		},
+		{
+			name: "random numbers",
+			samples: [][]float64{
+				{8, 35, 47, 61, 56, 69, 66, 74, 35, 69, 5, 38, 58, 40, 36, 12},
+				{79, 44, 57, 46, 11, 8, 53, 77, 13, 35, 38, 47, 73, 16, 26, 29},
+				{51, 76, 22, 55, 20, 63, 59, 66, 34, 58, 64, 16, 79, 7, 58, 28},
+			},
+			bounds: []float64{10, 20, 40, 80},
+			want:   []float64{44.44, 72.89, 76.44, 79.29},
+		},
+		{
+			name: "single histogram",
+			samples: [][]float64{
+				{6, 34, 30, 10, 20, 18, 26, 31, 4, 2, 33, 17, 30, 1, 18, 29},
+			},
+			bounds: []float64{10, 20, 40, 80},
+			want:   []float64{20, 36, 38, 39.6},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var vec HistogramVec
+			for _, sample := range tt.samples {
+				histogram := samples2Histogram(sample, tt.bounds)
+				vec = append(vec, &histogram)
+			}
+			var got []float64
+			for _, q := range []float64{0.5, 0.9, 0.95, 0.99} {
+				got = append(got, math.Round(vec.Quantile(q)*100)/100)
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Quantile() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestHistogramVec_Validate(t *testing.T) {
+	tests := []struct {
+		name string
+		vec  HistogramVec
+		want error
+	}{
+		{
+			name: "nil SampleCount",
+			vec: HistogramVec{
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(1), SampleSum: pointer.Float64Ptr(1.0)}},
+				&Histogram{&dto.Histogram{SampleSum: pointer.Float64Ptr(2.0)}},
+			},
+			want: fmt.Errorf("nil or empty histogram SampleCount"),
+		},
+		{
+			name: "valid HistogramVec",
+			vec: HistogramVec{
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(1), SampleSum: pointer.Float64Ptr(1.0)}},
+				&Histogram{&dto.Histogram{SampleCount: uint64Ptr(2), SampleSum: pointer.Float64Ptr(2.0)}},
+			},
+		},
+		{
+			name: "different bucket size",
+			vec: HistogramVec{
+				&Histogram{&dto.Histogram{
+					SampleCount: uint64Ptr(4),
+					SampleSum:   pointer.Float64Ptr(10.0),
+					Bucket: []*dto.Bucket{
+						{CumulativeCount: uint64Ptr(1), UpperBound: pointer.Float64Ptr(1)},
+						{CumulativeCount: uint64Ptr(2), UpperBound: pointer.Float64Ptr(2)},
+						{CumulativeCount: uint64Ptr(5), UpperBound: pointer.Float64Ptr(4)},
+					},
+				}},
+				&Histogram{&dto.Histogram{
+					SampleCount: uint64Ptr(3),
+					SampleSum:   pointer.Float64Ptr(8.0),
+					Bucket: []*dto.Bucket{
+						{CumulativeCount: uint64Ptr(1), UpperBound: pointer.Float64Ptr(2)},
+						{CumulativeCount: uint64Ptr(3), UpperBound: pointer.Float64Ptr(4)},
+					},
+				}},
+			},
+			want: fmt.Errorf("found different bucket size: expect 3, but got 2 at index 1"),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.vec.Validate(); fmt.Sprintf("%v", got) != fmt.Sprintf("%v", tt.want) {
+				t.Errorf("Validate() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

test/integration/scheduler_perf/util.go

@@ -183,7 +183,7 @@ type metricsCollectorConfig struct {
 }
 
 // metricsCollector collects metrics from legacyregistry.DefaultGatherer.Gather() endpoint.
-// Currently only Histrogram metrics are supported.
+// Currently only Histogram metrics are supported.
 type metricsCollector struct {
 	*metricsCollectorConfig
 	labels map[string]string
@@ -203,7 +203,7 @@ func (*metricsCollector) run(ctx context.Context) {
 func (pc *metricsCollector) collect() []DataItem {
 	var dataItems []DataItem
 	for _, metric := range pc.Metrics {
-		dataItem := collectHistogram(metric, pc.labels)
+		dataItem := collectHistogramVec(metric, pc.labels)
 		if dataItem != nil {
 			dataItems = append(dataItems, *dataItem)
 		}
@@ -211,26 +211,23 @@ func (pc *metricsCollector) collect() []DataItem {
 	return dataItems
 }
 
-func collectHistogram(metric string, labels map[string]string) *DataItem {
+func collectHistogramVec(metric string, labels map[string]string) *DataItem {
-	hist, err := testutil.GetHistogramFromGatherer(legacyregistry.DefaultGatherer, metric)
+	vec, err := testutil.GetHistogramVecFromGatherer(legacyregistry.DefaultGatherer, metric, nil)
 	if err != nil {
 		klog.Error(err)
 		return nil
 	}
-	if hist.Histogram == nil {
+
-		klog.Errorf("metric %q is not a Histogram metric", metric)
+	if err := vec.Validate(); err != nil {
-		return nil
-	}
-	if err := hist.Validate(); err != nil {
 		klog.Error(err)
 		return nil
 	}
 
-	q50 := hist.Quantile(0.50)
+	q50 := vec.Quantile(0.50)
-	q90 := hist.Quantile(0.90)
+	q90 := vec.Quantile(0.90)
-	q95 := hist.Quantile(0.95)
+	q95 := vec.Quantile(0.95)
-	q99 := hist.Quantile(0.99)
+	q99 := vec.Quantile(0.99)
-	avg := hist.Average()
+	avg := vec.Average()
 
 	msFactor := float64(time.Second) / float64(time.Millisecond)