fix(meminfo): account for optional unit field so values are accurate

[tjhop]

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.

[SuperQ]

I think this is a good idea, but it's going to silently change the results of the struct.

I think we need a breaking change here in order to signal to downstream code that the results are different.

Maybe we should rename the struct from Meminfo to MeminfoBytes in order to make it clear that the values returned are in bytes.

Or we add new struct fields like:

MemTotal: val
MemTotalBytes: scaledValue(val, unit)

[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

[SuperQ]

Thanks!

[SuperQ]

These are never bytes, as they're always unit of "pages".

See mm/hugetlb.c

[tjhop]

Makes sense -- I'll get rid of the Bytes fields for these. Since those fields in the kernel explicitly never emit a unit, they should always follow the len == 2 path and never be subject to the scaling logic, so I feel comfortable not adding in any explicit handling to avoid scaling those fields. Please lemme know if you feel differently

[tjhop]

Addressed in 8607561

[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.

[discordianfish]

Great, thanks!