From 1c71191aaf40d4bcabaf15c1aa222b7915a5f29c Mon Sep 17 00:00:00 2001 From: Ivan Izaguirre Date: Sat, 23 Mar 2019 19:14:08 +0100 Subject: [PATCH 1/4] Fixes #191: crash on empty metric name Signed-off-by: Ivan Izaguirre --- exporter.go | 6 +++++- exporter_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/exporter.go b/exporter.go index 7a6adb4..7fe55c5 100644 --- a/exporter.go +++ b/exporter.go @@ -280,7 +280,7 @@ type Exporter struct { func escapeMetricName(metricName string) string { // If a metric starts with a digit, prepend an underscore. - if metricName[0] >= '0' && metricName[0] <= '9' { + if len(metricName) > 0 && metricName[0] >= '0' && metricName[0] <= '9' { metricName = "_" + metricName } @@ -333,6 +333,10 @@ func (b *Exporter) handleEvent(event Event) { metricName := "" prometheusLabels := event.Labels() if present { + if mapping.Name == "" { + log.Debugf("The mapping for match \"%v\" generates an empty metric name.", mapping.Match) + return + } metricName = escapeMetricName(mapping.Name) for label, value := range labels { prometheusLabels[label] = value diff --git a/exporter_test.go b/exporter_test.go index f353c96..cb31724 100644 --- a/exporter_test.go +++ b/exporter_test.go @@ -56,6 +56,38 @@ func TestNegativeCounter(t *testing.T) { ex.Listen(events) } +// TestEmptyStringMetric validates when a metric name ends up +// being the empty string after applying the match replacements +// tha we don't panic the Exporter Listener. +func TestEmptyStringMetric(t *testing.T) { + events := make(chan Events) + go func() { + c := Events{ + &CounterEvent{ + metricName: "foo_bar", + value: 1, + }, + } + events <- c + close(events) + }() + + config := ` +mappings: +- match: .*_bar + match_type: regex + name: "${1}" +` + testMapper := &mapper.MetricMapper{} + err := testMapper.InitFromYAMLString(config) + if err != nil { + t.Fatalf("Config load error: %s %s", config, err) + } + + ex := NewExporter(testMapper) + ex.Listen(events) +} + // TestInvalidUtf8InDatadogTagValue validates robustness of exporter listener // against datadog tags with invalid tag values. // It sends the same tags first with a valid value, then with an invalid one. @@ -167,6 +199,7 @@ func TestEscapeMetricName(t *testing.T) { "with😱emoji": "with_emoji", "with.*.multiple": "with___multiple", "test.web-server.foo.bar": "test_web_server_foo_bar", + "": "", } for in, want := range scenarios { From c477c7703ffe066207f2ae2b9ee0bf3ecedb7c53 Mon Sep 17 00:00:00 2001 From: Ivan Izaguirre Date: Sat, 23 Mar 2019 19:47:25 +0100 Subject: [PATCH 2/4] Fix format to comply with go1.11 rules Signed-off-by: Ivan Izaguirre --- exporter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter_test.go b/exporter_test.go index cb31724..0ddc0c3 100644 --- a/exporter_test.go +++ b/exporter_test.go @@ -199,7 +199,7 @@ func TestEscapeMetricName(t *testing.T) { "with😱emoji": "with_emoji", "with.*.multiple": "with___multiple", "test.web-server.foo.bar": "test_web_server_foo_bar", - "": "", + "": "", } for in, want := range scenarios { From c9e5e94ed282fb7287d4d92b020456bebd4b24f6 Mon Sep 17 00:00:00 2001 From: Ivan Izaguirre Date: Tue, 26 Mar 2019 00:44:17 +0100 Subject: [PATCH 3/4] Adds a separate counter for events discarded due to errors Signed-off-by: Ivan Izaguirre --- exporter.go | 5 +++-- exporter_test.go | 25 +++++++++++++++++++++++++ telemetry.go | 8 ++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/exporter.go b/exporter.go index 7fe55c5..15c01b7 100644 --- a/exporter.go +++ b/exporter.go @@ -334,7 +334,8 @@ func (b *Exporter) handleEvent(event Event) { prometheusLabels := event.Labels() if present { if mapping.Name == "" { - log.Debugf("The mapping for match \"%v\" generates an empty metric name.", mapping.Match) + log.Debugf("The mapping of '%s' for match '%s' generates an empty metric name", event.MetricName(), mapping.Match) + errorEventStats.WithLabelValues("empty_metric_name").Inc() return } metricName = escapeMetricName(mapping.Name) @@ -352,7 +353,7 @@ func (b *Exporter) handleEvent(event Event) { // will cause the exporter to panic. Instead we will warn and continue to the next event. if event.Value() < 0.0 { log.Debugf("Counter %q is: '%f' (counter must be non-negative value)", metricName, event.Value()) - eventStats.WithLabelValues("illegal_negative_counter").Inc() + errorEventStats.WithLabelValues("illegal_negative_counter").Inc() return } diff --git a/exporter_test.go b/exporter_test.go index 0ddc0c3..8b223e2 100644 --- a/exporter_test.go +++ b/exporter_test.go @@ -52,8 +52,16 @@ func TestNegativeCounter(t *testing.T) { close(events) }() + errorCounter := errorEventStats.WithLabelValues("illegal_negative_counter") + prev := getTelemetryCounterValue(errorCounter) + ex := NewExporter(&mapper.MetricMapper{}) ex.Listen(events) + + updated := getTelemetryCounterValue(errorCounter) + if updated-prev != 1 { + t.Fatal("Empty metric name error event not counted") + } } // TestEmptyStringMetric validates when a metric name ends up @@ -84,8 +92,16 @@ mappings: t.Fatalf("Config load error: %s %s", config, err) } + errorCounter := errorEventStats.WithLabelValues("empty_metric_name") + prev := getTelemetryCounterValue(errorCounter) + ex := NewExporter(testMapper) ex.Listen(events) + + updated := getTelemetryCounterValue(errorCounter) + if updated-prev != 1 { + t.Fatal("Empty metric name error event not counted") + } } // TestInvalidUtf8InDatadogTagValue validates robustness of exporter listener @@ -388,3 +404,12 @@ func labelPairsAsLabels(pairs []*dto.LabelPair) (labels prometheus.Labels) { } return } + +func getTelemetryCounterValue(counter prometheus.Counter) float64 { + var metric dto.Metric + err := counter.Write(&metric) + if err != nil { + return 0.0 + } + return metric.Counter.GetValue() +} diff --git a/telemetry.go b/telemetry.go index 4e881aa..541140c 100644 --- a/telemetry.go +++ b/telemetry.go @@ -102,6 +102,13 @@ var ( }, []string{"type"}, ) + errorEventStats = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "statsd_exporter_events_error_total", + Help: "The total number of StatsD events discarded due to errors.", + }, + []string{"reason"}, + ) ) func init() { @@ -119,4 +126,5 @@ func init() { prometheus.MustRegister(configLoads) prometheus.MustRegister(mappingsCount) prometheus.MustRegister(conflictingEventStats) + prometheus.MustRegister(errorEventStats) } From 51e735c87801f60e494a7c47d24c33e4e29011ae Mon Sep 17 00:00:00 2001 From: Ivan Izaguirre Date: Tue, 26 Mar 2019 01:02:38 +0100 Subject: [PATCH 4/4] Fix test error message Signed-off-by: Ivan Izaguirre --- exporter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter_test.go b/exporter_test.go index 8b223e2..fc0d0b1 100644 --- a/exporter_test.go +++ b/exporter_test.go @@ -60,7 +60,7 @@ func TestNegativeCounter(t *testing.T) { updated := getTelemetryCounterValue(errorCounter) if updated-prev != 1 { - t.Fatal("Empty metric name error event not counted") + t.Fatal("Illegal negative counter error not counted") } }