introduce metric naming test (#3789)

* introduce metric naming test

Signed-off-by: zounengren <zounengren@cmss.chinamobile.com>

* Update metrics.go

Signed-off-by: zounengren <zounengren@cmss.chinamobile.com>

go.mod

@@ -30,6 +30,7 @@ require (
 	github.com/prometheus/client_golang v1.5.1
 	github.com/prometheus/client_model v0.2.0
 	github.com/prometheus/common v0.9.1
+	github.com/prometheus/prometheus v2.5.0+incompatible
 	github.com/tinylib/msgp v1.1.0 // indirect
 	github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 // indirect
 	go.etcd.io/etcd v0.5.0-alpha.5.0.20200306183522-221f0cc107cb

go.sum

@@ -367,6 +367,8 @@ github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsT
 github.com/prometheus/procfs v0.0.3/go.mod h1:4A/X28fw3Fc593LaREMrKMqOKvUAntwMDaekg4FpcdQ=
 github.com/prometheus/procfs v0.0.8 h1:+fpWZdT24pJBiqJdAwYBjPSk+5YmQzYNPYzQsdzLkt8=
 github.com/prometheus/procfs v0.0.8/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A=
+github.com/prometheus/prometheus v2.5.0+incompatible h1:7QPitgO2kOFG8ecuRn9O/4L9+10He72rVRJvMXrE9Hg=
+github.com/prometheus/prometheus v2.5.0+incompatible/go.mod h1:oAIUtOny2rjMX0OWN5vPR5/q/twIROJvdqnQKDdil/s=
 github.com/rainycape/memcache v0.0.0-20150622160815-1031fa0ce2f2/go.mod h1:7tZKcyumwBO6qip7RNQ5r77yrssm9bfCowcLEBcU5IA=
 github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a h1:9ZKAASQSHhDYGoxY8uLVpewe1GDZ2vu2Tr/vTdVAkFQ=
 github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=

man/coredns-dnssec.7

@@ -1,5 +1,5 @@
 .\" Generated by Mmark Markdown Processer - mmark.miek.nl
-.TH "COREDNS-DNSSEC" 7 "January 2020" "CoreDNS" "CoreDNS Plugins"
+.TH "COREDNS-DNSSEC" 7 "March 2020" "CoreDNS" "CoreDNS Plugins"
 
 .SH "NAME"
 .PP
@@ -72,7 +72,7 @@ RRSIGs. The default for \fBCAPACITY\fP is 10000.
 If monitoring is enabled (via the \fIprometheus\fP plugin) then the following metrics are exported:
 
 .IP \(bu 4
-\fB\fCcoredns_dnssec_cache_size{server, type}\fR - total elements in the cache, type is "signature".
+\fB\fCcoredns_dnssec_cache_entries{server, type}\fR - total elements in the cache, type is "signature".
 .IP \(bu 4
 \fB\fCcoredns_dnssec_cache_hits_total{server}\fR - Counter of cache hits.
 .IP \(bu 4

man/coredns-hosts.7

@@ -1,5 +1,5 @@
 .\" Generated by Mmark Markdown Processer - mmark.miek.nl
-.TH "COREDNS-HOSTS" 7 "January 2020" "CoreDNS" "CoreDNS Plugins"
+.TH "COREDNS-HOSTS" 7 "March 2020" "CoreDNS" "CoreDNS Plugins"
 
 .SH "NAME"
 .PP
@@ -98,7 +98,7 @@ queries for those zones will be subject to fallthrough.
 If monitoring is enabled (via the \fIprometheus\fP plugin) then the following metrics are exported:
 
 .IP \(bu 4
-\fB\fCcoredns_hosts_entries_count{}\fR - The combined number of entries in hosts and Corefile.
+\fB\fCcoredns_hosts_entries{}\fR - The combined number of entries in hosts and Corefile.
 .IP \(bu 4
 \fB\fCcoredns_hosts_reload_timestamp_seconds{}\fR - The timestamp of the last reload of hosts file.
 

plugin/autopath/metrics.go

@@ -7,6 +7,7 @@ import (
 )
 
 var (
+	// autoPathCount is counter of successfully autopath-ed queries.
 	autoPathCount = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Namespace: plugin.Namespace,
 		Subsystem: "autopath",

plugin/cache/handler.go

@@ -10,7 +10,6 @@ import (
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
-	"github.com/prometheus/client_golang/prometheus"
 )
 
 // ServeDNS implements the plugin.Handler interface.
@@ -128,47 +127,3 @@ func (c *Cache) exists(state request.Request) *item {
 	}
 	return nil
 }
-
-var (
-	cacheSize = prometheus.NewGaugeVec(prometheus.GaugeOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "cache",
-		Name:      "size",
-		Help:      "The number of elements in the cache.",
-	}, []string{"server", "type"})
-
-	cacheHits = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "cache",
-		Name:      "hits_total",
-		Help:      "The count of cache hits.",
-	}, []string{"server", "type"})
-
-	cacheMisses = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "cache",
-		Name:      "misses_total",
-		Help:      "The count of cache misses.",
-	}, []string{"server"})
-
-	cachePrefetches = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "cache",
-		Name:      "prefetch_total",
-		Help:      "The number of time the cache has prefetched a cached item.",
-	}, []string{"server"})
-
-	cacheDrops = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "cache",
-		Name:      "drops_total",
-		Help:      "The number responses that are not cached, because the reply is malformed.",
-	}, []string{"server"})
-
-	servedStale = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "cache",
-		Name:      "served_stale_total",
-		Help:      "The number of requests served from stale cache entries.",
-	}, []string{"server"})
-)

