From a0e42f8a6176ac792ebf658e697f4b364bd4042c Mon Sep 17 00:00:00 2001
From: Cameron Sparr <cameronsparr@gmail.com>
Date: Wed, 17 Aug 2016 16:20:32 +0100
Subject: [PATCH] Sanitize graphite characters in field names

also sanitize the names at a higher scope for better clarity

closes #1637
---
 CHANGELOG.md                                  |  1 +
 plugins/serializers/graphite/graphite.go      | 10 +-
 plugins/serializers/graphite/graphite_test.go | 92 ++++++++++++++++---
 3 files changed, 84 insertions(+), 19 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8725d298f..5bdd04142 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -138,6 +138,7 @@ consistent with the behavior of `collection_jitter`.
 - [#1600](https://github.com/influxdata/telegraf/issues/1600): Fix quoting with text values in postgresql_extensible plugin.
 - [#1425](https://github.com/influxdata/telegraf/issues/1425): Fix win_perf_counter "index out of range" panic.
 - [#1634](https://github.com/influxdata/telegraf/issues/1634): Fix ntpq panic when field is missing.
+- [#1637](https://github.com/influxdata/telegraf/issues/1637): Sanitize graphite output field names.
 
 ## v0.13.1 [2016-05-24]
 
diff --git a/plugins/serializers/graphite/graphite.go b/plugins/serializers/graphite/graphite.go
index 2cc4add56..6a6fd9cac 100644
--- a/plugins/serializers/graphite/graphite.go
+++ b/plugins/serializers/graphite/graphite.go
@@ -12,7 +12,7 @@ const DEFAULT_TEMPLATE = "host.tags.measurement.field"
 
 var (
 	fieldDeleter   = strings.NewReplacer(".FIELDNAME", "", "FIELDNAME.", "")
-	sanitizedChars = strings.NewReplacer("/", "-", "@", "-", "*", "-", " ", "_", "..", ".")
+	sanitizedChars = strings.NewReplacer("/", "-", "@", "-", "*", "-", " ", "_", "..", ".", `\`, "")
 )
 
 type GraphiteSerializer struct {
@@ -36,8 +36,8 @@ func (s *GraphiteSerializer) Serialize(metric telegraf.Metric) ([]string, error)
 		valueS := fmt.Sprintf("%#v", value)
 		point := fmt.Sprintf("%s %s %d",
 			// insert "field" section of template
-			InsertField(bucket, fieldName),
-			valueS,
+			sanitizedChars.Replace(InsertField(bucket, fieldName)),
+			sanitizedChars.Replace(valueS),
 			timestamp)
 		out = append(out, point)
 	}
@@ -100,9 +100,9 @@ func SerializeBucketName(
 	}
 
 	if prefix == "" {
-		return sanitizedChars.Replace(strings.Join(out, "."))
+		return strings.Join(out, ".")
 	}
-	return sanitizedChars.Replace(prefix + "." + strings.Join(out, "."))
+	return prefix + "." + strings.Join(out, ".")
 }
 
 // InsertField takes the bucket string from SerializeBucketName and replaces the
diff --git a/plugins/serializers/graphite/graphite_test.go b/plugins/serializers/graphite/graphite_test.go
index 50ba0e2e0..57196b861 100644
--- a/plugins/serializers/graphite/graphite_test.go
+++ b/plugins/serializers/graphite/graphite_test.go
@@ -160,6 +160,58 @@ func TestSerializeValueField2(t *testing.T) {
 	assert.Equal(t, expS, mS)
 }
 
+// test that fields with spaces get fixed.
+func TestSerializeFieldWithSpaces(t *testing.T) {
+	now := time.Now()
+	tags := map[string]string{
+		"host":       "localhost",
+		"cpu":        "cpu0",
+		"datacenter": "us-west-2",
+	}
+	fields := map[string]interface{}{
+		`field\ with\ spaces`: float64(91.5),
+	}
+	m, err := telegraf.NewMetric("cpu", tags, fields, now)
+	assert.NoError(t, err)
+
+	s := GraphiteSerializer{
+		Template: "host.tags.measurement.field",
+	}
+	mS, err := s.Serialize(m)
+	assert.NoError(t, err)
+
+	expS := []string{
+		fmt.Sprintf("localhost.cpu0.us-west-2.cpu.field_with_spaces 91.5 %d", now.Unix()),
+	}
+	assert.Equal(t, expS, mS)
+}
+
+// test that tags with spaces get fixed.
+func TestSerializeTagWithSpaces(t *testing.T) {
+	now := time.Now()
+	tags := map[string]string{
+		"host":       "localhost",
+		"cpu":        `cpu\ 0`,
+		"datacenter": "us-west-2",
+	}
+	fields := map[string]interface{}{
+		`field_with_spaces`: float64(91.5),
+	}
+	m, err := telegraf.NewMetric("cpu", tags, fields, now)
+	assert.NoError(t, err)
+
+	s := GraphiteSerializer{
+		Template: "host.tags.measurement.field",
+	}
+	mS, err := s.Serialize(m)
+	assert.NoError(t, err)
+
+	expS := []string{
+		fmt.Sprintf("localhost.cpu_0.us-west-2.cpu.field_with_spaces 91.5 %d", now.Unix()),
+	}
+	assert.Equal(t, expS, mS)
+}
+
 // test that a field named "value" gets ignored at beginning of template.
 func TestSerializeValueField3(t *testing.T) {
 	now := time.Now()
@@ -186,6 +238,32 @@ func TestSerializeValueField3(t *testing.T) {
 	assert.Equal(t, expS, mS)
 }
 
+// test that a field named "value" gets ignored at beginning of template.
+func TestSerializeValueField5(t *testing.T) {
+	now := time.Now()
+	tags := map[string]string{
+		"host":       "localhost",
+		"cpu":        "cpu0",
+		"datacenter": "us-west-2",
+	}
+	fields := map[string]interface{}{
+		"value": float64(91.5),
+	}
+	m, err := telegraf.NewMetric("cpu", tags, fields, now)
+	assert.NoError(t, err)
+
+	s := GraphiteSerializer{
+		Template: template5,
+	}
+	mS, err := s.Serialize(m)
+	assert.NoError(t, err)
+
+	expS := []string{
+		fmt.Sprintf("localhost.us-west-2.cpu0.cpu 91.5 %d", now.Unix()),
+	}
+	assert.Equal(t, expS, mS)
+}
+
 func TestSerializeMetricPrefix(t *testing.T) {
 	now := time.Now()
 	tags := map[string]string{
@@ -315,20 +393,6 @@ func TestTemplate4(t *testing.T) {
 	assert.Equal(t, expS, mS)
 }
 
-func TestTemplate5(t *testing.T) {
-	now := time.Now()
-	fields := map[string]interface{}{
-		"usage_idle": float64(91.5),
-	}
-	m, err := telegraf.NewMetric("cpu", defaultTags, fields, now)
-	assert.NoError(t, err)
-
-	mS := SerializeBucketName(m.Name(), m.Tags(), template5, "")
-
-	expS := "localhost.us-west-2.cpu0.cpu.FIELDNAME"
-	assert.Equal(t, expS, mS)
-}
-
 func TestTemplate6(t *testing.T) {
 	now := time.Now()
 	fields := map[string]interface{}{