Merge pull request #743 from prometheus/beorn7/test

Add linter helpers to testutil

Author: beorn7

prometheus/testutil/lint.go

@@ -0,0 +1,46 @@
+// 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 testutil
+
+import (
+	"fmt"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/testutil/promlint"
+)
+
+// 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) {
+	reg := prometheus.NewPedanticRegistry()
+	if err := reg.Register(c); err != nil {
+		return nil, fmt.Errorf("registering collector failed: %s", err)
+	}
+	return GatherAndLint(reg, metricNames...)
+}
+
+// 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) {
+	got, err := g.Gather()
+	if err != nil {
+		return nil, fmt.Errorf("gathering metrics failed: %s", err)
+	}
+	if metricNames != nil {
+		got = filterMetrics(got, metricNames)
+	}
+	return promlint.NewWithMetricFamilies(got).Lint()
+}

prometheus/testutil/lint_test.go

@@ -0,0 +1,71 @@
+// 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 testutil
+
+import (
+	"testing"
+
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+func TestCollectAndLintGood(t *testing.T) {
+	cnt := prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "some_total",
+			Help: "A value that represents a counter.",
+			ConstLabels: prometheus.Labels{
+				"label1": "value1",
+			},
+		},
+		[]string{"foo"},
+	)
+	cnt.WithLabelValues("bar")
+	cnt.WithLabelValues("baz")
+
+	problems, err := CollectAndLint(cnt)
+	if err != nil {
+		t.Error("Unexpected error:", err)
+	}
+	if len(problems) > 0 {
+		t.Error("Unexpected lint problems:", problems)
+	}
+}
+
+func TestCollectAndLintBad(t *testing.T) {
+	cnt := prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "someThing_ms",
+			Help: "A value that represents a counter.",
+			ConstLabels: prometheus.Labels{
+				"label1": "value1",
+			},
+		},
+		[]string{"fooBar"},
+	)
+	cnt.WithLabelValues("bar")
+	cnt.WithLabelValues("baz")
+
+	problems, err := CollectAndLint(cnt)
+	if err != nil {
+		t.Error("Unexpected error:", err)
+	}
+	if len(problems) < 5 {
+		// The exact nature of the lint problems found is tested within
+		// the promlint package itself. Here we only want to make sure
+		// that the collector successfully hits the linter and that at
+		// least the five problems that the linter could recognize at
+		// the time of writing this test are flagged.
+		t.Error("Not enough lint problems found.")
+	}
+}

prometheus/testutil/promlint/promlint.go

@@ -29,7 +29,13 @@ import (
 // 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 {
-	r io.Reader
+	// The linter will read metrics in the Prometheus text format from r and
+	// then lint it, _and_ it will lint the metrics provided directly as
+	// MetricFamily proto messages in mfs. Note, however, that the current
+	// constructor functions New and NewWithMetricFamilies only ever set one
+	// of them.
+	r   io.Reader
+	mfs []*dto.MetricFamily
 }
 
 // A Problem is an issue detected by a Linter.
@@ -42,40 +48,52 @@ type Problem struct {
 }
 
 // newProblem is helper function to create a Problem.
-func newProblem(mf dto.MetricFamily, text string) 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.
-// Only the Prometheus text exposition format is supported.
+// 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 {
 	return &Linter{
 		r: r,
 	}
 }
 
+// NewWithMetricFamilies creates a new Linter that reads from a slice of
+// MetricFamily protobuf messages.
+func NewWithMetricFamilies(mfs []*dto.MetricFamily) *Linter {
+	return &Linter{
+		mfs: mfs,
+	}
+}
+
 // 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
