From c39535eb56d751db3f9a83cc481d618a6fc36dc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Vila=C3=A7a?= Date: Thu, 13 Jul 2023 23:53:49 +0100 Subject: [PATCH] Refactor promlint validation structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: João Vilaça --- prometheus/testutil/lint.go | 5 +- prometheus/testutil/promlint/promlint.go | 297 +----------------- prometheus/testutil/promlint/promlint_test.go | 83 ++--- .../validations/counter_validations.go | 39 +++ .../validations/generic_name_validations.go | 100 ++++++ .../promlint/validations/help_validations.go | 28 ++ .../validations/histogram_validations.go | 62 ++++ .../testutil/promlint/validations/problem.go | 33 ++ .../testutil/promlint/validations/units.go | 118 +++++++ .../promlint/validations/validation.go | 31 ++ 10 files changed, 463 insertions(+), 333 deletions(-) create mode 100644 prometheus/testutil/promlint/validations/counter_validations.go create mode 100644 prometheus/testutil/promlint/validations/generic_name_validations.go create mode 100644 prometheus/testutil/promlint/validations/help_validations.go create mode 100644 prometheus/testutil/promlint/validations/histogram_validations.go create mode 100644 prometheus/testutil/promlint/validations/problem.go create mode 100644 prometheus/testutil/promlint/validations/units.go create mode 100644 prometheus/testutil/promlint/validations/validation.go diff --git a/prometheus/testutil/lint.go b/prometheus/testutil/lint.go index 8d2f05500..ec990c246 100644 --- a/prometheus/testutil/lint.go +++ b/prometheus/testutil/lint.go @@ -18,12 +18,13 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil/promlint" + "github.com/prometheus/client_golang/prometheus/testutil/promlint/validations" ) // CollectAndLint registers the provided Collector with a newly created pedantic // Registry. It then calls GatherAndLint with that Registry and with the // provided metricNames. -func CollectAndLint(c prometheus.Collector, metricNames ...string) ([]promlint.Problem, error) { +func CollectAndLint(c prometheus.Collector, metricNames ...string) ([]validations.Problem, error) { reg := prometheus.NewPedanticRegistry() if err := reg.Register(c); err != nil { return nil, fmt.Errorf("registering collector failed: %w", err) @@ -34,7 +35,7 @@ func CollectAndLint(c prometheus.Collector, metricNames ...string) ([]promlint.P // GatherAndLint gathers all metrics from the provided Gatherer and checks them // with the linter in the promlint package. If any metricNames are provided, // only metrics with those names are checked. -func GatherAndLint(g prometheus.Gatherer, metricNames ...string) ([]promlint.Problem, error) { +func GatherAndLint(g prometheus.Gatherer, metricNames ...string) ([]validations.Problem, error) { got, err := g.Gather() if err != nil { return nil, fmt.Errorf("gathering metrics failed: %w", err) diff --git a/prometheus/testutil/promlint/promlint.go b/prometheus/testutil/promlint/promlint.go index c8864b6c3..017409d0a 100644 --- a/prometheus/testutil/promlint/promlint.go +++ b/prometheus/testutil/promlint/promlint.go @@ -16,15 +16,13 @@ package promlint import ( "errors" - "fmt" "io" - "regexp" "sort" - "strings" + dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/expfmt" - dto "github.com/prometheus/client_model/go" + "github.com/prometheus/client_golang/prometheus/testutil/promlint/validations" ) // A Linter is a Prometheus metrics linter. It identifies issues with metric @@ -39,23 +37,6 @@ type Linter struct { mfs []*dto.MetricFamily } -// A Problem is an issue detected by a Linter. -type Problem struct { - // The name of the metric indicated by this Problem. - Metric string - - // A description of the issue for this Problem. - Text string -} - -// newProblem is helper function to create a Problem. -func newProblem(mf *dto.MetricFamily, text string) Problem { - return Problem{ - Metric: mf.GetName(), - Text: text, - } -} - // New creates a new Linter that reads an input stream of Prometheus metrics in // the Prometheus text exposition format. func New(r io.Reader) *Linter { @@ -75,8 +56,8 @@ func NewWithMetricFamilies(mfs []*dto.MetricFamily) *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) { - var problems []Problem +func (l *Linter) Lint() ([]validations.Problem, error) { + var problems []validations.Problem if l.r != nil { d := expfmt.NewDecoder(l.r, expfmt.FmtText) @@ -110,276 +91,12 @@ func (l *Linter) Lint() ([]Problem, error) { } // lint is the entry point for linting a single metric. -func lint(mf *dto.MetricFamily) []Problem { - fns := []func(mf *dto.MetricFamily) []Problem{ - lintHelp, - lintMetricUnits, - lintCounter, - lintHistogramSummaryReserved, - lintMetricTypeInName, - lintReservedChars, - lintCamelCase, - lintUnitAbbreviations, - } - - var problems []Problem - for _, fn := range fns { +func lint(mf *dto.MetricFamily) []validations.Problem { + var problems []validations.Problem + for _, fn := range validations.DefaultValidations { problems = append(problems, fn(mf)...) } // TODO(mdlayher): lint rules for specific metrics types. return problems } - -// lintHelp detects issues related to the help text for a metric. -func lintHelp(mf *dto.MetricFamily) []Problem { - var problems []Problem - - // Expect all metrics to have help text available. - if mf.Help == nil { - problems = append(problems, newProblem(mf, "no help text")) - } - - return problems -} - -// lintMetricUnits detects issues with metric unit names. -func lintMetricUnits(mf *dto.MetricFamily) []Problem { - var problems []Problem - - unit, base, ok := metricUnits(*mf.Name) - if !ok { - // No known units detected. - return nil - } - - // Unit is already a base unit. - if unit == base { - return nil - } - - problems = append(problems, newProblem(mf, fmt.Sprintf("use base unit %q instead of %q", base, unit))) - - return problems -} - -// lintCounter detects issues specific to counters, as well as patterns that should -// only be used with counters. -func lintCounter(mf *dto.MetricFamily) []Problem { - var problems []Problem - - isCounter := mf.GetType() == dto.MetricType_COUNTER - isUntyped := mf.GetType() == dto.MetricType_UNTYPED - hasTotalSuffix := strings.HasSuffix(mf.GetName(), "_total") - - switch { - case isCounter && !hasTotalSuffix: - problems = append(problems, newProblem(mf, `counter metrics should have "_total" suffix`)) - case !isUntyped && !isCounter && hasTotalSuffix: - problems = append(problems, newProblem(mf, `non-counter metrics should not have "_total" suffix`)) - } - - return problems -} - -// lintHistogramSummaryReserved detects when other types of metrics use names or labels -// reserved for use by histograms and/or summaries. -func lintHistogramSummaryReserved(mf *dto.MetricFamily) []Problem { - // These rules do not apply to untyped metrics. - t := mf.GetType() - if t == dto.MetricType_UNTYPED { - return nil - } - - var problems []Problem - - isHistogram := t == dto.MetricType_HISTOGRAM - isSummary := t == dto.MetricType_SUMMARY - - n := mf.GetName() - - if !isHistogram && strings.HasSuffix(n, "_bucket") { - problems = append(problems, newProblem(mf, `non-histogram metrics should not have "_bucket" suffix`)) - } - if !isHistogram && !isSummary && strings.HasSuffix(n, "_count") { - problems = append(problems, newProblem(mf, `non-histogram and non-summary metrics should not have "_count" suffix`)) - } - if !isHistogram && !isSummary && strings.HasSuffix(n, "_sum") { - problems = append(problems, newProblem(mf, `non-histogram and non-summary metrics should not have "_sum" suffix`)) - } - - for _, m := range mf.GetMetric() { - for _, l := range m.GetLabel() { - ln := l.GetName() - - if !isHistogram && ln == "le" { - problems = append(problems, newProblem(mf, `non-histogram metrics should not have "le" label`)) - } - if !isSummary && ln == "quantile" { - problems = append(problems, newProblem(mf, `non-summary metrics should not have "quantile" label`)) - } - } - } - - return problems -} - -// lintMetricTypeInName detects when metric types are included in the metric name. -func lintMetricTypeInName(mf *dto.MetricFamily) []Problem { - var problems []Problem - n := strings.ToLower(mf.GetName()) - - for i, t := range dto.MetricType_name { - if i == int32(dto.MetricType_UNTYPED) { - continue - } - - typename := strings.ToLower(t) - if strings.Contains(n, "_"+typename+"_") || strings.HasSuffix(n, "_"+typename) { - problems = append(problems, newProblem(mf, fmt.Sprintf(`metric name should not include type '%s'`, typename))) - } - } - return problems -} - -// lintReservedChars detects colons in metric names. -func lintReservedChars(mf *dto.MetricFamily) []Problem { - var problems []Problem - if strings.Contains(mf.GetName(), ":") { - problems = append(problems, newProblem(mf, "metric names should not contain ':'")) - } - return problems -} - -var camelCase = regexp.MustCompile(`[a-z][A-Z]`) - -// lintCamelCase detects metric names and label names written in camelCase. -func lintCamelCase(mf *dto.MetricFamily) []Problem { - var problems []Problem - if camelCase.FindString(mf.GetName()) != "" { - problems = append(problems, newProblem(mf, "metric names should be written in 'snake_case' not 'camelCase'")) - } - - for _, m := range mf.GetMetric() { - for _, l := range m.GetLabel() { - if camelCase.FindString(l.GetName()) != "" { - problems = append(problems, newProblem(mf, "label names should be written in 'snake_case' not 'camelCase'")) - } - } - } - return problems -} - -// lintUnitAbbreviations detects abbreviated units in the metric name. -func lintUnitAbbreviations(mf *dto.MetricFamily) []Problem { - var problems []Problem - n := strings.ToLower(mf.GetName()) - for _, s := range unitAbbreviations { - if strings.Contains(n, "_"+s+"_") || strings.HasSuffix(n, "_"+s) { - problems = append(problems, newProblem(mf, "metric names should not contain abbreviated units")) - } - } - return problems -} - -// metricUnits attempts to detect known unit types used as part of a metric name, -// e.g. "foo_bytes_total" or "bar_baz_milligrams". -func metricUnits(m string) (unit, base string, ok bool) { - ss := strings.Split(m, "_") - - for _, s := range ss { - if base, found := units[s]; found { - return s, base, true - } - - for _, p := range unitPrefixes { - if strings.HasPrefix(s, p) { - if base, found := units[s[len(p):]]; found { - return s, base, true - } - } - } - } - - return "", "", false -} - -// Units and their possible prefixes recognized by this library. More can be -// added over time as needed. -var ( - // map a unit to the appropriate base unit. - units = map[string]string{ - // Base units. - "amperes": "amperes", - "bytes": "bytes", - "celsius": "celsius", // Also allow Celsius because it is common in typical Prometheus use cases. - "grams": "grams", - "joules": "joules", - "kelvin": "kelvin", // SI base unit, used in special cases (e.g. color temperature, scientific measurements). - "meters": "meters", // Both American and international spelling permitted. - "metres": "metres", - "seconds": "seconds", - "volts": "volts", - - // Non base units. - // Time. - "minutes": "seconds", - "hours": "seconds", - "days": "seconds", - "weeks": "seconds", - // Temperature. - "kelvins": "kelvin", - "fahrenheit": "celsius", - "rankine": "celsius", - // Length. - "inches": "meters", - "yards": "meters", - "miles": "meters", - // Bytes. - "bits": "bytes", - // Energy. - "calories": "joules", - // Mass. - "pounds": "grams", - "ounces": "grams", - } - - unitPrefixes = []string{ - "pico", - "nano", - "micro", - "milli", - "centi", - "deci", - "deca", - "hecto", - "kilo", - "kibi", - "mega", - "mibi", - "giga", - "gibi", - "tera", - "tebi", - "peta", - "pebi", - } - - // Common abbreviations that we'd like to discourage. - unitAbbreviations = []string{ - "s", - "ms", - "us", - "ns", - "sec", - "b", - "kb", - "mb", - "gb", - "tb", - "pb", - "m", - "h", - "d", - } -) diff --git a/prometheus/testutil/promlint/promlint_test.go b/prometheus/testutil/promlint/promlint_test.go index 70b1908cd..b2df5d230 100644 --- a/prometheus/testutil/promlint/promlint_test.go +++ b/prometheus/testutil/promlint/promlint_test.go @@ -20,12 +20,13 @@ import ( "testing" "github.com/prometheus/client_golang/prometheus/testutil/promlint" + "github.com/prometheus/client_golang/prometheus/testutil/promlint/validations" ) type test struct { name string in string - problems []promlint.Problem + problems []validations.Problem } func TestLintNoHelpText(t *testing.T) { @@ -38,7 +39,7 @@ func TestLintNoHelpText(t *testing.T) { # TYPE go_goroutines gauge go_goroutines 24 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "go_goroutines", Text: msg, }}, @@ -50,7 +51,7 @@ go_goroutines 24 # TYPE go_goroutines gauge go_goroutines 24 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "go_goroutines", Text: msg, }}, @@ -64,7 +65,7 @@ go_goroutines 24 # TYPE go_threads gauge go_threads 10 `, - problems: []promlint.Problem{ + problems: []validations.Problem{ { Metric: "go_goroutines", Text: msg, @@ -91,7 +92,7 @@ func TestLintMetricUnits(t *testing.T) { tests := []struct { name string in string - problems []promlint.Problem + problems []validations.Problem }{ // good cases. { @@ -182,7 +183,7 @@ x_kelvin 10 # TYPE x_milliamperes untyped x_milliamperes 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_milliamperes", Text: `use base unit "amperes" instead of "milliamperes"`, }}, @@ -194,7 +195,7 @@ x_milliamperes 10 # TYPE x_gigabytes untyped x_gigabytes 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_gigabytes", Text: `use base unit "bytes" instead of "gigabytes"`, }}, @@ -206,7 +207,7 @@ x_gigabytes 10 # TYPE x_kilograms untyped x_kilograms 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_kilograms", Text: `use base unit "grams" instead of "kilograms"`, }}, @@ -218,7 +219,7 @@ x_kilograms 10 # TYPE x_nanocelsius untyped x_nanocelsius 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_nanocelsius", Text: `use base unit "celsius" instead of "nanocelsius"`, }}, @@ -230,7 +231,7 @@ x_nanocelsius 10 # TYPE x_kilometers untyped x_kilometers 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_kilometers", Text: `use base unit "meters" instead of "kilometers"`, }}, @@ -242,7 +243,7 @@ x_kilometers 10 # TYPE x_picometers untyped x_picometers 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_picometers", Text: `use base unit "meters" instead of "picometers"`, }}, @@ -254,7 +255,7 @@ x_picometers 10 # TYPE x_microseconds untyped x_microseconds 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_microseconds", Text: `use base unit "seconds" instead of "microseconds"`, }}, @@ -266,7 +267,7 @@ x_microseconds 10 # TYPE x_minutes untyped x_minutes 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_minutes", Text: `use base unit "seconds" instead of "minutes"`, }}, @@ -278,7 +279,7 @@ x_minutes 10 # TYPE x_hours untyped x_hours 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_hours", Text: `use base unit "seconds" instead of "hours"`, }}, @@ -290,7 +291,7 @@ x_hours 10 # TYPE x_days untyped x_days 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_days", Text: `use base unit "seconds" instead of "days"`, }}, @@ -302,7 +303,7 @@ x_days 10 # TYPE x_kelvins untyped x_kelvins 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_kelvins", Text: `use base unit "kelvin" instead of "kelvins"`, }}, @@ -314,7 +315,7 @@ x_kelvins 10 # TYPE thermometers_fahrenheit untyped thermometers_fahrenheit 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "thermometers_fahrenheit", Text: `use base unit "celsius" instead of "fahrenheit"`, }}, @@ -326,7 +327,7 @@ thermometers_fahrenheit 10 # TYPE thermometers_rankine untyped thermometers_rankine 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "thermometers_rankine", Text: `use base unit "celsius" instead of "rankine"`, }}, @@ -338,7 +339,7 @@ thermometers_rankine 10 # TYPE x_inches untyped x_inches 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_inches", Text: `use base unit "meters" instead of "inches"`, }}, @@ -350,7 +351,7 @@ x_inches 10 # TYPE x_yards untyped x_yards 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_yards", Text: `use base unit "meters" instead of "yards"`, }}, @@ -362,7 +363,7 @@ x_yards 10 # TYPE x_miles untyped x_miles 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_miles", Text: `use base unit "meters" instead of "miles"`, }}, @@ -374,7 +375,7 @@ x_miles 10 # TYPE x_bits untyped x_bits 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_bits", Text: `use base unit "bytes" instead of "bits"`, }}, @@ -386,7 +387,7 @@ x_bits 10 # TYPE x_calories untyped x_calories 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_calories", Text: `use base unit "joules" instead of "calories"`, }}, @@ -398,7 +399,7 @@ x_calories 10 # TYPE x_pounds untyped x_pounds 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_pounds", Text: `use base unit "grams" instead of "pounds"`, }}, @@ -410,7 +411,7 @@ x_pounds 10 # TYPE x_ounces untyped x_ounces 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_ounces", Text: `use base unit "grams" instead of "ounces"`, }}, @@ -443,7 +444,7 @@ func TestLintCounter(t *testing.T) { # TYPE x_bytes counter x_bytes 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_bytes", Text: `counter metrics should have "_total" suffix`, }}, @@ -455,7 +456,7 @@ x_bytes 10 # TYPE x_bytes_total gauge x_bytes_total 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_bytes_total", Text: `non-counter metrics should not have "_total" suffix`, }}, @@ -506,7 +507,7 @@ func TestLintHistogramSummaryReserved(t *testing.T) { # TYPE x_bytes_bucket gauge x_bytes_bucket 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_bytes_bucket", Text: `non-histogram metrics should not have "_bucket" suffix`, }}, @@ -518,7 +519,7 @@ x_bytes_bucket 10 # TYPE x_bytes_count gauge x_bytes_count 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_bytes_count", Text: `non-histogram and non-summary metrics should not have "_count" suffix`, }}, @@ -530,7 +531,7 @@ x_bytes_count 10 # TYPE x_bytes_sum gauge x_bytes_sum 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_bytes_sum", Text: `non-histogram and non-summary metrics should not have "_sum" suffix`, }}, @@ -542,7 +543,7 @@ x_bytes_sum 10 # TYPE x_bytes gauge x_bytes{le="1"} 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_bytes", Text: `non-histogram metrics should not have "le" label`, }}, @@ -554,7 +555,7 @@ x_bytes{le="1"} 10 # TYPE x_bytes gauge x_bytes{quantile="1"} 10 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "x_bytes", Text: `non-summary metrics should not have "quantile" label`, }}, @@ -579,7 +580,7 @@ tsdb_compaction_duration_bucket{le="+Inf",quantile="0.01"} 69 tsdb_compaction_duration_sum 28.740810936000006 tsdb_compaction_duration_count 69 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "tsdb_compaction_duration", Text: `non-summary metrics should not have "quantile" label`, }}, @@ -597,7 +598,7 @@ go_gc_duration_seconds{quantile="1",le="0.01"} 0.021754305 go_gc_duration_seconds_sum 1.769429004 go_gc_duration_seconds_count 5962 `, - problems: []promlint.Problem{{ + problems: []validations.Problem{{ Metric: "go_gc_duration_seconds", Text: `non-histogram metrics should not have "le" label`, }}, @@ -642,7 +643,7 @@ go_gc_duration_seconds_count 5962 } func TestLintMetricTypeInName(t *testing.T) { - genTest := func(n, t, err string, problems ...promlint.Problem) test { + genTest := func(n, t, err string, problems ...validations.Problem) test { return test{ name: fmt.Sprintf("%s with _%s suffix", t, t), in: fmt.Sprintf(` @@ -650,14 +651,14 @@ func TestLintMetricTypeInName(t *testing.T) { # TYPE %s %s %s 10 `, n, n, t, n), - problems: append(problems, promlint.Problem{ + problems: append(problems, validations.Problem{ Metric: n, Text: fmt.Sprintf(`metric name should not include type '%s'`, err), }), } } - twoProbTest := genTest("http_requests_counter", "counter", "counter", promlint.Problem{ + twoProbTest := genTest("http_requests_counter", "counter", "counter", validations.Problem{ Metric: "http_requests_counter", Text: `counter metrics should have "_total" suffix`, }) @@ -684,7 +685,7 @@ func TestLintReservedChars(t *testing.T) { # TYPE request_duration::_seconds histogram request_duration::_seconds 10 `, - problems: []promlint.Problem{ + problems: []validations.Problem{ { Metric: "request_duration::_seconds", Text: "metric names should not contain ':'", @@ -704,7 +705,7 @@ func TestLintCamelCase(t *testing.T) { # TYPE requestDuration_seconds histogram requestDuration_seconds 10 `, - problems: []promlint.Problem{ + problems: []validations.Problem{ { Metric: "requestDuration_seconds", Text: "metric names should be written in 'snake_case' not 'camelCase'", @@ -718,7 +719,7 @@ requestDuration_seconds 10 # TYPE request_duration_seconds histogram request_duration_seconds{httpService="foo"} 10 `, - problems: []promlint.Problem{ + problems: []validations.Problem{ { Metric: "request_duration_seconds", Text: "label names should be written in 'snake_case' not 'camelCase'", @@ -738,7 +739,7 @@ func TestLintUnitAbbreviations(t *testing.T) { # TYPE %s gauge %s 10 `, n, n, n), - problems: []promlint.Problem{ + problems: []validations.Problem{ { Metric: n, Text: "metric names should not contain abbreviated units", diff --git a/prometheus/testutil/promlint/validations/counter_validations.go b/prometheus/testutil/promlint/validations/counter_validations.go new file mode 100644 index 000000000..f8c9dc947 --- /dev/null +++ b/prometheus/testutil/promlint/validations/counter_validations.go @@ -0,0 +1,39 @@ +// Copyright 2020 The Prometheus 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 validations + +import ( + "strings" + + dto "github.com/prometheus/client_model/go" +) + +// lintCounter detects issues specific to counters, as well as patterns that should +// only be used with counters. +func lintCounter(mf *dto.MetricFamily) []Problem { + var problems []Problem + + isCounter := mf.GetType() == dto.MetricType_COUNTER + isUntyped := mf.GetType() == dto.MetricType_UNTYPED + hasTotalSuffix := strings.HasSuffix(mf.GetName(), "_total") + + switch { + case isCounter && !hasTotalSuffix: + problems = append(problems, newProblem(mf, `counter metrics should have "_total" suffix`)) + case !isUntyped && !isCounter && hasTotalSuffix: + problems = append(problems, newProblem(mf, `non-counter metrics should not have "_total" suffix`)) + } + + return problems +} diff --git a/prometheus/testutil/promlint/validations/generic_name_validations.go b/prometheus/testutil/promlint/validations/generic_name_validations.go new file mode 100644 index 000000000..673a89156 --- /dev/null +++ b/prometheus/testutil/promlint/validations/generic_name_validations.go @@ -0,0 +1,100 @@ +// Copyright 2020 The Prometheus 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 validations + +import ( + "fmt" + "regexp" + "strings" + + dto "github.com/prometheus/client_model/go" +) + +var camelCase = regexp.MustCompile(`[a-z][A-Z]`) + +// lintMetricUnits detects issues with metric unit names. +func lintMetricUnits(mf *dto.MetricFamily) []Problem { + var problems []Problem + + unit, base, ok := metricUnits(*mf.Name) + if !ok { + // No known units detected. + return nil + } + + // Unit is already a base unit. + if unit == base { + return nil + } + + problems = append(problems, newProblem(mf, fmt.Sprintf("use base unit %q instead of %q", base, unit))) + + return problems +} + +// lintMetricTypeInName detects when metric types are included in the metric name. +func lintMetricTypeInName(mf *dto.MetricFamily) []Problem { + var problems []Problem + n := strings.ToLower(mf.GetName()) + + for i, t := range dto.MetricType_name { + if i == int32(dto.MetricType_UNTYPED) { + continue + } + + typename := strings.ToLower(t) + if strings.Contains(n, "_"+typename+"_") || strings.HasSuffix(n, "_"+typename) { + problems = append(problems, newProblem(mf, fmt.Sprintf(`metric name should not include type '%s'`, typename))) + } + } + return problems +} + +// lintReservedChars detects colons in metric names. +func lintReservedChars(mf *dto.MetricFamily) []Problem { + var problems []Problem + if strings.Contains(mf.GetName(), ":") { + problems = append(problems, newProblem(mf, "metric names should not contain ':'")) + } + return problems +} + +// lintCamelCase detects metric names and label names written in camelCase. +func lintCamelCase(mf *dto.MetricFamily) []Problem { + var problems []Problem + if camelCase.FindString(mf.GetName()) != "" { + problems = append(problems, newProblem(mf, "metric names should be written in 'snake_case' not 'camelCase'")) + } + + for _, m := range mf.GetMetric() { + for _, l := range m.GetLabel() { + if camelCase.FindString(l.GetName()) != "" { + problems = append(problems, newProblem(mf, "label names should be written in 'snake_case' not 'camelCase'")) + } + } + } + return problems +} + +// lintUnitAbbreviations detects abbreviated units in the metric name. +func lintUnitAbbreviations(mf *dto.MetricFamily) []Problem { + var problems []Problem + n := strings.ToLower(mf.GetName()) + for _, s := range unitAbbreviations { + if strings.Contains(n, "_"+s+"_") || strings.HasSuffix(n, "_"+s) { + problems = append(problems, newProblem(mf, "metric names should not contain abbreviated units")) + } + } + return problems +} diff --git a/prometheus/testutil/promlint/validations/help_validations.go b/prometheus/testutil/promlint/validations/help_validations.go new file mode 100644 index 000000000..41f82855b --- /dev/null +++ b/prometheus/testutil/promlint/validations/help_validations.go @@ -0,0 +1,28 @@ +// Copyright 2020 The Prometheus 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 validations + +import dto "github.com/prometheus/client_model/go" + +// lintHelp detects issues related to the help text for a metric. +func lintHelp(mf *dto.MetricFamily) []Problem { + var problems []Problem + + // Expect all metrics to have help text available. + if mf.Help == nil { + problems = append(problems, newProblem(mf, "no help text")) + } + + return problems +} diff --git a/prometheus/testutil/promlint/validations/histogram_validations.go b/prometheus/testutil/promlint/validations/histogram_validations.go new file mode 100644 index 000000000..bb5e9dc46 --- /dev/null +++ b/prometheus/testutil/promlint/validations/histogram_validations.go @@ -0,0 +1,62 @@ +// Copyright 2020 The Prometheus 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 validations + +import ( + "strings" + + dto "github.com/prometheus/client_model/go" +) + +// lintHistogramSummaryReserved detects when other types of metrics use names or labels +// reserved for use by histograms and/or summaries. +func lintHistogramSummaryReserved(mf *dto.MetricFamily) []Problem { + // These rules do not apply to untyped metrics. + t := mf.GetType() + if t == dto.MetricType_UNTYPED { + return nil + } + + var problems []Problem + + isHistogram := t == dto.MetricType_HISTOGRAM + isSummary := t == dto.MetricType_SUMMARY + + n := mf.GetName() + + if !isHistogram && strings.HasSuffix(n, "_bucket") { + problems = append(problems, newProblem(mf, `non-histogram metrics should not have "_bucket" suffix`)) + } + if !isHistogram && !isSummary && strings.HasSuffix(n, "_count") { + problems = append(problems, newProblem(mf, `non-histogram and non-summary metrics should not have "_count" suffix`)) + } + if !isHistogram && !isSummary && strings.HasSuffix(n, "_sum") { + problems = append(problems, newProblem(mf, `non-histogram and non-summary metrics should not have "_sum" suffix`)) + } + + for _, m := range mf.GetMetric() { + for _, l := range m.GetLabel() { + ln := l.GetName() + + if !isHistogram && ln == "le" { + problems = append(problems, newProblem(mf, `non-histogram metrics should not have "le" label`)) + } + if !isSummary && ln == "quantile" { + problems = append(problems, newProblem(mf, `non-summary metrics should not have "quantile" label`)) + } + } + } + + return problems +} diff --git a/prometheus/testutil/promlint/validations/problem.go b/prometheus/testutil/promlint/validations/problem.go new file mode 100644 index 000000000..4c6d88b4f --- /dev/null +++ b/prometheus/testutil/promlint/validations/problem.go @@ -0,0 +1,33 @@ +// Copyright 2020 The Prometheus 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 validations + +import dto "github.com/prometheus/client_model/go" + +// A Problem is an issue detected by a linter. +type Problem struct { + // The name of the metric indicated by this Problem. + Metric string + + // A description of the issue for this Problem. + Text string +} + +// newProblem is helper function to create a Problem. +func newProblem(mf *dto.MetricFamily, text string) Problem { + return Problem{ + Metric: mf.GetName(), + Text: text, + } +} diff --git a/prometheus/testutil/promlint/validations/units.go b/prometheus/testutil/promlint/validations/units.go new file mode 100644 index 000000000..967977d2b --- /dev/null +++ b/prometheus/testutil/promlint/validations/units.go @@ -0,0 +1,118 @@ +// Copyright 2020 The Prometheus 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 validations + +import "strings" + +// Units and their possible prefixes recognized by this library. More can be +// added over time as needed. +var ( + // map a unit to the appropriate base unit. + units = map[string]string{ + // Base units. + "amperes": "amperes", + "bytes": "bytes", + "celsius": "celsius", // Also allow Celsius because it is common in typical Prometheus use cases. + "grams": "grams", + "joules": "joules", + "kelvin": "kelvin", // SI base unit, used in special cases (e.g. color temperature, scientific measurements). + "meters": "meters", // Both American and international spelling permitted. + "metres": "metres", + "seconds": "seconds", + "volts": "volts", + + // Non base units. + // Time. + "minutes": "seconds", + "hours": "seconds", + "days": "seconds", + "weeks": "seconds", + // Temperature. + "kelvins": "kelvin", + "fahrenheit": "celsius", + "rankine": "celsius", + // Length. + "inches": "meters", + "yards": "meters", + "miles": "meters", + // Bytes. + "bits": "bytes", + // Energy. + "calories": "joules", + // Mass. + "pounds": "grams", + "ounces": "grams", + } + + unitPrefixes = []string{ + "pico", + "nano", + "micro", + "milli", + "centi", + "deci", + "deca", + "hecto", + "kilo", + "kibi", + "mega", + "mibi", + "giga", + "gibi", + "tera", + "tebi", + "peta", + "pebi", + } + + // Common abbreviations that we'd like to discourage. + unitAbbreviations = []string{ + "s", + "ms", + "us", + "ns", + "sec", + "b", + "kb", + "mb", + "gb", + "tb", + "pb", + "m", + "h", + "d", + } +) + +// metricUnits attempts to detect known unit types used as part of a metric name, +// e.g. "foo_bytes_total" or "bar_baz_milligrams". +func metricUnits(m string) (unit, base string, ok bool) { + ss := strings.Split(m, "_") + + for _, s := range ss { + if base, found := units[s]; found { + return s, base, true + } + + for _, p := range unitPrefixes { + if strings.HasPrefix(s, p) { + if base, found := units[s[len(p):]]; found { + return s, base, true + } + } + } + } + + return "", "", false +} diff --git a/prometheus/testutil/promlint/validations/validation.go b/prometheus/testutil/promlint/validations/validation.go new file mode 100644 index 000000000..5d42f2847 --- /dev/null +++ b/prometheus/testutil/promlint/validations/validation.go @@ -0,0 +1,31 @@ +// Copyright 2020 The Prometheus 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 validations + +import ( + dto "github.com/prometheus/client_model/go" +) + +type Validation = func(mf *dto.MetricFamily) []Problem + +var DefaultValidations = []Validation{ + lintHelp, + lintMetricUnits, + lintCounter, + lintHistogramSummaryReserved, + lintMetricTypeInName, + lintReservedChars, + lintCamelCase, + lintUnitAbbreviations, +}