From bccef2856dd4f49e49400a8bb931ab2a1bba5a79 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. closes #1724 closes #1796 --- CHANGELOG.md | 1 + plugins/inputs/cgroup/README.md | 9 ++- plugins/inputs/cgroup/cgroup.go | 21 ++++--- plugins/inputs/cgroup/cgroup_linux.go | 5 +- plugins/inputs/cgroup/cgroup_test.go | 84 ++++++++++++--------------- 5 files changed, 58 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a0034618..5fdd4bde3 100644 --- a/CHANGELOG.md +++ b/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] diff --git a/plugins/inputs/cgroup/README.md b/plugins/inputs/cgroup/README.md index feb332dd9..e62c9b15e 100644 --- a/plugins/inputs/cgroup/README.md +++ b/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 -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.go b/plugins/inputs/cgroup/cgroup.go index e38b6a4c1..cc5e4b496 100644 --- a/plugins/inputs/cgroup/cgroup.go +++ b/plugins/inputs/cgroup/cgroup.go @@ -11,15 +11,18 @@ type CGroup struct { } var sampleConfig = ` - ## Directories in which to look for files, globs are supported. - # paths = [ - # "/cgroup/memory", - # "/cgroup/memory/child1", - # "/cgroup/memory/child2/*", - # ] - ## 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"] + ## Directories in which to look for files, globs are supported. + ## 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. + # paths = [ + # "/cgroup/memory", + # "/cgroup/memory/child1", + # "/cgroup/memory/child2/*", + # ] + ## 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 { 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) }