From b81e2d18f9b307b646617a2b2fdc22ca7d86f00d Mon Sep 17 00:00:00 2001 From: yoyinzyc Date: Tue, 23 Feb 2021 15:22:41 -0800 Subject: [PATCH] enforce metric cardinality check to counter metric. --- .../k8s.io/component-base/metrics/counter.go | 15 +++- .../component-base/metrics/counter_test.go | 77 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/component-base/metrics/counter.go b/staging/src/k8s.io/component-base/metrics/counter.go index 34c177fe1ac..addf680c87b 100644 --- a/staging/src/k8s.io/component-base/metrics/counter.go +++ b/staging/src/k8s.io/component-base/metrics/counter.go @@ -112,13 +112,20 @@ type CounterVec struct { func NewCounterVec(opts *CounterOpts, labels []string) *CounterVec { opts.StabilityLevel.setDefaults() + fqName := BuildFQName(opts.Namespace, opts.Subsystem, opts.Name) + allowListLock.RLock() + if allowList, ok := labelValueAllowLists[fqName]; ok { + opts.LabelValueAllowLists = allowList + } + allowListLock.RUnlock() + cv := &CounterVec{ CounterVec: noopCounterVec, CounterOpts: opts, originalLabels: labels, lazyMetric: lazyMetric{}, } - cv.lazyInit(cv, BuildFQName(opts.Namespace, opts.Subsystem, opts.Name)) + cv.lazyInit(cv, fqName) return cv } @@ -158,6 +165,9 @@ func (v *CounterVec) WithLabelValues(lvs ...string) CounterMetric { if !v.IsCreated() { return noop // return no-op counter } + if v.LabelValueAllowLists != nil { + v.LabelValueAllowLists.ConstrainToAllowedList(v.originalLabels, lvs) + } return v.CounterVec.WithLabelValues(lvs...) } @@ -169,6 +179,9 @@ func (v *CounterVec) With(labels map[string]string) CounterMetric { if !v.IsCreated() { return noop // return no-op counter } + if v.LabelValueAllowLists != nil { + v.LabelValueAllowLists.ConstrainLabelMap(labels) + } return v.CounterVec.With(labels) } diff --git a/staging/src/k8s.io/component-base/metrics/counter_test.go b/staging/src/k8s.io/component-base/metrics/counter_test.go index 9a0c951fbce..3595a57946d 100644 --- a/staging/src/k8s.io/component-base/metrics/counter_test.go +++ b/staging/src/k8s.io/component-base/metrics/counter_test.go @@ -208,3 +208,80 @@ func TestCounterVec(t *testing.T) { }) } } + +func TestCounterWithLabelValueAllowList(t *testing.T) { + labelAllowValues := map[string]string{ + "namespace_subsystem_metric_test_name,label_a": "allowed", + } + labels := []string{"label_a", "label_b"} + opts := &CounterOpts{ + Namespace: "namespace", + Name: "metric_test_name", + 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, + "allowed b2": 1, + }, + }, + { + desc: "Test unexpected input", + labelValues: [][]string{{"allowed", "b1"}, {"not_allowed", "b1"}}, + expectMetricValues: map[string]int{ + "allowed b1": 1, + "unexpected b1": 1, + }, + }, + } + + 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 := NewCounterVec(opts, labels) + registry.MustRegister(c) + + for _, lv := range test.labelValues { + c.WithLabelValues(lv...).Inc() + } + 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.GetCounter().GetValue()) + 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) + } + } + }) + } +}