Merge pull request #85444 from RainbowMango/pr_fix_metrics_cannot_enable_issue

Provided a mechanism to re-register hidden metrics
This commit is contained in:
Kubernetes Prow Robot 2019-11-25 20:27:10 -08:00 committed by GitHub
commit 5d3125e512
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 139 additions and 7 deletions

View File

@ -143,6 +143,19 @@ func (r *lazyMetric) Create(version *semver.Version) bool {
return r.IsCreated() 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 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.

View File

@ -32,6 +32,8 @@ import (
var ( var (
showHiddenOnce sync.Once showHiddenOnce sync.Once
showHidden atomic.Value 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 // 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() { func SetShowHidden() {
showHiddenOnce.Do(func() { showHiddenOnce.Do(func() {
showHidden.Store(true) 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. // will register with KubeRegistry.
type Registerable interface { type Registerable interface {
prometheus.Collector prometheus.Collector
// Create will mark deprecated state for the collector
Create(version *semver.Version) bool 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 // 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. // automatic behavior can be configured for metric versioning.
type kubeRegistry struct { type kubeRegistry struct {
PromRegistry 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 // 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) { if c.Create(&kr.version) {
return kr.PromRegistry.Register(c) return kr.PromRegistry.Register(c)
} }
kr.trackHiddenCollector(c)
return nil return nil
} }
@ -137,6 +154,8 @@ func (kr *kubeRegistry) MustRegister(cs ...Registerable) {
for _, c := range cs { for _, c := range cs {
if c.Create(&kr.version) { if c.Create(&kr.version) {
metrics = append(metrics, c) metrics = append(metrics, c)
} else {
kr.trackHiddenCollector(c)
} }
} }
kr.PromRegistry.MustRegister(metrics...) kr.PromRegistry.MustRegister(metrics...)
@ -204,11 +223,36 @@ func (kr *kubeRegistry) Gather() ([]*dto.MetricFamily, error) {
return kr.PromRegistry.Gather() 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 { func newKubeRegistry(v apimachineryversion.Info) *kubeRegistry {
r := &kubeRegistry{ r := &kubeRegistry{
PromRegistry: prometheus.NewRegistry(), PromRegistry: prometheus.NewRegistry(),
version: parseVersion(v), version: parseVersion(v),
} }
registriesLock.Lock()
defer registriesLock.Unlock()
registries = append(registries, r)
return r return r
} }
@ -216,5 +260,6 @@ func newKubeRegistry(v apimachineryversion.Info) *kubeRegistry {
// pre-registered. // pre-registered.
func NewKubeRegistry() KubeRegistry { func NewKubeRegistry() KubeRegistry {
r := newKubeRegistry(version.Get()) r := newKubeRegistry(version.Get())
return r return r
} }

View File

@ -17,10 +17,13 @@ limitations under the License.
package metrics package metrics
import ( import (
"strings"
"sync"
"testing" "testing"
"github.com/blang/semver" "github.com/blang/semver"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
apimachineryversion "k8s.io/apimachinery/pkg/version" apimachineryversion "k8s.io/apimachinery/pkg/version"
@ -204,12 +207,6 @@ func TestMustRegister(t *testing.T) {
registryVersion: &v115, registryVersion: &v115,
expectedPanics: []bool{false}, 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 { 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)
}
})
}
}