From 90dd4bbd4570560a4b72ca8525c99dc8fc1db163 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Wed, 27 Dec 2017 15:48:14 +0000 Subject: [PATCH] metrics: correctly register all metrics (#1335) After initial startup, see if prometheus is loaded and if so, register our metrics with it. Stop doing the init() func and just use the sync.Once so we don't double registrer our metrics. --- plugin/autopath/metrics.go | 10 +--------- plugin/autopath/setup.go | 14 +++++++++++++- plugin/cache/handler.go | 18 ++++++------------ plugin/cache/setup.go | 17 +++++++++++++++++ plugin/dnssec/handler.go | 19 +++++++------------ plugin/dnssec/setup.go | 17 +++++++++++++++++ plugin/proxy/metrics.go | 11 +---------- plugin/proxy/setup.go | 15 ++++++++++++++- 8 files changed, 76 insertions(+), 45 deletions(-) diff --git a/plugin/autopath/metrics.go b/plugin/autopath/metrics.go index 3901fde3a..2d32793bf 100644 --- a/plugin/autopath/metrics.go +++ b/plugin/autopath/metrics.go @@ -18,12 +18,4 @@ var ( }, []string{}) ) -// OnStartupMetrics sets up the metrics on startup. -func OnStartupMetrics() error { - metricsOnce.Do(func() { - prometheus.MustRegister(AutoPathCount) - }) - return nil -} - -var metricsOnce sync.Once +var once sync.Once diff --git a/plugin/autopath/setup.go b/plugin/autopath/setup.go index f4b908465..ac0f05ec5 100644 --- a/plugin/autopath/setup.go +++ b/plugin/autopath/setup.go @@ -5,6 +5,7 @@ import ( "github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/plugin" + "github.com/coredns/coredns/plugin/metrics" "github.com/mholt/caddy" "github.com/miekg/dns" @@ -24,7 +25,18 @@ func setup(c *caddy.Controller) error { return plugin.Error("autopath", err) } - c.OnStartup(OnStartupMetrics) + c.OnStartup(func() error { + once.Do(func() { + m := dnsserver.GetConfig(c).Handler("prometheus") + if m == nil { + return + } + if x, ok := m.(*metrics.Metrics); ok { + x.MustRegister(AutoPathCount) + } + }) + return nil + }) // Do this in OnStartup, so all plugin has been initialized. c.OnStartup(func() error { diff --git a/plugin/cache/handler.go b/plugin/cache/handler.go index b7824beba..df2c74e39 100644 --- a/plugin/cache/handler.go +++ b/plugin/cache/handler.go @@ -1,6 +1,7 @@ package cache import ( + "sync" "time" "github.com/coredns/coredns/plugin" @@ -84,38 +85,31 @@ func (c *Cache) get(now time.Time, qname string, qtype uint16, do bool) (*item, var ( cacheSize = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: plugin.Namespace, - Subsystem: subsystem, + Subsystem: "cache", Name: "size", Help: "The number of elements in the cache.", }, []string{"type"}) cacheCapacity = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: plugin.Namespace, - Subsystem: subsystem, + Subsystem: "cache", Name: "capacity", Help: "The cache's capacity.", }, []string{"type"}) cacheHits = prometheus.NewCounterVec(prometheus.CounterOpts{ Namespace: plugin.Namespace, - Subsystem: subsystem, + Subsystem: "cache", Name: "hits_total", Help: "The count of cache hits.", }, []string{"type"}) cacheMisses = prometheus.NewCounter(prometheus.CounterOpts{ Namespace: plugin.Namespace, - Subsystem: subsystem, + Subsystem: "cache", Name: "misses_total", Help: "The count of cache misses.", }) ) -const subsystem = "cache" - -func init() { - prometheus.MustRegister(cacheSize) - prometheus.MustRegister(cacheCapacity) - prometheus.MustRegister(cacheHits) - prometheus.MustRegister(cacheMisses) -} +var once sync.Once diff --git a/plugin/cache/setup.go b/plugin/cache/setup.go index d8ef9a8d7..d4d041ae8 100644 --- a/plugin/cache/setup.go +++ b/plugin/cache/setup.go @@ -7,6 +7,7 @@ import ( "github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/plugin" + "github.com/coredns/coredns/plugin/metrics" "github.com/coredns/coredns/plugin/pkg/cache" "github.com/mholt/caddy" @@ -29,6 +30,22 @@ func setup(c *caddy.Controller) error { return ca }) + c.OnStartup(func() error { + once.Do(func() { + m := dnsserver.GetConfig(c).Handler("prometheus") + if m == nil { + return + } + if x, ok := m.(*metrics.Metrics); ok { + x.MustRegister(cacheSize) + x.MustRegister(cacheCapacity) + x.MustRegister(cacheHits) + x.MustRegister(cacheMisses) + } + }) + return nil + }) + // Export the capacity for the metrics. This only happens once, because this is a re-load change only. cacheCapacity.WithLabelValues(Success).Set(float64(ca.pcap)) cacheCapacity.WithLabelValues(Denial).Set(float64(ca.ncap)) diff --git a/plugin/dnssec/handler.go b/plugin/dnssec/handler.go index 6fa2dd042..0fde35dd7 100644 --- a/plugin/dnssec/handler.go +++ b/plugin/dnssec/handler.go @@ -1,6 +1,8 @@ package dnssec import ( + "sync" + "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/request" @@ -42,28 +44,28 @@ func (d Dnssec) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) var ( cacheSize = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: plugin.Namespace, - Subsystem: subsystem, + Subsystem: "dnssec", Name: "cache_size", Help: "The number of elements in the dnssec cache.", }, []string{"type"}) cacheCapacity = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: plugin.Namespace, - Subsystem: subsystem, + Subsystem: "dnssec", Name: "cache_capacity", Help: "The dnssec cache's capacity.", }, []string{"type"}) cacheHits = prometheus.NewCounter(prometheus.CounterOpts{ Namespace: plugin.Namespace, - Subsystem: subsystem, + Subsystem: "dnssec", Name: "cache_hits_total", Help: "The count of cache hits.", }) cacheMisses = prometheus.NewCounter(prometheus.CounterOpts{ Namespace: plugin.Namespace, - Subsystem: subsystem, + Subsystem: "dnssec", Name: "cache_misses_total", Help: "The count of cache misses.", }) @@ -72,11 +74,4 @@ var ( // Name implements the Handler interface. func (d Dnssec) Name() string { return "dnssec" } -const subsystem = "dnssec" - -func init() { - prometheus.MustRegister(cacheSize) - prometheus.MustRegister(cacheCapacity) - prometheus.MustRegister(cacheHits) - prometheus.MustRegister(cacheMisses) -} +var once sync.Once diff --git a/plugin/dnssec/setup.go b/plugin/dnssec/setup.go index 012ba5b3d..26cdbe2f1 100644 --- a/plugin/dnssec/setup.go +++ b/plugin/dnssec/setup.go @@ -7,6 +7,7 @@ import ( "github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/plugin" + "github.com/coredns/coredns/plugin/metrics" "github.com/coredns/coredns/plugin/pkg/cache" "github.com/mholt/caddy" @@ -30,6 +31,22 @@ func setup(c *caddy.Controller) error { return New(zones, keys, next, ca) }) + c.OnStartup(func() error { + once.Do(func() { + m := dnsserver.GetConfig(c).Handler("prometheus") + if m == nil { + return + } + if x, ok := m.(*metrics.Metrics); ok { + x.MustRegister(cacheSize) + x.MustRegister(cacheCapacity) + x.MustRegister(cacheHits) + x.MustRegister(cacheMisses) + } + }) + return nil + }) + // Export the capacity for the metrics. This only happens once, because this is a re-load change only. cacheCapacity.WithLabelValues("signature").Set(float64(capacity)) diff --git a/plugin/proxy/metrics.go b/plugin/proxy/metrics.go index 96be50893..e0dd3fe98 100644 --- a/plugin/proxy/metrics.go +++ b/plugin/proxy/metrics.go @@ -25,15 +25,6 @@ var ( }, []string{"proto", "proxy_proto", "family", "to"}) ) -// OnStartupMetrics sets up the metrics on startup. This is done for all proxy protocols. -func OnStartupMetrics() error { - metricsOnce.Do(func() { - prometheus.MustRegister(RequestCount) - prometheus.MustRegister(RequestDuration) - }) - return nil -} - // familyToString returns the string form of either 1, or 2. Returns // empty string is not a known family func familyToString(f int) string { @@ -46,4 +37,4 @@ func familyToString(f int) string { return "" } -var metricsOnce sync.Once +var once sync.Once diff --git a/plugin/proxy/setup.go b/plugin/proxy/setup.go index bbe65c35d..eff7c63f4 100644 --- a/plugin/proxy/setup.go +++ b/plugin/proxy/setup.go @@ -3,6 +3,7 @@ package proxy import ( "github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/plugin" + "github.com/coredns/coredns/plugin/metrics" "github.com/mholt/caddy" ) @@ -28,7 +29,19 @@ func setup(c *caddy.Controller) error { return P }) - c.OnStartup(OnStartupMetrics) + c.OnStartup(func() error { + once.Do(func() { + m := dnsserver.GetConfig(c).Handler("prometheus") + if m == nil { + return + } + if x, ok := m.(*metrics.Metrics); ok { + x.MustRegister(RequestCount) + x.MustRegister(RequestDuration) + } + }) + return nil + }) for i := range upstreams { u := upstreams[i]