Merge pull request #2768 from JohnnyG235711/docker-0filesystem-fix

Added support for filesystem metrics on Docker

Author: bobbypage

container/docker/handler.go

@@ -398,11 +398,14 @@ func (h *dockerContainerHandler) getFsStats(stats *info.ContainerStats) error {
 		fsType string
 	)
 
+	var fsInfo *info.FsInfo
+
 	// Docker does not impose any filesystem limits for containers. So use capacity as limit.
 	for _, fs := range mi.Filesystems {
 		if fs.Device == device {
 			limit = fs.Capacity
 			fsType = fs.Type
+			fsInfo = &fs
 			break
 		}
 	}
@@ -413,11 +416,45 @@ func (h *dockerContainerHandler) getFsStats(stats *info.ContainerStats) error {
 	fsStat.Usage = usage.TotalUsageBytes
 	fsStat.Inodes = usage.InodeUsage
 
+	if fsInfo != nil {
+		fileSystems, err := h.fsInfo.GetGlobalFsInfo()
+
+		if err == nil {
+			addDiskStats(fileSystems, fsInfo, &fsStat)
+		} else {
+			klog.Errorf("Unable to obtain diskstats for filesystem %s: %v", fsStat.Device, err)
+		}
+	}
+
 	stats.Filesystem = append(stats.Filesystem, fsStat)
 
 	return nil
 }
 
