From 0e32c43e07dfa9f03f7695ad892bf8235f2e33f1 Mon Sep 17 00:00:00 2001 From: Han Kang Date: Thu, 18 Feb 2021 15:25:15 -0800 Subject: [PATCH] allow explicit disabling of metrics as an escape hatch. Change-Id: I92895987d140e0e94eea3e4dea872c298d82babb --- .../component-base/metrics/collector.go | 20 +++++------ .../k8s.io/component-base/metrics/metric.go | 23 +++++++++--- .../k8s.io/component-base/metrics/options.go | 10 ++++++ .../k8s.io/component-base/metrics/registry.go | 16 ++++++--- .../component-base/metrics/registry_test.go | 36 +++++++++++++++++++ 5 files changed, 87 insertions(+), 18 deletions(-) diff --git a/staging/src/k8s.io/component-base/metrics/collector.go b/staging/src/k8s.io/component-base/metrics/collector.go index 090342e1623..61ad951e5b9 100644 --- a/staging/src/k8s.io/component-base/metrics/collector.go +++ b/staging/src/k8s.io/component-base/metrics/collector.go @@ -49,10 +49,10 @@ type StableCollector interface { // is a convenient assistant for custom collectors. // It is recommend that inherit BaseStableCollector when implementing custom collectors. type BaseStableCollector struct { - descriptors map[string]*Desc // stores all descriptors by pair, these are collected from DescribeWithStability(). - registrable map[string]*Desc // stores registrable descriptors by pair, is a subset of descriptors. - hidden map[string]*Desc // stores hidden descriptors by pair, is a subset of descriptors. - self StableCollector + descriptors map[string]*Desc // stores all descriptors by pair, these are collected from DescribeWithStability(). + registerable map[string]*Desc // stores registerable descriptors by pair, is a subset of descriptors. + hidden map[string]*Desc // stores hidden descriptors by pair, is a subset of descriptors. + self StableCollector } // DescribeWithStability sends all descriptors to the provided channel. @@ -64,7 +64,7 @@ func (bsc *BaseStableCollector) DescribeWithStability(ch chan<- *Desc) { // Describe sends all descriptors to the provided channel. // It intend to be called by prometheus registry. func (bsc *BaseStableCollector) Describe(ch chan<- *prometheus.Desc) { - for _, d := range bsc.registrable { + for _, d := range bsc.registerable { ch <- d.toPrometheusDesc() } } @@ -128,11 +128,11 @@ func (bsc *BaseStableCollector) init(self StableCollector) { } func (bsc *BaseStableCollector) trackRegistrableDescriptor(d *Desc) { - if bsc.registrable == nil { - bsc.registrable = make(map[string]*Desc) + if bsc.registerable == nil { + bsc.registerable = make(map[string]*Desc) } - bsc.registrable[d.fqName] = d + bsc.registerable[d.fqName] = d } func (bsc *BaseStableCollector) trackHiddenDescriptor(d *Desc) { @@ -158,7 +158,7 @@ func (bsc *BaseStableCollector) Create(version *semver.Version, self StableColle } } - if len(bsc.registrable) > 0 { + if len(bsc.registerable) > 0 { return true } @@ -173,7 +173,7 @@ func (bsc *BaseStableCollector) ClearState() { } bsc.descriptors = nil - bsc.registrable = nil + bsc.registerable = nil bsc.hidden = nil bsc.self = nil } diff --git a/staging/src/k8s.io/component-base/metrics/metric.go b/staging/src/k8s.io/component-base/metrics/metric.go index bb1f6962709..2f7b880d862 100644 --- a/staging/src/k8s.io/component-base/metrics/metric.go +++ b/staging/src/k8s.io/component-base/metrics/metric.go @@ -87,10 +87,24 @@ func (r *lazyMetric) lazyInit(self kubeCollector, fqName string) { r.self = self } -// determineDeprecationStatus figures out whether the lazy metric should be deprecated or not. +// preprocessMetric figures out whether the lazy metric should be hidden or not. // This method takes a Version argument which should be the version of the binary in which -// this code is currently being executed. -func (r *lazyMetric) determineDeprecationStatus(version semver.Version) { +// this code is currently being executed. A metric can be hidden under two conditions: +// 1. if the metric is deprecated and is outside the grace period (i.e. has been +// deprecated for more than one release +// 2. if the metric is manually disabled via a CLI flag. +// +// Disclaimer: disabling a metric via a CLI flag has higher precedence than +// deprecation and will override show-hidden-metrics for the explicitly +// disabled metric. +func (r *lazyMetric) preprocessMetric(version semver.Version) { + disabledMetricsLock.RLock() + defer disabledMetricsLock.RUnlock() + // disabling metrics is higher in precedence than showing hidden metrics + if _, ok := disabledMetrics[r.fqName]; ok { + r.isHidden = true + return + } selfVersion := r.self.DeprecatedVersion() if selfVersion == nil { return @@ -99,6 +113,7 @@ func (r *lazyMetric) determineDeprecationStatus(version semver.Version) { if selfVersion.LTE(version) { r.isDeprecated = true } + if ShouldShowHidden() { klog.Warningf("Hidden metrics (%s) have been manually overridden, showing this very deprecated metric.", r.fqName) return @@ -126,7 +141,7 @@ func (r *lazyMetric) IsDeprecated() bool { // created. func (r *lazyMetric) Create(version *semver.Version) bool { if version != nil { - r.determineDeprecationStatus(*version) + r.preprocessMetric(*version) } // let's not create if this metric is slated to be hidden if r.IsHidden() { diff --git a/staging/src/k8s.io/component-base/metrics/options.go b/staging/src/k8s.io/component-base/metrics/options.go index c15dd9b4d20..087ea34b270 100644 --- a/staging/src/k8s.io/component-base/metrics/options.go +++ b/staging/src/k8s.io/component-base/metrics/options.go @@ -28,6 +28,7 @@ import ( // Options has all parameters needed for exposing metrics from components type Options struct { ShowHiddenMetricsForVersion string + DisabledMetrics []string } // NewOptions returns default metrics options @@ -56,12 +57,21 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) { "The format is ., e.g.: '1.16'. "+ "The purpose of this format is make sure you have the opportunity to notice if the next release hides additional metrics, "+ "rather than being surprised when they are permanently removed in the release after that.") + fs.StringSliceVar(&o.DisabledMetrics, + "disabled-metrics", + o.DisabledMetrics, + "This flag provides an escape hatch for misbehaving metrics. "+ + "You must provide the fully qualified metric name in order to disable it.") } // Apply applies parameters into global configuration of metrics. func (o *Options) Apply() { if o != nil && len(o.ShowHiddenMetricsForVersion) > 0 { SetShowHidden() + // set disabled metrics + for _, metricName := range o.DisabledMetrics { + SetDisabledMetric(metricName) + } } } diff --git a/staging/src/k8s.io/component-base/metrics/registry.go b/staging/src/k8s.io/component-base/metrics/registry.go index 06a5c6503cf..b608fb568f0 100644 --- a/staging/src/k8s.io/component-base/metrics/registry.go +++ b/staging/src/k8s.io/component-base/metrics/registry.go @@ -30,10 +30,12 @@ import ( ) var ( - showHiddenOnce sync.Once - showHidden atomic.Value - registries []*kubeRegistry // stores all registries created by NewKubeRegistry() - registriesLock sync.RWMutex + showHiddenOnce sync.Once + disabledMetricsLock sync.RWMutex + showHidden atomic.Value + registries []*kubeRegistry // stores all registries created by NewKubeRegistry() + registriesLock sync.RWMutex + disabledMetrics = map[string]struct{}{} ) // shouldHide be used to check if a specific metric with deprecated version should be hidden @@ -61,6 +63,12 @@ func ValidateShowHiddenMetricsVersion(v string) []error { return nil } +func SetDisabledMetric(name string) { + disabledMetricsLock.Lock() + defer disabledMetricsLock.Unlock() + disabledMetrics[name] = struct{}{} +} + // SetShowHidden will enable showing hidden metrics. This will no-opt // after the initial call func SetShowHidden() { 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 c7fb7d4297f..a6331b22f20 100644 --- a/staging/src/k8s.io/component-base/metrics/registry_test.go +++ b/staging/src/k8s.io/component-base/metrics/registry_test.go @@ -515,3 +515,39 @@ func TestRegistryReset(t *testing.T) { t.Fatal(err) } } + +func TestDisabledMetrics(t *testing.T) { + SetDisabledMetric("should_be_disabled") + currentVersion := apimachineryversion.Info{ + Major: "1", + Minor: "17", + GitVersion: "v1.17.1-alpha-1.12345", + } + registry := newKubeRegistry(currentVersion) + disabledMetric := NewCounterVec(&CounterOpts{ + Name: "should_be_disabled", + Help: "this metric should be disabled", + }, []string{"label"}) + // gauges cannot be reset + enabledMetric := NewGauge(&GaugeOpts{ + Name: "should_be_enabled", + Help: "this metric should not be disabled", + }) + + registry.MustRegister(disabledMetric) + registry.MustRegister(enabledMetric) + disabledMetric.WithLabelValues("one").Inc() + disabledMetric.WithLabelValues("two").Inc() + disabledMetric.WithLabelValues("two").Inc() + enabledMetric.Inc() + + enabledMetricOutput := ` + # HELP should_be_enabled [ALPHA] this metric should not be disabled + # TYPE should_be_enabled gauge + should_be_enabled 1 +` + + if err := testutil.GatherAndCompare(registry, strings.NewReader(enabledMetricOutput), "should_be_disabled", "should_be_enabled"); err != nil { + t.Fatal(err) + } +}