Repository: prometheus/procfs
Pull request #569 (2023-09-15 13:37:16)

fix(meminfo): account for optional unit field so values are accurate
Addresses: #565

Previously, this function was ignoring the optional unit field, leading to incorrect results on systems that do report the field. This uses the humanize lib used elsewhere within the Prometheus ecosystem to normalize the value to bytes, including when a unit is provided.

tjhop: I had similar thoughts and was considering suggesting a complimentary struct/function for `MeminfoBytes`, `parseMeminfoBytes()`, `fs.MeminfoBytes()`, etc so that the existing code would continue to work as expected. 

I think adding in new struct fields for the scaled values to live alongside the raw/unscaled values is a clever solution. I'll work on it 👍
tjhop: @SuperQ I revamped a bit to try the shadow struct fields, lemme know your thoughts
tjhop: Amended the last commit to fix an accidental deleted line (thank you tests). Force pushed to fix. Only the last commit was amended, so I believe the approval should still be valid since that commit wasn't touched. 

Patch:
diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
index 8ace97b..15cf547 100644
--- a/.github/workflows/golangci-lint.yml
+++ b/.github/workflows/golangci-lint.yml
@@ -18,7 +18,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout repository
-        uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4.0.0
+        uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
       - name: install Go
         uses: actions/setup-go@6edd4406fa81c3da01a34fa6f6343087c207a568 # v3.5.0
         with:
diff --git a/meminfo.go b/meminfo.go
index eaf00e2..db1daea 100644
--- a/meminfo.go
+++ b/meminfo.go
@@ -140,6 +140,54 @@ type Meminfo struct {
 	DirectMap4k       *uint64
 	DirectMap2M       *uint64
 	DirectMap1G       *uint64
+
+	// The struct fields below are the byte-normalized counterparts to the
+	// existing struct fields. Values are normalized using the optional
+	// unit field in the meminfo line.
+	MemTotalBytes          *uint64
+	MemFreeBytes           *uint64
+	MemAvailableBytes      *uint64
+	BuffersBytes           *uint64
+	CachedBytes            *uint64
+	SwapCachedBytes        *uint64
+	ActiveBytes            *uint64
+	InactiveBytes          *uint64
+	ActiveAnonBytes        *uint64
+	InactiveAnonBytes      *uint64
+	ActiveFileBytes        *uint64
+	InactiveFileBytes      *uint64
+	UnevictableBytes       *uint64
+	MlockedBytes           *uint64
+	SwapTotalBytes         *uint64
+	SwapFreeBytes          *uint64
+	DirtyBytes             *uint64
+	WritebackBytes         *uint64
+	AnonPagesBytes         *uint64
+	MappedBytes            *uint64
+	ShmemBytes             *uint64
+	SlabBytes              *uint64
+	SReclaimableBytes      *uint64
+	SUnreclaimBytes        *uint64
+	KernelStackBytes       *uint64
+	PageTablesBytes        *uint64
+	NFSUnstableBytes       *uint64
+	BounceBytes            *uint64
+	WritebackTmpBytes      *uint64
+	CommitLimitBytes       *uint64
+	CommittedASBytes       *uint64
+	VmallocTotalBytes      *uint64
+	VmallocUsedBytes       *uint64
+	VmallocChunkBytes      *uint64
+	HardwareCorruptedBytes *uint64
+	AnonHugePagesBytes     *uint64
+	ShmemHugePagesBytes    *uint64
+	ShmemPmdMappedBytes    *uint64
+	CmaTotalBytes          *uint64
+	CmaFreeBytes           *uint64
+	HugepagesizeBytes      *uint64
+	DirectMap4kBytes       *uint64
+	DirectMap2MBytes       *uint64
+	DirectMap1GBytes       *uint64
 }
 
 // Meminfo returns an information about current kernel/system memory statistics.
