Merge pull request #86477 from RainbowMango/pr_introduce_promlint

Introduce promlint to guarantee metrics follow Prometheus best practices

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"],

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),
+	}
+}

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])
+			}
+		})
+	}
+}