From 22520f4c7bdc8fb5cff9353a48866b4b6b38e1a5 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Tue, 18 Jul 2017 08:37:03 -0700 Subject: [PATCH 1/4] Move value check before metric get If someone is emitting negative counters we shouldn't even register the metric for them --- exporter.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/exporter.go b/exporter.go index 581718c..1618329 100644 --- a/exporter.go +++ b/exporter.go @@ -233,10 +233,6 @@ func (b *Exporter) Listen(e <-chan Events) { switch ev := event.(type) { case *CounterEvent: - counter := b.Counters.Get( - b.suffix(metricName, "counter"), - prometheusLabels, - ) // We don't accept negative values for counters. Incrementing the counter with a negative number // will cause the exporter to panic. Instead we will warn and continue to the next event. if event.Value() < 0.0 { @@ -244,6 +240,11 @@ func (b *Exporter) Listen(e <-chan Events) { continue } + counter := b.Counters.Get( + b.suffix(metricName, "counter"), + prometheusLabels, + ) + counter.Add(event.Value()) eventStats.WithLabelValues("counter").Inc() From d9aa6e2867f5163939e987ea4c49dd443ceb5fff Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Tue, 18 Jul 2017 08:42:12 -0700 Subject: [PATCH 2/4] Don't crash on conflicting metric names This patch simply moves the error message from a log.Fatalf() to a log.Errorf() to continue on. Fixes #63 --- exporter.go | 55 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/exporter.go b/exporter.go index 1618329..96efbbe 100644 --- a/exporter.go +++ b/exporter.go @@ -69,7 +69,7 @@ func NewCounterContainer() *CounterContainer { } } -func (c *CounterContainer) Get(metricName string, labels prometheus.Labels) prometheus.Counter { +func (c *CounterContainer) Get(metricName string, labels prometheus.Labels) (prometheus.Counter, error) { hash := hashNameAndLabels(metricName, labels) counter, ok := c.Elements[hash] if !ok { @@ -80,10 +80,10 @@ func (c *CounterContainer) Get(metricName string, labels prometheus.Labels) prom }) c.Elements[hash] = counter if err := prometheus.Register(counter); err != nil { - log.Fatalf(regErrF, metricName, err) + return nil, err } } - return counter + return counter, nil } type GaugeContainer struct { @@ -96,7 +96,7 @@ func NewGaugeContainer() *GaugeContainer { } } -func (c *GaugeContainer) Get(metricName string, labels prometheus.Labels) prometheus.Gauge { +func (c *GaugeContainer) Get(metricName string, labels prometheus.Labels) (prometheus.Gauge, error) { hash := hashNameAndLabels(metricName, labels) gauge, ok := c.Elements[hash] if !ok { @@ -107,10 +107,10 @@ func (c *GaugeContainer) Get(metricName string, labels prometheus.Labels) promet }) c.Elements[hash] = gauge if err := prometheus.Register(gauge); err != nil { - log.Fatalf(regErrF, metricName, err) + return nil, err } } - return gauge + return gauge, nil } type SummaryContainer struct { @@ -123,7 +123,7 @@ func NewSummaryContainer() *SummaryContainer { } } -func (c *SummaryContainer) Get(metricName string, labels prometheus.Labels) prometheus.Summary { +func (c *SummaryContainer) Get(metricName string, labels prometheus.Labels) (prometheus.Summary, error) { hash := hashNameAndLabels(metricName, labels) summary, ok := c.Elements[hash] if !ok { @@ -135,10 +135,10 @@ func (c *SummaryContainer) Get(metricName string, labels prometheus.Labels) prom }) c.Elements[hash] = summary if err := prometheus.Register(summary); err != nil { - log.Fatalf(regErrF, metricName, err) + return nil, err } } - return summary + return summary, nil } type Event interface { @@ -240,37 +240,48 @@ func (b *Exporter) Listen(e <-chan Events) { continue } - counter := b.Counters.Get( + counter, err := b.Counters.Get( b.suffix(metricName, "counter"), prometheusLabels, ) + if err == nil { + counter.Add(event.Value()) - counter.Add(event.Value()) - - eventStats.WithLabelValues("counter").Inc() + eventStats.WithLabelValues("counter").Inc() + } else { + log.Errorf(regErrF, metricName, err) + } case *GaugeEvent: - gauge := b.Gauges.Get( + gauge, err := b.Gauges.Get( b.suffix(metricName, "gauge"), prometheusLabels, ) - if ev.relative { - gauge.Add(event.Value()) + if err == nil { + if ev.relative { + gauge.Add(event.Value()) + } else { + gauge.Set(event.Value()) + } + + eventStats.WithLabelValues("gauge").Inc() } else { - gauge.Set(event.Value()) + log.Errorf(regErrF, metricName, err) } - eventStats.WithLabelValues("gauge").Inc() - case *TimerEvent: - summary := b.Summaries.Get( + summary, err := b.Summaries.Get( b.suffix(metricName, "timer"), prometheusLabels, ) - summary.Observe(event.Value()) + if err == nil { + summary.Observe(event.Value()) - eventStats.WithLabelValues("timer").Inc() + eventStats.WithLabelValues("timer").Inc() + } else { + log.Errorf(regErrF, metricName, err) + } default: log.Errorln("Unsupported event type") From c1791d7ecb59ca2db92417dfee13b1bce07e10f9 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Tue, 18 Jul 2017 08:48:51 -0700 Subject: [PATCH 3/4] Ensure we log errors every time there is a problem Move addition of metric to "Elements" until after a successful registry with prometheus, otherwise we'll continue to increment a metric which isn't registered --- exporter.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exporter.go b/exporter.go index 96efbbe..0a963d2 100644 --- a/exporter.go +++ b/exporter.go @@ -78,10 +78,10 @@ func (c *CounterContainer) Get(metricName string, labels prometheus.Labels) (pro Help: defaultHelp, ConstLabels: labels, }) - c.Elements[hash] = counter if err := prometheus.Register(counter); err != nil { return nil, err } + c.Elements[hash] = counter } return counter, nil } @@ -105,10 +105,10 @@ func (c *GaugeContainer) Get(metricName string, labels prometheus.Labels) (prome Help: defaultHelp, ConstLabels: labels, }) - c.Elements[hash] = gauge if err := prometheus.Register(gauge); err != nil { return nil, err } + c.Elements[hash] = gauge } return gauge, nil } @@ -133,10 +133,10 @@ func (c *SummaryContainer) Get(metricName string, labels prometheus.Labels) (pro Help: defaultHelp, ConstLabels: labels, }) - c.Elements[hash] = summary if err := prometheus.Register(summary); err != nil { return nil, err } + c.Elements[hash] = summary } return summary, nil } From bbd5227a1c169bff308a4fd4c2020b1c7a6ff22b Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Tue, 18 Jul 2017 08:56:27 -0700 Subject: [PATCH 4/4] Add metric for total number of conflicting events --- exporter.go | 3 +++ telemetry.go | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/exporter.go b/exporter.go index 0a963d2..0603dc7 100644 --- a/exporter.go +++ b/exporter.go @@ -250,6 +250,7 @@ func (b *Exporter) Listen(e <-chan Events) { eventStats.WithLabelValues("counter").Inc() } else { log.Errorf(regErrF, metricName, err) + conflictingEventStats.WithLabelValues("counter").Inc() } case *GaugeEvent: @@ -268,6 +269,7 @@ func (b *Exporter) Listen(e <-chan Events) { eventStats.WithLabelValues("gauge").Inc() } else { log.Errorf(regErrF, metricName, err) + conflictingEventStats.WithLabelValues("gauge").Inc() } case *TimerEvent: @@ -281,6 +283,7 @@ func (b *Exporter) Listen(e <-chan Events) { eventStats.WithLabelValues("timer").Inc() } else { log.Errorf(regErrF, metricName, err) + conflictingEventStats.WithLabelValues("timer").Inc() } default: diff --git a/telemetry.go b/telemetry.go index c668c7d..ae07fcf 100644 --- a/telemetry.go +++ b/telemetry.go @@ -47,6 +47,13 @@ var ( Name: "statsd_exporter_loaded_mappings", Help: "The current number of configured metric mappings.", }) + conflictingEventStats = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "statsd_exporter_events_conflict_total", + Help: "The total number of StatsD events with conflicting names.", + }, + []string{"type"}, + ) ) func init() { @@ -54,4 +61,5 @@ func init() { prometheus.MustRegister(networkStats) prometheus.MustRegister(configLoads) prometheus.MustRegister(mappingsCount) + prometheus.MustRegister(conflictingEventStats) }