From 932e0ca918fd8f36c34e7fb81caa9ec9e0e6d333 Mon Sep 17 00:00:00 2001 From: Ririsoft Date: Wed, 21 Sep 2016 08:00:36 +0200 Subject: [PATCH] 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 53f40063b31fd9ef3d92e7fc22e821d0f71ac46d. --- plugins/inputs/cgroup/README.md | 5 +- plugins/inputs/cgroup/cgroup_linux.go | 5 +- plugins/inputs/cgroup/cgroup_test.go | 84 ++++++++++++--------------- 3 files changed, 41 insertions(+), 53 deletions(-) diff --git a/plugins/inputs/cgroup/README.md b/plugins/inputs/cgroup/README.md index feb332dd9..ab06342bf 100644 --- a/plugins/inputs/cgroup/README.md +++ b/plugins/inputs/cgroup/README.md @@ -33,9 +33,8 @@ KEY1 VAL1\n ### Tags: -Measurements don't have any specific tags unless you define them at the telegraf level (defaults). We -used to have the path listed as a tag, but to keep cardinality in check it's easier to move this -value to a field. Thanks @sebito91! +All measurements have the following tags: + - path ### Configuration: diff --git a/plugins/inputs/cgroup/cgroup_linux.go b/plugins/inputs/cgroup/cgroup_linux.go index ecaf8126d..e8ba6f881 100644 --- a/plugins/inputs/cgroup/cgroup_linux.go +++ b/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 } diff --git a/plugins/inputs/cgroup/cgroup_test.go b/plugins/inputs/cgroup/cgroup_test.go index ff9b8d7a8..206b51f6d 100644 --- a/plugins/inputs/cgroup/cgroup_test.go +++ b/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{}{ - "memory.limit_in_bytes": 223372036854771712, - "path": "testdata/memory/group_2", + tags = map[string]string{ + "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{}{ - "memory.limit_in_bytes": 223372036854771712, - "path": "testdata/memory/group_1/group_1_2", + tags = map[string]string{ + "path": "testdata/memory/group_1/group_1_2", } + acc.AssertContainsTaggedFields(t, "cgroup", fields, tags) - fieldsThree := map[string]interface{}{ - "memory.limit_in_bytes": 223372036854771712, - "path": "testdata/memory/group_2", + tags = map[string]string{ + "path": "testdata/memory/group_2", } - - assertContainsFields(&acc, t, "cgroup", []map[string]interface{}{fields, fieldsTwo, fieldsThree}) + acc.AssertContainsTaggedFields(t, "cgroup", fields, tags) } // ====================================================================== @@ -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{}{ - "memory.limit_in_bytes": 223372036854771712, - "path": "testdata/memory/group_2/group_1_1", + tags = map[string]string{ + "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) }