Merge pull request kubernetes#85444 from RainbowMango/pr_fix_metrics_cannot_enable_issue

Provided a mechanism to re-register hidden metrics

Author: k8s-ci-robot

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

@@ -143,6 +143,19 @@ func (r *lazyMetric) Create(version *semver.Version) bool {
 	return r.IsCreated()
 }
 
+// ClearState will clear all the states marked by Create.
+// It intends to be used for re-register a hidden metric.
+func (r *lazyMetric) ClearState() {
+	r.createLock.Lock()
+	defer r.createLock.Unlock()
+
+	r.isDeprecated = false
+	r.isHidden = false
+	r.isCreated = false
+	r.markDeprecationOnce = *(new(sync.Once))
+	r.createOnce = *(new(sync.Once))
+}
+
 /*
 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.

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

@@ -32,6 +32,8 @@ import (
 var (
 	showHiddenOnce sync.Once
 	showHidden     atomic.Value
+	registries     []*kubeRegistry // stores all registries created by NewKubeRegistry()
+	registriesLock sync.RWMutex
 )
 
 // shouldHide be used to check if a specific metric with deprecated version should be hidden
@@ -77,6 +79,11 @@ func ValidateShowHiddenMetricsVersion(v string) []error {
 func SetShowHidden() {
 	showHiddenOnce.Do(func() {
 		showHidden.Store(true)
+
+		// re-register collectors that has been hidden in phase of last registry.
+		for _, r := range registries {
+			r.enableHiddenCollectors()
+		}
 	})
 }
 
@@ -91,7 +98,12 @@ func ShouldShowHidden() bool {
 // will register with KubeRegistry.
 type Registerable interface {
 	prometheus.Collector
+
+	// Create will mark deprecated state for the collector
 	Create(version *semver.Version) bool
+
+	// ClearState will clear all the states marked by Create.
+	ClearState()
 }
 
 // KubeRegistry is an interface which implements a subset of prometheus.Registerer and
@@ -114,7 +126,9 @@ type KubeRegistry interface {
 // automatic behavior can be configured for metric versioning.
 type kubeRegistry struct {
 	PromRegistry
-	version semver.Version
+	version              semver.Version
+	hiddenCollectors     []Registerable // stores all collectors that has been hidden
+	hiddenCollectorsLock sync.RWMutex
 }
 
 // Register registers a new Collector to be included in metrics
@@ -126,6 +140,9 @@ func (kr *kubeRegistry) Register(c Registerable) error {
 	if c.Create(&kr.version) {
 		return kr.PromRegistry.Register(c)
 	}
+
+	kr.trackHiddenCollector(c)
+
 	return nil
 }
 
@@ -137,6 +154,8 @@ func (kr *kubeRegistry) MustRegister(cs ...Registerable) {
 	for _, c := range cs {
 		if c.Create(&kr.version) {
 			metrics = append(metrics, c)
+		} else {
+			kr.trackHiddenCollector(c)
 		}
 	}
 	kr.PromRegistry.MustRegister(metrics...)
@@ -204,17 +223,43 @@ func (kr *kubeRegistry) Gather() ([]*dto.MetricFamily, error) {
 	return kr.PromRegistry.Gather()
 }
 
+// trackHiddenCollector stores all hidden collectors.
+func (kr *kubeRegistry) trackHiddenCollector(c Registerable) {
+	kr.hiddenCollectorsLock.Lock()
+	defer kr.hiddenCollectorsLock.Unlock()
+
+	kr.hiddenCollectors = append(kr.hiddenCollectors, c)
+}
+
+// enableHiddenCollectors will re-register all of the hidden collectors.
+func (kr *kubeRegistry) enableHiddenCollectors() {
+	kr.hiddenCollectorsLock.Lock()
+	defer kr.hiddenCollectorsLock.Unlock()
+
+	for _, c := range kr.hiddenCollectors {
+		c.ClearState()
+		kr.MustRegister(c)
+	}
+	kr.hiddenCollectors = nil
+}
+
 func newKubeRegistry(v apimachineryversion.Info) *kubeRegistry {
 	r := &kubeRegistry{
 		PromRegistry: prometheus.NewRegistry(),
 		version:      parseVersion(v),
 	}
+
+	registriesLock.Lock()
+	defer registriesLock.Unlock()
+	registries = append(registries, r)
+
 	return r
 }
 
 // NewKubeRegistry creates a new vanilla Registry without any Collectors
 // pre-registered.
 func NewKubeRegistry() KubeRegistry {
 	r := newKubeRegistry(version.Get())
+
 	return r
 }

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

@@ -17,10 +17,13 @@ limitations under the License.
 package metrics
 
 import (
+	"strings"
+	"sync"
 	"testing"
 
 	"github.com/blang/semver"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/stretchr/testify/assert"
 
 	apimachineryversion "k8s.io/apimachinery/pkg/version"
@@ -204,12 +207,6 @@ func TestMustRegister(t *testing.T) {
 			registryVersion: &v115,
 			expectedPanics:  []bool{false},
 		},
-		{
-			desc:            "test must registering same hidden metric",
-			metrics:         []*Counter{alphaHiddenCounter, alphaHiddenCounter},
-			registryVersion: &v115,
-			expectedPanics:  []bool{false, false}, // hidden metrics no-opt
-		},
 	}
 
 	for _, test := range tests {
@@ -317,3 +314,80 @@ func TestValidateShowHiddenMetricsVersion(t *testing.T) {
 		})
 	}
 }
+
+func TestEnableHiddenMetrics(t *testing.T) {
+	currentVersion := apimachineryversion.Info{
+		Major:      "1",
+		Minor:      "17",
+		GitVersion: "v1.17.1-alpha-1.12345",
+	}
+
+	var tests = []struct {
+		name           string
+		fqName         string
+		counter        *Counter
+		mustRegister   bool
+		expectedMetric string
+	}{
+		{
+			name:   "hide by register",
+			fqName: "hidden_metric_register",
+			counter: NewCounter(&CounterOpts{
+				Name:              "hidden_metric_register",
+				Help:              "counter help",
+				StabilityLevel:    STABLE,
+				DeprecatedVersion: "1.16.0",
+			}),
+			mustRegister: false,
+			expectedMetric: `
+				# HELP hidden_metric_register [STABLE] (Deprecated since 1.16.0) counter help
+				# TYPE hidden_metric_register counter
+				hidden_metric_register 1
+				`,
+		},
+		{
+			name:   "hide by must register",
+			fqName: "hidden_metric_must_register",
+			counter: NewCounter(&CounterOpts{
+				Name:              "hidden_metric_must_register",
+				Help:              "counter help",
+				StabilityLevel:    STABLE,
+				DeprecatedVersion: "1.16.0",
+			}),
+			mustRegister: true,
+			expectedMetric: `
+				# HELP hidden_metric_must_register [STABLE] (Deprecated since 1.16.0) counter help
+				# TYPE hidden_metric_must_register counter
+				hidden_metric_must_register 1
+				`,
+		},
+	}
+
+	for _, test := range tests {
+		tc := test
+		t.Run(tc.name, func(t *testing.T) {
+			registry := newKubeRegistry(currentVersion)
+			if tc.mustRegister {
+				registry.MustRegister(tc.counter)
+			} else {
+				_ = registry.Register(tc.counter)
+			}
+
+			tc.counter.Inc() // no-ops, because counter hasn't been initialized
+			if err := testutil.GatherAndCompare(registry, strings.NewReader(""), tc.fqName); err != nil {
+				t.Fatal(err)
+			}
+
+			SetShowHidden()
+			defer func() {
+				showHiddenOnce = *new(sync.Once)
+				showHidden.Store(false)
+			}()
+
+			tc.counter.Inc()
+			if err := testutil.GatherAndCompare(registry, strings.NewReader(tc.expectedMetric), tc.fqName); err != nil {
+				t.Fatal(err)
+			}
+		})
+	}
+}