Merge pull request #77 from fearful-symmetry/fix-parser-v2-iostat

Fix io.stat parsing errors

metric/system/cgroup/cgv2/io.go

@@ -23,12 +23,14 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
+	"strings"
 
 	"github.com/elastic/elastic-agent-libs/logp"
 	"github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/cgcommon"
 )
 
-//IOSubsystem is the replacement for the bulkio controller in cgroupsV1
+// IOSubsystem is the replacement for the bulkio controller in cgroupsV1
 type IOSubsystem struct {
 	ID   string `json:"id,omitempty"`   // ID of the cgroup.
 	Path string `json:"path,omitempty"` // Path to the cgroup relative to the cgroup subsystem's mountpoint.
@@ -88,31 +90,81 @@ func getIOStats(path string, resolveDevIDs bool) (map[string]IOStat, error) {
 
 	sc := bufio.NewScanner(f)
 	for sc.Scan() {
-		devMetric := IOStat{}
-		var major, minor uint64
-		_, err := fmt.Sscanf(sc.Text(), "%d:%d rbytes=%d wbytes=%d rios=%d wios=%d dbytes=%d dios=%d", &major, &minor, &devMetric.Read.Bytes, &devMetric.Write.Bytes, &devMetric.Read.IOs, &devMetric.Write.IOs, &devMetric.Discarded.Bytes, &devMetric.Discarded.IOs)
+		devices, metrics, foundMetrics, err := parseStatLine(sc.Text(), resolveDevIDs)
 		if err != nil {
-			return stats, fmt.Errorf("error scanning file: %s: %w", file, err)
+			return nil, fmt.Errorf("error parsing line in file: %w", err)
 		}
+		if !foundMetrics {
+			continue
+		}
+		for _, dev := range devices {
+			stats[dev] = metrics
+		}
+	}
 
-		// try to find the device name associated with the major/minor pair
-		// This isn't guaranteed to work, for a number of reasons, so we'll need to fall back
-		var found bool
-		var devName string
-		if resolveDevIDs {
-			found, devName, err = fetchDeviceName(major, minor)
+	return stats, nil
+}
+
+// parses a single line in io.stat; a bit complicated, since these files are more complex then they look.
+// returns a list of device names associated with the metrics, the metric set, and a bool indicating if metrics were found
+func parseStatLine(line string, resolveDevIDs bool) ([]string, IOStat, bool, error) {
+	devIds := []string{}
+	stats := IOStat{}
+	foundMetrics := false
+	// cautiously iterate over a line to find the components
+	// under certain conditions, the stat.io will combine different loopback devices onto a single line,
+	// 7:7 7:6 7:5 7:4 rbytes=556032 wbytes=0 rios=78 wios=0 dbytes=0 dios=0
+	// we can also get lines without metrics, like
+	//  7:7 7:6 7:5 7:4
+	for _, component := range strings.Split(line, " ") {
+		if strings.Contains(component, ":") {
+			var major, minor uint64
+			_, err := fmt.Sscanf(component, "%d:%d", &major, &minor)
 			if err != nil {
-				return nil, fmt.Errorf("error looking up device ID %d:%d %w", major, minor, err)
+				return nil, IOStat{}, false, fmt.Errorf("could not read device ID: %s: %w", component, err)
+			}
+
+			var found bool
+			var devName string
+			// try to find the device name associated with the major/minor pair
+			// This isn't guaranteed to work, for a number of reasons, so we'll need to fall back
+			if resolveDevIDs {
+				found, devName, _ = fetchDeviceName(major, minor)
+			}
+
+			if found {
+				devIds = append(devIds, devName)
+			} else {
+				devIds = append(devIds, component)
+			}
+		} else if strings.Contains(component, "=") {
+			foundMetrics = true
+			counterSplit := strings.Split(component, "=")
+			if len(counterSplit) < 2 {
+				continue
+			}
+			name := counterSplit[0]
+			counter, err := strconv.ParseUint(counterSplit[1], 10, 64)
+			if err != nil {
+				return nil, IOStat{}, false, fmt.Errorf("error parsing counter '%s' in stat: %w", counterSplit[1], err)
+			}
+			switch name {
+			case "rbytes":
+				stats.Read.Bytes = counter
+			case "wbytes":
+				stats.Write.Bytes = counter
+			case "rios":
+				stats.Read.IOs = counter
+			case "wios":
+				stats.Write.IOs = counter
+			case "dbytes":
+				stats.Discarded.Bytes = counter
+			case "dios":
+				stats.Discarded.IOs = counter
 			}
-		}
 
-		if found {
-			stats[devName] = devMetric
-		} else {
-			idKey := fmt.Sprintf("%d:%d", major, minor)
-			stats[idKey] = devMetric
 		}
-	}
 
-	return stats, nil
+	}
+	return devIds, stats, foundMetrics, nil
 }

metric/system/cgroup/cgv2/v2_test.go

@@ -24,6 +24,8 @@ import (
 )
 
 const v2Path = "../testdata/docker/sys/fs/cgroup/system.slice/docker-1c8fa019edd4b9d4b2856f4932c55929c5c118c808ed5faee9a135ca6e84b039.scope"
+const ubuntu = "../testdata/io_statfiles/ubuntu"
+const ubuntu2 = "../testdata/io_statfiles/ubuntu2"
 
 func TestGetIO(t *testing.T) {
 	ioTest := IOSubsystem{}
@@ -46,6 +48,125 @@ func TestGetIO(t *testing.T) {
 	assert.Equal(t, goodStat, ioTest.Stats)
 }
 
+func TestIostatFilesDuplicatedDeviceMetrics(t *testing.T) {
+	ioTest := IOSubsystem{}
+	err := ioTest.Get(ubuntu, false)
+	assert.NoError(t, err, "error in Get")
+
+	goodStat := map[string]IOStat{
+		"7:7": {
+			Read: IOMetric{
+				Bytes: 556032,
+				IOs:   78,
+			},
+			Write: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+			Discarded: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+		},
+		"7:6": {
+			Read: IOMetric{
+				Bytes: 556032,
+				IOs:   78,
+			},
+			Write: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+			Discarded: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+		},
+		"7:5": {
+			Read: IOMetric{
+				Bytes: 556032,
+				IOs:   78,
+			},
+			Write: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+			Discarded: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+		},
+		"7:4": {
+			Read: IOMetric{
+				Bytes: 556032,
+				IOs:   78,
+			},
+			Write: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+			Discarded: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+		},
+		"7:3": {
+			Read: IOMetric{
+				Bytes: 21017600,
+				IOs:   629,
+			},
+			Write: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+			Discarded: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+		},
+	}
+
+	assert.Equal(t, goodStat, ioTest.Stats)
+}
+
+func TestIOStatDeviceWithNoMetrics(t *testing.T) {
+	ioTest := IOSubsystem{}
+	err := ioTest.Get(ubuntu2, false)
+	assert.NoError(t, err, "error in Get")
+
+	goodStat := map[string]IOStat{
+		"253:0": {
+			Read: IOMetric{
+				Bytes: 45931053056,
+				IOs:   1078394,
+			},
+			Write: IOMetric{
+				Bytes: 211814596608,
+				IOs:   21426614,
+			},
+			Discarded: IOMetric{
+				Bytes: 0,
+				IOs:   0,
+			},
+		},
+		"259:0": {
+			Read: IOMetric{
+				Bytes: 48963873792,
+				IOs:   1315370,
+			},
+			Write: IOMetric{
+				Bytes: 217588278272,
+				IOs:   15358572,
+			},
+			Discarded: IOMetric{
+				Bytes: 3222265856,
+				IOs:   24,
+			},
+		},
+	}
+	assert.Equal(t, goodStat, ioTest.Stats)
+}
+
 func TestGetMem(t *testing.T) {
 	mem := MemorySubsystem{}
 	err := mem.Get(v2Path)

metric/system/cgroup/testdata/io_statfiles/ubuntu/io.stat

@@ -0,0 +1,2 @@
+7:7 7:6 7:5 7:4 rbytes=556032 wbytes=0 rios=78 wios=0 dbytes=0 dios=0
+7:3 rbytes=21017600 wbytes=0 rios=629 wios=0 dbytes=0 dios=0

metric/system/cgroup/testdata/io_statfiles/ubuntu2/io.stat

@@ -0,0 +1,3 @@
+253:0 rbytes=45931053056 wbytes=211814596608 rios=1078394 wios=21426614 dbytes=0 dios=0
+259:0 rbytes=48963873792 wbytes=217588278272 rios=1315370 wios=15358572 dbytes=3222265856 dios=24
+7:7 7:6 7:5 7:4 7:3 7:2 7:1 7:0