From 9ec270804a0c056bb6df39c51a2c58fab4d5f321 Mon Sep 17 00:00:00 2001 From: RainbowMango Date: Sat, 21 Sep 2019 11:10:30 +0800 Subject: [PATCH 1/2] Handle metrics.StabilityLevel default value better. Provide a method setDefault() to StabilityLevel type. Update bazel by hack/update-bazel.sh --- .../src/k8s.io/component-base/metrics/BUILD | 1 + .../src/k8s.io/component-base/metrics/opts.go | 10 +++ .../component-base/metrics/opts_test.go | 61 +++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 staging/src/k8s.io/component-base/metrics/opts_test.go diff --git a/staging/src/k8s.io/component-base/metrics/BUILD b/staging/src/k8s.io/component-base/metrics/BUILD index b1b78e6b3a3..f97d0c5c97d 100644 --- a/staging/src/k8s.io/component-base/metrics/BUILD +++ b/staging/src/k8s.io/component-base/metrics/BUILD @@ -41,6 +41,7 @@ go_test( "counter_test.go", "gauge_test.go", "histogram_test.go", + "opts_test.go", "registry_test.go", "summary_test.go", "version_parser_test.go", diff --git a/staging/src/k8s.io/component-base/metrics/opts.go b/staging/src/k8s.io/component-base/metrics/opts.go index 4fd1048b167..496f2470f7d 100644 --- a/staging/src/k8s.io/component-base/metrics/opts.go +++ b/staging/src/k8s.io/component-base/metrics/opts.go @@ -53,6 +53,16 @@ const ( STABLE StabilityLevel = "STABLE" ) +// setDefaults takes 'ALPHA' in case of empty. +func (sl *StabilityLevel) setDefaults() { + switch *sl { + case "": + *sl = ALPHA + default: + // no-op, since we have a StabilityLevel already + } +} + // CounterOpts is an alias for Opts. See there for doc comments. type CounterOpts KubeOpts diff --git a/staging/src/k8s.io/component-base/metrics/opts_test.go b/staging/src/k8s.io/component-base/metrics/opts_test.go new file mode 100644 index 00000000000..d009829a6b7 --- /dev/null +++ b/staging/src/k8s.io/component-base/metrics/opts_test.go @@ -0,0 +1,61 @@ +/* +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 ( + "testing" +) + +func TestDefaultStabilityLevel(t *testing.T) { + var tests = []struct { + name string + inputValue StabilityLevel + expectValue StabilityLevel + expectPanic bool + }{ + { + name: "empty should take ALPHA by default", + inputValue: "", + expectValue: ALPHA, + expectPanic: false, + }, + { + name: "ALPHA remain unchanged", + inputValue: ALPHA, + expectValue: ALPHA, + expectPanic: false, + }, + { + name: "STABLE remain unchanged", + inputValue: STABLE, + expectValue: STABLE, + expectPanic: false, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + var stability = tc.inputValue + + stability.setDefaults() + if stability != tc.expectValue { + t.Errorf("Got %s, expected: %v ", stability, tc.expectValue) + } + }) + } +} From 4127525e9b9dd0e601415af58605f1485a2db468 Mon Sep 17 00:00:00 2001 From: RainbowMango Date: Sat, 21 Sep 2019 11:19:24 +0800 Subject: [PATCH 2/2] Migrate stability level handle functionality overall metrics package --- staging/src/k8s.io/component-base/metrics/counter.go | 12 ++++-------- staging/src/k8s.io/component-base/metrics/gauge.go | 12 ++++-------- .../src/k8s.io/component-base/metrics/histogram.go | 12 ++++-------- staging/src/k8s.io/component-base/metrics/summary.go | 12 ++++-------- 4 files changed, 16 insertions(+), 32 deletions(-) diff --git a/staging/src/k8s.io/component-base/metrics/counter.go b/staging/src/k8s.io/component-base/metrics/counter.go index 17386ed5a84..08901963adf 100644 --- a/staging/src/k8s.io/component-base/metrics/counter.go +++ b/staging/src/k8s.io/component-base/metrics/counter.go @@ -34,10 +34,8 @@ type Counter struct { // However, the object returned will not measure anything unless the collector is first // registered, since the metric is lazily instantiated. func NewCounter(opts *CounterOpts) *Counter { - // todo: handle defaulting better - if opts.StabilityLevel == "" { - opts.StabilityLevel = ALPHA - } + opts.StabilityLevel.setDefaults() + kc := &Counter{ CounterOpts: opts, lazyMetric: lazyMetric{}, @@ -86,10 +84,8 @@ type CounterVec struct { // However, the object returned will not measure anything unless the collector is first // registered, since the metric is lazily instantiated. func NewCounterVec(opts *CounterOpts, labels []string) *CounterVec { - // todo: handle defaulting better - if opts.StabilityLevel == "" { - opts.StabilityLevel = ALPHA - } + opts.StabilityLevel.setDefaults() + cv := &CounterVec{ CounterVec: noopCounterVec, CounterOpts: opts, diff --git a/staging/src/k8s.io/component-base/metrics/gauge.go b/staging/src/k8s.io/component-base/metrics/gauge.go index 82b982d9d6c..020f387acfd 100644 --- a/staging/src/k8s.io/component-base/metrics/gauge.go +++ b/staging/src/k8s.io/component-base/metrics/gauge.go @@ -34,10 +34,8 @@ type Gauge struct { // However, the object returned will not measure anything unless the collector is first // registered, since the metric is lazily instantiated. func NewGauge(opts *GaugeOpts) *Gauge { - // todo: handle defaulting better - if opts.StabilityLevel == "" { - opts.StabilityLevel = ALPHA - } + opts.StabilityLevel.setDefaults() + kc := &Gauge{ GaugeOpts: opts, lazyMetric: lazyMetric{}, @@ -86,10 +84,8 @@ type GaugeVec struct { // However, the object returned will not measure anything unless the collector is first // registered, since the metric is lazily instantiated. func NewGaugeVec(opts *GaugeOpts, labels []string) *GaugeVec { - // todo: handle defaulting better - if opts.StabilityLevel == "" { - opts.StabilityLevel = ALPHA - } + opts.StabilityLevel.setDefaults() + cv := &GaugeVec{ GaugeVec: noopGaugeVec, GaugeOpts: opts, diff --git a/staging/src/k8s.io/component-base/metrics/histogram.go b/staging/src/k8s.io/component-base/metrics/histogram.go index 6aa95953ed4..58ad36bbd33 100644 --- a/staging/src/k8s.io/component-base/metrics/histogram.go +++ b/staging/src/k8s.io/component-base/metrics/histogram.go @@ -46,10 +46,8 @@ type Histogram struct { // NewHistogram returns an object which is Histogram-like. However, nothing // will be measured until the histogram is registered somewhere. func NewHistogram(opts *HistogramOpts) *Histogram { - // todo: handle defaulting better - if opts.StabilityLevel == "" { - opts.StabilityLevel = ALPHA - } + opts.StabilityLevel.setDefaults() + h := &Histogram{ HistogramOpts: opts, lazyMetric: lazyMetric{}, @@ -98,10 +96,8 @@ type HistogramVec struct { // prometheus.HistogramVec object. However, the object returned will not measure // anything unless the collector is first registered, since the metric is lazily instantiated. func NewHistogramVec(opts *HistogramOpts, labels []string) *HistogramVec { - // todo: handle defaulting better - if opts.StabilityLevel == "" { - opts.StabilityLevel = ALPHA - } + opts.StabilityLevel.setDefaults() + v := &HistogramVec{ HistogramVec: noopHistogramVec, HistogramOpts: opts, diff --git a/staging/src/k8s.io/component-base/metrics/summary.go b/staging/src/k8s.io/component-base/metrics/summary.go index ff573782245..338e5085d07 100644 --- a/staging/src/k8s.io/component-base/metrics/summary.go +++ b/staging/src/k8s.io/component-base/metrics/summary.go @@ -37,10 +37,8 @@ type Summary struct { // // DEPRECATED: as per the metrics overhaul KEP func NewSummary(opts *SummaryOpts) *Summary { - // todo: handle defaulting better - if opts.StabilityLevel == "" { - opts.StabilityLevel = ALPHA - } + opts.StabilityLevel.setDefaults() + s := &Summary{ SummaryOpts: opts, lazyMetric: lazyMetric{}, @@ -93,10 +91,8 @@ type SummaryVec struct { // // DEPRECATED: as per the metrics overhaul KEP func NewSummaryVec(opts *SummaryOpts, labels []string) *SummaryVec { - // todo: handle defaulting better - if opts.StabilityLevel == "" { - opts.StabilityLevel = ALPHA - } + opts.StabilityLevel.setDefaults() + v := &SummaryVec{ SummaryOpts: opts, originalLabels: labels,