Files
dns/test/metric_naming_test.go
zooqueen 66369c5a2d build the gate the way the binary is built, and fix what that exposed
Nine packages failed. Eight of them for one reason: `go test ./...` ran without
the tags the Makefile ships with (GOTAGS = grpcnotrace metrics), and
luxfi/metric is `//go:build !metrics` — every counter, gauge and histogram is a
no-op unless that tag is set. So metrics, metrics/vars and the kubernetes
object counters were all asserting on numbers that could never move:

  RequestCount was not incremented. Before: 0.000000, After: 0.000000

The shipping binary does set it, so this was never a production defect — but a
gate that builds something other than what ships cannot see one either.

The rest were real, and none of them are about metrics:

  test/metric_naming_test.go declared a local `type metric` while importing
    the package named metric, which every other file here imports too. The
    local type is the newcomer; renamed it visitor, for the ast.Visitor it is.
    Its BuildFQName call went back to prometheus, which is whose name format is
    under test — luxfi/metric has no three-part equivalent, only
    AppendNamespace(namespace, name).

  Three %q verbs applied to numbers — an MX preference, an Rcode, a SOA
    serial. Newer vet rejects them, and `go test` runs vet, so three packages
    failed to build without ever reaching a test.

  TestImportOrdering walked operator/, which is its own module
    (github.com/hanzoai/dns-operator) and imports no coredns, so the
    std -> coredns -> 3rd shape it checks for has no middle group to find.
    Skipped like pb/ and vendor/ already are, for the same reason.

Eight of the nine pass now. The ninth is test/, where seven metrics
integration tests scrape /metrics and find nothing — the counters record but
are not exposed. That is the other half of the luxfi/metric migration and
wants its own change.
2026-08-10 20:08:49 -07:00

171 lines
3.7 KiB
Go
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
package test
import (
"go/ast"
"go/parser"
"go/token"
"os"
"path/filepath"
"strconv"
"strings"
"testing"
"github.com/coredns/coredns/plugin"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil/promlint"
dto "github.com/prometheus/client_model/go"
)
func TestMetricNaming(t *testing.T) {
walker := validMetricWalker{}
err := filepath.Walk("..", walker.walk)
if err != nil {
t.Fatal(err)
}
if len(walker.Metrics) > 0 {
l := promlint.NewWithMetricFamilies(walker.Metrics)
problems, err := l.Lint()
if err != nil {
t.Fatalf("Link found error: %s", err)
}
if len(problems) > 0 {
t.Fatalf("A slice of Problems indicating any issues found in the metrics stream: %s", problems)
}
}
}
type validMetricWalker struct {
Metrics []*dto.MetricFamily
}
func (w *validMetricWalker) walk(path string, info os.FileInfo, _ error) error {
// only for regular files, not starting with a . and those that are go files.
if !info.Mode().IsRegular() {
return nil
}
// Is it appropriate to compare the file name equals metrics.go directly?
if strings.HasPrefix(path, "../.") {
return nil
}
if strings.HasSuffix(path, "_test.go") {
return nil
}
if !strings.HasSuffix(path, ".go") {
return nil
}
fs := token.NewFileSet()
f, err := parser.ParseFile(fs, path, nil, parser.AllErrors)
if err != nil {
return err
}
l := &visitor{}
ast.Walk(l, f)
if l.Metric != nil {
w.Metrics = append(w.Metrics, l.Metric)
}
return nil
}
// visitor collects the metric family declared in one parsed file. Named for
// what it does — it is an ast.Visitor — because `metric` is the package this
// file, and every other file here, imports from luxfi.
type visitor struct {
Metric *dto.MetricFamily
}
func (l *visitor) Visit(n ast.Node) ast.Visitor {
if n == nil {
return nil
}
ce, ok := n.(*ast.CallExpr)
if !ok {
return l
}
se, ok := ce.Fun.(*ast.SelectorExpr)
if !ok {
return l
}
id, ok := se.X.(*ast.Ident)
if !ok {
return l
}
if id.Name != "prometheus" { //prometheus
return l
}
var metricsType dto.MetricType
switch se.Sel.Name {
case "NewCounterVec", "NewCounter":
metricsType = dto.MetricType_COUNTER
case "NewGaugeVec", "NewGauge":
metricsType = dto.MetricType_GAUGE
case "NewHistogramVec", "NewHistogram":
metricsType = dto.MetricType_HISTOGRAM
case "NewSummaryVec", "NewSummary":
metricsType = dto.MetricType_SUMMARY
default:
return l
}
// Check first arg, that should have basic lit with capital
if len(ce.Args) < 1 {
return l
}
bl, ok := ce.Args[0].(*ast.CompositeLit)
if !ok {
return l
}
// parse Namespace Subsystem Name Help
var subsystem, name, help string
for _, elt := range bl.Elts {
expr, ok := elt.(*ast.KeyValueExpr)
if !ok {
continue
}
object, ok := expr.Key.(*ast.Ident)
if !ok {
continue
}
value, ok := expr.Value.(*ast.BasicLit)
if !ok {
continue
}
// remove quotes
stringLiteral, err := strconv.Unquote(value.Value)
if err != nil {
return l
}
switch object.Name {
case "Subsystem":
subsystem = stringLiteral
case "Name":
name = stringLiteral
case "Help":
help = stringLiteral
}
}
// validate metrics field
if len(name) == 0 || len(help) == 0 {
return l
}
// prometheus's own join, because that is exactly what is under test here:
// the name goes straight to promlint as a dto.MetricFamily, and luxfi/metric
// has no three-part equivalent (AppendNamespace takes a namespace and a name,
// not a subsystem between them).
metricName := prometheus.BuildFQName(plugin.Namespace, subsystem, name)
l.Metric = &dto.MetricFamily{
Name: &metricName,
Help: &help,
Type: &metricsType,
}
return l
}