plugin/cache/metrics.go

@@ -0,0 +1,52 @@
+package cache
+
+import (
+	"github.com/coredns/coredns/plugin"
+
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+var (
+	// cacheSize is total elements in the cache by cache type.
+	cacheSize = prometheus.NewGaugeVec(prometheus.GaugeOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "cache",
+		Name:      "size",
+		Help:      "The number of elements in the cache.",
+	}, []string{"server", "type"})
+	// cacheHits is counter of cache hits by cache type.
+	cacheHits = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "cache",
+		Name:      "hits_total",
+		Help:      "The count of cache hits.",
+	}, []string{"server", "type"})
+	// cacheMisses is the counter of cache misses.
+	cacheMisses = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "cache",
+		Name:      "misses_total",
+		Help:      "The count of cache misses.",
+	}, []string{"server"})
+	// cachePrefetches is the number of time the cache has prefetched a cached item.
+	cachePrefetches = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "cache",
+		Name:      "prefetch_total",
+		Help:      "The number of time the cache has prefetched a cached item.",
+	}, []string{"server"})
+	// cacheDrops is the number responses that are not cached, because the reply is malformed.
+	cacheDrops = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "cache",
+		Name:      "drops_total",
+		Help:      "The number responses that are not cached, because the reply is malformed.",
+	}, []string{"server"})
+	// servedStale is the number of requests served from stale cache entries.
+	servedStale = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "cache",
+		Name:      "served_stale_total",
+		Help:      "The number of requests served from stale cache entries.",
+	}, []string{"server"})
+)

plugin/dnssec/README.md

@@ -53,7 +53,7 @@ used (See [bugs](#bugs)).
 
 If monitoring is enabled (via the *prometheus* plugin) then the following metrics are exported:
 
-* `coredns_dnssec_cache_size{server, type}` - total elements in the cache, type is "signature".
+* `coredns_dnssec_cache_entries{server, type}` - total elements in the cache, type is "signature".
 * `coredns_dnssec_cache_hits_total{server}` - Counter of cache hits.
 * `coredns_dnssec_cache_misses_total{server}` - Counter of cache misses.
 

plugin/dnssec/handler.go

@@ -8,7 +8,6 @@ import (
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
-	"github.com/prometheus/client_golang/prometheus"
 )
 
 // ServeDNS implements the plugin.Handler interface.
@@ -47,28 +46,5 @@ func (d Dnssec) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	return plugin.NextOrFailure(d.Name(), d.Next, ctx, w, r)
 }
 
-var (
-	cacheSize = prometheus.NewGaugeVec(prometheus.GaugeOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "dnssec",
-		Name:      "cache_size",
-		Help:      "The number of elements in the dnssec cache.",
-	}, []string{"server", "type"})
-
-	cacheHits = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "dnssec",
-		Name:      "cache_hits_total",
-		Help:      "The count of cache hits.",
-	}, []string{"server"})
-
-	cacheMisses = prometheus.NewCounterVec(prometheus.CounterOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "dnssec",
-		Name:      "cache_misses_total",
-		Help:      "The count of cache misses.",
-	}, []string{"server"})
-)
-
 // Name implements the Handler interface.
 func (d Dnssec) Name() string { return "dnssec" }

