From a5c4780e96d96a7e71d433752ac8bdef789d43e0 Mon Sep 17 00:00:00 2001 From: yoyinzyc Date: Wed, 3 Mar 2021 15:08:29 -0800 Subject: [PATCH] enforce metric cardinality check to summary metric --- .../k8s.io/component-base/metrics/summary.go | 15 +++- .../component-base/metrics/summary_test.go | 77 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/component-base/metrics/summary.go b/staging/src/k8s.io/component-base/metrics/summary.go index 5d3be8def62..4732b02e308 100644 --- a/staging/src/k8s.io/component-base/metrics/summary.go +++ b/staging/src/k8s.io/component-base/metrics/summary.go @@ -99,12 +99,19 @@ type SummaryVec struct { func NewSummaryVec(opts *SummaryOpts, labels []string) *SummaryVec { 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 := &SummaryVec{ SummaryOpts: opts, originalLabels: labels, lazyMetric: lazyMetric{}, } - v.lazyInit(v, BuildFQName(opts.Namespace, opts.Subsystem, opts.Name)) + v.lazyInit(v, fqName) return v } @@ -139,6 +146,9 @@ func (v *SummaryVec) WithLabelValues(lvs ...string) ObserverMetric { if !v.IsCreated() { return noop } + if v.LabelValueAllowLists != nil { + v.LabelValueAllowLists.ConstrainToAllowedList(v.originalLabels, lvs) + } return v.SummaryVec.WithLabelValues(lvs...) } @@ -150,6 +160,9 @@ func (v *SummaryVec) With(labels map[string]string) ObserverMetric { if !v.IsCreated() { return noop } + if v.LabelValueAllowLists != nil { + v.LabelValueAllowLists.ConstrainLabelMap(labels) + } return v.SummaryVec.With(labels) } diff --git a/staging/src/k8s.io/component-base/metrics/summary_test.go b/staging/src/k8s.io/component-base/metrics/summary_test.go index 752ae5d5992..86ab5badefb 100644 --- a/staging/src/k8s.io/component-base/metrics/summary_test.go +++ b/staging/src/k8s.io/component-base/metrics/summary_test.go @@ -198,3 +198,80 @@ func TestSummaryVec(t *testing.T) { }) } } + +func TestSummaryWithLabelValueAllowList(t *testing.T) { + labelAllowValues := map[string]string{ + "namespace_subsystem_metric_allowlist_test,label_a": "allowed", + } + labels := []string{"label_a", "label_b"} + opts := &SummaryOpts{ + 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, + "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 := NewSummaryVec(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.GetSummary().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) + } + } + }) + } +}