New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add linter helpers to testutil #743
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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.") | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would comment that this is Not relevant to package users There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would actually work with both having values, just that you don't have a constructor doing so (but we might have in the future). I'll add a comment explaining that. |
||
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 { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would prefer
len(metricNames) > 0
, less surprising.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking was that the linter will never recognize fewer problems in future versions, but it might recognize more. I changed the comment to clarify that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx!