From 2c71cedb79aa2e04c16e3bf9624626b42e9ff886 Mon Sep 17 00:00:00 2001 From: TJ Hoplock Date: Wed, 13 Sep 2023 13:35:24 -0400 Subject: [PATCH 1/3] 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. To try and maintain existing behavior, the fixed/unit-scaled values are stored as new struct fields. Signed-off-by: TJ Hoplock --- meminfo.go | 221 ++++++++++++++++++++++++++++++++++++------------ meminfo_test.go | 43 ++++++++++ 2 files changed, 212 insertions(+), 52 deletions(-) diff --git a/meminfo.go b/meminfo.go index eaf00e224..cccdfcd0e 100644 --- a/meminfo.go +++ b/meminfo.go @@ -140,6 +140,58 @@ 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 + HugePagesTotalBytes *uint64 + HugePagesFreeBytes *uint64 + HugePagesRsvdBytes *uint64 + HugePagesSurpBytes *uint64 + HugepagesizeBytes *uint64 + DirectMap4kBytes *uint64 + DirectMap2MBytes *uint64 + DirectMap1GBytes *uint64 } // Meminfo returns an information about current kernel/system memory statistics. @@ -162,114 +214,179 @@ 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) if err != nil { return nil, err } + val = v + + 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 + m.HugePagesTotalBytes = &valBytes case "HugePages_Free:": - m.HugePagesFree = &v + m.HugePagesFree = &val + m.HugePagesFreeBytes = &valBytes case "HugePages_Rsvd:": - m.HugePagesRsvd = &v + m.HugePagesRsvd = &val + m.HugePagesRsvdBytes = &valBytes case "HugePages_Surp:": - m.HugePagesSurp = &v + m.HugePagesSurp = &val + m.HugePagesSurpBytes = &valBytes 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 } } diff --git a/meminfo_test.go b/meminfo_test.go index 860f1773c..5a4b761f4 100644 --- a/meminfo_test.go +++ b/meminfo_test.go @@ -62,6 +62,49 @@ 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), + HugePagesTotalBytes: newuint64(0), + HugePagesFreeBytes: newuint64(0), + HugePagesRsvdBytes: newuint64(0), + HugePagesSurpBytes: newuint64(0), + HugepagesizeBytes: newuint64(2097152), + DirectMap4kBytes: newuint64(93323264), + DirectMap2MBytes: newuint64(16424894464), } have, err := getProcFixtures(t).Meminfo() From f7b4621f8166ce550ca135fe0d64ba5df23f40d8 Mon Sep 17 00:00:00 2001 From: TJ Hoplock Date: Mon, 25 Sep 2023 23:54:42 -0400 Subject: [PATCH 2/3] ref(meminfo): simplify variable assignment Addresses PR feedback https://github.com/prometheus/procfs/pull/569#discussion_r1336227047 Signed-off-by: TJ Hoplock --- meminfo.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/meminfo.go b/meminfo.go index cccdfcd0e..ec00a6246 100644 --- a/meminfo.go +++ b/meminfo.go @@ -217,13 +217,11 @@ func parseMemInfo(r io.Reader) (*Meminfo, error) { fields := strings.Fields(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 } - val = v - switch len(fields) { case 2: // No unit present, use the parsed the value as bytes directly. From 325db766b91801edb487e6103ea6477978454bb4 Mon Sep 17 00:00:00 2001 From: TJ Hoplock Date: Tue, 26 Sep 2023 09:11:56 -0400 Subject: [PATCH 3/3] fix(meminfo): remove `bytes` versions of fields that aren't bytes Signed-off-by: TJ Hoplock --- meminfo.go | 8 -------- meminfo_test.go | 4 ---- 2 files changed, 12 deletions(-) diff --git a/meminfo.go b/meminfo.go index ec00a6246..db1daea5f 100644 --- a/meminfo.go +++ b/meminfo.go @@ -184,10 +184,6 @@ type Meminfo struct { ShmemPmdMappedBytes *uint64 CmaTotalBytes *uint64 CmaFreeBytes *uint64 - HugePagesTotalBytes *uint64 - HugePagesFreeBytes *uint64 - HugePagesRsvdBytes *uint64 - HugePagesSurpBytes *uint64 HugepagesizeBytes *uint64 DirectMap4kBytes *uint64 DirectMap2MBytes *uint64 @@ -363,16 +359,12 @@ func parseMemInfo(r io.Reader) (*Meminfo, error) { m.CmaFreeBytes = &valBytes case "HugePages_Total:": m.HugePagesTotal = &val - m.HugePagesTotalBytes = &valBytes case "HugePages_Free:": m.HugePagesFree = &val - m.HugePagesFreeBytes = &valBytes case "HugePages_Rsvd:": m.HugePagesRsvd = &val - m.HugePagesRsvdBytes = &valBytes case "HugePages_Surp:": m.HugePagesSurp = &val - m.HugePagesSurpBytes = &valBytes case "Hugepagesize:": m.Hugepagesize = &val m.HugepagesizeBytes = &valBytes diff --git a/meminfo_test.go b/meminfo_test.go index 5a4b761f4..2d2b238e1 100644 --- a/meminfo_test.go +++ b/meminfo_test.go @@ -98,10 +98,6 @@ func TestMeminfo(t *testing.T) { VmallocChunkBytes: newuint64(35184269148160), HardwareCorruptedBytes: newuint64(0), AnonHugePagesBytes: newuint64(12582912), - HugePagesTotalBytes: newuint64(0), - HugePagesFreeBytes: newuint64(0), - HugePagesRsvdBytes: newuint64(0), - HugePagesSurpBytes: newuint64(0), HugepagesizeBytes: newuint64(2097152), DirectMap4kBytes: newuint64(93323264), DirectMap2MBytes: newuint64(16424894464),