Finish making Gauge and TimingHistogram implement the same interface

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

@@ -18,6 +18,7 @@ package metrics
 
 import (
 	"context"
+
 	"github.com/blang/semver/v4"
 	"github.com/prometheus/client_golang/prometheus"
 	dto "github.com/prometheus/client_model/go"
@@ -106,9 +107,14 @@ type CounterVec struct {
 	originalLabels []string
 }
 
-// NewCounterVec returns an object which satisfies the kubeCollector and CounterVecMetric interfaces.
+var _ kubeCollector = &CounterVec{}
+
+// TODO: make this true: var _ CounterVecMetric = &CounterVec{}
+
+// NewCounterVec returns an object which satisfies the kubeCollector and (almost) CounterVecMetric interfaces.
 // However, the object returned will not measure anything unless the collector is first
-// registered, since the metric is lazily instantiated.
+// registered, since the metric is lazily instantiated, and only members extracted after
+// registration will actually measure anything.
 func NewCounterVec(opts *CounterOpts, labels []string) *CounterVec {
 	opts.StabilityLevel.setDefaults()
 
@@ -149,13 +155,16 @@ func (v *CounterVec) initializeDeprecatedMetric() {
 	v.initializeMetric()
 }
 
-// Default Prometheus behavior actually results in the creation of a new metric
+// Default Prometheus Vec behavior is that member extraction results in creation of a new element
-// if a metric with the unique label values is not found in the underlying stored metricMap.
+// if one with the unique label values is not found in the underlying stored metricMap.
 // This means  that if this function is called but the underlying metric is not registered
 // (which means it will never be exposed externally nor consumed), the metric will exist in memory
 // for perpetuity (i.e. throughout application lifecycle).
 //
 // For reference: https://github.com/prometheus/client_golang/blob/v0.9.2/prometheus/counter.go#L179-L197
+//
+// In contrast, the Vec behavior in this package is that member extraction before registration
+// returns a permanent noop object.
 
 // WithLabelValues returns the Counter for the given slice of label
 // values (same order as the VariableLabels in Desc). If that combination of

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

@@ -18,6 +18,7 @@ package metrics
 
 import (
 	"context"
+
 	"github.com/blang/semver/v4"
 	"github.com/prometheus/client_golang/prometheus"
 
@@ -33,7 +34,11 @@ type Gauge struct {
 	selfCollector
 }
 
-// NewGauge returns an object which satisfies the kubeCollector and KubeGauge interfaces.
+var _ GaugeMetric = &Gauge{}
+var _ Registerable = &Gauge{}
+var _ kubeCollector = &Gauge{}
+
+// NewGauge returns an object which satisfies the kubeCollector, Registerable, and Gauge interfaces.
 // However, the object returned will not measure anything unless the collector is first
 // registered, since the metric is lazily instantiated.
 func NewGauge(opts *GaugeOpts) *Gauge {
@@ -88,9 +93,14 @@ type GaugeVec struct {
 	originalLabels []string
 }
 
-// NewGaugeVec returns an object which satisfies the kubeCollector and KubeGaugeVec interfaces.
+var _ GaugeVecMetric = &GaugeVec{}
+var _ Registerable = &GaugeVec{}
+var _ kubeCollector = &GaugeVec{}
+
+// NewGaugeVec returns an object which satisfies the kubeCollector, Registerable, and GaugeVecMetric interfaces.
 // However, the object returned will not measure anything unless the collector is first
-// registered, since the metric is lazily instantiated.
+// registered, since the metric is lazily instantiated, and only members extracted after
+// registration will actually measure anything.
 func NewGaugeVec(opts *GaugeOpts, labels []string) *GaugeVec {
 	opts.StabilityLevel.setDefaults()
 
@@ -130,26 +140,55 @@ func (v *GaugeVec) initializeDeprecatedMetric() {
 	v.initializeMetric()
 }
 
-// Default Prometheus behavior actually results in the creation of a new metric
+func (v *GaugeVec) WithLabelValuesChecked(lvs ...string) (GaugeMetric, error) {
-// if a metric with the unique label values is not found in the underlying stored metricMap.
+	if v.IsHidden() {
+		return noop, nil
+	}
+	if !v.IsCreated() {
+		return noop, errNotRegistered // return no-op gauge
+	}
+	if v.LabelValueAllowLists != nil {
+		v.LabelValueAllowLists.ConstrainToAllowedList(v.originalLabels, lvs)
+	}
+	elt, err := v.GaugeVec.GetMetricWithLabelValues(lvs...)
+	return elt, err
+}
+
+// Default Prometheus Vec behavior is that member extraction results in creation of a new element
+// if one with the unique label values is not found in the underlying stored metricMap.
 // This means  that if this function is called but the underlying metric is not registered
 // (which means it will never be exposed externally nor consumed), the metric will exist in memory
 // for perpetuity (i.e. throughout application lifecycle).
 //
 // For reference: https://github.com/prometheus/client_golang/blob/v0.9.2/prometheus/gauge.go#L190-L208
+//
+// In contrast, the Vec behavior in this package is that member extraction before registration
+// returns a permanent noop object.
 
 // WithLabelValues returns the GaugeMetric for the given slice of label
 // values (same order as the VariableLabels in Desc). If that combination of
 // label values is accessed for the first time, a new GaugeMetric is created IFF the gaugeVec
 // has been registered to a metrics registry.
 func (v *GaugeVec) WithLabelValues(lvs ...string) GaugeMetric {
+	ans, err := v.WithLabelValuesChecked(lvs...)
+	if err == nil || ErrIsNotRegistered(err) {
+		return ans
+	}
+	panic(err)
+}
+
+func (v *GaugeVec) WithChecked(labels map[string]string) (GaugeMetric, error) {
+	if v.IsHidden() {
+		return noop, nil
+	}
 	if !v.IsCreated() {
-		return noop // return no-op gauge
+		return noop, errNotRegistered // return no-op gauge
 	}
 	if v.LabelValueAllowLists != nil {
-		v.LabelValueAllowLists.ConstrainToAllowedList(v.originalLabels, lvs)
+		v.LabelValueAllowLists.ConstrainLabelMap(labels)
 	}
-	return v.GaugeVec.WithLabelValues(lvs...)
+	elt, err := v.GaugeVec.GetMetricWith(labels)
+	return elt, err
 }
 
 // With returns the GaugeMetric for the given Labels map (the label names
@@ -157,13 +196,11 @@ func (v *GaugeVec) WithLabelValues(lvs ...string) GaugeMetric {
 // accessed for the first time, a new GaugeMetric is created IFF the gaugeVec has
 // been registered to a metrics registry.
 func (v *GaugeVec) With(labels map[string]string) GaugeMetric {
-	if !v.IsCreated() {
+	ans, err := v.WithChecked(labels)
-		return noop // return no-op gauge
+	if err == nil || ErrIsNotRegistered(err) {
+		return ans
 	}
-	if v.LabelValueAllowLists != nil {
+	panic(err)
-		v.LabelValueAllowLists.ConstrainLabelMap(labels)
-	}
-	return v.GaugeVec.With(labels)
 }
 
 // Delete deletes the metric where the variable labels are the same as those
@@ -219,6 +256,10 @@ func (v *GaugeVec) WithContext(ctx context.Context) *GaugeVecWithContext {
 	}
 }
 
+func (v *GaugeVec) InterfaceWithContext(ctx context.Context) GaugeVecMetric {
+	return v.WithContext(ctx)
+}
+
 // GaugeVecWithContext is the wrapper of GaugeVec with context.
 type GaugeVecWithContext struct {
 	*GaugeVec

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

@@ -101,7 +101,10 @@ type HistogramVec struct {
 
 // NewHistogramVec returns an object which satisfies kubeCollector and wraps the
 // prometheus.HistogramVec object. However, the object returned will not measure
-// anything unless the collector is first registered, since the metric is lazily instantiated.
+// anything unless the collector is first registered, since the metric is lazily instantiated,
+// and only members extracted after
+// registration will actually measure anything.
+
 func NewHistogramVec(opts *HistogramOpts, labels []string) *HistogramVec {
 	opts.StabilityLevel.setDefaults()
 
@@ -137,13 +140,16 @@ func (v *HistogramVec) initializeDeprecatedMetric() {
 	v.initializeMetric()
 }
 
-// Default Prometheus behavior actually results in the creation of a new metric
+// Default Prometheus Vec behavior is that member extraction results in creation of a new element
-// if a metric with the unique label values is not found in the underlying stored metricMap.
+// if one with the unique label values is not found in the underlying stored metricMap.
 // This means  that if this function is called but the underlying metric is not registered
 // (which means it will never be exposed externally nor consumed), the metric will exist in memory
 // for perpetuity (i.e. throughout application lifecycle).
 //
 // For reference: https://github.com/prometheus/client_golang/blob/v0.9.2/prometheus/histogram.go#L460-L470
+//
+// In contrast, the Vec behavior in this package is that member extraction before registration
+// returns a permanent noop object.
 
 // WithLabelValues returns the ObserverMetric for the given slice of label
 // values (same order as the VariableLabels in Desc). If that combination of

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

@@ -18,6 +18,7 @@ package metrics
 
 import (
 	"context"
+
 	"github.com/blang/semver/v4"
 	"github.com/prometheus/client_golang/prometheus"
 )
@@ -93,7 +94,9 @@ type SummaryVec struct {
 
 // NewSummaryVec returns an object which satisfies kubeCollector and wraps the
 // prometheus.SummaryVec object. However, the object returned will not measure
-// anything unless the collector is first registered, since the metric is lazily instantiated.
+// anything unless the collector is first registered, since the metric is lazily instantiated,
+// and only members extracted after
+// registration will actually measure anything.
 //
 // DEPRECATED: as per the metrics overhaul KEP
 func NewSummaryVec(opts *SummaryOpts, labels []string) *SummaryVec {
@@ -130,13 +133,16 @@ func (v *SummaryVec) initializeDeprecatedMetric() {
 	v.initializeMetric()
 }
 
-// Default Prometheus behavior actually results in the creation of a new metric
+// Default Prometheus Vec behavior is that member extraction results in creation of a new element
-// if a metric with the unique label values is not found in the underlying stored metricMap.
+// if one with the unique label values is not found in the underlying stored metricMap.
 // This means  that if this function is called but the underlying metric is not registered
 // (which means it will never be exposed externally nor consumed), the metric will exist in memory
 // for perpetuity (i.e. throughout application lifecycle).
 //
-// For reference: https://github.com/prometheus/client_golang/blob/v0.9.2/prometheus/summary.go#L485-L495
+// For reference: https://github.com/prometheus/client_golang/blob/v0.9.2/prometheus/histogram.go#L460-L470
+//
+// In contrast, the Vec behavior in this package is that member extraction before registration
+// returns a permanent noop object.
 
 // WithLabelValues returns the ObserverMetric for the given slice of label
 // values (same order as the VariableLabels in Desc). If that combination of

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

@@ -34,6 +34,10 @@ type TimingHistogram struct {
 	selfCollector
 }
 
+var _ GaugeMetric = &TimingHistogram{}
+var _ Registerable = &TimingHistogram{}
+var _ kubeCollector = &TimingHistogram{}
+
 // NewTimingHistogram returns an object which is TimingHistogram-like. However, nothing
 // will be measured until the histogram is registered somewhere.
 func NewTimingHistogram(opts *TimingHistogramOpts) *TimingHistogram {
@@ -89,9 +93,9 @@ func (h *TimingHistogram) WithContext(ctx context.Context) GaugeMetric {
 	return h.GaugeMetric
 }
 
-// timingHistogramVec is the internal representation of our wrapping struct around prometheus
+// TimingHistogramVec is the internal representation of our wrapping struct around prometheus
 // TimingHistogramVecs.
-type timingHistogramVec struct {
+type TimingHistogramVec struct {
 	*promext.TimingHistogramVec
 	*TimingHistogramOpts
 	nowFunc func() time.Time
@@ -99,15 +103,19 @@ type timingHistogramVec struct {
 	originalLabels []string
 }
 
-// NewTimingHistogramVec returns an object which satisfies kubeCollector and
+var _ GaugeVecMetric = &TimingHistogramVec{}
-// wraps the promext.timingHistogramVec object.  Note well the way that
+var _ Registerable = &TimingHistogramVec{}
+var _ kubeCollector = &TimingHistogramVec{}
+
+// NewTimingHistogramVec returns an object which satisfies the kubeCollector, Registerable, and GaugeVecMetric interfaces
+// and wraps an underlying promext.TimingHistogramVec object.  Note well the way that
 // behavior depends on registration and whether this is hidden.
-func NewTimingHistogramVec(opts *TimingHistogramOpts, labels []string) PreContextAndRegisterableGaugeMetricVec {
+func NewTimingHistogramVec(opts *TimingHistogramOpts, labels []string) *TimingHistogramVec {
 	return NewTestableTimingHistogramVec(time.Now, opts, labels)
 }
 
 // NewTestableTimingHistogramVec adds injection of the clock.
-func NewTestableTimingHistogramVec(nowFunc func() time.Time, opts *TimingHistogramOpts, labels []string) PreContextAndRegisterableGaugeMetricVec {
+func NewTestableTimingHistogramVec(nowFunc func() time.Time, opts *TimingHistogramOpts, labels []string) *TimingHistogramVec {
 	opts.StabilityLevel.setDefaults()
 
 	fqName := BuildFQName(opts.Namespace, opts.Subsystem, opts.Name)
@@ -117,7 +125,7 @@ func NewTestableTimingHistogramVec(nowFunc func() time.Time, opts *TimingHistogr
 	}
 	allowListLock.RUnlock()
 
-	v := &timingHistogramVec{
+	v := &TimingHistogramVec{
 		TimingHistogramVec:  noopTimingHistogramVec,
 		TimingHistogramOpts: opts,
 		nowFunc:             nowFunc,
@@ -129,16 +137,16 @@ func NewTestableTimingHistogramVec(nowFunc func() time.Time, opts *TimingHistogr
 }
 
 // DeprecatedVersion returns a pointer to the Version or nil
-func (v *timingHistogramVec) DeprecatedVersion() *semver.Version {
+func (v *TimingHistogramVec) DeprecatedVersion() *semver.Version {
 	return parseSemver(v.TimingHistogramOpts.DeprecatedVersion)
 }
 
-func (v *timingHistogramVec) initializeMetric() {
+func (v *TimingHistogramVec) initializeMetric() {
 	v.TimingHistogramOpts.annotateStabilityLevel()
 	v.TimingHistogramVec = promext.NewTestableTimingHistogramVec(v.nowFunc, v.TimingHistogramOpts.toPromHistogramOpts(), v.originalLabels...)
 }
 
-func (v *timingHistogramVec) initializeDeprecatedMetric() {
+func (v *TimingHistogramVec) initializeDeprecatedMetric() {
 	v.TimingHistogramOpts.markDeprecated()
 	v.initializeMetric()
 }
@@ -152,7 +160,7 @@ func (v *timingHistogramVec) initializeDeprecatedMetric() {
 // 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) {
+func (v *TimingHistogramVec) WithLabelValuesChecked(lvs ...string) (GaugeMetric, error) {
 	if v.IsHidden() {
 		return noop, nil
 	}
@@ -171,7 +179,7 @@ func (v *timingHistogramVec) WithLabelValuesChecked(lvs ...string) (GaugeMetric,
 // 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 {
+func (v *TimingHistogramVec) WithLabelValues(lvs ...string) GaugeMetric {
 	ans, err := v.WithLabelValuesChecked(lvs...)
 	if err == nil || ErrIsNotRegistered(err) {
 		return ans
@@ -188,7 +196,7 @@ func (v *timingHistogramVec) WithLabelValues(lvs ...string) GaugeMetric {
 // 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) {
+func (v *TimingHistogramVec) WithChecked(labels map[string]string) (GaugeMetric, error) {
 	if v.IsHidden() {
 		return noop, nil
 	}
@@ -206,7 +214,7 @@ func (v *timingHistogramVec) WithChecked(labels map[string]string) (GaugeMetric,
 // 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 {
+func (v *TimingHistogramVec) With(labels map[string]string) GaugeMetric {
 	ans, err := v.WithChecked(labels)
 	if err == nil || ErrIsNotRegistered(err) {
 		return ans
@@ -221,7 +229,7 @@ func (v *timingHistogramVec) With(labels map[string]string) GaugeMetric {
 // with those of the VariableLabels in Desc. However, such inconsistent Labels
 // can never match an actual metric, so the method will always return false in
 // that case.
-func (v *timingHistogramVec) Delete(labels map[string]string) bool {
+func (v *TimingHistogramVec) Delete(labels map[string]string) bool {
 	if !v.IsCreated() {
 		return false // since we haven't created the metric, we haven't deleted a metric with the passed in values
 	}
@@ -229,7 +237,7 @@ func (v *timingHistogramVec) Delete(labels map[string]string) bool {
 }
 
 // Reset deletes all metrics in this vector.
-func (v *timingHistogramVec) Reset() {
+func (v *TimingHistogramVec) Reset() {
 	if !v.IsCreated() {
 		return
 	}
@@ -237,17 +245,17 @@ func (v *timingHistogramVec) Reset() {
 	v.TimingHistogramVec.Reset()
 }
 
-// WithContext returns wrapped timingHistogramVec with context
+// WithContext returns wrapped TimingHistogramVec with context
-func (v *timingHistogramVec) WithContext(ctx context.Context) GaugeMetricVec {
+func (v *TimingHistogramVec) InterfaceWithContext(ctx context.Context) GaugeVecMetric {
 	return &TimingHistogramVecWithContext{
 		ctx:                ctx,
-		timingHistogramVec: v,
+		TimingHistogramVec: v,
 	}
 }
 
-// TimingHistogramVecWithContext is the wrapper of timingHistogramVec with context.
+// TimingHistogramVecWithContext is the wrapper of TimingHistogramVec with context.
 // Currently the context is ignored.
 type TimingHistogramVecWithContext struct {
-	*timingHistogramVec
+	*TimingHistogramVec
 	ctx context.Context
 }

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

@@ -17,7 +17,6 @@ limitations under the License.
 package metrics
 
 import (
-	"context"
 	"errors"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -68,12 +67,18 @@ type GaugeMetric interface {
 	SetToCurrentTime()
 }
 
-// GaugeMetricVec is a collection of Gauges that differ only in label values.
+// GaugeVecMetric is a collection of Gauges that differ only in label values.
-// This is really just one Metric.
+type GaugeVecMetric interface {
-// It might be better called GaugeVecMetric, but that pattern of name is already
+	// Default Prometheus Vec behavior is that member extraction results in creation of a new element
-// taken by the other pattern --- which is treacherous.  The treachery is that
+	// if one with the unique label values is not found in the underlying stored metricMap.
-// WithLabelValues can return an object that is permanently broken (i.e., a noop).
+	// This means  that if this function is called but the underlying metric is not registered
-type GaugeMetricVec interface {
+	// (which means it will never be exposed externally nor consumed), the metric would exist in memory
+	// for perpetuity (i.e. throughout application lifecycle).
+	//
+	// For reference: https://github.com/prometheus/client_golang/blob/v0.9.2/prometheus/gauge.go#L190-L208
+	//
+	// In contrast, the Vec behavior in this package is that member extraction before registration
+	// returns a permanent noop object.
 
 	// WithLabelValuesChecked, if called on a hidden vector,
 	// will return a noop gauge and a nil error.
@@ -120,26 +125,6 @@ type GaugeMetricVec interface {
 	Reset()
 }
 
-// PreContextGaugeMetricVec is something that can construct a GaugeMetricVec
-// that uses a given Context.
-type PreContextGaugeMetricVec interface {
-	// WithContext creates a GaugeMetricVec that uses the given Context
-	WithContext(ctx context.Context) GaugeMetricVec
-}
-
-// RegisterableGaugeMetricVec is the intersection of Registerable and GaugeMetricVec
-type RegisterableGaugeMetricVec interface {
-	Registerable
-	GaugeMetricVec
-}
-
-// PreContextAndRegisterableGaugeMetricVec is the intersection of
-// PreContextGaugeMetricVec and RegisterableGaugeMetricVec
-type PreContextAndRegisterableGaugeMetricVec interface {
-	PreContextGaugeMetricVec
-	RegisterableGaugeMetricVec
-}
-
 // ObserverMetric captures individual observations.
 type ObserverMetric interface {
 	Observe(float64)