+// issues found in the metrics stream. The slice is sorted by metric name
 // and issue description.
 func (l *Linter) Lint() ([]Problem, error) {
-	// TODO(mdlayher): support for protobuf exposition format?
-	d := expfmt.NewDecoder(l.r, expfmt.FmtText)
-
 	var problems []Problem
 
-	var mf dto.MetricFamily
-	for {
-		if err := d.Decode(&mf); err != nil {
-			if err == io.EOF {
-				break
+	if l.r != nil {
+		d := expfmt.NewDecoder(l.r, expfmt.FmtText)
+
+		mf := &dto.MetricFamily{}
+		for {
+			if err := d.Decode(mf); err != nil {
+				if err == io.EOF {
+					break
+				}
+
+				return nil, err
 			}
 
-			return nil, err
+			problems = append(problems, lint(mf)...)
 		}
-
+	}
+	for _, mf := range l.mfs {
 		problems = append(problems, lint(mf)...)
 	}
 
@@ -91,8 +109,8 @@ 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{
+func lint(mf *dto.MetricFamily) []Problem {
+	fns := []func(mf *dto.MetricFamily) []Problem{
 		lintHelp,
 		lintMetricUnits,
 		lintCounter,
@@ -113,7 +131,7 @@ func lint(mf dto.MetricFamily) []Problem {
 }
 
 // lintHelp detects issues related to the help text for a metric.
-func lintHelp(mf dto.MetricFamily) []Problem {
+func lintHelp(mf *dto.MetricFamily) []Problem {
 	var problems []Problem
 
 	// Expect all metrics to have help text available.
@@ -125,7 +143,7 @@ func lintHelp(mf dto.MetricFamily) []Problem {
 }
 
 // lintMetricUnits detects issues with metric unit names.
-func lintMetricUnits(mf dto.MetricFamily) []Problem {
+func lintMetricUnits(mf *dto.MetricFamily) []Problem {
 	var problems []Problem
 
 	unit, base, ok := metricUnits(*mf.Name)
@@ -146,7 +164,7 @@ func lintMetricUnits(mf dto.MetricFamily) []Problem {
 
 // lintCounter detects issues specific to counters, as well as patterns that should
 // only be used with counters.
-func lintCounter(mf dto.MetricFamily) []Problem {
+func lintCounter(mf *dto.MetricFamily) []Problem {
 	var problems []Problem
 
 	isCounter := mf.GetType() == dto.MetricType_COUNTER
@@ -165,7 +183,7 @@ func lintCounter(mf dto.MetricFamily) []Problem {
 
 // 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 {
+func lintHistogramSummaryReserved(mf *dto.MetricFamily) []Problem {
 	// These rules do not apply to untyped metrics.
 	t := mf.GetType()
 	if t == dto.MetricType_UNTYPED {
@@ -206,7 +224,7 @@ func lintHistogramSummaryReserved(mf dto.MetricFamily) []Problem {
 }
 
 // lintMetricTypeInName detects when metric types are included in the metric name.
-func lintMetricTypeInName(mf dto.MetricFamily) []Problem {
+func lintMetricTypeInName(mf *dto.MetricFamily) []Problem {
 	var problems []Problem
 	n := strings.ToLower(mf.GetName())
 
@@ -224,7 +242,7 @@ func lintMetricTypeInName(mf dto.MetricFamily) []Problem {
 }
 
 // lintReservedChars detects colons in metric names.
-func lintReservedChars(mf dto.MetricFamily) []Problem {
+func lintReservedChars(mf *dto.MetricFamily) []Problem {
 	var problems []Problem
 	if strings.Contains(mf.GetName(), ":") {
 		problems = append(problems, newProblem(mf, "metric names should not contain ':'"))
@@ -235,7 +253,7 @@ func lintReservedChars(mf dto.MetricFamily) []Problem {
 var camelCase = regexp.MustCompile(`[a-z][A-Z]`)
 
 // lintCamelCase detects metric names and label names written in camelCase.
-func lintCamelCase(mf dto.MetricFamily) []Problem {
+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'"))
@@ -252,7 +270,7 @@ func lintCamelCase(mf dto.MetricFamily) []Problem {
 }
 
 // lintUnitAbbreviations detects abbreviated units in the metric name.
-func lintUnitAbbreviations(mf dto.MetricFamily) []Problem {
+func lintUnitAbbreviations(mf *dto.MetricFamily) []Problem {
 	var problems []Problem
 	n := strings.ToLower(mf.GetName())
 	for _, s := range unitAbbreviations {

prometheus/testutil/testutil.go

@@ -31,6 +31,10 @@
 // testing custom prometheus.Collector implementations and in particular whole
 // exporters, i.e. programs that retrieve telemetry data from a 3rd party source
 // and convert it into Prometheus metrics.
+//
+// In a similar pattern, CollectAndLint and GatherAndLint can be used to detect
+// metrics that have issues with their name, type, or metadata without being
+// necessarily invalid, e.g. a counter with a name missing the “_total” suffix.
 package testutil
 
 import (
@@ -136,8 +140,8 @@ func CollectAndCount(c prometheus.Collector) int {
 }
 
 // CollectAndCompare registers the provided Collector with a newly created
-// pedantic Registry. It then does the same as GatherAndCompare, gathering the
-// metrics from the pedantic Registry.
+// pedantic Registry. It then calls GatherAndCompare with that Registry and with
+// the provided metricNames.
 func CollectAndCompare(c prometheus.Collector, expected io.Reader, metricNames ...string) error {
 	reg := prometheus.NewPedanticRegistry()
 	if err := reg.Register(c); err != nil {