From a4ec884af56dbd750f1b46837934191ae9880988 Mon Sep 17 00:00:00 2001 From: Leandro Piccilli Date: Tue, 14 Feb 2017 00:56:48 +0100 Subject: [PATCH 1/4] Check if tag value is empty before allocation --- metric/metric.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/metric/metric.go b/metric/metric.go index 0a2ca41b6e282..12c9e433012b3 100644 --- a/metric/metric.go +++ b/metric/metric.go @@ -44,8 +44,12 @@ 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 - taglen += 2 + len(escape(k, "tagkey")) + len(escape(v, "tagval")) + vsize := len(escape(v, "tagval")) + if vsize > 0 { + taglen += 2 + len(escape(k, "tagkey")) + vsize + } else { + delete(tags, k) + } } m.tags = make([]byte, taglen) From 41b7d84075b3482c7b740bb45a9879f9b3ed5db7 Mon Sep 17 00:00:00 2001 From: Leandro Piccilli Date: Tue, 14 Feb 2017 00:57:40 +0100 Subject: [PATCH 2/4] Fix prometheus input test --- plugins/inputs/prometheus/parser_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/plugins/inputs/prometheus/parser_test.go b/plugins/inputs/prometheus/parser_test.go index fcd32ad43bbd7..4f2a8516f4b31 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 From 37c7202b14b36c5169ea484f7c93cf275529de53 Mon Sep 17 00:00:00 2001 From: Leandro Piccilli Date: Tue, 14 Feb 2017 00:58:28 +0100 Subject: [PATCH 3/4] Fix nagios parse test --- plugins/parsers/nagios/parser_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/parsers/nagios/parser_test.go b/plugins/parsers/nagios/parser_test.go index ee21ea1174ca4..b1e3d6fddc58c 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) { From 4d28c8580be87d833e673c320394c9fb4f747613 Mon Sep 17 00:00:00 2001 From: Leandro Piccilli Date: Wed, 15 Feb 2017 07:49:59 +0100 Subject: [PATCH 4/4] Update and add test --- metric/metric.go | 11 ++++++----- metric/metric_test.go | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/metric/metric.go b/metric/metric.go index 12c9e433012b3..edf3b77949d70 100644 --- a/metric/metric.go +++ b/metric/metric.go @@ -44,17 +44,18 @@ func New( // pre-allocate exact size of the tags slice taglen := 0 for k, v := range tags { - vsize := len(escape(v, "tagval")) - if vsize > 0 { - taglen += 2 + len(escape(k, "tagkey")) + vsize - } else { - delete(tags, k) + 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 f133a507ca129..dd231f8c46a6f 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) + +}