From 75a6d09348ec542e09da4763093ffd00a0f039c4 Mon Sep 17 00:00:00 2001 From: RainbowMango Date: Fri, 20 Dec 2019 17:34:34 +0800 Subject: [PATCH 1/2] Introduce promlint to guarantee metrics follow Prometheus best practices. --- .../metrics/testutil/promlint.go | 110 ++++++++++++++++++ .../metrics/testutil/promlint_test.go | 61 ++++++++++ 2 files changed, 171 insertions(+) create mode 100644 staging/src/k8s.io/component-base/metrics/testutil/promlint.go create mode 100644 staging/src/k8s.io/component-base/metrics/testutil/promlint_test.go diff --git a/staging/src/k8s.io/component-base/metrics/testutil/promlint.go b/staging/src/k8s.io/component-base/metrics/testutil/promlint.go new file mode 100644 index 00000000000..bc61008bb91 --- /dev/null +++ b/staging/src/k8s.io/component-base/metrics/testutil/promlint.go @@ -0,0 +1,110 @@ +/* +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. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testutil + +import ( + "io" + + "github.com/prometheus/client_golang/prometheus/testutil/promlint" +) + +// exceptionMetrics is an exception list of metrics which violates promlint rules. +// +// The original entries come from the existing metrics when we introduce promlint. +// We setup this list for allow and not fail on the current violations. +// Generally speaking, you need to fix the problem for a new metric rather than add it into the list. +var exceptionMetrics = []string{ + // kube-apiserver + "aggregator_openapi_v2_regeneration_count", + "apiserver_admission_step_admission_duration_seconds_summary", + "apiserver_current_inflight_requests", + "apiserver_longrunning_gauge", + "apiserver_request_total", + "authenticated_user_requests", + "authentication_attempts", + "get_token_count", + "get_token_fail_count", + "ssh_tunnel_open_count", + "ssh_tunnel_open_fail_count", + + // kube-controller-manager + "attachdetach_controller_forced_detaches", + "authenticated_user_requests", + "authentication_attempts", + "get_token_count", + "get_token_fail_count", + "kubernetes_build_info", + "node_collector_evictions_number", + + // kube-proxy + "kubernetes_build_info", + + // kube-scheduler + "scheduler_total_preemption_attempts", + + // kubelet-resource-v1alpha1 + "container_cpu_usage_seconds_total", + "node_cpu_usage_seconds_total", +} + +// A Problem is an issue detected by a Linter. +type Problem promlint.Problem + +// A Linter is a Prometheus metrics linter. It identifies issues with metric +// names, types, and metadata, and reports them to the caller. +type Linter struct { + promLinter *promlint.Linter +} + +// Lint performs a linting pass, returning a slice of Problems indicating any +// issues found in the metrics stream. The slice is sorted by metric name +// and issue description. +func (l *Linter) Lint() ([]Problem, error) { + promProblems, err := l.promLinter.Lint() + if err != nil { + return nil, err + } + + // Ignore problems those in exception list + problems := make([]Problem, 0, len(promProblems)) + for i := range promProblems { + if !l.shouldIgnore(promProblems[i].Metric) { + problems = append(problems, Problem(promProblems[i])) + } + } + + return problems, nil +} + +// shouldIgnore returns true if metric in the exception list, otherwise returns false. +func (l *Linter) shouldIgnore(metricName string) bool { + for i := range exceptionMetrics { + if metricName == exceptionMetrics[i] { + return true + } + } + + return false +} + +// NewPromLinter creates a new Linter that reads an input stream of Prometheus metrics. +// Only the text exposition format is supported. +func NewPromLinter(r io.Reader) *Linter { + return &Linter{ + promLinter: promlint.New(r), + } +} diff --git a/staging/src/k8s.io/component-base/metrics/testutil/promlint_test.go b/staging/src/k8s.io/component-base/metrics/testutil/promlint_test.go new file mode 100644 index 00000000000..49e8698a545 --- /dev/null +++ b/staging/src/k8s.io/component-base/metrics/testutil/promlint_test.go @@ -0,0 +1,61 @@ +/* +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. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testutil + +import ( + "strings" + "testing" +) + +func TestLinter(t *testing.T) { + var tests = []struct { + name string + metric string + expect string + }{ + { + name: "problematic metric should be reported", + metric: ` + # HELP test_problematic_total [ALPHA] non-counter metrics should not have total suffix + # TYPE test_problematic_total gauge + test_problematic_total{some_label="some_value"} 1 + `, + expect: `non-counter metrics should not have "_total" suffix`, + }, + // Don't need to test metrics in exception list, they will be covered by e2e test. + // In addition, we don't need to update this test when we remove metrics from exception list in the future. + } + + for _, test := range tests { + tc := test + t.Run(tc.name, func(t *testing.T) { + linter := NewPromLinter(strings.NewReader(tc.metric)) + problems, err := linter.Lint() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(problems) == 0 { + t.Fatalf("expecte a problem but got none") + } + + if problems[0].Text != tc.expect { + t.Fatalf("expect: %s, but got: %s", tc.expect, problems[0]) + } + }) + } +} From 1d44f63b9dd9e7a6f9f5164d317bdef6bc288e5d Mon Sep 17 00:00:00 2001 From: RainbowMango Date: Tue, 19 May 2020 15:49:37 +0800 Subject: [PATCH 2/2] Update vendor by hack/update-vendor.sh. --- staging/src/k8s.io/component-base/metrics/testutil/BUILD | 3 +++ 1 file changed, 3 insertions(+) diff --git a/staging/src/k8s.io/component-base/metrics/testutil/BUILD b/staging/src/k8s.io/component-base/metrics/testutil/BUILD index ba04be0ae8d..9d7adc8cc48 100644 --- a/staging/src/k8s.io/component-base/metrics/testutil/BUILD +++ b/staging/src/k8s.io/component-base/metrics/testutil/BUILD @@ -4,6 +4,7 @@ go_library( name = "go_default_library", srcs = [ "metrics.go", + "promlint.go", "testutil.go", ], importmap = "k8s.io/kubernetes/vendor/k8s.io/component-base/metrics/testutil", @@ -13,6 +14,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/version:go_default_library", "//staging/src/k8s.io/component-base/metrics:go_default_library", "//vendor/github.com/prometheus/client_golang/prometheus/testutil:go_default_library", + "//vendor/github.com/prometheus/client_golang/prometheus/testutil/promlint:go_default_library", "//vendor/github.com/prometheus/client_model/go:go_default_library", "//vendor/github.com/prometheus/common/expfmt:go_default_library", "//vendor/github.com/prometheus/common/model:go_default_library", @@ -37,6 +39,7 @@ go_test( name = "go_default_test", srcs = [ "metrics_test.go", + "promlint_test.go", "testutil_test.go", ], embed = [":go_default_library"],