From df402e885d4fe461b9bfef78ade52077b7bde26d Mon Sep 17 00:00:00 2001 From: Leandro Piccilli Date: Tue, 14 Feb 2017 00:56:48 +0100 Subject: [PATCH] Check if tag value is empty before allocation closes #2390 closes #2404 --- CHANGELOG.md | 1 + metric/metric.go | 7 ++++++- metric/metric_test.go | 23 +++++++++++++++++++++++ plugins/inputs/prometheus/parser_test.go | 8 +++----- plugins/parsers/nagios/parser_test.go | 2 +- 5 files changed, 34 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aaf94adb..a49f5e8f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ be deprecated eventually. - [#2341](https://github.com/influxdata/telegraf/issues/2341): telegraf swallowing panics in --test mode. - [#2358](https://github.com/influxdata/telegraf/pull/2358): Create pidfile with 644 permissions & defer file deletion. - [#2282](https://github.com/influxdata/telegraf/issues/2282): Reloading telegraf freezes prometheus output. +- [#2390](https://github.com/influxdata/telegraf/issues/2390): Empty tag value causes error on InfluxDB output. ## v1.2.1 [2017-02-01] diff --git a/metric/metric.go b/metric/metric.go index 0a2ca41b6..edf3b7794 100644 --- a/metric/metric.go +++ b/metric/metric.go @@ -44,13 +44,18 @@ func New( // pre-allocate exact size of the tags slice taglen := 0 for k, v := range tags { - // TODO check that length of tag key & value are > 0 + if len(k) == 0 || len(v) == 0 { + continue + } taglen += 2 + len(escape(k, "tagkey")) + len(escape(v, "tagval")) } m.tags = make([]byte, taglen) i := 0 for k, v := range tags { + if len(k) == 0 || len(v) == 0 { + continue + } m.tags[i] = ',' i++ i += copy(m.tags[i:], escape(k, "tagkey")) diff --git a/metric/metric_test.go b/metric/metric_test.go index f133a507c..dd231f8c4 100644 --- a/metric/metric_test.go +++ b/metric/metric_test.go @@ -625,3 +625,26 @@ func TestNewMetricFailNaN(t *testing.T) { _, err := New("cpu", tags, fields, now) assert.NoError(t, err) } + +func TestEmptyTagValueOrKey(t *testing.T) { + now := time.Now() + + tags := map[string]string{ + "host": "localhost", + "emptytag": "", + "": "valuewithoutkey", + } + fields := map[string]interface{}{ + "usage_idle": float64(99), + } + m, err := New("cpu", tags, fields, now) + + assert.True(t, m.HasTag("host")) + assert.False(t, m.HasTag("emptytag")) + assert.Equal(t, + fmt.Sprintf("cpu,host=localhost usage_idle=99 %d\n", now.UnixNano()), + m.String()) + + assert.NoError(t, err) + +} diff --git a/plugins/inputs/prometheus/parser_test.go b/plugins/inputs/prometheus/parser_test.go index fcd32ad43..4f2a8516f 100644 --- a/plugins/inputs/prometheus/parser_test.go +++ b/plugins/inputs/prometheus/parser_test.go @@ -111,11 +111,9 @@ func TestParseValidPrometheus(t *testing.T) { "gauge": float64(1), }, metrics[0].Fields()) assert.Equal(t, map[string]string{ - "osVersion": "CentOS Linux 7 (Core)", - "dockerVersion": "1.8.2", - "kernelVersion": "3.10.0-229.20.1.el7.x86_64", - "cadvisorRevision": "", - "cadvisorVersion": "", + "osVersion": "CentOS Linux 7 (Core)", + "dockerVersion": "1.8.2", + "kernelVersion": "3.10.0-229.20.1.el7.x86_64", }, metrics[0].Tags()) // Counter value diff --git a/plugins/parsers/nagios/parser_test.go b/plugins/parsers/nagios/parser_test.go index ee21ea117..b1e3d6fdd 100644 --- a/plugins/parsers/nagios/parser_test.go +++ b/plugins/parsers/nagios/parser_test.go @@ -67,7 +67,7 @@ func TestParseValidOutput(t *testing.T) { assert.Equal(t, map[string]interface{}{ "value": float64(0.008457), }, metrics[0].Fields()) - assert.Equal(t, map[string]string{"unit": ""}, metrics[0].Tags()) + assert.Equal(t, map[string]string{}, metrics[0].Tags()) } func TestParseInvalidOutput(t *testing.T) {