From cc44150054bd435ce6309db197ff3bdc66b95172 Mon Sep 17 00:00:00 2001 From: Nikolay Denev Date: Tue, 18 Apr 2017 19:42:58 +0100 Subject: [PATCH] Simplify system.DiskUsage() (#2630) --- plugins/inputs/system/cpu.go | 2 +- plugins/inputs/system/disk.go | 5 +- plugins/inputs/system/disk_test.go | 108 +++++++++++++++++++++++++++++ plugins/inputs/system/memory.go | 5 +- plugins/inputs/system/mock_PS.go | 44 ++++++++++++ plugins/inputs/system/net.go | 2 +- plugins/inputs/system/netstat.go | 2 +- plugins/inputs/system/ps.go | 72 +++++++++++++------ 8 files changed, 212 insertions(+), 28 deletions(-) diff --git a/plugins/inputs/system/cpu.go b/plugins/inputs/system/cpu.go index 3ed2606fa..e6aa9f22d 100644 --- a/plugins/inputs/system/cpu.go +++ b/plugins/inputs/system/cpu.go @@ -121,7 +121,7 @@ func init() { return &CPUStats{ PerCPU: true, TotalCPU: true, - ps: &systemPS{}, + ps: newSystemPS(), } }) } diff --git a/plugins/inputs/system/disk.go b/plugins/inputs/system/disk.go index 004466f83..46f2219a7 100644 --- a/plugins/inputs/system/disk.go +++ b/plugins/inputs/system/disk.go @@ -219,11 +219,12 @@ func (s *DiskIOStats) diskTags(devName string) map[string]string { } func init() { + ps := newSystemPS() inputs.Add("disk", func() telegraf.Input { - return &DiskStats{ps: &systemPS{}} + return &DiskStats{ps: ps} }) inputs.Add("diskio", func() telegraf.Input { - return &DiskIOStats{ps: &systemPS{}, SkipSerialNumber: true} + return &DiskIOStats{ps: ps, SkipSerialNumber: true} }) } diff --git a/plugins/inputs/system/disk_test.go b/plugins/inputs/system/disk_test.go index fc0ff4d0d..5ba4d041f 100644 --- a/plugins/inputs/system/disk_test.go +++ b/plugins/inputs/system/disk_test.go @@ -1,14 +1,122 @@ package system import ( + "os" "testing" "github.com/influxdata/telegraf/testutil" "github.com/shirou/gopsutil/disk" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) +type MockFileInfo struct { + os.FileInfo +} + +func TestDiskUsage(t *testing.T) { + mck := &mock.Mock{} + mps := MockPSDisk{&systemPS{&mockDiskUsage{mck}}, mck} + defer mps.AssertExpectations(t) + + var acc testutil.Accumulator + var err error + + psAll := []disk.PartitionStat{ + { + Device: "/dev/sda", + Mountpoint: "/", + Fstype: "ext4", + Opts: "", + }, + { + Device: "/dev/sdb", + Mountpoint: "/home", + Fstype: "ext4", + Opts: "", + }, + } + duAll := []disk.UsageStat{ + { + Path: "/", + Fstype: "ext4", + Total: 128, + Free: 23, + Used: 100, + InodesTotal: 1234, + InodesFree: 234, + InodesUsed: 1000, + }, + { + Path: "/home", + Fstype: "ext4", + Total: 256, + Free: 46, + Used: 200, + InodesTotal: 2468, + InodesFree: 468, + InodesUsed: 2000, + }, + } + + mps.On("Partitions", true).Return(psAll, nil) + mps.On("OSGetenv", "HOST_MOUNT_PREFIX").Return("") + mps.On("OSStat", "/").Return(MockFileInfo{}, nil) + mps.On("OSStat", "/home").Return(MockFileInfo{}, nil) + mps.On("PSDiskUsage", "/").Return(&duAll[0], nil) + mps.On("PSDiskUsage", "/home").Return(&duAll[1], nil) + + err = (&DiskStats{ps: mps}).Gather(&acc) + require.NoError(t, err) + + numDiskMetrics := acc.NFields() + expectedAllDiskMetrics := 14 + assert.Equal(t, expectedAllDiskMetrics, numDiskMetrics) + + tags1 := map[string]string{ + "path": "/", + "fstype": "ext4", + "device": "sda", + } + tags2 := map[string]string{ + "path": "/home", + "fstype": "ext4", + "device": "sdb", + } + + fields1 := map[string]interface{}{ + "total": uint64(128), + "used": uint64(100), + "free": uint64(23), + "inodes_total": uint64(1234), + "inodes_free": uint64(234), + "inodes_used": uint64(1000), + "used_percent": float64(81.30081300813008), + } + fields2 := map[string]interface{}{ + "total": uint64(256), + "used": uint64(200), + "free": uint64(46), + "inodes_total": uint64(2468), + "inodes_free": uint64(468), + "inodes_used": uint64(2000), + "used_percent": float64(81.30081300813008), + } + acc.AssertContainsTaggedFields(t, "disk", fields1, tags1) + acc.AssertContainsTaggedFields(t, "disk", fields2, tags2) + + // We expect 6 more DiskMetrics 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, expectedAllDiskMetrics+7, acc.NFields()) + + // We should see all the diskpoints as MountPoints includes both + // / and /home + err = (&DiskStats{ps: &mps, MountPoints: []string{"/", "/home"}}).Gather(&acc) + assert.Equal(t, 2*expectedAllDiskMetrics+7, acc.NFields()) +} + func TestDiskStats(t *testing.T) { var mps MockPS defer mps.AssertExpectations(t) diff --git a/plugins/inputs/system/memory.go b/plugins/inputs/system/memory.go index 26dc550f8..3f679b36c 100644 --- a/plugins/inputs/system/memory.go +++ b/plugins/inputs/system/memory.go @@ -73,11 +73,12 @@ func (s *SwapStats) Gather(acc telegraf.Accumulator) error { } func init() { + ps := newSystemPS() inputs.Add("mem", func() telegraf.Input { - return &MemStats{ps: &systemPS{}} + return &MemStats{ps: ps} }) inputs.Add("swap", func() telegraf.Input { - return &SwapStats{ps: &systemPS{}} + return &SwapStats{ps: ps} }) } diff --git a/plugins/inputs/system/mock_PS.go b/plugins/inputs/system/mock_PS.go index a83a8b803..d5093f031 100644 --- a/plugins/inputs/system/mock_PS.go +++ b/plugins/inputs/system/mock_PS.go @@ -1,6 +1,8 @@ package system import ( + "os" + "github.com/stretchr/testify/mock" "github.com/shirou/gopsutil/cpu" @@ -13,6 +15,16 @@ import ( type MockPS struct { mock.Mock + PSDiskDeps +} + +type MockPSDisk struct { + *systemPS + *mock.Mock +} + +type mockDiskUsage struct { + *mock.Mock } func (m *MockPS) LoadAvg() (*load.AvgStat, error) { @@ -96,3 +108,35 @@ func (m *MockPS) NetConnections() ([]net.ConnectionStat, error) { return r0, r1 } + +func (m *mockDiskUsage) Partitions(all bool) ([]disk.PartitionStat, error) { + ret := m.Called(all) + + r0 := ret.Get(0).([]disk.PartitionStat) + r1 := ret.Error(1) + + return r0, r1 +} + +func (m *mockDiskUsage) OSGetenv(key string) string { + ret := m.Called(key) + return ret.Get(0).(string) +} + +func (m *mockDiskUsage) OSStat(name string) (os.FileInfo, error) { + ret := m.Called(name) + + r0 := ret.Get(0).(os.FileInfo) + r1 := ret.Error(1) + + return r0, r1 +} + +func (m *mockDiskUsage) PSDiskUsage(path string) (*disk.UsageStat, error) { + ret := m.Called(path) + + r0 := ret.Get(0).(*disk.UsageStat) + r1 := ret.Error(1) + + return r0, r1 +} diff --git a/plugins/inputs/system/net.go b/plugins/inputs/system/net.go index 3f89176fb..f47a2cc6c 100644 --- a/plugins/inputs/system/net.go +++ b/plugins/inputs/system/net.go @@ -105,6 +105,6 @@ func (s *NetIOStats) Gather(acc telegraf.Accumulator) error { func init() { inputs.Add("net", func() telegraf.Input { - return &NetIOStats{ps: &systemPS{}} + return &NetIOStats{ps: newSystemPS()} }) } diff --git a/plugins/inputs/system/netstat.go b/plugins/inputs/system/netstat.go index 98b729bbe..1699e0808 100644 --- a/plugins/inputs/system/netstat.go +++ b/plugins/inputs/system/netstat.go @@ -66,6 +66,6 @@ func (s *NetStats) Gather(acc telegraf.Accumulator) error { func init() { inputs.Add("netstat", func() telegraf.Input { - return &NetStats{ps: &systemPS{}} + return &NetStats{ps: newSystemPS()} }) } diff --git a/plugins/inputs/system/ps.go b/plugins/inputs/system/ps.go index 20e01742a..979a3b164 100644 --- a/plugins/inputs/system/ps.go +++ b/plugins/inputs/system/ps.go @@ -23,6 +23,13 @@ type PS interface { NetConnections() ([]net.ConnectionStat, error) } +type PSDiskDeps interface { + Partitions(all bool) ([]disk.PartitionStat, error) + OSGetenv(key string) string + OSStat(name string) (os.FileInfo, error) + PSDiskUsage(path string) (*disk.UsageStat, error) +} + func add(acc telegraf.Accumulator, name string, val float64, tags map[string]string) { if val >= 0 { @@ -30,7 +37,15 @@ func add(acc telegraf.Accumulator, } } -type systemPS struct{} +func newSystemPS() *systemPS { + return &systemPS{&systemPSDisk{}} +} + +type systemPS struct { + PSDiskDeps +} + +type systemPSDisk struct{} func (s *systemPS) CPUTimes(perCPU, totalCPU bool) ([]cpu.TimesStat, error) { var cpuTimes []cpu.TimesStat @@ -55,7 +70,7 @@ func (s *systemPS) DiskUsage( mountPointFilter []string, fstypeExclude []string, ) ([]*disk.UsageStat, []*disk.PartitionStat, error) { - parts, err := disk.Partitions(true) + parts, err := s.Partitions(true) if err != nil { return nil, nil, err } @@ -74,35 +89,34 @@ func (s *systemPS) DiskUsage( var partitions []*disk.PartitionStat for i := range parts { - p := parts[i] if len(mountPointFilter) > 0 { // If the mount point is not a member of the filter set, // don't gather info on it. - _, ok := mountPointFilterSet[p.Mountpoint] - if !ok { + if _, ok := mountPointFilterSet[p.Mountpoint]; !ok { continue } } - mountpoint := os.Getenv("HOST_MOUNT_PREFIX") + p.Mountpoint - if _, err := os.Stat(mountpoint); err == nil { - du, err := disk.Usage(mountpoint) - if err != nil { - return nil, nil, err - } - du.Path = p.Mountpoint - // If the mount point is a member of the exclude set, - // don't gather info on it. - _, ok := fstypeExcludeSet[p.Fstype] - if ok { - continue - } - du.Fstype = p.Fstype - usage = append(usage, du) - partitions = append(partitions, &p) + // If the mount point is a member of the exclude set, + // don't gather info on it. + if _, ok := fstypeExcludeSet[p.Fstype]; ok { + continue } + + mountpoint := s.OSGetenv("HOST_MOUNT_PREFIX") + p.Mountpoint + if _, err := s.OSStat(mountpoint); err != nil { + continue + } + du, err := s.PSDiskUsage(mountpoint) + if err != nil { + continue + } + du.Path = p.Mountpoint + du.Fstype = p.Fstype + usage = append(usage, du) + partitions = append(partitions, &p) } return usage, partitions, nil @@ -136,3 +150,19 @@ func (s *systemPS) VMStat() (*mem.VirtualMemoryStat, error) { func (s *systemPS) SwapStat() (*mem.SwapMemoryStat, error) { return mem.SwapMemory() } + +func (s *systemPSDisk) Partitions(all bool) ([]disk.PartitionStat, error) { + return disk.Partitions(all) +} + +func (s *systemPSDisk) OSGetenv(key string) string { + return os.Getenv(key) +} + +func (s *systemPSDisk) OSStat(name string) (os.FileInfo, error) { + return os.Stat(name) +} + +func (s *systemPSDisk) PSDiskUsage(path string) (*disk.UsageStat, error) { + return disk.Usage(path) +}