From e96f7a9b1240e3eee9ccebf5f3e8b1844ac52f3a Mon Sep 17 00:00:00 2001
From: Cameron Sparr <cameronsparr@gmail.com>
Date: Mon, 10 Oct 2016 15:35:15 +0100
Subject: [PATCH] graphite parser, handle multiple templates empty filter

Previously, the graphite parser would simply overwrite any template that
had an identical filter to a previous template. This included the empty
filter.

Now we will still overwrite, but first we will sort to make sure that
the most "specific" template always matches.

closes #1731
---
 CHANGELOG.md                            |  1 +
 docs/DATA_FORMATS_INPUT.md              | 10 +++
 plugins/parsers/graphite/parser.go      | 81 +++++++++++++++++++------
 plugins/parsers/graphite/parser_test.go | 42 +++++++++++++
 4 files changed, 114 insertions(+), 20 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8a054d39..b65c68c3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -58,6 +58,7 @@ continue sending logs to /var/log/telegraf/telegraf.log.
 - [#1854](https://github.com/influxdata/telegraf/pull/1853): SQL Server waitstats truncation bug.
 - [#1810](https://github.com/influxdata/telegraf/issues/1810): Fix logparser common log format: numbers in ident.
 - [#1793](https://github.com/influxdata/telegraf/pull/1793): Fix JSON Serialization in OpenTSDB output.
+- [#1731](https://github.com/influxdata/telegraf/issues/1731): Fix Graphite template ordering, use most specific.
 
 ## v1.0.1 [2016-09-26]
 
diff --git a/docs/DATA_FORMATS_INPUT.md b/docs/DATA_FORMATS_INPUT.md
index 2e3a479a..c14752d9 100644
--- a/docs/DATA_FORMATS_INPUT.md
+++ b/docs/DATA_FORMATS_INPUT.md
@@ -232,6 +232,16 @@ us.west.cpu.load 100
 => cpu.load,region=us.west value=100
 ```
 
+Multiple templates can also be specified, but these should be differentiated
+using _filters_ (see below for more details)
+
+```toml
+templates = [
+    "*.*.* region.region.measurement", # <- all 3-part measurements will match this one.
+    "*.*.*.* region.region.host.measurement", # <- all 4-part measurements will match this one.
+]
+```
+
 #### Field Templates:
 
 The field keyword tells Telegraf to give the metric that field name.
diff --git a/plugins/parsers/graphite/parser.go b/plugins/parsers/graphite/parser.go
index d371274d..4a3c21df 100644
--- a/plugins/parsers/graphite/parser.go
+++ b/plugins/parsers/graphite/parser.go
@@ -57,38 +57,34 @@ func NewGraphiteParser(
 	defaultTemplate, _ := NewTemplate("measurement*", nil, p.Separator)
 	matcher.AddDefaultTemplate(defaultTemplate)
 
+	tmplts := parsedTemplates{}
 	for _, pattern := range p.Templates {
-		template := pattern
-		filter := ""
+		tmplt := parsedTemplate{}
+		tmplt.template = pattern
 		// Format is [filter] <template> [tag1=value1,tag2=value2]
 		parts := strings.Fields(pattern)
 		if len(parts) < 1 {
 			continue
 		} else if len(parts) >= 2 {
 			if strings.Contains(parts[1], "=") {
-				template = parts[0]
+				tmplt.template = parts[0]
+				tmplt.tagstring = parts[1]
 			} else {
-				filter = parts[0]
-				template = parts[1]
-			}
-		}
-
-		// Parse out the default tags specific to this template
-		tags := map[string]string{}
-		if strings.Contains(parts[len(parts)-1], "=") {
-			tagStrs := strings.Split(parts[len(parts)-1], ",")
-			for _, kv := range tagStrs {
-				parts := strings.Split(kv, "=")
-				tags[parts[0]] = parts[1]
+				tmplt.filter = parts[0]
+				tmplt.template = parts[1]
+				if len(parts) > 2 {
+					tmplt.tagstring = parts[2]
+				}
 			}
 		}
+		tmplts = append(tmplts, tmplt)
+	}
 
-		tmpl, err1 := NewTemplate(template, tags, p.Separator)
-		if err1 != nil {
-			err = err1
-			break
+	sort.Sort(tmplts)
+	for _, tmplt := range tmplts {
+		if err := p.addToMatcher(tmplt); err != nil {
+			return nil, err
 		}
-		matcher.Add(filter, tmpl)
 	}
 
 	if err != nil {
@@ -98,6 +94,24 @@ func NewGraphiteParser(
 	}
 }
 
+func (p *GraphiteParser) addToMatcher(tmplt parsedTemplate) error {
+	// Parse out the default tags specific to this template
+	tags := map[string]string{}
+	if tmplt.tagstring != "" {
+		for _, kv := range strings.Split(tmplt.tagstring, ",") {
+			parts := strings.Split(kv, "=")
+			tags[parts[0]] = parts[1]
+		}
+	}
+
+	tmpl, err := NewTemplate(tmplt.template, tags, p.Separator)
+	if err != nil {
+		return err
+	}
+	p.matcher.Add(tmplt.filter, tmpl)
+	return nil
+}
+
 func (p *GraphiteParser) Parse(buf []byte) ([]telegraf.Metric, error) {
 	// parse even if the buffer begins with a newline
 	buf = bytes.TrimPrefix(buf, []byte("\n"))
@@ -465,3 +479,30 @@ func (n *nodes) Less(j, k int) bool {
 
 func (n *nodes) Swap(i, j int) { (*n)[i], (*n)[j] = (*n)[j], (*n)[i] }
 func (n *nodes) Len() int      { return len(*n) }
+
+type parsedTemplate struct {
+	template  string
+	filter    string
+	tagstring string
+}
+type parsedTemplates []parsedTemplate
+
+func (e parsedTemplates) Less(j, k int) bool {
+	if len(e[j].filter) == 0 && len(e[k].filter) == 0 {
+		nj := len(strings.Split(e[j].template, "."))
+		nk := len(strings.Split(e[k].template, "."))
+		return nj < nk
+	}
+	if len(e[j].filter) == 0 {
+		return true
+	}
+	if len(e[k].filter) == 0 {
+		return false
+	}
+
+	nj := len(strings.Split(e[j].template, "."))
+	nk := len(strings.Split(e[k].template, "."))
+	return nj < nk
+}
+func (e parsedTemplates) Swap(i, j int) { e[i], e[j] = e[j], e[i] }
+func (e parsedTemplates) Len() int      { return len(e) }
diff --git a/plugins/parsers/graphite/parser_test.go b/plugins/parsers/graphite/parser_test.go
index 55f1a9e2..9665c2c4 100644
--- a/plugins/parsers/graphite/parser_test.go
+++ b/plugins/parsers/graphite/parser_test.go
@@ -747,6 +747,48 @@ func TestApplyTemplateGreedyField(t *testing.T) {
 	}
 }
 
+func TestApplyTemplateOverSpecific(t *testing.T) {
+	p, err := NewGraphiteParser(
+		".",
+		[]string{
+			"measurement.host.metric.metric.metric",
+		},
+		nil,
+	)
+	assert.NoError(t, err)
+
+	measurement, tags, _, err := p.ApplyTemplate("net.server001.a.b 2")
+	assert.Equal(t, "net", measurement)
+	assert.Equal(t,
+		map[string]string{"host": "server001", "metric": "a.b"},
+		tags)
+}
+
+func TestApplyTemplateMostSpecificTemplate(t *testing.T) {
+	p, err := NewGraphiteParser(
+		".",
+		[]string{
+			"measurement.host.metric",
+			"measurement.host.metric.metric.metric",
+			"measurement.host.metric.metric",
+		},
+		nil,
+	)
+	assert.NoError(t, err)
+
+	measurement, tags, _, err := p.ApplyTemplate("net.server001.a.b.c 2")
+	assert.Equal(t, "net", measurement)
+	assert.Equal(t,
+		map[string]string{"host": "server001", "metric": "a.b.c"},
+		tags)
+
+	measurement, tags, _, err = p.ApplyTemplate("net.server001.a.b 2")
+	assert.Equal(t, "net", measurement)
+	assert.Equal(t,
+		map[string]string{"host": "server001", "metric": "a.b"},
+		tags)
+}
+
 // Test Helpers
 func errstr(err error) string {
 	if err != nil {
-- 
GitLab