plugin/dnssec/metrics.go

@@ -0,0 +1,31 @@
+package dnssec
+
+import (
+	"github.com/coredns/coredns/plugin"
+
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+var (
+	// cacheSize is the number of elements in the dnssec cache.
+	cacheSize = prometheus.NewGaugeVec(prometheus.GaugeOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "dnssec",
+		Name:      "cache_entries",
+		Help:      "The number of elements in the dnssec cache.",
+	}, []string{"server", "type"})
+	// cacheHits is the count of cache hits.
+	cacheHits = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "dnssec",
+		Name:      "cache_hits_total",
+		Help:      "The count of cache hits.",
+	}, []string{"server"})
+	// cacheMisses is the count of cache misses.
+	cacheMisses = prometheus.NewCounterVec(prometheus.CounterOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "dnssec",
+		Name:      "cache_misses_total",
+		Help:      "The count of cache misses.",
+	}, []string{"server"})
+)

plugin/hosts/README.md

@@ -76,7 +76,7 @@ hosts [FILE [ZONES...]] {
 
 If monitoring is enabled (via the *prometheus* plugin) then the following metrics are exported:
 
-- `coredns_hosts_entries_count{}` - The combined number of entries in hosts and Corefile.
+- `coredns_hosts_entries{}` - The combined number of entries in hosts and Corefile.
 - `coredns_hosts_reload_timestamp_seconds{}` - The timestamp of the last reload of hosts file.
 
 ## Examples

plugin/hosts/hostsfile.go

@@ -17,8 +17,6 @@ import (
 	"time"
 
 	"github.com/coredns/coredns/plugin"
-
-	"github.com/prometheus/client_golang/prometheus"
 )
 
 // parseIP calls discards any v6 zone info, before calling net.ParseIP.
@@ -256,19 +254,3 @@ func (h *Hostsfile) LookupStaticAddr(addr string) []string {
 	copy(hostsCp[len(hosts1):], hosts2)
 	return hostsCp
 }
-
-var (
-	hostsEntries = prometheus.NewGaugeVec(prometheus.GaugeOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "hosts",
-		Name:      "entries_count",
-		Help:      "The combined number of entries in hosts and Corefile.",
-	}, []string{})
-
-	hostsReloadTime = prometheus.NewGauge(prometheus.GaugeOpts{
-		Namespace: plugin.Namespace,
-		Subsystem: "hosts",
-		Name:      "reload_timestamp_seconds",
-		Help:      "The timestamp of the last reload of hosts file.",
-	})
-)

plugin/hosts/metrics.go

@@ -0,0 +1,24 @@
+package hosts
+
+import (
+	"github.com/coredns/coredns/plugin"
+
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+var (
+	// hostsEntries is the combined number of entries in hosts and Corefile.
+	hostsEntries = prometheus.NewGaugeVec(prometheus.GaugeOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "hosts",
+		Name:      "entries",
+		Help:      "The combined number of entries in hosts and Corefile.",
+	}, []string{})
+	// hostsReloadTime is the timestamp of the last reload of hosts file.
+	hostsReloadTime = prometheus.NewGauge(prometheus.GaugeOpts{
+		Namespace: plugin.Namespace,
+		Subsystem: "hosts",
+		Name:      "reload_timestamp_seconds",
+		Help:      "The timestamp of the last reload of hosts file.",
+	})
+)

plugin/reload/metrics.go

