From 2a86d4ca69ffa206bbe211d786117130b5b5c875 Mon Sep 17 00:00:00 2001 From: yoyinzyc Date: Wed, 3 Mar 2021 15:06:54 -0800 Subject: [PATCH] enforce metric cardinality check to histogram metric --- .../component-base/metrics/histogram.go | 15 +++- .../component-base/metrics/histogram_test.go | 77 +++++++++++++++++++ .../src/k8s.io/component-base/metrics/opts.go | 48 ++++++------ 3 files changed, 116 insertions(+), 24 deletions(-) diff --git a/staging/src/k8s.io/component-base/metrics/histogram.go b/staging/src/k8s.io/component-base/metrics/histogram.go index 90f21766e9f..9dd75388b33 100644 --- a/staging/src/k8s.io/component-base/metrics/histogram.go +++ b/staging/src/k8s.io/component-base/metrics/histogram.go @@ -104,13 +104,20 @@ type HistogramVec struct { func NewHistogramVec(opts *HistogramOpts, labels []string) *HistogramVec { opts.StabilityLevel.setDefaults() + fqName := BuildFQName(opts.Namespace, opts.Subsystem, opts.Name) + allowListLock.RLock() + if allowList, ok := labelValueAllowLists[fqName]; ok { + opts.LabelValueAllowLists = allowList + } + allowListLock.RUnlock() + v := &HistogramVec{ HistogramVec: noopHistogramVec, HistogramOpts: opts, originalLabels: labels, lazyMetric: lazyMetric{}, } - v.lazyInit(v, BuildFQName(opts.Namespace, opts.Subsystem, opts.Name)) + v.lazyInit(v, fqName) return v } @@ -145,6 +152,9 @@ func (v *HistogramVec) WithLabelValues(lvs ...string) ObserverMetric { if !v.IsCreated() { return noop } + if v.LabelValueAllowLists != nil { + v.LabelValueAllowLists.ConstrainToAllowedList(v.originalLabels, lvs) + } return v.HistogramVec.WithLabelValues(lvs...) } @@ -156,6 +166,9 @@ func (v *HistogramVec) With(labels map[string]string) ObserverMetric { if !v.IsCreated() { return noop } + if v.LabelValueAllowLists != nil { + v.LabelValueAllowLists.ConstrainLabelMap(labels) + } return v.HistogramVec.With(labels) } diff --git a/staging/src/k8s.io/component-base/metrics/histogram_test.go b/staging/src/k8s.io/component-base/metrics/histogram_test.go index 762441eebd1..99f5dcf808b 100644 --- a/staging/src/k8s.io/component-base/metrics/histogram_test.go +++ b/staging/src/k8s.io/component-base/metrics/histogram_test.go @@ -204,3 +204,80 @@ func TestHistogramVec(t *testing.T) { }) } } + +func TestHistogramWithLabelValueAllowList(t *testing.T) { + labelAllowValues := map[string]string{ + "namespace_subsystem_metric_allowlist_test,label_a": "allowed", + } + labels := []string{"label_a", "label_b"} + opts := &HistogramOpts{ + Namespace: "namespace", + Name: "metric_allowlist_test", + Subsystem: "subsystem", + } + var tests = []struct { + desc string + labelValues [][]string + expectMetricValues map[string]int + }{ + { + desc: "Test no unexpected input", + labelValues: [][]string{{"allowed", "b1"}, {"allowed", "b2"}}, + expectMetricValues: map[string]int{ + "allowed b1": 1.0, + "allowed b2": 1.0, + }, + }, + { + desc: "Test unexpected input", + labelValues: [][]string{{"allowed", "b1"}, {"not_allowed", "b1"}}, + expectMetricValues: map[string]int{ + "allowed b1": 1.0, + "unexpected b1": 1.0, + }, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + SetLabelAllowListFromCLI(labelAllowValues) + registry := newKubeRegistry(apimachineryversion.Info{ + Major: "1", + Minor: "15", + GitVersion: "v1.15.0-alpha-1.12345", + }) + c := NewHistogramVec(opts, labels) + registry.MustRegister(c) + + for _, lv := range test.labelValues { + c.WithLabelValues(lv...).Observe(1.0) + } + mfs, err := registry.Gather() + assert.Nil(t, err, "Gather failed %v", err) + + for _, mf := range mfs { + if *mf.Name != BuildFQName(opts.Namespace, opts.Subsystem, opts.Name) { + continue + } + mfMetric := mf.GetMetric() + + for _, m := range mfMetric { + var aValue, bValue string + for _, l := range m.Label { + if *l.Name == "label_a" { + aValue = *l.Value + } + if *l.Name == "label_b" { + bValue = *l.Value + } + } + labelValuePair := aValue + " " + bValue + expectedValue, ok := test.expectMetricValues[labelValuePair] + assert.True(t, ok, "Got unexpected label values, lable_a is %v, label_b is %v", aValue, bValue) + actualValue := int(m.GetHistogram().GetSampleCount()) + assert.Equalf(t, expectedValue, actualValue, "Got %v, wanted %v as the count while setting label_a to %v and label b to %v", actualValue, expectedValue, aValue, bValue) + } + } + }) + } +} diff --git a/staging/src/k8s.io/component-base/metrics/opts.go b/staging/src/k8s.io/component-base/metrics/opts.go index 395913936c6..04203b74e0a 100644 --- a/staging/src/k8s.io/component-base/metrics/opts.go +++ b/staging/src/k8s.io/component-base/metrics/opts.go @@ -148,16 +148,17 @@ func (o *GaugeOpts) toPromGaugeOpts() prometheus.GaugeOpts { // and can safely be left at their zero value, although it is strongly // encouraged to set a Help string. type HistogramOpts struct { - Namespace string - Subsystem string - Name string - Help string - ConstLabels map[string]string - Buckets []float64 - DeprecatedVersion string - deprecateOnce sync.Once - annotateOnce sync.Once - StabilityLevel StabilityLevel + Namespace string + Subsystem string + Name string + Help string + ConstLabels map[string]string + Buckets []float64 + DeprecatedVersion string + deprecateOnce sync.Once + annotateOnce sync.Once + StabilityLevel StabilityLevel + LabelValueAllowLists *MetricLabelAllowList } // Modify help description on the metric description. @@ -194,19 +195,20 @@ func (o *HistogramOpts) toPromHistogramOpts() prometheus.HistogramOpts { // a help string and to explicitly set the Objectives field to the desired value // as the default value will change in the upcoming v0.10 of the library. type SummaryOpts struct { - Namespace string - Subsystem string - Name string - Help string - ConstLabels map[string]string - Objectives map[float64]float64 - MaxAge time.Duration - AgeBuckets uint32 - BufCap uint32 - DeprecatedVersion string - deprecateOnce sync.Once - annotateOnce sync.Once - StabilityLevel StabilityLevel + Namespace string + Subsystem string + Name string + Help string + ConstLabels map[string]string + Objectives map[float64]float64 + MaxAge time.Duration + AgeBuckets uint32 + BufCap uint32 + DeprecatedVersion string + deprecateOnce sync.Once + annotateOnce sync.Once + StabilityLevel StabilityLevel + LabelValueAllowLists *MetricLabelAllowList } // Modify help description on the metric description.