diff --git a/staging/src/k8s.io/component-base/metrics/testutil/promlint.go b/staging/src/k8s.io/component-base/metrics/testutil/promlint.go index 9e9e479520b..2a8d19b7b3a 100644 --- a/staging/src/k8s.io/component-base/metrics/testutil/promlint.go +++ b/staging/src/k8s.io/component-base/metrics/testutil/promlint.go @@ -17,7 +17,9 @@ limitations under the License. package testutil import ( + "fmt" "io" + "strings" "github.com/prometheus/client_golang/prometheus/testutil/promlint" ) @@ -28,14 +30,26 @@ import ( // 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{ + // k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/egressselector + "apiserver_egress_dialer_dial_failure_count", // counter metrics should have "_total" suffix + + // k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset + "apiserver_flowcontrol_current_inqueue_requests", // label names should be written in 'snake_case' not 'camelCase', + "apiserver_flowcontrol_current_executing_requests", // label names should be written in 'snake_case' not 'camelCase' + "apiserver_flowcontrol_rejected_requests_total", // label names should be written in 'snake_case' not 'camelCase' + + // k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/healthz + "apiserver_request_total", // label names should be written in 'snake_case' not 'camelCase' + + // k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/filters + "authenticated_user_requests", // counter metrics should have "_total" suffix + "authentication_attempts", // counter metrics should have "_total" suffix + // 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", @@ -49,14 +63,22 @@ var exceptionMetrics = []string{ "get_token_fail_count", "node_collector_evictions_number", - // kubelet-resource-v1alpha1 - "container_cpu_usage_seconds_total", - "node_cpu_usage_seconds_total", + // k8s.io/kubernetes/pkg/kubelet/server/stats + "container_cpu_usage_seconds_total", // non-counter metrics should not have "_total" suffix + "node_cpu_usage_seconds_total", // non-counter metrics should not have "_total" suffix + + // k8s.io/kubernetes/pkg/kubelet/pleg + "kubelet_running_container_count", // non-histogram and non-summary metrics should not have "_count" suffix + "kubelet_running_pod_count", // non-histogram and non-summary metrics should not have "_count" suffix } // A Problem is an issue detected by a Linter. type Problem promlint.Problem +func (p *Problem) String() string { + return fmt.Sprintf("%s:%s", p.Metric, p.Text) +} + // 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 { @@ -101,3 +123,42 @@ func NewPromLinter(r io.Reader) *Linter { promLinter: promlint.New(r), } } + +func mergeProblems(problems []Problem) string { + var problemsMsg []string + + for index := range problems { + problemsMsg = append(problemsMsg, problems[index].String()) + } + + return strings.Join(problemsMsg, ",") +} + +// shouldIgnore returns true if metric in the exception list, otherwise returns false. +func shouldIgnore(metricName string) bool { + for i := range exceptionMetrics { + if metricName == exceptionMetrics[i] { + return true + } + } + + return false +} + +// getLintError will ignore the metrics in exception list and converts lint problem to error. +func getLintError(problems []promlint.Problem) error { + var filteredProblems []Problem + for _, problem := range problems { + if shouldIgnore(problem.Metric) { + continue + } + + filteredProblems = append(filteredProblems, Problem(problem)) + } + + if len(filteredProblems) == 0 { + return nil + } + + return fmt.Errorf("lint error: %s", mergeProblems(filteredProblems)) +} 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 index 49e8698a545..72913c33bcb 100644 --- a/staging/src/k8s.io/component-base/metrics/testutil/promlint_test.go +++ b/staging/src/k8s.io/component-base/metrics/testutil/promlint_test.go @@ -59,3 +59,46 @@ func TestLinter(t *testing.T) { }) } } + +func TestMergeProblems(t *testing.T) { + problemOne := Problem{ + Metric: "metric_one", + Text: "problem one", + } + problemTwo := Problem{ + Metric: "metric_two", + Text: "problem two", + } + + var tests = []struct { + name string + problems []Problem + expected string + }{ + { + name: "no problem", + problems: nil, + expected: "", + }, + { + name: "one problem", + problems: []Problem{problemOne}, + expected: "metric_one:problem one", + }, + { + name: "more than one problem", + problems: []Problem{problemOne, problemTwo}, + expected: "metric_one:problem one,metric_two:problem two", + }, + } + + for _, test := range tests { + tc := test + t.Run(tc.name, func(t *testing.T) { + got := mergeProblems(tc.problems) + if tc.expected != got { + t.Errorf("expected: %s, but got: %s", tc.expected, got) + } + }) + } +} diff --git a/staging/src/k8s.io/component-base/metrics/testutil/testutil.go b/staging/src/k8s.io/component-base/metrics/testutil/testutil.go index eefef4aa025..439045989ce 100644 --- a/staging/src/k8s.io/component-base/metrics/testutil/testutil.go +++ b/staging/src/k8s.io/component-base/metrics/testutil/testutil.go @@ -30,6 +30,14 @@ import ( // pedantic Registry. It then does the same as GatherAndCompare, gathering the // metrics from the pedantic Registry. func CollectAndCompare(c metrics.Collector, expected io.Reader, metricNames ...string) error { + lintProblems, err := testutil.CollectAndLint(c, metricNames...) + if err != nil { + return err + } + if err := getLintError(lintProblems); err != nil { + return err + } + return testutil.CollectAndCompare(c, expected, metricNames...) } @@ -38,6 +46,14 @@ func CollectAndCompare(c metrics.Collector, expected io.Reader, metricNames ...s // exposition format. If any metricNames are provided, only metrics with those // names are compared. func GatherAndCompare(g metrics.Gatherer, expected io.Reader, metricNames ...string) error { + lintProblems, err := testutil.GatherAndLint(g, metricNames...) + if err != nil { + return err + } + if err := getLintError(lintProblems); err != nil { + return err + } + return testutil.GatherAndCompare(g, expected, metricNames...) } diff --git a/staging/src/k8s.io/component-base/metrics/testutil/testutil_test.go b/staging/src/k8s.io/component-base/metrics/testutil/testutil_test.go index 9123cb55185..61af603aee8 100644 --- a/staging/src/k8s.io/component-base/metrics/testutil/testutil_test.go +++ b/staging/src/k8s.io/component-base/metrics/testutil/testutil_test.go @@ -27,13 +27,13 @@ func TestNewFakeKubeRegistry(t *testing.T) { registryVersion := "1.18.0" counter := metrics.NewCounter( &metrics.CounterOpts{ - Name: "test_counter_name", + Name: "test_normal_total", Help: "counter help", }, ) deprecatedCounter := metrics.NewCounter( &metrics.CounterOpts{ - Name: "test_deprecated_counter", + Name: "test_deprecated_total", Help: "counter help", DeprecatedVersion: "1.18.0", }, @@ -55,18 +55,18 @@ func TestNewFakeKubeRegistry(t *testing.T) { name: "normal", metric: counter, expected: ` - # HELP test_counter_name [ALPHA] counter help - # TYPE test_counter_name counter - test_counter_name 0 + # HELP test_normal_total [ALPHA] counter help + # TYPE test_normal_total counter + test_normal_total 0 `, }, { name: "deprecated", metric: deprecatedCounter, expected: ` - # HELP test_deprecated_counter [ALPHA] (Deprecated since 1.18.0) counter help - # TYPE test_deprecated_counter counter - test_deprecated_counter 0 + # HELP test_deprecated_total [ALPHA] (Deprecated since 1.18.0) counter help + # TYPE test_deprecated_total counter + test_deprecated_total 0 `, }, {