From a3c846b73e4b93a90c6c82b57f64958619b5bb31 Mon Sep 17 00:00:00 2001 From: subhachandrachandra Date: Fri, 21 Aug 2015 15:15:19 -0700 Subject: [PATCH 1/4] Fixed total memory reporting for Darwin systems. hw.memsize is reported as bytes instead of pages. --- plugins/system/ps/mem/mem_darwin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/system/ps/mem/mem_darwin.go b/plugins/system/ps/mem/mem_darwin.go index 5d2ff7e3e..43da44d1d 100644 --- a/plugins/system/ps/mem/mem_darwin.go +++ b/plugins/system/ps/mem/mem_darwin.go @@ -53,7 +53,7 @@ func VirtualMemory() (*VirtualMemoryStat, error) { } ret := &VirtualMemoryStat{ - Total: parsed[0] * p, + Total: parsed[0], Free: parsed[1] * p, } From 13ee9ff37be54226cf23bd44b34016be7db6d38f Mon Sep 17 00:00:00 2001 From: subhachandrachandra Date: Fri, 21 Aug 2015 16:08:54 -0700 Subject: [PATCH 2/4] Fixed memory reporting for Linux systems /proc/meminfo reports memory in KiloBytes and so needs a multiplier of 1024 instead of 1000. The kernel reports in terms of pages and the proc filesystem is left shifting by 2 for 4KB pages to get KB. Since this is a binary shift, Bytes will need to shift by 10 and so get multiplied by 1024. From the kernel code. PAGE_SHIFT = 12 for 4KB pages "MemTotal: %8lu kB\n", K(i.totalram) --- plugins/system/ps/mem/mem_linux.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/system/ps/mem/mem_linux.go b/plugins/system/ps/mem/mem_linux.go index a0505b5f5..42a49a2b6 100644 --- a/plugins/system/ps/mem/mem_linux.go +++ b/plugins/system/ps/mem/mem_linux.go @@ -30,17 +30,17 @@ func VirtualMemory() (*VirtualMemoryStat, error) { } switch key { case "MemTotal": - ret.Total = t * 1000 + ret.Total = t * 1024 case "MemFree": - ret.Free = t * 1000 + ret.Free = t * 1024 case "Buffers": - ret.Buffers = t * 1000 + ret.Buffers = t * 1024 case "Cached": - ret.Cached = t * 1000 + ret.Cached = t * 1024 case "Active": - ret.Active = t * 1000 + ret.Active = t * 1024 case "Inactive": - ret.Inactive = t * 1000 + ret.Inactive = t * 1024 } } ret.Available = ret.Free + ret.Buffers + ret.Cached From e6ea09f4825d073094eec6763716923b478876ae Mon Sep 17 00:00:00 2001 From: subhachandrachandra Date: Wed, 7 Oct 2015 14:42:11 -0700 Subject: [PATCH 3/4] Added Mountpoints and SkipInodeUsage options to the Disk plugin to control which mountpoint stats get reported for and to skip inode stats. --- plugins/system/disk.go | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/plugins/system/disk.go b/plugins/system/disk.go index 74348bdbe..cac219757 100644 --- a/plugins/system/disk.go +++ b/plugins/system/disk.go @@ -8,13 +8,27 @@ import ( type DiskStats struct { ps PS + + Mountpoints []string + SkipInodeUsage bool } func (_ *DiskStats) Description() string { return "Read metrics about disk usage by mount point" } -func (_ *DiskStats) SampleConfig() string { return "" } +var diskSampleConfig = ` + # By default, telegraf gather stats for all mountpoints and for inodes. + # Setting mountpoints will restrict the stats to the specified ones. + # mountpoints. + # Mountpoints=["/"] + # Setting SkipInodeUsage will skip the reporting of inode stats. + # SkipInodeUsage=true +` + +func (_ *DiskStats) SampleConfig() string { + return diskSampleConfig +} func (s *DiskStats) Gather(acc plugins.Accumulator) error { disks, err := s.ps.DiskUsage() @@ -22,7 +36,16 @@ func (s *DiskStats) Gather(acc plugins.Accumulator) error { return fmt.Errorf("error getting disk usage info: %s", err) } + mPoints := make(map[string]bool) + for _, mp := range s.Mountpoints { + mPoints[mp] = true + } + for _, du := range disks { + _, member := mPoints[ du.Path ] + if !member { + continue + } tags := map[string]string{ "path": du.Path, "fstype": du.Fstype, @@ -30,9 +53,11 @@ func (s *DiskStats) Gather(acc plugins.Accumulator) error { acc.Add("total", du.Total, tags) acc.Add("free", du.Free, tags) acc.Add("used", du.Total-du.Free, tags) - acc.Add("inodes_total", du.InodesTotal, tags) - acc.Add("inodes_free", du.InodesFree, tags) - acc.Add("inodes_used", du.InodesTotal-du.InodesFree, tags) + if !s.SkipInodeUsage { + acc.Add("inodes_total", du.InodesTotal, tags) + acc.Add("inodes_free", du.InodesFree, tags) + acc.Add("inodes_used", du.InodesTotal-du.InodesFree, tags) + } } return nil From cf1dcfe37c44e3814a9a5cf7c0ba2af8537b8e6f Mon Sep 17 00:00:00 2001 From: subhachandrachandra Date: Thu, 8 Oct 2015 14:17:04 -0700 Subject: [PATCH 4/4] Dropped SkipInodeUsage option as "drop" achieves the same results. Fixed a bug in restricting Disk reporting to specific mountpoints Added tests for the Disk.Mountpoints option Fixed minor bug in usage of assert for the cpu tests where expected and actual values were swapped. --- plugins/system/disk.go | 31 ++++++++------- plugins/system/system_test.go | 72 ++++++++++++++++++++++++++--------- 2 files changed, 69 insertions(+), 34 deletions(-) diff --git a/plugins/system/disk.go b/plugins/system/disk.go index cac219757..6d65139c5 100644 --- a/plugins/system/disk.go +++ b/plugins/system/disk.go @@ -10,7 +10,6 @@ type DiskStats struct { ps PS Mountpoints []string - SkipInodeUsage bool } func (_ *DiskStats) Description() string { @@ -18,16 +17,14 @@ func (_ *DiskStats) Description() string { } var diskSampleConfig = ` - # By default, telegraf gather stats for all mountpoints and for inodes. + # By default, telegraf gather stats for all mountpoints. # Setting mountpoints will restrict the stats to the specified ones. # mountpoints. # Mountpoints=["/"] - # Setting SkipInodeUsage will skip the reporting of inode stats. - # SkipInodeUsage=true -` +` -func (_ *DiskStats) SampleConfig() string { - return diskSampleConfig +func (_ *DiskStats) SampleConfig() string { + return diskSampleConfig } func (s *DiskStats) Gather(acc plugins.Accumulator) error { @@ -36,14 +33,18 @@ func (s *DiskStats) Gather(acc plugins.Accumulator) error { return fmt.Errorf("error getting disk usage info: %s", err) } + var restrictMpoints bool mPoints := make(map[string]bool) - for _, mp := range s.Mountpoints { - mPoints[mp] = true + if len(s.Mountpoints) != 0 { + restrictMpoints = true + for _, mp := range s.Mountpoints { + mPoints[mp] = true + } } for _, du := range disks { - _, member := mPoints[ du.Path ] - if !member { + _, member := mPoints[du.Path] + if restrictMpoints && !member { continue } tags := map[string]string{ @@ -53,11 +54,9 @@ func (s *DiskStats) Gather(acc plugins.Accumulator) error { acc.Add("total", du.Total, tags) acc.Add("free", du.Free, tags) acc.Add("used", du.Total-du.Free, tags) - if !s.SkipInodeUsage { - acc.Add("inodes_total", du.InodesTotal, tags) - acc.Add("inodes_free", du.InodesFree, tags) - acc.Add("inodes_used", du.InodesTotal-du.InodesFree, tags) - } + acc.Add("inodes_total", du.InodesTotal, tags) + acc.Add("inodes_free", du.InodesFree, tags) + acc.Add("inodes_used", du.InodesTotal-du.InodesFree, tags) } return nil diff --git a/plugins/system/system_test.go b/plugins/system/system_test.go index 68f546add..219ea5462 100644 --- a/plugins/system/system_test.go +++ b/plugins/system/system_test.go @@ -51,16 +51,26 @@ func TestSystemStats_GenerateStats(t *testing.T) { mps.On("CPUTimes").Return([]cpu.CPUTimesStat{cts}, nil) - du := &disk.DiskUsageStat{ - Path: "/", - Fstype: "ext4", - Total: 128, - Free: 23, - InodesTotal: 1234, - InodesFree: 234, + du := []*disk.DiskUsageStat{ + { + Path: "/", + Fstype: "ext4", + Total: 128, + Free: 23, + InodesTotal: 1234, + InodesFree: 234, + }, + { + Path: "/home", + Fstype: "ext4", + Total: 256, + Free: 46, + InodesTotal: 2468, + InodesFree: 468, + }, } - mps.On("DiskUsage").Return([]*disk.DiskUsageStat{du}, nil) + mps.On("DiskUsage").Return(du, nil) diskio := disk.DiskIOCountersStat{ ReadCount: 888, @@ -128,7 +138,7 @@ func TestSystemStats_GenerateStats(t *testing.T) { numCPUPoints := len(acc.Points) - preCPUPoints expectedCPUPoints := 10 - assert.Equal(t, numCPUPoints, expectedCPUPoints) + assert.Equal(t, expectedCPUPoints, numCPUPoints) // Computed values are checked with delta > 0 becasue of floating point arithmatic // imprecision @@ -153,7 +163,7 @@ func TestSystemStats_GenerateStats(t *testing.T) { numCPUPoints = len(acc.Points) - (preCPUPoints + numCPUPoints) expectedCPUPoints = 20 - assert.Equal(t, numCPUPoints, expectedCPUPoints) + assert.Equal(t, expectedCPUPoints, numCPUPoints) assertContainsTaggedFloat(t, acc, "time_user", 11.4, 0, cputags) assertContainsTaggedFloat(t, acc, "time_system", 10.9, 0, cputags) @@ -177,20 +187,46 @@ func TestSystemStats_GenerateStats(t *testing.T) { assertContainsTaggedFloat(t, acc, "usage_guest", 4.8, 0.0005, cputags) assertContainsTaggedFloat(t, acc, "usage_guest_nice", 2.2, 0.0005, cputags) - err = (&DiskStats{&mps}).Gather(&acc) + preDiskPoints := len(acc.Points) + + err = (&DiskStats{ps: &mps}).Gather(&acc) require.NoError(t, err) - tags := map[string]string{ + numDiskPoints := len(acc.Points) - preDiskPoints + expectedAllDiskPoints := 12 + assert.Equal(t, expectedAllDiskPoints, numDiskPoints) + + tags1 := map[string]string{ "path": "/", "fstype": "ext4", } + tags2 := map[string]string{ + "path": "/home", + "fstype": "ext4", + } - assert.True(t, acc.CheckTaggedValue("total", uint64(128), tags)) - assert.True(t, acc.CheckTaggedValue("used", uint64(105), tags)) - assert.True(t, acc.CheckTaggedValue("free", uint64(23), tags)) - assert.True(t, acc.CheckTaggedValue("inodes_total", uint64(1234), tags)) - assert.True(t, acc.CheckTaggedValue("inodes_free", uint64(234), tags)) - assert.True(t, acc.CheckTaggedValue("inodes_used", uint64(1000), tags)) + assert.True(t, acc.CheckTaggedValue("total", uint64(128), tags1)) + assert.True(t, acc.CheckTaggedValue("used", uint64(105), tags1)) + assert.True(t, acc.CheckTaggedValue("free", uint64(23), tags1)) + assert.True(t, acc.CheckTaggedValue("inodes_total", uint64(1234), tags1)) + assert.True(t, acc.CheckTaggedValue("inodes_free", uint64(234), tags1)) + assert.True(t, acc.CheckTaggedValue("inodes_used", uint64(1000), tags1)) + assert.True(t, acc.CheckTaggedValue("total", uint64(256), tags2)) + assert.True(t, acc.CheckTaggedValue("used", uint64(210), tags2)) + assert.True(t, acc.CheckTaggedValue("free", uint64(46), tags2)) + assert.True(t, acc.CheckTaggedValue("inodes_total", uint64(2468), tags2)) + assert.True(t, acc.CheckTaggedValue("inodes_free", uint64(468), tags2)) + assert.True(t, acc.CheckTaggedValue("inodes_used", uint64(2000), tags2)) + + // We expect 6 more DiskPoints to show up with an explicit match on "/" + // and /home not matching the /dev in Mountpoints + err = (&DiskStats{ps: &mps, Mountpoints: []string{"/", "/dev"}}).Gather(&acc) + assert.Equal(t, preDiskPoints+expectedAllDiskPoints+6, len(acc.Points)) + + // We should see all the diskpoints as Mountpoints includes both + // / and /home + err = (&DiskStats{ps: &mps, Mountpoints: []string{"/", "/home"}}).Gather(&acc) + assert.Equal(t, preDiskPoints+2*expectedAllDiskPoints+6, len(acc.Points)) err = (&NetIOStats{ps: &mps, skipChecks: true}).Gather(&acc) require.NoError(t, err)