From 10865d994445c31e19c2aeb6b946af3b41a2eb4a Mon Sep 17 00:00:00 2001 From: RainbowMango Date: Tue, 19 Nov 2019 10:30:02 +0800 Subject: [PATCH] Provided a mechanism to re-register hidden metrics. --- .../k8s.io/component-base/metrics/metric.go | 13 +++ .../k8s.io/component-base/metrics/registry.go | 47 +++++++++- .../component-base/metrics/registry_test.go | 86 +++++++++++++++++-- 3 files changed, 139 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/component-base/metrics/metric.go b/staging/src/k8s.io/component-base/metrics/metric.go index 10398e396b6..015627b5990 100644 --- a/staging/src/k8s.io/component-base/metrics/metric.go +++ b/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. diff --git a/staging/src/k8s.io/component-base/metrics/registry.go b/staging/src/k8s.io/component-base/metrics/registry.go index fa1854bd69c..ed018332c73 100644 --- a/staging/src/k8s.io/component-base/metrics/registry.go +++ b/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,11 +223,36 @@ 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 } @@ -216,5 +260,6 @@ func newKubeRegistry(v apimachineryversion.Info) *kubeRegistry { // pre-registered. func NewKubeRegistry() KubeRegistry { r := newKubeRegistry(version.Get()) + return r } diff --git a/staging/src/k8s.io/component-base/metrics/registry_test.go b/staging/src/k8s.io/component-base/metrics/registry_test.go index 2e232949799..9058b295bde 100644 --- a/staging/src/k8s.io/component-base/metrics/registry_test.go +++ b/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) + } + }) + } +}