From ef0b2dfbf49d36d7c31d57fd603b2f7db652360d Mon Sep 17 00:00:00 2001 From: CatherineF-dev Date: Wed, 10 Nov 2021 03:23:54 +0000 Subject: [PATCH 1/9] Fix metrics AlreadyRegisteredError on TestRecordOperation and TestGetHistogramVecFromGatherer unit test --- .../kuberuntime/instrumented_services_test.go | 23 ++++++++++++------- .../metrics/testutil/metrics_test.go | 15 ++++-------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/pkg/kubelet/kuberuntime/instrumented_services_test.go b/pkg/kubelet/kuberuntime/instrumented_services_test.go index d027414b650..3d3e7e240f4 100644 --- a/pkg/kubelet/kuberuntime/instrumented_services_test.go +++ b/pkg/kubelet/kuberuntime/instrumented_services_test.go @@ -1,12 +1,9 @@ /* Copyright 2016 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. @@ -19,20 +16,27 @@ package kuberuntime import ( "net" "net/http" + "sync" "testing" "time" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/stretchr/testify/assert" - "k8s.io/component-base/metrics/legacyregistry" + compbasemetrics "k8s.io/component-base/metrics" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" "k8s.io/kubernetes/pkg/kubelet/metrics" ) +var registerMetrics sync.Once + func TestRecordOperation(t *testing.T) { - legacyregistry.MustRegister(metrics.RuntimeOperations) - legacyregistry.MustRegister(metrics.RuntimeOperationsDuration) - legacyregistry.MustRegister(metrics.RuntimeOperationsErrors) + // Use local registry + var registry = compbasemetrics.NewKubeRegistry() + registry.MustRegister(metrics.RuntimeOperations) + registry.MustRegister(metrics.RuntimeOperationsDuration) + registry.MustRegister(metrics.RuntimeOperationsErrors) l, err := net.Listen("tcp", "127.0.0.1:0") assert.NoError(t, err) @@ -41,7 +45,8 @@ func TestRecordOperation(t *testing.T) { prometheusURL := "http://" + l.Addr().String() + "/metrics" mux := http.NewServeMux() //lint:ignore SA1019 ignore deprecated warning until we move off of global registries - mux.Handle("/metrics", legacyregistry.Handler()) + handler := promhttp.InstrumentMetricHandler(prometheus.DefaultRegisterer, promhttp.HandlerFor(registry, promhttp.HandlerOpts{})) + mux.Handle("/metrics", handler) server := &http.Server{ Addr: l.Addr().String(), Handler: mux, @@ -61,6 +66,8 @@ func TestRecordOperation(t *testing.T) { assert.HTTPBodyContains(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { mux.ServeHTTP(w, r) }), "GET", prometheusURL, nil, runtimeOperationsDurationExpected) + + registry.Reset() } func TestInstrumentedVersion(t *testing.T) { diff --git a/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go b/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go index 3ee3a28736e..6b0a077cfab 100644 --- a/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go +++ b/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go @@ -1,12 +1,9 @@ /* Copyright 2020 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. @@ -20,13 +17,11 @@ import ( "fmt" "math" "reflect" - "sync" "testing" "github.com/google/go-cmp/cmp" dto "github.com/prometheus/client_model/go" "k8s.io/component-base/metrics" - "k8s.io/component-base/metrics/legacyregistry" "k8s.io/utils/pointer" ) @@ -517,7 +512,6 @@ func TestHistogramVec_Validate(t *testing.T) { } func TestGetHistogramVecFromGatherer(t *testing.T) { - var registerMetrics sync.Once tests := []struct { name string lvMap map[string]string @@ -576,16 +570,17 @@ func TestGetHistogramVecFromGatherer(t *testing.T) { Buckets: buckets, } vec := metrics.NewHistogramVec(HistogramOpts, labels) - registerMetrics.Do(func() { - legacyregistry.MustRegister(vec) - }) + // Use local registry + var registry = metrics.NewKubeRegistry() + var gather metrics.Gatherer = registry + registry.MustRegister(vec) // Observe two metrics with same value for label1 but different value of label2. vec.WithLabelValues("value1-0", "value2-0").Observe(1.5) vec.WithLabelValues("value1-0", "value2-1").Observe(2.5) vec.WithLabelValues("value1-1", "value2-0").Observe(3.5) vec.WithLabelValues("value1-1", "value2-1").Observe(4.5) metricName := fmt.Sprintf("%s_%s_%s", HistogramOpts.Namespace, HistogramOpts.Subsystem, HistogramOpts.Name) - histogramVec, _ := GetHistogramVecFromGatherer(legacyregistry.DefaultGatherer, metricName, tt.lvMap) + histogramVec, _ := GetHistogramVecFromGatherer(gather, metricName, tt.lvMap) if diff := cmp.Diff(tt.wantVec, histogramVec); diff != "" { t.Errorf("Got unexpected HistogramVec (-want +got):\n%s", diff) } From 8290400e9ca73b9619f66ce3ff293190b8ad72f1 Mon Sep 17 00:00:00 2001 From: CatherineF-dev Date: Wed, 10 Nov 2021 03:29:13 +0000 Subject: [PATCH 2/9] format --- pkg/kubelet/kuberuntime/instrumented_services_test.go | 3 +++ .../src/k8s.io/component-base/metrics/testutil/metrics_test.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/pkg/kubelet/kuberuntime/instrumented_services_test.go b/pkg/kubelet/kuberuntime/instrumented_services_test.go index 3d3e7e240f4..ea966a2568a 100644 --- a/pkg/kubelet/kuberuntime/instrumented_services_test.go +++ b/pkg/kubelet/kuberuntime/instrumented_services_test.go @@ -1,9 +1,12 @@ /* Copyright 2016 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. diff --git a/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go b/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go index 6b0a077cfab..c289b6a4902 100644 --- a/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go +++ b/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go @@ -1,9 +1,12 @@ /* Copyright 2020 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. From 744785ee40076e326d62a2be227dcbddfe66ab7a Mon Sep 17 00:00:00 2001 From: CatherineF-dev Date: Fri, 12 Nov 2021 02:12:49 +0000 Subject: [PATCH 3/9] remove prometheus.DefaultRegisterer --- pkg/kubelet/kuberuntime/instrumented_services_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/kubelet/kuberuntime/instrumented_services_test.go b/pkg/kubelet/kuberuntime/instrumented_services_test.go index ea966a2568a..aa5ab10f9f2 100644 --- a/pkg/kubelet/kuberuntime/instrumented_services_test.go +++ b/pkg/kubelet/kuberuntime/instrumented_services_test.go @@ -23,7 +23,6 @@ import ( "testing" "time" - "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/stretchr/testify/assert" @@ -48,7 +47,7 @@ func TestRecordOperation(t *testing.T) { prometheusURL := "http://" + l.Addr().String() + "/metrics" mux := http.NewServeMux() //lint:ignore SA1019 ignore deprecated warning until we move off of global registries - handler := promhttp.InstrumentMetricHandler(prometheus.DefaultRegisterer, promhttp.HandlerFor(registry, promhttp.HandlerOpts{})) + handler := promhttp.HandlerFor(registry, promhttp.HandlerOpts{}) mux.Handle("/metrics", handler) server := &http.Server{ Addr: l.Addr().String(), From 03f7a8d7d26eb42ecf065546ba4a6038100417b4 Mon Sep 17 00:00:00 2001 From: CatherineF-dev Date: Fri, 12 Nov 2021 03:49:58 +0000 Subject: [PATCH 4/9] add local registry.Reset() --- .../src/k8s.io/component-base/metrics/testutil/metrics_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go b/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go index c289b6a4902..4f916c155b5 100644 --- a/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go +++ b/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go @@ -587,6 +587,8 @@ func TestGetHistogramVecFromGatherer(t *testing.T) { if diff := cmp.Diff(tt.wantVec, histogramVec); diff != "" { t.Errorf("Got unexpected HistogramVec (-want +got):\n%s", diff) } + + registry.Reset() }) } } From a8324a3bb750e02a2cdf0ad6d9935f7c783a375c Mon Sep 17 00:00:00 2001 From: CatherineF-dev Date: Fri, 12 Nov 2021 03:52:19 +0000 Subject: [PATCH 5/9] clean --- pkg/kubelet/kuberuntime/instrumented_services_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/kubelet/kuberuntime/instrumented_services_test.go b/pkg/kubelet/kuberuntime/instrumented_services_test.go index aa5ab10f9f2..cdc31b142c4 100644 --- a/pkg/kubelet/kuberuntime/instrumented_services_test.go +++ b/pkg/kubelet/kuberuntime/instrumented_services_test.go @@ -19,7 +19,6 @@ package kuberuntime import ( "net" "net/http" - "sync" "testing" "time" @@ -31,8 +30,6 @@ import ( "k8s.io/kubernetes/pkg/kubelet/metrics" ) -var registerMetrics sync.Once - func TestRecordOperation(t *testing.T) { // Use local registry var registry = compbasemetrics.NewKubeRegistry() From a30af261f1abd2da6deb4c7e2a7ffda224559712 Mon Sep 17 00:00:00 2001 From: CatherineF-dev Date: Fri, 12 Nov 2021 15:03:44 +0000 Subject: [PATCH 6/9] remove lint --- pkg/kubelet/kuberuntime/instrumented_services_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/kubelet/kuberuntime/instrumented_services_test.go b/pkg/kubelet/kuberuntime/instrumented_services_test.go index cdc31b142c4..0702ef638f6 100644 --- a/pkg/kubelet/kuberuntime/instrumented_services_test.go +++ b/pkg/kubelet/kuberuntime/instrumented_services_test.go @@ -43,7 +43,6 @@ func TestRecordOperation(t *testing.T) { prometheusURL := "http://" + l.Addr().String() + "/metrics" mux := http.NewServeMux() - //lint:ignore SA1019 ignore deprecated warning until we move off of global registries handler := promhttp.HandlerFor(registry, promhttp.HandlerOpts{}) mux.Handle("/metrics", handler) server := &http.Server{ From 49d341aa2b9fe7e6266173fb846e1efcc2cf9ec9 Mon Sep 17 00:00:00 2001 From: CatherineF-dev Date: Fri, 12 Nov 2021 23:03:38 +0000 Subject: [PATCH 7/9] Use defer in non-loop --- pkg/kubelet/kuberuntime/instrumented_services_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/kubelet/kuberuntime/instrumented_services_test.go b/pkg/kubelet/kuberuntime/instrumented_services_test.go index 0702ef638f6..b343320c8a2 100644 --- a/pkg/kubelet/kuberuntime/instrumented_services_test.go +++ b/pkg/kubelet/kuberuntime/instrumented_services_test.go @@ -33,6 +33,7 @@ import ( func TestRecordOperation(t *testing.T) { // Use local registry var registry = compbasemetrics.NewKubeRegistry() + defer registry.Reset() registry.MustRegister(metrics.RuntimeOperations) registry.MustRegister(metrics.RuntimeOperationsDuration) registry.MustRegister(metrics.RuntimeOperationsErrors) @@ -64,8 +65,6 @@ func TestRecordOperation(t *testing.T) { assert.HTTPBodyContains(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { mux.ServeHTTP(w, r) }), "GET", prometheusURL, nil, runtimeOperationsDurationExpected) - - registry.Reset() } func TestInstrumentedVersion(t *testing.T) { From d9737eabf49f7e228eb32bf70ef2936804b0694c Mon Sep 17 00:00:00 2001 From: CatherineF-dev Date: Fri, 12 Nov 2021 23:09:51 +0000 Subject: [PATCH 8/9] Use HandlerFor --- pkg/kubelet/kuberuntime/instrumented_services_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/kubelet/kuberuntime/instrumented_services_test.go b/pkg/kubelet/kuberuntime/instrumented_services_test.go index b343320c8a2..e95d6bdb74a 100644 --- a/pkg/kubelet/kuberuntime/instrumented_services_test.go +++ b/pkg/kubelet/kuberuntime/instrumented_services_test.go @@ -22,7 +22,6 @@ import ( "testing" "time" - "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/stretchr/testify/assert" compbasemetrics "k8s.io/component-base/metrics" @@ -33,6 +32,7 @@ import ( func TestRecordOperation(t *testing.T) { // Use local registry var registry = compbasemetrics.NewKubeRegistry() + var gather compbasemetrics.Gatherer = registry defer registry.Reset() registry.MustRegister(metrics.RuntimeOperations) registry.MustRegister(metrics.RuntimeOperationsDuration) @@ -44,7 +44,7 @@ func TestRecordOperation(t *testing.T) { prometheusURL := "http://" + l.Addr().String() + "/metrics" mux := http.NewServeMux() - handler := promhttp.HandlerFor(registry, promhttp.HandlerOpts{}) + handler := compbasemetrics.HandlerFor(gather, compbasemetrics.HandlerOpts{}) mux.Handle("/metrics", handler) server := &http.Server{ Addr: l.Addr().String(), From 5646120fbbe5fe07e64a1a1a5680af8564b3d682 Mon Sep 17 00:00:00 2001 From: CatherineF-dev Date: Tue, 16 Nov 2021 18:57:24 +0000 Subject: [PATCH 9/9] Use Reset at first --- pkg/kubelet/kuberuntime/instrumented_services_test.go | 4 +++- .../k8s.io/component-base/metrics/testutil/metrics_test.go | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/kubelet/kuberuntime/instrumented_services_test.go b/pkg/kubelet/kuberuntime/instrumented_services_test.go index e95d6bdb74a..7919c552890 100644 --- a/pkg/kubelet/kuberuntime/instrumented_services_test.go +++ b/pkg/kubelet/kuberuntime/instrumented_services_test.go @@ -33,11 +33,13 @@ func TestRecordOperation(t *testing.T) { // Use local registry var registry = compbasemetrics.NewKubeRegistry() var gather compbasemetrics.Gatherer = registry - defer registry.Reset() + registry.MustRegister(metrics.RuntimeOperations) registry.MustRegister(metrics.RuntimeOperationsDuration) registry.MustRegister(metrics.RuntimeOperationsErrors) + registry.Reset() + l, err := net.Listen("tcp", "127.0.0.1:0") assert.NoError(t, err) defer l.Close() diff --git a/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go b/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go index 4f916c155b5..c289b6a4902 100644 --- a/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go +++ b/staging/src/k8s.io/component-base/metrics/testutil/metrics_test.go @@ -587,8 +587,6 @@ func TestGetHistogramVecFromGatherer(t *testing.T) { if diff := cmp.Diff(tt.wantVec, histogramVec); diff != "" { t.Errorf("Got unexpected HistogramVec (-want +got):\n%s", diff) } - - registry.Reset() }) } }