make method names more succinct, improve documentation for posterity

pkg/util/metrics/counter.go

@@ -53,13 +53,13 @@ func (c *kubeCounter) setPrometheusCounter(counter prometheus.Counter) {
 	c.initSelfCollection(counter)
 }
 
-// GetDeprecatedVersion returns a pointer to the Version or nil
+// DeprecatedVersion returns a pointer to the Version or nil
-func (c *kubeCounter) GetDeprecatedVersion() *semver.Version {
+func (c *kubeCounter) DeprecatedVersion() *semver.Version {
 	return c.CounterOpts.DeprecatedVersion
 }
 
 // initializeMetric invocation creates the actual underlying Counter. Until this method is called
-// our underlying counter is a no-op.
+// the underlying counter is a no-op.
 func (c *kubeCounter) initializeMetric() {
 	c.CounterOpts.annotateStabilityLevel()
 	// this actually creates the underlying prometheus counter.
@@ -67,13 +67,13 @@ func (c *kubeCounter) initializeMetric() {
 }
 
 // initializeDeprecatedMetric invocation creates the actual (but deprecated) Counter. Until this method
-// is called our underlying counter is a no-op.
+// is called the underlying counter is a no-op.
 func (c *kubeCounter) initializeDeprecatedMetric() {
 	c.CounterOpts.markDeprecated()
 	c.initializeMetric()
 }
 
-// kubeCounterVec is our internal representation of our wrapping struct around prometheus
+// kubeCounterVec is the internal representation of our wrapping struct around prometheus
 // counterVecs. kubeCounterVec implements both KubeCollector and KubeCounterVec.
 type kubeCounterVec struct {
 	*prometheus.CounterVec
@@ -96,30 +96,34 @@ func NewCounterVec(opts CounterOpts, labels []string) *kubeCounterVec {
 	return cv
 }
 
-// GetDeprecatedVersion returns a pointer to the Version or nil
+// DeprecatedVersion returns a pointer to the Version or nil
-func (v *kubeCounterVec) GetDeprecatedVersion() *semver.Version {
+func (v *kubeCounterVec) DeprecatedVersion() *semver.Version {
 	return v.CounterOpts.DeprecatedVersion
 }
 
 // initializeMetric invocation creates the actual underlying CounterVec. Until this method is called
-// our underlying counterVec is a no-op.
+// the underlying counterVec is a no-op.
 func (v *kubeCounterVec) initializeMetric() {
 	v.CounterVec = prometheus.NewCounterVec(v.CounterOpts.toPromCounterOpts(), v.originalLabels)
 }
 
-// initializeMetric invocation creates the actual (but deprecated) CounterVec. Until this method is called
+// initializeDeprecatedMetric invocation creates the actual (but deprecated) CounterVec. Until this method is called
-// our underlying counterVec is a no-op.
+// the underlying counterVec is a no-op.
 func (v *kubeCounterVec) initializeDeprecatedMetric() {
 	v.CounterOpts.markDeprecated()
 	v.initializeMetric()
 }
 
 // Default Prometheus behavior actually results in the creation of a new metric
-// if a metric with the unique label values is not found in the underlying stored metricMap. This
+// if a metric with the unique label values is not found in the underlying stored metricMap.
-// is undesirable for us, since we want a way to turn OFF metrics which end up turning into memory
+// This means  that if this function is called but the underlying metric is not registered
-// leaks.
+// (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/master/prometheus/counter.go#L148-L177
+// For reference: https://github.com/prometheus/client_golang/blob/v0.9.2/prometheus/counter.go#L179-L197
+//
+// This method returns a no-op metric if the metric is not actually created/registered, avoiding that
+// memory leak.
 func (v *kubeCounterVec) WithLabelValues(lvs ...string) KubeCounter {
 	if !v.IsCreated() {
 		return noop // return no-op counter

pkg/util/metrics/counter_test.go

@@ -101,7 +101,7 @@ func TestCounter(t *testing.T) {
 				}
 			}
 
-			// let's increment the counter N number of times and verify that the metric retains the count correctly
+			// increment the counter N number of times and verify that the metric retains the count correctly
 			numberOfTimesToIncrement := 3
 			for i := 0; i < numberOfTimesToIncrement; i++ {
 				c.Inc()
@@ -188,7 +188,7 @@ func TestCounterVec(t *testing.T) {
 			if err != nil {
 				t.Fatalf("Gather failed %v", err)
 			}
-			// we no-opt here when we don't have any metric families (i.e. when the metric is hidden)
+			// this no-opts here when there are no metric families (i.e. when the metric is hidden)
 			for _, mf := range mfs {
 				if len(mf.GetMetric()) != 1 {
 					t.Errorf("Got %v metrics, wanted 1 as the count", len(mf.GetMetric()))
@@ -206,7 +206,7 @@ func TestCounterVec(t *testing.T) {
 				t.Fatalf("Gather failed %v", err)
 			}
 
-			// we no-opt here when we don't have any metric families (i.e. when the metric is hidden)
+			// this no-opts here when there are no metric families (i.e. when the metric is hidden)
 			for _, mf := range mfs {
 				if len(mf.GetMetric()) != 3 {
 					t.Errorf("Got %v metrics, wanted 3 as the count", len(mf.GetMetric()))

pkg/util/metrics/metric.go

@@ -24,39 +24,41 @@ import (
 	"sync"
 )
 
-/**
+/*
- * This extends the prometheus.Collector interface so that we can customize the metric
+This extends the prometheus.Collector interface to allow customization of the metric
- * registration process. Specifically, we defer metric initialization until ActuallyCreate
+registration process. Defer metric initialization until Create() is called, which then
- * is called, which then delegates to the underlying metric's initializeMetric or
+delegates to the underlying metric's initializeMetric or initializeDeprecatedMetric
- * initializeDeprecatedMetric method call depending on whether the metric is deprecated or not.
+method call depending on whether the metric is deprecated or not.
  */
 type KubeCollector interface {
 	Collector
 	LazyMetric
-	GetDeprecatedVersion() *semver.Version
+	DeprecatedVersion() *semver.Version
 	// Each collector metric should provide an initialization function
 	// for both deprecated and non-deprecated variants of a metric. This
-	// is necessary since we are now deferring metric instantiation
+	// is necessary since metric instantiation will be deferred
 	// until the metric is actually registered somewhere.
 	initializeMetric()
 	initializeDeprecatedMetric()
 }
 
-// LazyMetric defines our registration functionality. We expect LazyMetric
+/*
-// objects to lazily instantiate metrics (i.e defer metric instantiation until when
+LazyMetric defines our registration functionality. LazyMetric objects are expected
-// ActuallyCreate is explicitly called).
+to lazily instantiate metrics (i.e defer metric instantiation until when
+the Create() function is explicitly called).
+ */
 type LazyMetric interface {
-	ActuallyCreate(*semver.Version) bool
+	Create(*semver.Version) bool
 	IsCreated() bool
 	IsHidden() bool
 	IsDeprecated() bool
 }
 
 /*
- * lazyMetric implements LazyMetric. A lazy metric is lazy because it waits until metric
+lazyMetric implements LazyMetric. A lazy metric is lazy because it waits until metric
- * registration time before instantiation. Add it as an anonymous field to a struct that
+registration time before instantiation. Add it as an anonymous field to a struct that
- * implements KubeCollector to get deferred registration behavior. You must call lazyInit
+implements KubeCollector to get deferred registration behavior. You must call lazyInit
- * with the KubeCollector itself as an argument.
+with the KubeCollector itself as an argument.
  */
 type lazyMetric struct {
 	isDeprecated        bool
@@ -78,11 +80,11 @@ func (r *lazyMetric) lazyInit(self KubeCollector) {
 	r.self = self
 }
 
-// determineDeprecationStatus figures out whether our lazy metric should be deprecated or not. It takes
+// determineDeprecationStatus figures out whether the lazy metric should be deprecated or not.
-// a Version argument which should be the version of the binary in which this code is currently being
+// This method takes a Version argument which should be the version of the binary in which
-// executed.
+// this code is currently being executed.
 func (r *lazyMetric) determineDeprecationStatus(version semver.Version) {
-	selfVersion := r.self.GetDeprecatedVersion()
+	selfVersion := r.self.DeprecatedVersion()
 	if selfVersion == nil {
 		return
 	}
@@ -105,13 +107,12 @@ func (r *lazyMetric) IsDeprecated() bool {
 	return r.isDeprecated
 }
 
-// Defer initialization of metric until we know if we actually need to
+// Create forces the initialization of metric which has been deferred until
-// register the thing. This wrapper just allows us to consolidate the
+// the point at which this method is invoked. This method will determine whether
-// syncOnce logic in a single spot and toggle the flag, since this
+// the metric is deprecated or hidden, no-opting if the metric should be considered
-// behavior will be consistent across metrics.
+// hidden. Furthermore, this function no-opts and returns true if metric is already
-//
+// created.
-// This no-opts and returns true if metric is already created.
+func (r *lazyMetric) Create(version *semver.Version) bool {
-func (r *lazyMetric) ActuallyCreate(version *semver.Version) bool {
 	if version != nil {
 		r.determineDeprecationStatus(*version)
 	}
@@ -130,11 +131,11 @@ func (r *lazyMetric) ActuallyCreate(version *semver.Version) bool {
 	return r.IsCreated()
 }
 
-/**
+/*
- * This code is directly lifted from the prometheus codebase. It's a convenience struct which
+This code is directly lifted from the prometheus codebase. It's a convenience struct which
- * allows you satisfy the Collector interface automatically if you already satisfy the Metric interface.
+allows you satisfy the Collector interface automatically if you already satisfy the Metric interface.
- *
+
- * For reference: https://github.com/prometheus/client_golang/blob/65d3a96fbaa7c8c9535d7133d6d98cd50eed4db8/prometheus/collector.go#L98-L120
+For reference: https://github.com/prometheus/client_golang/blob/v0.9.2/prometheus/collector.go#L98-L120
  */
 type selfCollector struct {
 	metric prometheus.Metric

pkg/util/metrics/opts.go

@@ -23,9 +23,9 @@ import (
 	"sync"
 )
 
-// KubeOpts is superset struct for prometheus.Opts. We choose not to embed
+// KubeOpts is superset struct for prometheus.Opts. The prometheus Opts structure
-// the prometheus Opts structure here because that would change struct initialization
+// is purposefully not embedded here because that would change struct initialization
-// in the manner to which people are currently accustomed.
+// in the manner which people are currently accustomed.
 //
 // Name must be set to a non-empty string. DeprecatedVersion is defined only
 // if the metric for which this options applies is, in fact, deprecated.

pkg/util/metrics/registry.go

@@ -45,7 +45,7 @@ func MustRegister(cs ...KubeCollector) {
 }
 
 func (kr *KubeRegistry) Register(c KubeCollector) error {
-	if c.ActuallyCreate(&kr.version) {
+	if c.Create(&kr.version) {
 		return kr.PromRegistry.Register(c)
 	}
 	return nil
@@ -54,7 +54,7 @@ func (kr *KubeRegistry) Register(c KubeCollector) error {
 func (kr *KubeRegistry) MustRegister(cs ...KubeCollector) {
 	metrics := make([]prometheus.Collector, 0, len(cs))
 	for _, c := range cs {
-		if c.ActuallyCreate(&kr.version) {
+		if c.Create(&kr.version) {
 			metrics = append(metrics, c)
 		}
 	}