Simplify system.DiskUsage() (#2630)

plugins/inputs/system/cpu.go

@@ -121,7 +121,7 @@ func init() {
 		return &CPUStats{
 			PerCPU:   true,
 			TotalCPU: true,
-			ps:       &systemPS{},
+			ps:       newSystemPS(),
 		}
 	})
 }

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}
 	})
 }

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)

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}
 	})
 }

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
+}

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()}
 	})
 }

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()}
 	})
 }

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,36 +89,35 @@ 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 := mountPointFilterSet[p.Mountpoint]; !ok {
-			if !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 := fstypeExcludeSet[p.Fstype]; ok {
-			if 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)
+}