diff --git a/staging/src/k8s.io/component-base/metrics/timing_histogram.go b/staging/src/k8s.io/component-base/metrics/timing_histogram.go index ab05cc03a31..6d790248810 100644 --- a/staging/src/k8s.io/component-base/metrics/timing_histogram.go +++ b/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) { - gm, _ := v.WithLabelValues(labelValues...) - gm.Set(value) -} - -func (v *timingHistogramVec) Inc(labelValues ...string) { - gm, _ := v.WithLabelValues(labelValues...) - gm.Inc() -} - -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) { +// WithLabelValuesChecked, 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) WithLabelValuesChecked(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 -// 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) With(labels map[string]string) (GaugeMetric, error) { +// WithLabelValues calls WithLabelValuesChecked +// 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) WithLabelValues(lvs ...string) GaugeMetric { + 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 diff --git a/staging/src/k8s.io/component-base/metrics/timing_histogram_test.go b/staging/src/k8s.io/component-base/metrics/timing_histogram_test.go index 43b5a1489ac..b397ca44e34 100644 --- a/staging/src/k8s.io/component-base/metrics/timing_histogram_test.go +++ b/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.Set(v0, "2", "3") + c.WithLabelValues("1", "3").Set(v0) + c.WithLabelValues("2", "3").Set(v0) dt1 := time.Nanosecond t1 := t0.Add(dt1) clk.SetTime(t1) - c.Add(5.0, "1", "2") - c.Add(5.0, "1", "3") - c.Add(5.0, "2", "3") + c.WithLabelValues("1", "2").Add(5.0) + c.WithLabelValues("1", "3").Add(5.0) + 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 + } +} diff --git a/staging/src/k8s.io/component-base/metrics/wrappers.go b/staging/src/k8s.io/component-base/metrics/wrappers.go index 840a18c48e1..9ca4c8cb217 100644 --- a/staging/src/k8s.io/component-base/metrics/wrappers.go +++ b/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) - IncForLabels(labels map[string]string) - DecForLabels(labels map[string]string) - AddForLabels(delta float64, labels map[string]string) - SetToCurrentTimeForLabels(labels map[string]string) + // WithLabelValuesChecked, 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. + WithLabelValuesChecked(labelValues ...string) (GaugeMetric, error) - // 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. - WithLabelValues(labelValues ...string) (GaugeMetric, error) + // WithLabelValues calls WithLabelValuesChecked + // and handles errors as follows. + // An error that passes ErrIsNotRegistered is ignored + // and the noop gauge is returned; + // all other errors cause a panic. + WithLabelValues(labelValues ...string) GaugeMetric - // With, 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. - With(labels map[string]string) (GaugeMetric, error) + // 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. + 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.