Skip to content

Commit

Permalink
Merge pull request #99385 from YoyinZyc/unbounded_metric
Browse files Browse the repository at this point in the history
Metric cardinality enforcement
  • Loading branch information
k8s-ci-robot committed Mar 3, 2021
2 parents 5b0d045 + b81e2d1 commit b7d146b
Show file tree
Hide file tree
Showing 6 changed files with 334 additions and 12 deletions.
15 changes: 14 additions & 1 deletion staging/src/k8s.io/component-base/metrics/counter.go
Expand Up @@ -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
}

Expand Down Expand Up @@ -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...)
}

Expand All @@ -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)
}

Expand Down
77 changes: 77 additions & 0 deletions staging/src/k8s.io/component-base/metrics/counter_test.go
Expand Up @@ -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)
}
}
})
}
}
36 changes: 34 additions & 2 deletions staging/src/k8s.io/component-base/metrics/options.go
Expand Up @@ -18,6 +18,7 @@ package metrics

import (
"fmt"
"regexp"

"github.com/blang/semver"
"github.com/spf13/pflag"
Expand All @@ -29,6 +30,7 @@ import (
type Options struct {
ShowHiddenMetricsForVersion string
DisabledMetrics []string
AllowListMapping map[string]string
}

// NewOptions returns default metrics options
Expand All @@ -38,12 +40,20 @@ func NewOptions() *Options {

// Validate validates metrics flags options.
func (o *Options) Validate() []error {
var errs []error
err := validateShowHiddenMetricsVersion(parseVersion(version.Get()), o.ShowHiddenMetricsForVersion)
if err != nil {
return []error{err}
errs = append(errs, err)
}

return nil
if err := validateAllowMetricLabel(o.AllowListMapping); err != nil {
errs = append(errs, err)
}

if len(errs) == 0 {
return nil
}
return errs
}

// AddFlags adds flags for exposing component metrics.
Expand All @@ -63,6 +73,10 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) {
"This flag provides an escape hatch for misbehaving metrics. "+
"You must provide the fully qualified metric name in order to disable it. "+
"Disclaimer: disabling metrics is higher in precedence than showing hidden metrics.")
fs.StringToStringVar(&o.AllowListMapping, "allow-metric-labels", o.AllowListMapping,
"The map from metric-label to value allow-list of this label. The key's format is <MetricName>,<LabelName>. "+
"The value's format is <allowed_value>,<allowed_value>..."+
"e.g. metric1,label1='v1,v2,v3', metric1,label2='v1,v2,v3' metric2,label1='v1,v2,v3'.")
}

// Apply applies parameters into global configuration of metrics.
Expand All @@ -77,6 +91,9 @@ func (o *Options) Apply() {
for _, metricName := range o.DisabledMetrics {
SetDisabledMetric(metricName)
}
if o.AllowListMapping != nil {
SetLabelAllowListFromCLI(o.AllowListMapping)
}
}

func validateShowHiddenMetricsVersion(currentVersion semver.Version, targetVersionStr string) error {
Expand All @@ -91,3 +108,18 @@ func validateShowHiddenMetricsVersion(currentVersion semver.Version, targetVersi

return nil
}

func validateAllowMetricLabel(allowListMapping map[string]string) error {
if allowListMapping == nil {
return nil
}
metricNameRegex := `[a-zA-Z_:][a-zA-Z0-9_:]*`
labelRegex := `[a-zA-Z_][a-zA-Z0-9_]*`
for k := range allowListMapping {
reg := regexp.MustCompile(metricNameRegex + `,` + labelRegex)
if reg.FindString(k) != k {
return fmt.Errorf("--allow-metric-labels must has a list of kv pair with format `metricName:labelName=labelValue, labelValue,...`")
}
}
return nil
}
67 changes: 67 additions & 0 deletions staging/src/k8s.io/component-base/metrics/options_test.go
@@ -0,0 +1,67 @@
/*
Copyright 2021 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package metrics

import "testing"

func TestValidateAllowMetricLabel(t *testing.T) {
var tests = []struct {
name string
input map[string]string
expectedError bool
}{
{
"validated",
map[string]string{
"metric_name,label_name": "labelValue1,labelValue2",
},
false,
},
{
"metric name is not valid",
map[string]string{
"-metric_name,label_name": "labelValue1,labelValue2",
},
true,
},
{
"label name is not valid",
map[string]string{
"metric_name,:label_name": "labelValue1,labelValue2",
},
true,
},
{
"no label name",
map[string]string{
"metric_name": "labelValue1,labelValue2",
},
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateAllowMetricLabel(tt.input)
if err == nil && tt.expectedError {
t.Error("Got error is nil, wanted error is not nil")
}
if err != nil && !tt.expectedError {
t.Errorf("Got error is %v, wanted no error", err)
}
})
}
}
72 changes: 63 additions & 9 deletions staging/src/k8s.io/component-base/metrics/opts.go
Expand Up @@ -18,10 +18,17 @@ package metrics

import (
"fmt"
"strings"
"sync"
"time"

"github.com/prometheus/client_golang/prometheus"
"k8s.io/apimachinery/pkg/util/sets"
)

var (
labelValueAllowLists = map[string]*MetricLabelAllowList{}
allowListLock sync.RWMutex
)

// KubeOpts is superset struct for prometheus.Opts. The prometheus Opts structure
Expand All @@ -31,15 +38,16 @@ import (
// Name must be set to a non-empty string. DeprecatedVersion is defined only
// if the metric for which this options applies is, in fact, deprecated.
type KubeOpts struct {
Namespace string
Subsystem string
Name string
Help string
ConstLabels map[string]string
DeprecatedVersion string
deprecateOnce sync.Once
annotateOnce sync.Once
StabilityLevel StabilityLevel
Namespace string
Subsystem string
Name string
Help string
ConstLabels map[string]string
DeprecatedVersion string
deprecateOnce sync.Once
annotateOnce sync.Once
StabilityLevel StabilityLevel
LabelValueAllowLists *MetricLabelAllowList
}

// BuildFQName joins the given three name components by "_". Empty name
Expand Down Expand Up @@ -243,3 +251,49 @@ func (o *SummaryOpts) toPromSummaryOpts() prometheus.SummaryOpts {
BufCap: o.BufCap,
}
}

type MetricLabelAllowList struct {
labelToAllowList map[string]sets.String
}

func (allowList *MetricLabelAllowList) ConstrainToAllowedList(labelNameList, labelValueList []string) {
for index, value := range labelValueList {
name := labelNameList[index]
if allowValues, ok := allowList.labelToAllowList[name]; ok {
if !allowValues.Has(value) {
labelValueList[index] = "unexpected"
}
}
}
}

func (allowList *MetricLabelAllowList) ConstrainLabelMap(labels map[string]string) {
for name, value := range labels {
if allowValues, ok := allowList.labelToAllowList[name]; ok {
if !allowValues.Has(value) {
labels[name] = "unexpected"
}
}
}
}

func SetLabelAllowListFromCLI(allowListMapping map[string]string) {
allowListLock.Lock()
defer allowListLock.Unlock()
for metricLabelName, labelValues := range allowListMapping {
metricName := strings.Split(metricLabelName, ",")[0]
labelName := strings.Split(metricLabelName, ",")[1]
valueSet := sets.NewString(strings.Split(labelValues, ",")...)

allowList, ok := labelValueAllowLists[metricName]
if ok {
allowList.labelToAllowList[labelName] = valueSet
} else {
labelToAllowList := make(map[string]sets.String)
labelToAllowList[labelName] = valueSet
labelValueAllowLists[metricName] = &MetricLabelAllowList{
labelToAllowList,
}
}
}
}

0 comments on commit b7d146b

Please sign in to comment.