@@ -8,13 +8,14 @@ import (
 
 // Metrics for the reload plugin
 var (
-	FailedCount = prometheus.NewCounter(prometheus.CounterOpts{
+	// failedCount is the counter of the number of failed reload attempts.
+	failedCount = prometheus.NewCounter(prometheus.CounterOpts{
 		Namespace: plugin.Namespace,
 		Subsystem: "reload",
 		Name:      "failed_total",
 		Help:      "Counter of the number of failed reload attempts.",
 	})
-
+	// reloadInfo is record the hash value during reload.
 	reloadInfo = prometheus.NewGaugeVec(prometheus.GaugeOpts{
 		Namespace: plugin.Namespace,
 		Subsystem: "reload",

plugin/reload/reload.go

@@ -107,7 +107,7 @@ func hook(event caddy.EventName, info interface{}) error {
 					reloadInfo.WithLabelValues("md5", hex.EncodeToString(md5sum[:])).Set(1)
 					if err != nil {
 						log.Errorf("Corefile changed but reload failed: %s", err)
-						FailedCount.Add(1)
+						failedCount.Add(1)
 						continue
 					}
 					// we are done, if the plugin was not set used, then it is not.

plugin/reload/setup.go

@@ -71,7 +71,7 @@ func setup(c *caddy.Controller) error {
 	once.Do(func() {
 		caddy.RegisterEventHook("reload", hook)
 		c.OnRestart(func() error {
-			metrics.MustRegister(c, reloadInfo, FailedCount)
+			metrics.MustRegister(c, reloadInfo, failedCount)
 			return nil
 		})
 	})

plugin/template/metrics.go

@@ -9,18 +9,21 @@ import (
 )
 
 var (
+	// templateMatchesCount is the counter of template regex matches.
 	templateMatchesCount = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Namespace: plugin.Namespace,
 		Subsystem: "template",
 		Name:      "matches_total",
 		Help:      "Counter of template regex matches.",
 	}, []string{"server", "zone", "class", "type"})
+	// templateFailureCount is the counter of go template failures.
 	templateFailureCount = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Namespace: plugin.Namespace,
 		Subsystem: "template",
 		Name:      "template_failures_total",
 		Help:      "Counter of go template failures.",
 	}, []string{"server", "zone", "class", "type", "section", "template"})
+	// templateRRFailureCount is the counter of mis-templated RRs.
 	templateRRFailureCount = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Namespace: plugin.Namespace,
 		Subsystem: "template",

test/metric_naming_test.go

@@ -0,0 +1,172 @@
+package test
+
+import (
+	"bytes"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/coredns/coredns/plugin"
+
+	dto "github.com/prometheus/client_model/go"
+	"github.com/prometheus/common/expfmt"
+	"github.com/prometheus/prometheus/util/promlint"
+)
+
+func TestMetricNaming(t *testing.T) {
+
+	walker := validMetricWalker{}
+	err := filepath.Walk("..", walker.walk)
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(walker.Metrics) > 0 {
+
+		buf := &bytes.Buffer{}
+		encoder := expfmt.NewEncoder(buf, expfmt.FmtText)
+		for _, mf := range walker.Metrics {
+			if err := encoder.Encode(mf); err != nil {
+				t.Fatalf("Encoding and sending metric family: %s", err)
+			}
+		}
+
+		l := promlint.New(buf)
+		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 := &metric{}
+	ast.Walk(l, f)
+	if l.Metric != nil {
+		w.Metrics = append(w.Metrics, l.Metric)
+	}
+	return nil
+}
+
+type metric struct {
+	Metric *dto.MetricFamily
+}
+
+func (l metric) 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
+		}
+		switch object.Name {
+		case "Subsystem":
+			subsystem = value.Value
+		case "Name":
+			name = value.Value
+		case "Help":
+			help = value.Value
+		}
+	}
+
+	// validate metrics field
+	if len(name) == 0 || len(help) == 0 {
+		return l
+	}
+
+	var metricName string
+	if len(subsystem) > 0 {
+		metricName = strings.Join([]string{plugin.Namespace, subsystem, name}, "_")
+	} else {
+		metricName = strings.Join([]string{plugin.Namespace, name}, "_")
+	}
+	l.Metric = &dto.MetricFamily{
+		Name: &metricName,
+		Help: &help,
+		Type: &metricsType,
+	}
+	return l
+}