Revert "Moving cgroup path name to field from tag to reduce cardinality (#1457)"

This was introducing a regression with influxdb output, leading to
collision an points missing.
This reverts commit 53f40063b3.

closes #1724
closes #1796

CHANGELOG.md

@@ -60,6 +60,7 @@ continue sending logs to /var/log/telegraf/telegraf.log.
 - [#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.
 - [#1836](https://github.com/influxdata/telegraf/pull/1836): Fix snmp table field initialization for non-automatic table.
+- [#1724](https://github.com/influxdata/telegraf/issues/1724): cgroups path being parsed as metric.
 
 ## v1.0.1 [2016-09-26]
 

plugins/inputs/cgroup/README.md

@@ -2,6 +2,10 @@
 
 This input plugin will capture specific statistics per cgroup.
 
+Consider restricting paths to the set of cgroups you really
+want to monitor if you have a large number of cgroups, to avoid
+any cardinality issues.
+
 Following file formats are supported:
 
 * Single value
@@ -33,9 +37,8 @@ KEY1 VAL1\n
 
 ### Tags:
 
-Measurements don't have any specific tags unless you define them at the telegraf level (defaults). We
+All measurements have the following tags:
-used to have the path listed as a tag, but to keep cardinality in check it's easier to move this 
+  - path
-value to a field. Thanks @sebito91!
 
 
 ### Configuration:

plugins/inputs/cgroup/cgroup.go

@@ -11,15 +11,18 @@ type CGroup struct {
 }
 
 var sampleConfig = `
-    ## Directories in which to look for files, globs are supported.
+  ## Directories in which to look for files, globs are supported.
-	# paths = [
+  ## Consider restricting paths to the set of cgroups you really
-	#   "/cgroup/memory",
+  ## want to monitor if you have a large number of cgroups, to avoid
-	#   "/cgroup/memory/child1",
+  ## any cardinality issues.
-	#   "/cgroup/memory/child2/*",
+  # paths = [
-	# ]
+  #   "/cgroup/memory",
-	## cgroup stat fields, as file names, globs are supported.
+  #   "/cgroup/memory/child1",
-	## these file names are appended to each path from above.
+  #   "/cgroup/memory/child2/*",
-	# files = ["memory.*usage*", "memory.limit_in_bytes"]
+  # ]
+  ## cgroup stat fields, as file names, globs are supported.
+  ## these file names are appended to each path from above.
+  # files = ["memory.*usage*", "memory.limit_in_bytes"]
 `
 
 func (g *CGroup) SampleConfig() string {

plugins/inputs/cgroup/cgroup_linux.go

@@ -56,9 +56,10 @@ func (g *CGroup) gatherDir(dir string, acc telegraf.Accumulator) error {
 			return err
 		}
 	}
-	fields["path"] = dir
 
-	acc.AddFields(metricName, fields, nil)
+	tags := map[string]string{"path": dir}
+
+	acc.AddFields(metricName, fields, tags)
 
 	return nil
 }

plugins/inputs/cgroup/cgroup_test.go

@@ -3,13 +3,10 @@
 package cgroup
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/influxdata/telegraf/testutil"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"reflect"
 )
 
 var cg1 = &CGroup{
@@ -24,32 +21,15 @@ var cg1 = &CGroup{
 	},
 }
 
-func assertContainsFields(a *testutil.Accumulator, t *testing.T, measurement string, fieldSet []map[string]interface{}) {
-	a.Lock()
-	defer a.Unlock()
-
-	numEquals := 0
-	for _, p := range a.Metrics {
-		if p.Measurement == measurement {
-			for _, fields := range fieldSet {
-				if reflect.DeepEqual(fields, p.Fields) {
-					numEquals++
-				}
-			}
-		}
-	}
-
-	if numEquals != len(fieldSet) {
-		assert.Fail(t, fmt.Sprintf("only %d of %d are equal", numEquals, len(fieldSet)))
-	}
-}
-
 func TestCgroupStatistics_1(t *testing.T) {
 	var acc testutil.Accumulator
 
 	err := cg1.Gather(&acc)
 	require.NoError(t, err)
 
+	tags := map[string]string{
+		"path": "testdata/memory",
+	}
 	fields := map[string]interface{}{
 		"memory.stat.cache":           1739362304123123123,
 		"memory.stat.rss":             1775325184,
@@ -62,9 +42,8 @@ func TestCgroupStatistics_1(t *testing.T) {
 		"memory.limit_in_bytes":       223372036854771712,
 		"memory.use_hierarchy":        "12-781",
 		"notify_on_release":           0,
-		"path":                        "testdata/memory",
 	}
-	assertContainsFields(&acc, t, "cgroup", []map[string]interface{}{fields})
+	acc.AssertContainsTaggedFields(t, "cgroup", fields, tags)
 }
 
 // ======================================================================
@@ -80,14 +59,16 @@ func TestCgroupStatistics_2(t *testing.T) {
 	err := cg2.Gather(&acc)
 	require.NoError(t, err)
 
+	tags := map[string]string{
+		"path": "testdata/cpu",
+	}
 	fields := map[string]interface{}{
 		"cpuacct.usage_percpu.0": -1452543795404,
 		"cpuacct.usage_percpu.1": 1376681271659,
 		"cpuacct.usage_percpu.2": 1450950799997,
 		"cpuacct.usage_percpu.3": -1473113374257,
-		"path": "testdata/cpu",
 	}
-	assertContainsFields(&acc, t, "cgroup", []map[string]interface{}{fields})
+	acc.AssertContainsTaggedFields(t, "cgroup", fields, tags)
 }
 
 // ======================================================================
@@ -103,16 +84,18 @@ func TestCgroupStatistics_3(t *testing.T) {
 	err := cg3.Gather(&acc)
 	require.NoError(t, err)
 
+	tags := map[string]string{
+		"path": "testdata/memory/group_1",
+	}
 	fields := map[string]interface{}{
 		"memory.limit_in_bytes": 223372036854771712,
-		"path":                  "testdata/memory/group_1",
 	}
+	acc.AssertContainsTaggedFields(t, "cgroup", fields, tags)
 
-	fieldsTwo := map[string]interface{}{
+	tags = map[string]string{
-		"memory.limit_in_bytes": 223372036854771712,
+		"path": "testdata/memory/group_2",
-		"path":                  "testdata/memory/group_2",
 	}
-	assertContainsFields(&acc, t, "cgroup", []map[string]interface{}{fields, fieldsTwo})
+	acc.AssertContainsTaggedFields(t, "cgroup", fields, tags)
 }
 
 // ======================================================================
@@ -128,22 +111,23 @@ func TestCgroupStatistics_4(t *testing.T) {
 	err := cg4.Gather(&acc)
 	require.NoError(t, err)
 
+	tags := map[string]string{
+		"path": "testdata/memory/group_1/group_1_1",
+	}
 	fields := map[string]interface{}{
 		"memory.limit_in_bytes": 223372036854771712,
-		"path":                  "testdata/memory/group_1/group_1_1",
 	}
+	acc.AssertContainsTaggedFields(t, "cgroup", fields, tags)
 
-	fieldsTwo := map[string]interface{}{
+	tags = map[string]string{
-		"memory.limit_in_bytes": 223372036854771712,
+		"path": "testdata/memory/group_1/group_1_2",
-		"path":                  "testdata/memory/group_1/group_1_2",
 	}
+	acc.AssertContainsTaggedFields(t, "cgroup", fields, tags)
 
-	fieldsThree := map[string]interface{}{
+	tags = map[string]string{
-		"memory.limit_in_bytes": 223372036854771712,
+		"path": "testdata/memory/group_2",
-		"path":                  "testdata/memory/group_2",
 	}
-
+	acc.AssertContainsTaggedFields(t, "cgroup", fields, tags)
-	assertContainsFields(&acc, t, "cgroup", []map[string]interface{}{fields, fieldsTwo, fieldsThree})
 }
 
 // ======================================================================
@@ -159,16 +143,18 @@ func TestCgroupStatistics_5(t *testing.T) {
 	err := cg5.Gather(&acc)
 	require.NoError(t, err)
 
+	tags := map[string]string{
+		"path": "testdata/memory/group_1/group_1_1",
+	}
 	fields := map[string]interface{}{
 		"memory.limit_in_bytes": 223372036854771712,
-		"path":                  "testdata/memory/group_1/group_1_1",
 	}
+	acc.AssertContainsTaggedFields(t, "cgroup", fields, tags)
 
-	fieldsTwo := map[string]interface{}{
+	tags = map[string]string{
-		"memory.limit_in_bytes": 223372036854771712,
+		"path": "testdata/memory/group_2/group_1_1",
-		"path":                  "testdata/memory/group_2/group_1_1",
 	}
-	assertContainsFields(&acc, t, "cgroup", []map[string]interface{}{fields, fieldsTwo})
+	acc.AssertContainsTaggedFields(t, "cgroup", fields, tags)
 }
 
 // ======================================================================
@@ -184,11 +170,13 @@ func TestCgroupStatistics_6(t *testing.T) {
 	err := cg6.Gather(&acc)
 	require.NoError(t, err)
 
+	tags := map[string]string{
+		"path": "testdata/memory",
+	}
 	fields := map[string]interface{}{
 		"memory.usage_in_bytes":      3513667584,
 		"memory.use_hierarchy":       "12-781",
 		"memory.kmem.limit_in_bytes": 9223372036854771712,
-		"path": "testdata/memory",
 	}
-	assertContainsFields(&acc, t, "cgroup", []map[string]interface{}{fields})
+	acc.AssertContainsTaggedFields(t, "cgroup", fields, tags)
 }