+func addDiskStats(fileSystems []fs.Fs, fsInfo *info.FsInfo, fsStats *info.FsStats) {
+	if fsInfo == nil {
+		return
+	}
+
+	for _, fileSys := range fileSystems {
+		if fsInfo.DeviceMajor == fileSys.DiskStats.Major &&
+			fsInfo.DeviceMinor == fileSys.DiskStats.Minor {
+			fsStats.ReadsCompleted = fileSys.DiskStats.ReadsCompleted
+			fsStats.ReadsMerged = fileSys.DiskStats.ReadsMerged
+			fsStats.SectorsRead = fileSys.DiskStats.SectorsRead
+			fsStats.ReadTime = fileSys.DiskStats.ReadTime
+			fsStats.WritesCompleted = fileSys.DiskStats.WritesCompleted
+			fsStats.WritesMerged = fileSys.DiskStats.WritesMerged
+			fsStats.SectorsWritten = fileSys.DiskStats.SectorsWritten
+			fsStats.WriteTime = fileSys.DiskStats.WriteTime
+			fsStats.IoInProgress = fileSys.DiskStats.IoInProgress
+			fsStats.IoTime = fileSys.DiskStats.IoTime
+			fsStats.WeightedIoTime = fileSys.DiskStats.WeightedIoTime
+			break
+		}
+	}
+}
+
 // TODO(vmarmol): Get from libcontainer API instead of cgroup manager when we don't have to support older Dockers.
 func (h *dockerContainerHandler) GetStats() (*info.ContainerStats, error) {
 	stats, err := h.libcontainerHandler.GetStats()

container/docker/handler_test.go

@@ -24,6 +24,9 @@ import (
 
 	"github.com/docker/docker/api/types/container"
 	"github.com/stretchr/testify/assert"
+
+	"github.com/google/cadvisor/fs"
+	info "github.com/google/cadvisor/info/v1"
 )
 
 func TestStorageDirDetectionWithOldVersions(t *testing.T) {
@@ -144,3 +147,76 @@ func TestDockerEnvWhitelist(t *testing.T) {
 	as.Equal(rawEnvsMatchWithEmptyWhitelist, emptyExpected)
 
 }
+
+func TestAddDiskStatsCheck(t *testing.T) {
+	var readsCompleted, readsMerged, sectorsRead, readTime, writesCompleted, writesMerged, sectorsWritten,
+		writeTime, ioInProgress, ioTime, weightedIoTime uint64 = 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
+
+	fileSystem := fs.Fs{
+		DiskStats: fs.DiskStats{
+			ReadsCompleted:  readsCompleted,
+			ReadsMerged:     readsMerged,
+			SectorsRead:     sectorsRead,
+			ReadTime:        readTime,
+			WritesCompleted: writesCompleted,
+			WritesMerged:    writesMerged,
+			SectorsWritten:  sectorsWritten,
+			WriteTime:       writeTime,
+			IoInProgress:    ioInProgress,
+			IoTime:          ioTime,
+			WeightedIoTime:  weightedIoTime,
+		},
+	}
+
+	fileSystems := []fs.Fs{fileSystem}
+
+	var fsStats info.FsStats
+	addDiskStats(fileSystems, nil, &fsStats)
+}
+
+func TestAddDiskStats(t *testing.T) {
+	// Arrange
+	as := assert.New(t)
+	var readsCompleted, readsMerged, sectorsRead, readTime, writesCompleted, writesMerged, sectorsWritten,
+		writeTime, ioInProgress, ioTime, weightedIoTime uint64 = 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
+	var fsStats info.FsStats
+
+	fsInfo := info.FsInfo{
+		DeviceMajor: 4,
+		DeviceMinor: 64,
+	}
+
+	fileSystem := fs.Fs{
+		DiskStats: fs.DiskStats{
+			ReadsCompleted:  readsCompleted,
+			ReadsMerged:     readsMerged,
+			SectorsRead:     sectorsRead,
+			ReadTime:        readTime,
+			WritesCompleted: writesCompleted,
+			WritesMerged:    writesMerged,
+			SectorsWritten:  sectorsWritten,
+			WriteTime:       writeTime,
+			IoInProgress:    ioInProgress,
+			IoTime:          ioTime,
+			WeightedIoTime:  weightedIoTime,
+		},
+	}
+
+	fileSystems := []fs.Fs{fileSystem}
+
+	// Act
+	addDiskStats(fileSystems, &fsInfo, &fsStats)
+
+	// Assert
+	as.Equal(readsCompleted, fileSystem.DiskStats.ReadsCompleted, "ReadsCompleted metric should be %d but was %d", readsCompleted, fileSystem.DiskStats.ReadsCompleted)
+	as.Equal(readsMerged, fileSystem.DiskStats.ReadsMerged, "ReadsMerged metric should be %d but was %d", readsMerged, fileSystem.DiskStats.ReadsMerged)
+	as.Equal(sectorsRead, fileSystem.DiskStats.SectorsRead, "SectorsRead metric should be %d but was %d", sectorsRead, fileSystem.DiskStats.SectorsRead)
+	as.Equal(readTime, fileSystem.DiskStats.ReadTime, "ReadTime metric should be %d but was %d", readTime, fileSystem.DiskStats.ReadTime)
+	as.Equal(writesCompleted, fileSystem.DiskStats.WritesCompleted, "WritesCompleted metric should be %d but was %d", writesCompleted, fileSystem.DiskStats.WritesCompleted)
+	as.Equal(writesMerged, fileSystem.DiskStats.WritesMerged, "WritesMerged metric should be %d but was %d", writesMerged, fileSystem.DiskStats.WritesMerged)
+	as.Equal(sectorsWritten, fileSystem.DiskStats.SectorsWritten, "SectorsWritten metric should be %d but was %d", sectorsWritten, fileSystem.DiskStats.SectorsWritten)
+	as.Equal(writeTime, fileSystem.DiskStats.WriteTime, "WriteTime metric should be %d but was %d", writeTime, fileSystem.DiskStats.WriteTime)
+	as.Equal(ioInProgress, fileSystem.DiskStats.IoInProgress, "IoInProgress metric should be %d but was %d", ioInProgress, fileSystem.DiskStats.IoInProgress)
+	as.Equal(ioTime, fileSystem.DiskStats.IoTime, "IoTime metric should be %d but was %d", ioTime, fileSystem.DiskStats.IoTime)
+	as.Equal(weightedIoTime, fileSystem.DiskStats.WeightedIoTime, "WeightedIoTime metric should be %d but was %d", weightedIoTime, fileSystem.DiskStats.WeightedIoTime)
+}

fs/fs.go

@@ -488,6 +488,17 @@ func getDiskStatsMap(diskStatsFile string) (map[string]DiskStats, error) {
 				return nil, err
 			}
 		}
+
+		major64, err := strconv.ParseUint(words[0], 10, 64)
+		if err != nil {
+			return nil, err
+		}
+
+		minor64, err := strconv.ParseUint(words[1], 10, 64)
+		if err != nil {
+			return nil, err
+		}
+
 		diskStats := DiskStats{
 			MajorNum:        devInfo[0],
 			MinorNum:        devInfo[1],
@@ -502,6 +513,8 @@ func getDiskStatsMap(diskStatsFile string) (map[string]DiskStats, error) {
 			IoInProgress:    stats[8],
 			IoTime:          stats[9],
 			WeightedIoTime:  stats[10],
+			Major:           major64,
+			Minor:           minor64,
 		}
 		diskStatsMap[deviceName] = diskStats
 	}

fs/fs_test.go

@@ -43,7 +43,7 @@ func TestMountInfoFromDir(t *testing.T) {
 func TestGetDiskStatsMap(t *testing.T) {
 	diskStatsMap, err := getDiskStatsMap("test_resources/diskstats")
 	if err != nil {
-		t.Errorf("Error calling getDiskStatMap %s", err)
+		t.Errorf("Error calling getDiskStatsMap %s", err)
 	}
 	if len(diskStatsMap) != 30 {
 		t.Errorf("diskStatsMap %+v not valid", diskStatsMap)
@@ -95,7 +95,7 @@ func TestGetDiskStatsMap(t *testing.T) {
 func TestGetDiskStatsMapMajorMinorNum(t *testing.T) {
 	diskStatsMap, err := getDiskStatsMap("test_resources/diskstats")
 	if err != nil {
-		t.Errorf("Error calling getDiskStatMap %s", err)
+		t.Errorf("Error calling getDiskStatsMap %s", err)
 	}
 	if len(diskStatsMap) != 30 {
 		t.Errorf("diskStatsMap %+v not valid", diskStatsMap)

fs/types.go

@@ -77,6 +77,8 @@ type DiskStats struct {
 	IoInProgress    uint64
 	IoTime          uint64
 	WeightedIoTime  uint64
+	Major           uint64
+	Minor           uint64
 }
 
 type UsageInfo struct {