Proposed amendment of #94672

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

@@ -218,15 +218,13 @@ func (o *SamplingHistogramOpts) annotateStabilityLevel() {
 // convenience function to allow easy transformation to the prometheus
 // counterpart. This will do more once we have a proper label abstraction
 func (o *SamplingHistogramOpts) toPromSamplingHistogramOpts() promext.SamplingHistogramOpts {
-	return promext.SamplingHistogramOpts{
-		HistogramOpts: prometheus.HistogramOpts{
-			Namespace:   o.Namespace,
-			Subsystem:   o.Subsystem,
-			Name:        o.Name,
-			Help:        o.Help,
-			ConstLabels: o.ConstLabels,
-			Buckets:     o.Buckets,
-		},
+	return promext.SamplingHistogramOpts{ 
+		Namespace:      o.Namespace,
+		Subsystem:      o.Subsystem,
+		Name:           o.Name,
+		Help:           o.Help,
+		ConstLabels:    o.ConstLabels,
+		Buckets:        o.Buckets,
 		InitialValue:   o.InitialValue,
 		SamplingPeriod: o.SamplingPeriod,
 	}

staging/src/k8s.io/component-base/metrics/prometheusextension/sampling-histogram.go

@@ -18,6 +18,8 @@ package prometheusextension
 
 import (
 	"fmt"
+	"math"
+	"sort"
 	"sync"
 	"time"
 
@@ -43,7 +45,18 @@ type SamplingHistogram interface {
 // SamplingHistogram metric.  This builds on the options for creating
 // a Histogram metric.
 type SamplingHistogramOpts struct {
-	prometheus.HistogramOpts
+	Namespace   string
+	Subsystem   string
+	Name        string
+	Help        string
+	ConstLabels prometheus.Labels
+
+	// Buckets defines the buckets into which observations are counted. Each
+	// element in the slice is the upper inclusive bound of a bucket. The
+	// values must be sorted in strictly increasing order. There is no need
+	// to add a highest bucket with +Inf bound, it will be added
+	// implicitly. The default value is DefBuckets.
+	Buckets []float64
 
 	// The initial value of the variable.
 	InitialValue float64
@@ -69,28 +82,55 @@ func NewTestableSamplingHistogram(clock clock.Clock, opts SamplingHistogramOpts)
 	return newSamplingHistogram(clock, desc, opts)
 }
 
-func newSamplingHistogram(clock clock.Clock, desc *prometheus.Desc, opts SamplingHistogramOpts, labelValues ...string) (SamplingHistogram, error) {
+func newSamplingHistogram(clock clock.Clock, desc *prometheus.Desc, opts SamplingHistogramOpts) (SamplingHistogram, error) {
 	if opts.SamplingPeriod <= 0 {
 		return nil, fmt.Errorf("the given sampling period was %v but must be positive", opts.SamplingPeriod)
 	}
+	if len(opts.Buckets) == 0 {
+		opts.Buckets = prometheus.DefBuckets
+	}
+
+	for i, upperBound := range opts.Buckets {
+		if i < len(opts.Buckets)-1 {
+			if upperBound >= opts.Buckets[i+1] {
+				return nil, fmt.Errorf(
+					"histogram buckets must be in increasing order: %f >= %f",
+					upperBound, opts.Buckets[i+1],
+				)
+			}
+		} else {
+			if math.IsInf(upperBound, +1) {
+				// The +Inf bucket is implicit. Remove it here.
+				opts.Buckets = opts.Buckets[:i]
+			}
+		}
+	}
+
 	return &samplingHistogram{
 		samplingPeriod:  opts.SamplingPeriod,
-		histogram:       prometheus.NewHistogram(opts.HistogramOpts),
+		desc:            desc,
 		clock:           clock,
 		lastSampleIndex: clock.Now().UnixNano() / int64(opts.SamplingPeriod),
 		value:           opts.InitialValue,
+		buckets:         make([]uint64, len(opts.Buckets)),
+		upperBounds:     opts.Buckets,
 	}, nil
 }
 
 type samplingHistogram struct {
 	samplingPeriod time.Duration
-	histogram      prometheus.Histogram
+	desc           *prometheus.Desc
 	clock          clock.Clock
 	lock           sync.Mutex
 
 	// identifies the last sampling period completed
 	lastSampleIndex int64
 	value           float64
+
+	sum         float64
+	count       uint64
+	buckets     []uint64
+	upperBounds []float64
 }
 
 var _ SamplingHistogram = &samplingHistogram{}
@@ -104,34 +144,50 @@ func (sh *samplingHistogram) Add(delta float64) {
 }
 
 func (sh *samplingHistogram) Update(updateFn func(float64) float64) {
-	oldValue, numSamples := func() (float64, int64) {
-		sh.lock.Lock()
-		defer sh.lock.Unlock()
-		newSampleIndex := sh.clock.Now().UnixNano() / int64(sh.samplingPeriod)
-		deltaIndex := newSampleIndex - sh.lastSampleIndex
-		sh.lastSampleIndex = newSampleIndex
-		oldValue := sh.value
-		sh.value = updateFn(sh.value)
-		return oldValue, deltaIndex
-	}()
-	for i := int64(0); i < numSamples; i++ {
-		sh.histogram.Observe(oldValue)
+	sh.lock.Lock()
+	defer sh.lock.Unlock()
+
+	newSampleIndex := sh.clock.Now().UnixNano() / int64(sh.samplingPeriod)
+	deltaIndex := uint64(newSampleIndex - sh.lastSampleIndex)
+	sh.lastSampleIndex = newSampleIndex
+	oldValue := sh.value
+	sh.value = updateFn(sh.value)
+
+	// Increment the actual histogram parts.
+	sh.count += deltaIndex
+	sh.sum += oldValue * float64(deltaIndex)
+	i := sort.SearchFloat64s(sh.upperBounds, oldValue)
+	if i < len(sh.buckets) {
+		sh.buckets[i] += deltaIndex
 	}
 }
 
 func (sh *samplingHistogram) Desc() *prometheus.Desc {
-	return sh.histogram.Desc()
+	return sh.desc
 }
 
 func (sh *samplingHistogram) Write(dest *dto.Metric) error {
-	return sh.histogram.Write(dest)
+	sh.lock.Lock()
+	defer sh.lock.Unlock()
+
+	buckets := make(map[float64]uint64, len(sh.buckets))
+	var cumCount uint64
+	for i, count := range sh.buckets {
+		cumCount += count
+		buckets[sh.upperBounds[i]] = cumCount
+	}
+	metric, err := prometheus.NewConstHistogram(sh.desc, sh.count, sh.sum, buckets)
+	if err != nil {
+		return err
+	}
+	return metric.Write(dest)
 }
 
 func (sh *samplingHistogram) Describe(ch chan<- *prometheus.Desc) {
-	sh.histogram.Describe(ch)
+	ch <- sh.desc
 }
 
 func (sh *samplingHistogram) Collect(ch chan<- prometheus.Metric) {
 	sh.Add(0)
-	sh.histogram.Collect(ch)
+	ch <- sh
 }

staging/src/k8s.io/component-base/metrics/prometheusextension/sampling-histogram_test.go

@@ -28,14 +28,12 @@ import (
 func TestSamplingHistogram(t *testing.T) {
 	clk := testclock.NewFakeClock(time.Unix(time.Now().Unix(), 999999990))
 	sh, err := NewTestableSamplingHistogram(clk, SamplingHistogramOpts{
-		HistogramOpts: prometheus.HistogramOpts{
-			Namespace:   "test",
-			Subsystem:   "func",
-			Name:        "one",
-			Help:        "a helpful string",
-			ConstLabels: map[string]string{"l1": "v1", "l2": "v2"},
-			Buckets:     []float64{0, 1, 2},
-		},
+		Namespace:      "test",
+		Subsystem:      "func",
+		Name:           "one",
+		Help:           "a helpful string",
+		ConstLabels:    map[string]string{"l1": "v1", "l2": "v2"},
+		Buckets:        []float64{0, 1, 2},
 		InitialValue:   1,
 		SamplingPeriod: time.Second,
 	})
@@ -119,7 +117,7 @@ func expectHistogram(t *testing.T, when string, sh SamplingHistogram, buckets ..
 		}
 		expectedCount := buckets[i]
 		if expectedCount != *(mhBucket.CumulativeCount) {
-			t.Errorf("%s, bucket %d's count was %d rather than %d", when, i, mhBucket.CumulativeCount, expectedCount)
+			t.Errorf("%s, bucket %d's count was %d rather than %d", when, i, *(mhBucket.CumulativeCount), expectedCount)
 		}
 	}
 }