Make more ordinary and add benchmarks of wrapped timing histograms

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

@@ -143,61 +143,16 @@ func (v *timingHistogramVec) initializeDeprecatedMetric() {
 	v.initializeMetric()
 }
 
-func (v *timingHistogramVec) Set(value float64, labelValues ...string) {
+// WithLabelValuesChecked, if called on a hidden vector,
-	gm, _ := v.WithLabelValues(labelValues...)
+// will return a noop gauge and a nil error.
-	gm.Set(value)
+// If called before this vector has been registered in
-}
+// at least one registry, will return a noop gauge and
-
+// an error that passes ErrIsNotRegistered.
-func (v *timingHistogramVec) Inc(labelValues ...string) {
+// If called with a syntactic problem in the labels, will
-	gm, _ := v.WithLabelValues(labelValues...)
+// return a noop gauge and an error about the labels.
-	gm.Inc()
+// If none of the above apply, this method will return
-}
+// the appropriate vector member and a nil error.
-
+func (v *timingHistogramVec) WithLabelValuesChecked(lvs ...string) (GaugeMetric, error) {
-func (v *timingHistogramVec) Dec(labelValues ...string) {
-	gm, _ := v.WithLabelValues(labelValues...)
-	gm.Dec()
-}
-
-func (v *timingHistogramVec) Add(delta float64, labelValues ...string) {
-	gm, _ := v.WithLabelValues(labelValues...)
-	gm.Add(delta)
-}
-func (v *timingHistogramVec) SetToCurrentTime(labelValues ...string) {
-	gm, _ := v.WithLabelValues(labelValues...)
-	gm.SetToCurrentTime()
-}
-
-func (v *timingHistogramVec) SetForLabels(value float64, labels map[string]string) {
-	gm, _ := v.With(labels)
-	gm.Set(value)
-}
-
-func (v *timingHistogramVec) IncForLabels(labels map[string]string) {
-	gm, _ := v.With(labels)
-	gm.Inc()
-}
-
-func (v *timingHistogramVec) DecForLabels(labels map[string]string) {
-	gm, _ := v.With(labels)
-	gm.Dec()
-}
-
-func (v *timingHistogramVec) AddForLabels(delta float64, labels map[string]string) {
-	gm, _ := v.With(labels)
-	gm.Add(delta)
-}
-func (v *timingHistogramVec) SetToCurrentTimeForLabels(labels map[string]string) {
-	gm, _ := v.With(labels)
-	gm.SetToCurrentTime()
-}
-
-// WithLabelValues, if called after this vector has been
-// registered in at least one registry and this vector is not
-// hidden, will return a GaugeMetric that is NOT a noop along
-// with nil error.  If called on a hidden vector then it will
-// return a noop and a nil error.  Otherwise it returns a noop
-// and an error that passes ErrIsNotRegistered.
-func (v *timingHistogramVec) WithLabelValues(lvs ...string) (GaugeMetric, error) {
 	if v.IsHidden() {
 		return noop, nil
 	}
@@ -207,16 +162,33 @@ func (v *timingHistogramVec) WithLabelValues(lvs ...string) (GaugeMetric, error)
 	if v.LabelValueAllowLists != nil {
 		v.LabelValueAllowLists.ConstrainToAllowedList(v.originalLabels, lvs)
 	}
-	return v.TimingHistogramVec.WithLabelValues(lvs...).(GaugeMetric), nil
+	ops, err := v.TimingHistogramVec.GetMetricWithLabelValues(lvs...)
+	return ops.(GaugeMetric), err
 }
 
-// With, if called after this vector has been
+// WithLabelValues calls WithLabelValuesChecked
-// registered in at least one registry and this vector is not
+// and handles errors as follows.
-// hidden, will return a GaugeMetric that is NOT a noop along
+// An error that passes ErrIsNotRegistered is ignored
-// with nil error.  If called on a hidden vector then it will
+// and the noop gauge is returned;
-// return a noop and a nil error.  Otherwise it returns a noop
+// all other errors cause a panic.
-// and an error that passes ErrIsNotRegistered.
+func (v *timingHistogramVec) WithLabelValues(lvs ...string) GaugeMetric {
-func (v *timingHistogramVec) With(labels map[string]string) (GaugeMetric, error) {
+	ans, err := v.WithLabelValuesChecked(lvs...)
+	if err == nil || ErrIsNotRegistered(err) {
+		return ans
+	}
+	panic(err)
+}
+
+// WithChecked, if called on a hidden vector,
+// will return a noop gauge and a nil error.
+// If called before this vector has been registered in
+// at least one registry, will return a noop gauge and
+// an error that passes ErrIsNotRegistered.
+// If called with a syntactic problem in the labels, will
+// return a noop gauge and an error about the labels.
+// If none of the above apply, this method will return
+// the appropriate vector member and a nil error.
+func (v *timingHistogramVec) WithChecked(labels map[string]string) (GaugeMetric, error) {
 	if v.IsHidden() {
 		return noop, nil
 	}
@@ -226,7 +198,20 @@ func (v *timingHistogramVec) With(labels map[string]string) (GaugeMetric, error)
 	if v.LabelValueAllowLists != nil {
 		v.LabelValueAllowLists.ConstrainLabelMap(labels)
 	}
-	return v.TimingHistogramVec.With(labels).(GaugeMetric), nil
+	ops, err := v.TimingHistogramVec.GetMetricWith(labels)
+	return ops.(GaugeMetric), err
+}
+
+// With calls WithChecked and handles errors as follows.
+// An error that passes ErrIsNotRegistered is ignored
+// and the noop gauge is returned;
+// all other errors cause a panic.
+func (v *timingHistogramVec) With(labels map[string]string) GaugeMetric {
+	ans, err := v.WithChecked(labels)
+	if err == nil || ErrIsNotRegistered(err) {
+		return ans
+	}
+	panic(err)
 }
 
 // Delete deletes the metric where the variable labels are the same as those

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

@@ -221,7 +221,7 @@ func TestTimingHistogramVec(t *testing.T) {
 			c := NewTestableTimingHistogramVec(clk.Now, test.TimingHistogramOpts, test.labels)
 			registry.MustRegister(c)
 			var v0 float64 = 3
-			cm1, err := c.WithLabelValues("1", "2")
+			cm1, err := c.WithLabelValuesChecked("1", "2")
 			if err != nil {
 				t.Error(err)
 			}
@@ -251,14 +251,14 @@ func TestTimingHistogramVec(t *testing.T) {
 			}
 
 			// let's exercise the metric and verify it still works
-			c.Set(v0, "1", "3")
+			c.WithLabelValues("1", "3").Set(v0)
-			c.Set(v0, "2", "3")
+			c.WithLabelValues("2", "3").Set(v0)
 			dt1 := time.Nanosecond
 			t1 := t0.Add(dt1)
 			clk.SetTime(t1)
-			c.Add(5.0, "1", "2")
+			c.WithLabelValues("1", "2").Add(5.0)
-			c.Add(5.0, "1", "3")
+			c.WithLabelValues("1", "3").Add(5.0)
-			c.Add(5.0, "2", "3")
+			c.WithLabelValues("2", "3").Add(5.0)
 			ms, err = registry.Gather()
 			assert.Nil(t, err, "Gather failed %v", err)
 
@@ -324,7 +324,7 @@ func TestTimingHistogramWithLabelValueAllowList(t *testing.T) {
 			registry.MustRegister(c)
 			var v0 float64 = 13
 			for _, lv := range test.labelValues {
-				c.Set(v0, lv...)
+				c.WithLabelValues(lv...).Set(v0)
 			}
 
 			dt1 := 3 * time.Hour
@@ -332,7 +332,7 @@ func TestTimingHistogramWithLabelValueAllowList(t *testing.T) {
 			clk.SetTime(t1)
 
 			for _, lv := range test.labelValues {
-				c.Add(1.0, lv...)
+				c.WithLabelValues(lv...).Add(1.0)
 			}
 			mfs, err := registry.Gather()
 			assert.Nil(t, err, "Gather failed %v", err)
@@ -368,3 +368,75 @@ func TestTimingHistogramWithLabelValueAllowList(t *testing.T) {
 		})
 	}
 }
+
+func BenchmarkTimingHistogram(b *testing.B) {
+	b.StopTimer()
+	now := time.Now()
+	th := NewTestableTimingHistogram(func() time.Time { return now }, &TimingHistogramOpts{
+		Namespace:    "testns",
+		Subsystem:    "testsubsys",
+		Name:         "testhist",
+		Help:         "Me",
+		Buckets:      []float64{1, 2, 4, 8, 16},
+		InitialValue: 3,
+	})
+	registry := NewKubeRegistry()
+	registry.MustRegister(th)
+	var x int
+	b.StartTimer()
+	for i := 0; i < b.N; i++ {
+		now = now.Add(time.Duration(31-x) * time.Microsecond)
+		th.Set(float64(x))
+		x = (x + i) % 23
+	}
+}
+
+func BenchmarkTimingHistogramVecEltCached(b *testing.B) {
+	b.StopTimer()
+	now := time.Now()
+	hv := NewTestableTimingHistogramVec(func() time.Time { return now }, &TimingHistogramOpts{
+		Namespace:    "testns",
+		Subsystem:    "testsubsys",
+		Name:         "testhist",
+		Help:         "Me",
+		Buckets:      []float64{1, 2, 4, 8, 16},
+		InitialValue: 3,
+	},
+		[]string{"label1", "label2"})
+	registry := NewKubeRegistry()
+	registry.MustRegister(hv)
+	th, err := hv.WithLabelValuesChecked("v1", "v2")
+	if err != nil {
+		b.Error(err)
+	}
+	var x int
+	b.StartTimer()
+	for i := 0; i < b.N; i++ {
+		now = now.Add(time.Duration(31-x) * time.Microsecond)
+		th.Set(float64(x))
+		x = (x + i) % 23
+	}
+}
+
+func BenchmarkTimingHistogramVecEltFetched(b *testing.B) {
+	b.StopTimer()
+	now := time.Now()
+	hv := NewTestableTimingHistogramVec(func() time.Time { return now }, &TimingHistogramOpts{
+		Namespace:    "testns",
+		Subsystem:    "testsubsys",
+		Name:         "testhist",
+		Help:         "Me",
+		Buckets:      []float64{1, 2, 4, 8, 16},
+		InitialValue: 3,
+	},
+		[]string{"label1", "label2"})
+	registry := NewKubeRegistry()
+	registry.MustRegister(hv)
+	var x int
+	b.StartTimer()
+	for i := 0; i < b.N; i++ {
+		now = now.Add(time.Duration(31-x) * time.Microsecond)
+		hv.WithLabelValues("v1", "v2").Set(float64(x))
+		x = (x + i) % 60
+	}
+}

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

@@ -74,33 +74,41 @@ type GaugeMetric interface {
 // taken by the other pattern --- which is treacherous.  The treachery is that
 // WithLabelValues can return an object that is permanently broken (i.e., a noop).
 type GaugeMetricVec interface {
-	Set(value float64, labelValues ...string)
-	Inc(labelValues ...string)
-	Dec(labelValues ...string)
-	Add(delta float64, labelValues ...string)
-	SetToCurrentTime(labelValues ...string)
 
-	SetForLabels(value float64, labels map[string]string)
+	// WithLabelValuesChecked, if called on a hidden vector,
-	IncForLabels(labels map[string]string)
+	// will return a noop gauge and a nil error.
-	DecForLabels(labels map[string]string)
+	// If called before this vector has been registered in
-	AddForLabels(delta float64, labels map[string]string)
+	// at least one registry, will return a noop gauge and
-	SetToCurrentTimeForLabels(labels map[string]string)
+	// an error that passes ErrIsNotRegistered.
+	// If called with a syntactic problem in the labels, will
+	// return a noop gauge and an error about the labels.
+	// If none of the above apply, this method will return
+	// the appropriate vector member and a nil error.
+	WithLabelValuesChecked(labelValues ...string) (GaugeMetric, error)
 
-	// WithLabelValues, if called after this vector has been
+	// WithLabelValues calls WithLabelValuesChecked
-	// registered in at least one registry and this vector is not
+	// and handles errors as follows.
-	// hidden, will return a GaugeMetric that is NOT a noop along
+	// An error that passes ErrIsNotRegistered is ignored
-	// with nil error.  If called on a hidden vector then it will
+	// and the noop gauge is returned;
-	// return a noop and a nil error.  Otherwise it returns a noop
+	// all other errors cause a panic.
-	// and an error that passes ErrIsNotRegistered.
+	WithLabelValues(labelValues ...string) GaugeMetric
-	WithLabelValues(labelValues ...string) (GaugeMetric, error)
 
-	// With, if called after this vector has been
+	// WithChecked, if called on a hidden vector,
-	// registered in at least one registry and this vector is not
+	// will return a noop gauge and a nil error.
-	// hidden, will return a GaugeMetric that is NOT a noop along
+	// If called before this vector has been registered in
-	// with nil error.  If called on a hidden vector then it will
+	// at least one registry, will return a noop gauge and
-	// return a noop and a nil error.  Otherwise it returns a noop
+	// an error that passes ErrIsNotRegistered.
-	// and an error that passes ErrIsNotRegistered.
+	// If called with a syntactic problem in the labels, will
-	With(labels map[string]string) (GaugeMetric, error)
+	// return a noop gauge and an error about the labels.
+	// If none of the above apply, this method will return
+	// the appropriate vector member and a nil error.
+	WithChecked(labels map[string]string) (GaugeMetric, error)
+
+	// With calls WithChecked and handles errors as follows.
+	// An error that passes ErrIsNotRegistered is ignored
+	// and the noop gauge is returned;
+	// all other errors cause a panic.
+	With(labels map[string]string) GaugeMetric
 
 	// Delete asserts that the vec should have no member for the given label set.
 	// The returned bool indicates whether there was a change.