Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add support for custom validations in promlint #1311

Merged
merged 3 commits into from Oct 10, 2023
Merged

Add support for custom validations in promlint #1311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c39535e
Refactor promlint validation structure
machadovilaca Jul 13, 2023
83b0350
Add support for custom validations in promlint
machadovilaca Jul 13, 2023
adc08b1
Keep backwards compatibility
machadovilaca Aug 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 33 additions & 0 deletions prometheus/testutil/promlint/problem.go
View file
Open in desktop
@@ -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 promlint

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,
}
}
308 changes: 23 additions & 285 deletions prometheus/testutil/promlint/promlint.go
View file
Open in desktop
Expand Up @@ -16,15 +16,11 @@ package promlint

import (
"errors"
"fmt"
"io"
"regexp"
"sort"
"strings"

"github.com/prometheus/common/expfmt"

dto "github.com/prometheus/client_model/go"
"github.com/prometheus/common/expfmt"
)

// A Linter is a Prometheus metrics linter. It identifies issues with metric
Expand All @@ -37,23 +33,8 @@ type Linter struct {
// of them.
r io.Reader
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,
}
customValidations []Validation
}

// New creates a new Linter that reads an input stream of Prometheus metrics in
Expand All @@ -72,6 +53,14 @@ func NewWithMetricFamilies(mfs []*dto.MetricFamily) *Linter {
}
}

// AddCustomValidations adds custom validations to the linter.
func (l *Linter) AddCustomValidations(vs ...Validation) {
if l.customValidations == nil {
l.customValidations = make([]Validation, 0, len(vs))
}
l.customValidations = append(l.customValidations, vs...)
}

// 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.
Expand All @@ -91,11 +80,11 @@ func (l *Linter) Lint() ([]Problem, error) {
return nil, err
}

problems = append(problems, lint(mf)...)
problems = append(problems, l.lint(mf)...)
}
}
for _, mf := range l.mfs {
problems = append(problems, lint(mf)...)
problems = append(problems, l.lint(mf)...)
}

// Ensure deterministic output.
Expand All @@ -110,276 +99,25 @@ 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 {
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 {
func (l *Linter) lint(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
for _, fn := range defaultValidations {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why, why not merging those slices (default and custom) and not repeating the code?

Not a blocker, that's the only nit I would improve, merging anyway.

errs := fn(mf)
for _, err := range errs {
problems = append(problems, newProblem(mf, err.Error()))
}

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'"))
if l.customValidations != nil {
for _, fn := range l.customValidations {
errs := fn(mf)
for _, err := range errs {
problems = append(problems, newProblem(mf, err.Error()))
}
}
}
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"))
}
}
// TODO(mdlayher): lint rules for specific metrics types.
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",
}
)