diff --git a/staging/src/k8s.io/component-base/metrics/collector.go b/staging/src/k8s.io/component-base/metrics/collector.go index 49394b784dd..9604ab18fa7 100644 --- a/staging/src/k8s.io/component-base/metrics/collector.go +++ b/staging/src/k8s.io/component-base/metrics/collector.go @@ -43,8 +43,9 @@ type StableCollector interface { // is a convenient assistant for custom collectors. // It is recommend that inherit BaseStableCollector when implementing custom collectors. type BaseStableCollector struct { - descriptors []*Desc // stores all Desc collected from DescribeWithStability(). - registrable []*Desc // stores registrable Desc(not be hidden), is a subset of descriptors. + 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 } @@ -88,7 +89,19 @@ func (bsc *BaseStableCollector) Collect(ch chan<- prometheus.Metric) { } func (bsc *BaseStableCollector) add(d *Desc) { - bsc.descriptors = append(bsc.descriptors, d) + if len(d.fqName) == 0 { + panic("nameless metrics will be not allowed") + } + + if bsc.descriptors == nil { + bsc.descriptors = make(map[string]*Desc) + } + + if _, exist := bsc.descriptors[d.fqName]; exist { + panic(fmt.Sprintf("duplicate metrics (%s) will be not allowed", d.fqName)) + } + + bsc.descriptors[d.fqName] = d } // Init intends to be called by registry. @@ -108,6 +121,22 @@ func (bsc *BaseStableCollector) init(self StableCollector) { } } +func (bsc *BaseStableCollector) trackRegistrableDescriptor(d *Desc) { + if bsc.registrable == nil { + bsc.registrable = make(map[string]*Desc) + } + + bsc.registrable[d.fqName] = d +} + +func (bsc *BaseStableCollector) trackHiddenDescriptor(d *Desc) { + if bsc.hidden == nil { + bsc.hidden = make(map[string]*Desc) + } + + bsc.hidden[d.fqName] = d +} + // Create intends to be called by registry. // Create will return true as long as there is one or more metrics not be hidden. // Otherwise return false, that means the whole collector will be ignored by registry. @@ -117,9 +146,9 @@ func (bsc *BaseStableCollector) Create(version *semver.Version, self StableColle for _, d := range bsc.descriptors { d.create(version) if d.IsHidden() { - // do nothing for hidden metrics + bsc.trackHiddenDescriptor(d) } else { - bsc.registrable = append(bsc.registrable, d) + bsc.trackRegistrableDescriptor(d) } } diff --git a/staging/src/k8s.io/component-base/metrics/collector_test.go b/staging/src/k8s.io/component-base/metrics/collector_test.go index db6c77c44a5..03f33067b04 100644 --- a/staging/src/k8s.io/component-base/metrics/collector_test.go +++ b/staging/src/k8s.io/component-base/metrics/collector_test.go @@ -17,10 +17,13 @@ limitations under the License. package metrics import ( + "fmt" "strings" "testing" "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/assert" + apimachineryversion "k8s.io/apimachinery/pkg/version" ) @@ -92,3 +95,42 @@ func TestBaseCustomCollector(t *testing.T) { t.Fatal(err) } } + +func TestInvalidCustomCollector(t *testing.T) { + var currentVersion = apimachineryversion.Info{ + Major: "1", + Minor: "17", + GitVersion: "v1.17.0-alpha-1.12345", + } + var namelessDesc = NewDesc("", "this is a nameless metric", nil, nil, ALPHA, "") + var duplicatedDescA = NewDesc("test_duplicated_metric", "this is a duplicated metric A", nil, nil, ALPHA, "") + var duplicatedDescB = NewDesc("test_duplicated_metric", "this is a duplicated metric B", nil, nil, ALPHA, "") + + var tests = []struct { + name string + descriptors []*Desc + panicStr string + }{ + { + name: "nameless metric will be not allowed", + descriptors: []*Desc{namelessDesc}, + panicStr: "nameless metrics will be not allowed", + }, + { + name: "duplicated metric will be not allowed", + descriptors: []*Desc{duplicatedDescA, duplicatedDescB}, + panicStr: fmt.Sprintf("duplicate metrics (%s) will be not allowed", duplicatedDescA.fqName), + }, + } + + for _, test := range tests { + tc := test + t.Run(tc.name, func(t *testing.T) { + registry := newKubeRegistry(currentVersion) + customCollector := newTestCustomCollector(tc.descriptors...) + assert.Panics(t, func() { + registry.CustomMustRegister(customCollector) + }, tc.panicStr) + }) + } +}