@@ -162,114 +210,173 @@ func parseMemInfo(r io.Reader) (*Meminfo, error) {
 	var m Meminfo
 	s := bufio.NewScanner(r)
 	for s.Scan() {
-		// Each line has at least a name and value; we ignore the unit.
 		fields := strings.Fields(s.Text())
-		if len(fields) < 2 {
-			return nil, fmt.Errorf("%w: Malformed line %q", ErrFileParse, s.Text())
-		}
+		var val, valBytes uint64
 
-		v, err := strconv.ParseUint(fields[1], 0, 64)
+		val, err := strconv.ParseUint(fields[1], 0, 64)
 		if err != nil {
 			return nil, err
 		}
 
+		switch len(fields) {
+		case 2:
+			// No unit present, use the parsed the value as bytes directly.
+			valBytes = val
+		case 3:
+			// Unit present in optional 3rd field, convert it to
+			// bytes. The only unit supported within the Linux
+			// kernel is `kB`.
+			if fields[2] != "kB" {
+				return nil, fmt.Errorf("%w: Unsupported unit in optional 3rd field %q", ErrFileParse, fields[2])
+			}
+
+			valBytes = 1024 * val
+
+		default:
+			return nil, fmt.Errorf("%w: Malformed line %q", ErrFileParse, s.Text())
+		}
+
 		switch fields[0] {
 		case "MemTotal:":
-			m.MemTotal = &v
+			m.MemTotal = &val
+			m.MemTotalBytes = &valBytes
 		case "MemFree:":
-			m.MemFree = &v
+			m.MemFree = &val
+			m.MemFreeBytes = &valBytes
 		case "MemAvailable:":
-			m.MemAvailable = &v
+			m.MemAvailable = &val
+			m.MemAvailableBytes = &valBytes
 		case "Buffers:":
-			m.Buffers = &v
+			m.Buffers = &val
+			m.BuffersBytes = &valBytes
 		case "Cached:":
-			m.Cached = &v
+			m.Cached = &val
+			m.CachedBytes = &valBytes
 		case "SwapCached:":
-			m.SwapCached = &v
+			m.SwapCached = &val
+			m.SwapCachedBytes = &valBytes
 		case "Active:":
-			m.Active = &v
+			m.Active = &val
+			m.ActiveBytes = &valBytes
 		case "Inactive:":
-			m.Inactive = &v
+			m.Inactive = &val
+			m.InactiveBytes = &valBytes
 		case "Active(anon):":
-			m.ActiveAnon = &v
+			m.ActiveAnon = &val
+			m.ActiveAnonBytes = &valBytes
 		case "Inactive(anon):":
-			m.InactiveAnon = &v
+			m.InactiveAnon = &val
+			m.InactiveAnonBytes = &valBytes
 		case "Active(file):":
-			m.ActiveFile = &v
+			m.ActiveFile = &val
+			m.ActiveFileBytes = &valBytes
 		case "Inactive(file):":
-			m.InactiveFile = &v
+			m.InactiveFile = &val
+			m.InactiveFileBytes = &valBytes
 		case "Unevictable:":
-			m.Unevictable = &v
+			m.Unevictable = &val
+			m.UnevictableBytes = &valBytes
 		case "Mlocked:":
-			m.Mlocked = &v
+			m.Mlocked = &val
+			m.MlockedBytes = &valBytes
 		case "SwapTotal:":
-			m.SwapTotal = &v
+			m.SwapTotal = &val
+			m.SwapTotalBytes = &valBytes
 		case "SwapFree:":
-			m.SwapFree = &v
+			m.SwapFree = &val
+			m.SwapFreeBytes = &valBytes
 		case "Dirty:":
-			m.Dirty = &v
+			m.Dirty = &val
+			m.DirtyBytes = &valBytes
 		case "Writeback:":
-			m.Writeback = &v
+			m.Writeback = &val
+			m.WritebackBytes = &valBytes
 		case "AnonPages:":
-			m.AnonPages = &v
+			m.AnonPages = &val
+			m.AnonPagesBytes = &valBytes
 		case "Mapped:":
-			m.Mapped = &v
+			m.Mapped = &val
+			m.MappedBytes = &valBytes
 		case "Shmem:":
-			m.Shmem = &v
+			m.Shmem = &val
+			m.ShmemBytes = &valBytes
 		case "Slab:":
-			m.Slab = &v
+			m.Slab = &val
+			m.SlabBytes = &valBytes
 		case "SReclaimable:":
-			m.SReclaimable = &v
+			m.SReclaimable = &val
+			m.SReclaimableBytes = &valBytes
 		case "SUnreclaim:":
-			m.SUnreclaim = &v
+			m.SUnreclaim = &val
+			m.SUnreclaimBytes = &valBytes
 		case "KernelStack:":
-			m.KernelStack = &v
+			m.KernelStack = &val
+			m.KernelStackBytes = &valBytes
 		case "PageTables:":
-			m.PageTables = &v
+			m.PageTables = &val
+			m.PageTablesBytes = &valBytes
 		case "NFS_Unstable:":
-			m.NFSUnstable = &v
+			m.NFSUnstable = &val
+			m.NFSUnstableBytes = &valBytes
 		case "Bounce:":
-			m.Bounce = &v
+			m.Bounce = &val
+			m.BounceBytes = &valBytes
 		case "WritebackTmp:":
-			m.WritebackTmp = &v
+			m.WritebackTmp = &val
+			m.WritebackTmpBytes = &valBytes
 		case "CommitLimit:":
-			m.CommitLimit = &v
+			m.CommitLimit = &val
+			m.CommitLimitBytes = &valBytes
 		case "Committed_AS:":
-			m.CommittedAS = &v
+			m.CommittedAS = &val
+			m.CommittedASBytes = &valBytes
 		case "VmallocTotal:":
-			m.VmallocTotal = &v
+			m.VmallocTotal = &val
+			m.VmallocTotalBytes = &valBytes
 		case "VmallocUsed:":
-			m.VmallocUsed = &v
+			m.VmallocUsed = &val
+			m.VmallocUsedBytes = &valBytes
 		case "VmallocChunk:":
-			m.VmallocChunk = &v
+			m.VmallocChunk = &val
+			m.VmallocChunkBytes = &valBytes
 		case "HardwareCorrupted:":
-			m.HardwareCorrupted = &v
+			m.HardwareCorrupted = &val
+			m.HardwareCorruptedBytes = &valBytes
 		case "AnonHugePages:":
-			m.AnonHugePages = &v
+			m.AnonHugePages = &val
+			m.AnonHugePagesBytes = &valBytes
 		case "ShmemHugePages:":
-			m.ShmemHugePages = &v
+			m.ShmemHugePages = &val
+			m.ShmemHugePagesBytes = &valBytes
 		case "ShmemPmdMapped:":
-			m.ShmemPmdMapped = &v
+			m.ShmemPmdMapped = &val
+			m.ShmemPmdMappedBytes = &valBytes
 		case "CmaTotal:":
-			m.CmaTotal = &v
+			m.CmaTotal = &val
+			m.CmaTotalBytes = &valBytes
 		case "CmaFree:":
-			m.CmaFree = &v
+			m.CmaFree = &val
+			m.CmaFreeBytes = &valBytes
 		case "HugePages_Total:":
-			m.HugePagesTotal = &v
+			m.HugePagesTotal = &val
 		case "HugePages_Free:":
-			m.HugePagesFree = &v
+			m.HugePagesFree = &val
 		case "HugePages_Rsvd:":
-			m.HugePagesRsvd = &v
+			m.HugePagesRsvd = &val
 		case "HugePages_Surp:":
-			m.HugePagesSurp = &v
+			m.HugePagesSurp = &val
 		case "Hugepagesize:":
-			m.Hugepagesize = &v
+			m.Hugepagesize = &val
+			m.HugepagesizeBytes = &valBytes
 		case "DirectMap4k:":
-			m.DirectMap4k = &v
+			m.DirectMap4k = &val
+			m.DirectMap4kBytes = &valBytes
 		case "DirectMap2M:":
-			m.DirectMap2M = &v
+			m.DirectMap2M = &val
+			m.DirectMap2MBytes = &valBytes
 		case "DirectMap1G:":
-			m.DirectMap1G = &v
+			m.DirectMap1G = &val
+			m.DirectMap1GBytes = &valBytes
 		}
 	}
 


Test patch:
diff --git a/meminfo_test.go b/meminfo_test.go
index 860f177..2d2b238 100644
--- a/meminfo_test.go
+++ b/meminfo_test.go
@@ -62,6 +62,45 @@ func TestMeminfo(t *testing.T) {
 		Hugepagesize:      newuint64(2048),
 		DirectMap4k:       newuint64(91136),
 		DirectMap2M:       newuint64(16039936),
+
+		MemTotalBytes:          newuint64(16042172416),
+		MemFreeBytes:           newuint64(450891776),
+		BuffersBytes:           newuint64(1044611072),
+		CachedBytes:            newuint64(12295823360),
+		SwapCachedBytes:        newuint64(0),
+		ActiveBytes:            newuint64(6923546624),
+		InactiveBytes:          newuint64(6689492992),
+		ActiveAnonBytes:        newuint64(273670144),
+		InactiveAnonBytes:      newuint64(274432),
+		ActiveFileBytes:        newuint64(6649876480),
+		InactiveFileBytes:      newuint64(6689218560),
+		UnevictableBytes:       newuint64(0),
+		MlockedBytes:           newuint64(0),
+		SwapTotalBytes:         newuint64(0),
+		SwapFreeBytes:          newuint64(0),
+		DirtyBytes:             newuint64(786432),
+		WritebackBytes:         newuint64(0),
+		AnonPagesBytes:         newuint64(272605184),
+		MappedBytes:            newuint64(45264896),
+		ShmemBytes:             newuint64(1339392),
+		SlabBytes:              newuint64(1850638336),
+		SReclaimableBytes:      newuint64(1779838976),
+		SUnreclaimBytes:        newuint64(70799360),
+		KernelStackBytes:       newuint64(1654784),
+		PageTablesBytes:        newuint64(5414912),
+		NFSUnstableBytes:       newuint64(0),
+		BounceBytes:            newuint64(0),
+		WritebackTmpBytes:      newuint64(0),
+		CommitLimitBytes:       newuint64(8021086208),
+		CommittedASBytes:       newuint64(543584256),
+		VmallocTotalBytes:      newuint64(35184372087808),
+		VmallocUsedBytes:       newuint64(37474304),
+		VmallocChunkBytes:      newuint64(35184269148160),
+		HardwareCorruptedBytes: newuint64(0),
+		AnonHugePagesBytes:     newuint64(12582912),
+		HugepagesizeBytes:      newuint64(2097152),
+		DirectMap4kBytes:       newuint64(93323264),
+		DirectMap2MBytes:       newuint64(16424894464),
 	}
 
 	have, err := getProcFixtures(t).Meminfo()