diff --git a/staging/src/k8s.io/component-base/metrics/BUILD b/staging/src/k8s.io/component-base/metrics/BUILD index be2c221c2c2..b4545eb53dd 100644 --- a/staging/src/k8s.io/component-base/metrics/BUILD +++ b/staging/src/k8s.io/component-base/metrics/BUILD @@ -40,6 +40,7 @@ go_test( srcs = [ "collector_test.go", "counter_test.go", + "desc_test.go", "gauge_test.go", "histogram_test.go", "opts_test.go", diff --git a/staging/src/k8s.io/component-base/metrics/collector.go b/staging/src/k8s.io/component-base/metrics/collector.go index f720249068c..49394b784dd 100644 --- a/staging/src/k8s.io/component-base/metrics/collector.go +++ b/staging/src/k8s.io/component-base/metrics/collector.go @@ -115,26 +115,12 @@ func (bsc *BaseStableCollector) Create(version *semver.Version, self StableColle bsc.init(self) for _, d := range bsc.descriptors { - if version != nil { - d.determineDeprecationStatus(*version) + d.create(version) + if d.IsHidden() { + // do nothing for hidden metrics + } else { + bsc.registrable = append(bsc.registrable, d) } - - d.createOnce.Do(func() { - d.createLock.Lock() - defer d.createLock.Unlock() - - if d.IsHidden() { - // do nothing for hidden metrics - } else if d.IsDeprecated() { - d.initializeDeprecatedDesc() - bsc.registrable = append(bsc.registrable, d) - d.isCreated = true - } else { - d.initialize() - bsc.registrable = append(bsc.registrable, d) - d.isCreated = true - } - }) } if len(bsc.registrable) > 0 { diff --git a/staging/src/k8s.io/component-base/metrics/desc.go b/staging/src/k8s.io/component-base/metrics/desc.go index 2d4b0793ec1..8c4b22c0fc3 100644 --- a/staging/src/k8s.io/component-base/metrics/desc.go +++ b/staging/src/k8s.io/component-base/metrics/desc.go @@ -18,7 +18,6 @@ package metrics import ( "fmt" - "strings" "sync" "github.com/blang/semver" @@ -42,7 +41,8 @@ type Desc struct { variableLabels []string // promDesc is the descriptor used by every Prometheus Metric. - promDesc *prometheus.Desc + promDesc *prometheus.Desc + annotatedHelp string // stabilityLevel represents the API guarantees for a given defined metric. stabilityLevel StabilityLevel @@ -73,6 +73,7 @@ func NewDesc(fqName string, help string, variableLabels []string, constLabels La d := &Desc{ fqName: fqName, help: help, + annotatedHelp: help, variableLabels: variableLabels, constLabels: constLabels, stabilityLevel: stabilityLevel, @@ -86,6 +87,7 @@ func NewDesc(fqName string, help string, variableLabels []string, constLabels La // String formats the Desc as a string. // The stability metadata maybe annotated in 'HELP' section if called after registry, // otherwise not. +// e.g. "Desc{fqName: "normal_stable_descriptor", help: "[STABLE] this is a stable descriptor", constLabels: {}, variableLabels: []}" func (d *Desc) String() string { if d.isCreated { return d.promDesc.String() @@ -136,22 +138,76 @@ func (d *Desc) IsDeprecated() bool { return d.isDeprecated } +// IsCreated returns if metric has been created. +func (d *Desc) IsCreated() bool { + d.createLock.RLock() + defer d.createLock.RUnlock() + + return d.isCreated +} + +// create forces the initialization of Desc which has been deferred until +// the point at which this method is invoked. This method will determine whether +// the Desc is deprecated or hidden, no-opting if the Desc should be considered +// hidden. Furthermore, this function no-opts and returns true if Desc is already +// created. +func (d *Desc) create(version *semver.Version) bool { + if version != nil { + d.determineDeprecationStatus(*version) + } + + // let's not create if this metric is slated to be hidden + if d.IsHidden() { + return false + } + d.createOnce.Do(func() { + d.createLock.Lock() + defer d.createLock.Unlock() + + d.isCreated = true + if d.IsDeprecated() { + d.initializeDeprecatedDesc() + } else { + d.initialize() + } + }) + return d.IsCreated() +} + +// ClearState will clear all the states marked by Create. +// It intends to be used for re-register a hidden metric. +func (d *Desc) ClearState() { + d.isDeprecated = false + d.isHidden = false + d.isCreated = false + + d.markDeprecationOnce = *new(sync.Once) + d.createOnce = *new(sync.Once) + d.deprecateOnce = *new(sync.Once) + d.hideOnce = *new(sync.Once) + d.annotateOnce = *new(sync.Once) + + d.annotatedHelp = d.help + d.promDesc = nil +} + func (d *Desc) markDeprecated() { d.deprecateOnce.Do(func() { - d.help = fmt.Sprintf("(Deprecated since %s) %s", d.deprecatedVersion, d.help) + d.annotatedHelp = fmt.Sprintf("(Deprecated since %s) %s", d.deprecatedVersion, d.annotatedHelp) }) } func (d *Desc) annotateStabilityLevel() { d.annotateOnce.Do(func() { - d.help = fmt.Sprintf("[%v] %v", d.stabilityLevel, d.help) + d.annotatedHelp = fmt.Sprintf("[%v] %v", d.stabilityLevel, d.annotatedHelp) }) } func (d *Desc) initialize() { d.annotateStabilityLevel() + // this actually creates the underlying prometheus desc. - d.promDesc = prometheus.NewDesc(d.fqName, d.help, d.variableLabels, prometheus.Labels(d.constLabels)) + d.promDesc = prometheus.NewDesc(d.fqName, d.annotatedHelp, d.variableLabels, prometheus.Labels(d.constLabels)) } func (d *Desc) initializeDeprecatedDesc() { @@ -165,9 +221,5 @@ func (d *Desc) initializeDeprecatedDesc() { // 1. Desc `D` is registered to registry 'A' in TestA (Note: `D` maybe created) // 2. Desc `D` is registered to registry 'B' in TestB (Note: since 'D' has been created once, thus will be ignored by registry 'B') func (d *Desc) GetRawDesc() *Desc { - // remove stability from help if any - stabilityStr := fmt.Sprintf("[%v] ", d.stabilityLevel) - rawHelp := strings.Replace(d.help, stabilityStr, "", -1) - - return NewDesc(d.fqName, rawHelp, d.variableLabels, d.constLabels, d.stabilityLevel, d.deprecatedVersion) + return NewDesc(d.fqName, d.help, d.variableLabels, d.constLabels, d.stabilityLevel, d.deprecatedVersion) } diff --git a/staging/src/k8s.io/component-base/metrics/desc_test.go b/staging/src/k8s.io/component-base/metrics/desc_test.go new file mode 100644 index 00000000000..f91fdd90412 --- /dev/null +++ b/staging/src/k8s.io/component-base/metrics/desc_test.go @@ -0,0 +1,164 @@ +/* +Copyright 2019 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 ( + "reflect" + "strings" + "testing" + + "k8s.io/apimachinery/pkg/version" +) + +func TestDescCreate(t *testing.T) { + currentVersion := parseVersion(version.Info{ + Major: "1", + Minor: "17", + GitVersion: "v1.17.0-alpha-1.12345", + }) + + var tests = []struct { + name string + fqName string + help string + stabilityLevel StabilityLevel + deprecatedVersion string + + shouldCreate bool + expectedAnnotatedHelp string + }{ + { + name: "alpha descriptor should be created", + fqName: "normal_alpha_descriptor", + help: "this is an alpha descriptor", + stabilityLevel: ALPHA, + deprecatedVersion: "", + shouldCreate: true, + expectedAnnotatedHelp: "[ALPHA] this is an alpha descriptor", + }, + { + name: "stable descriptor should be created", + fqName: "normal_stable_descriptor", + help: "this is a stable descriptor", + stabilityLevel: STABLE, + deprecatedVersion: "", + shouldCreate: true, + expectedAnnotatedHelp: "[STABLE] this is a stable descriptor", + }, + { + name: "deprecated descriptor should be created", + fqName: "deprecated_stable_descriptor", + help: "this is a deprecated descriptor", + stabilityLevel: STABLE, + deprecatedVersion: "1.17.0", + shouldCreate: true, + expectedAnnotatedHelp: "[STABLE] (Deprecated since 1.17.0) this is a deprecated descriptor", + }, + { + name: "hidden descriptor should not be created", + fqName: "hidden_stable_descriptor", + help: "this is a hidden descriptor", + stabilityLevel: STABLE, + deprecatedVersion: "1.16.0", + shouldCreate: false, + expectedAnnotatedHelp: "this is a hidden descriptor", // hidden descriptor shall not be annotated. + }, + } + + for _, test := range tests { + tc := test + t.Run(tc.name, func(t *testing.T) { + desc := NewDesc(tc.fqName, tc.help, nil, nil, tc.stabilityLevel, tc.deprecatedVersion) + + if desc.IsCreated() { + t.Fatal("Descriptor should not be created by default.") + } + + desc.create(¤tVersion) + desc.create(¤tVersion) // we can safely create a descriptor over and over again. + + if desc.IsCreated() != tc.shouldCreate { + t.Fatalf("expected create state: %v, but got: %v", tc.shouldCreate, desc.IsCreated()) + } + + if !strings.Contains(desc.String(), tc.expectedAnnotatedHelp) { + t.Fatalf("expected annotated help: %s, but not in descriptor: %s", tc.expectedAnnotatedHelp, desc.String()) + } + }) + } +} + +func TestDescClearState(t *testing.T) { + currentVersion := parseVersion(version.Info{ + Major: "1", + Minor: "17", + GitVersion: "v1.17.0-alpha-1.12345", + }) + + var tests = []struct { + name string + fqName string + help string + stabilityLevel StabilityLevel + deprecatedVersion string + }{ + { + name: "alpha descriptor", + fqName: "normal_alpha_descriptor", + help: "this is an alpha descriptor", + stabilityLevel: ALPHA, + deprecatedVersion: "", + }, + { + name: "stable descriptor", + fqName: "normal_stable_descriptor", + help: "this is a stable descriptor", + stabilityLevel: STABLE, + deprecatedVersion: "", + }, + { + name: "deprecated descriptor", + fqName: "deprecated_stable_descriptor", + help: "this is a deprecated descriptor", + stabilityLevel: STABLE, + deprecatedVersion: "1.17.0", + }, + { + name: "hidden descriptor", + fqName: "hidden_stable_descriptor", + help: "this is a hidden descriptor", + stabilityLevel: STABLE, + deprecatedVersion: "1.16.0", + }, + } + + for _, test := range tests { + tc := test + t.Run(tc.name, func(t *testing.T) { + descA := NewDesc(tc.fqName, tc.help, nil, nil, tc.stabilityLevel, tc.deprecatedVersion) + descB := NewDesc(tc.fqName, tc.help, nil, nil, tc.stabilityLevel, tc.deprecatedVersion) + + descA.create(¤tVersion) + descA.ClearState() + + // create + if !reflect.DeepEqual(*descA, *descB) { + t.Fatal("descriptor state hasn't be cleaned up") + } + }) + } +}