parseMemInfo() ignores unit, leading to incorrect results from /proc/meminfo

[tjhop]

Hi all :wave:

Is there a reason that the parseMemInfo() function ignores the optional unit field? This can lead to incorrect results on systems that do supply the field, as the units are obviously not adjusted accordingly.

A small example program/output: meminfo.go

package main

import (
    "bufio"
    "fmt"
    "os"
    "strings"

    "github.com/dustin/go-humanize"
    "github.com/prometheus/procfs"
)

func main() {
    fmt.Println("Meminfo sample entries:")

    meminfoFile, err := os.Open("/proc/meminfo")
    if err != nil {
        panic(err)
    }
    defer meminfoFile.Close()

    scanner := bufio.NewScanner(meminfoFile)
    for scanner.Scan() {
        line := scanner.Text()
        fields := strings.Fields(line)
        key := strings.TrimSuffix(fields[0], ":")
        if key == "MemTotal" || key == "MemFree" {
            fmt.Println(line)

            val, err := humanize.ParseBytes(fmt.Sprintf("%s %s", fields[1], fields[2]))
            if err != nil {
                panic(err)
            }
            fmt.Printf("%s humanized: %s\n", key, humanize.Bytes(val))
        }
    }

    if scanner.Err() != nil {
        if err != nil {
            panic(err)
        }
    }

    fmt.Println()
    fmt.Println("procfs lib values:")
    fs, err := procfs.NewFS("/proc")
    if err != nil {
        panic(err)
    }

    meminfo, err := fs.Meminfo()
    if err != nil {
        panic(err)
    }

    fmt.Printf("MemTotal: %d bytes\n", *meminfo.MemTotal)
    fmt.Printf("MemTotal humanized: %s\n", humanize.Bytes(*meminfo.MemTotal))
    fmt.Printf("MemFree: %d bytes\n", *meminfo.MemFree)
    fmt.Printf("MemFree humanized: %s\n", humanize.Bytes(*meminfo.MemFree))
}

output:

/tmp/procfs -> go run meminfo.go
Meminfo sample entries:
MemTotal:       32588196 kB
MemTotal humanized: 33 GB
MemFree:         9787012 kB
MemFree humanized: 9.8 GB

procfs lib values:
MemTotal: 32588196 bytes
MemTotal humanized: 33 MB
MemFree: 9787012 bytes
MemFree humanized: 9.8 MB

This also looks like a good reason why the node exporter chooses to do (similar) but custom parsing of /proc/meminfo to account for the optional unit field, rather than use this library for meminfo as it's used in other collectors.

Using the humanize lib that's used elsewhere in other prometheus projects, I have what I believe to be a functional commit here that I'd be happy to contribute if this is determined to be a bug/should be fixed.

Example output with the patched lib:

/tmp/procfs -> go run meminfo.go
Meminfo sample entries:
MemTotal:       32588196 kB
MemTotal humanized: 33 GB
MemFree:         9685964 kB
MemFree humanized: 9.7 GB

procfs lib values:
MemTotal: 32588196000 bytes
MemTotal humanized: 33 GB
MemFree: 9685964000 bytes
MemFree humanized: 9.7 GB

[discordianfish]

Guess we were under the assumption that it's always the same unit. If not, that's an issue..

[tjhop]

No worries, I've opened a PR to fix it :+1:

[SuperQ]

The humanize library produces incorrect results, as the kernel is exposing kB to mean 1024. But the humanize library kB means 1000.

[tjhop]

Closing since #569 was merged