ref!: convert linux meminfo implementation to use procfs lib (prometh…

…eus#3049)

* ref!: convert linux meminfo implementation to use procfs lib

Part of prometheus#2957

Prometheus' procfs lib supports collecting memory info and we're using a
new enough version of the lib that has it available, so this converts
the meminfo collector for Linux to use data from procfs lib instead. The
bits I've touched for darwin/openbsd/netbsd are with intent to preserve
the original struct implementation/backwards compatibility.

Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>

* fix: meminfo debug log unsupported value

Fixes:

```
ts=2024-06-11T19:04:55.591Z caller=meminfo.go:44 level=debug collector=meminfo msg="Set node_mem" memInfo="unsupported value type"
```

Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>

* fix: don't coerce nil Meminfo entries to 0, leave out if nil

Nil entries in procfs.Meminfo fields indicate that the value isn't
present on the system. Coercing those nil values to `0` introduces new
metrics on systems that should not be present and can break some
queries.

Addresses PR feedback:
prometheus#3049 (comment)
prometheus#3049 (comment)

Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>

---------

Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>
Signed-off-by: Vitaly Zhuravlev <v-zhuravlev@users.noreply.github.com>

collector/meminfo.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
 )
@@ -30,19 +29,10 @@ const (
 	memInfoSubsystem = "memory"
 )
 
-type meminfoCollector struct {
-	logger log.Logger
-}
-
 func init() {
 	registerCollector("meminfo", defaultEnabled, NewMeminfoCollector)
 }
 
-// NewMeminfoCollector returns a new Collector exposing memory stats.
-func NewMeminfoCollector(logger log.Logger) (Collector, error) {
-	return &meminfoCollector{logger}, nil
-}
-
 // Update calls (*meminfoCollector).getMemInfo to get the platform specific
 // memory metrics.
 func (c *meminfoCollector) Update(ch chan<- prometheus.Metric) error {
@@ -51,7 +41,7 @@ func (c *meminfoCollector) Update(ch chan<- prometheus.Metric) error {
 	if err != nil {
 		return fmt.Errorf("couldn't get meminfo: %w", err)
 	}
-	level.Debug(c.logger).Log("msg", "Set node_mem", "memInfo", memInfo)
+	level.Debug(c.logger).Log("msg", "Set node_mem", "memInfo", fmt.Sprintf("%v", memInfo))
 	for k, v := range memInfo {
 		if strings.HasSuffix(k, "_total") {
 			metricType = prometheus.CounterValue

collector/meminfo_darwin.go

@@ -26,9 +26,21 @@ import (
 	"fmt"
 	"unsafe"
 
+	"github.com/go-kit/log"
 	"golang.org/x/sys/unix"
 )
 
+type meminfoCollector struct {
+	logger log.Logger
+}
+
+// NewMeminfoCollector returns a new Collector exposing memory stats.
+func NewMeminfoCollector(logger log.Logger) (Collector, error) {
+	return &meminfoCollector{
+		logger: logger,
+	}, nil
+}
+
 func (c *meminfoCollector) getMemInfo() (map[string]float64, error) {
 	host := C.mach_host_self()
 	infoCount := C.mach_msg_type_number_t(C.HOST_VM_INFO64_COUNT)

collector/meminfo_linux.go

@@ -17,59 +17,189 @@
 package collector
 
 import (
-	"bufio"
 	"fmt"
-	"io"
-	"os"
-	"regexp"
-	"strconv"
-	"strings"
-)
 
-var (
-	reParens = regexp.MustCompile(`\((.*)\)`)
+	"github.com/go-kit/log"
+	"github.com/prometheus/procfs"
 )
 
-func (c *meminfoCollector) getMemInfo() (map[string]float64, error) {
-	file, err := os.Open(procFilePath("meminfo"))
+type meminfoCollector struct {
+	fs     procfs.FS
+	logger log.Logger
+}
+
+// NewMeminfoCollector returns a new Collector exposing memory stats.
+func NewMeminfoCollector(logger log.Logger) (Collector, error) {
+	fs, err := procfs.NewFS(*procPath)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to open procfs: %w", err)
 	}
-	defer file.Close()
 
-	return parseMemInfo(file)
+	return &meminfoCollector{
+		logger: logger,
+		fs:     fs,
+	}, nil
 }
 
-func parseMemInfo(r io.Reader) (map[string]float64, error) {
-	var (
-		memInfo = map[string]float64{}
-		scanner = bufio.NewScanner(r)
-	)
+func (c *meminfoCollector) getMemInfo() (map[string]float64, error) {
+	meminfo, err := c.fs.Meminfo()
+	if err != nil {
+		return nil, fmt.Errorf("Failed to get memory info: %s", err)
+	}
+
+	metrics := make(map[string]float64)
 
-	for scanner.Scan() {
-		line := scanner.Text()
-		parts := strings.Fields(line)
-		// Workaround for empty lines occasionally occur in CentOS 6.2 kernel 3.10.90.
-		if len(parts) == 0 {
-			continue
-		}
-		fv, err := strconv.ParseFloat(parts[1], 64)
-		if err != nil {
-			return nil, fmt.Errorf("invalid value in meminfo: %w", err)
-		}
-		key := parts[0][:len(parts[0])-1] // remove trailing : from key
-		// Active(anon) -> Active_anon
-		key = reParens.ReplaceAllString(key, "_${1}")
-		switch len(parts) {
-		case 2: // no unit
-		case 3: // has unit, we presume kB
-			fv *= 1024
-			key = key + "_bytes"
-		default:
-			return nil, fmt.Errorf("invalid line in meminfo: %s", line)
-		}
-		memInfo[key] = fv
+	if meminfo.ActiveBytes != nil {
+		metrics["Active_bytes"] = float64(*meminfo.ActiveBytes)
+	}
+	if meminfo.ActiveAnonBytes != nil {
+		metrics["Active_anon_bytes"] = float64(*meminfo.ActiveAnonBytes)
+	}
+	if meminfo.ActiveFileBytes != nil {
+		metrics["Active_file_bytes"] = float64(*meminfo.ActiveFileBytes)
+	}
+	if meminfo.AnonHugePagesBytes != nil {
+		metrics["AnonHugePages_bytes"] = float64(*meminfo.AnonHugePagesBytes)
+	}
+	if meminfo.AnonPagesBytes != nil {
+		metrics["AnonPages_bytes"] = float64(*meminfo.AnonPagesBytes)
+	}
+	if meminfo.BounceBytes != nil {
+		metrics["Bounce_bytes"] = float64(*meminfo.BounceBytes)
+	}
+	if meminfo.BuffersBytes != nil {
+		metrics["Buffers_bytes"] = float64(*meminfo.BuffersBytes)
+	}
+	if meminfo.CachedBytes != nil {
+		metrics["Cached_bytes"] = float64(*meminfo.CachedBytes)
+	}
+	if meminfo.CmaFreeBytes != nil {
+		metrics["CmaFree_bytes"] = float64(*meminfo.CmaFreeBytes)
+	}
+	if meminfo.CmaTotalBytes != nil {
+		metrics["CmaTotal_bytes"] = float64(*meminfo.CmaTotalBytes)
+	}
+	if meminfo.CommitLimitBytes != nil {
+		metrics["CommitLimit_bytes"] = float64(*meminfo.CommitLimitBytes)
+	}
+	if meminfo.CommittedASBytes != nil {
+		metrics["Committed_AS_bytes"] = float64(*meminfo.CommittedASBytes)
+	}
+	if meminfo.DirectMap1GBytes != nil {
+		metrics["DirectMap1G_bytes"] = float64(*meminfo.DirectMap1GBytes)
+	}
+	if meminfo.DirectMap2MBytes != nil {
+		metrics["DirectMap2M_bytes"] = float64(*meminfo.DirectMap2MBytes)
+	}
+	if meminfo.DirectMap4kBytes != nil {
+		metrics["DirectMap4k_bytes"] = float64(*meminfo.DirectMap4kBytes)
+	}
+	if meminfo.DirtyBytes != nil {
+		metrics["Dirty_bytes"] = float64(*meminfo.DirtyBytes)
+	}
+	if meminfo.HardwareCorruptedBytes != nil {
+		metrics["HardwareCorrupted_bytes"] = float64(*meminfo.HardwareCorruptedBytes)
+	}
+	if meminfo.HugepagesizeBytes != nil {
+		metrics["Hugepagesize_bytes"] = float64(*meminfo.HugepagesizeBytes)
+	}
+	if meminfo.InactiveBytes != nil {
+		metrics["Inactive_bytes"] = float64(*meminfo.InactiveBytes)
+	}
+	if meminfo.InactiveAnonBytes != nil {
+		metrics["Inactive_anon_bytes"] = float64(*meminfo.InactiveAnonBytes)
+	}
+	if meminfo.InactiveFileBytes != nil {
+		metrics["Inactive_file_bytes"] = float64(*meminfo.InactiveFileBytes)
+	}
+	if meminfo.KernelStackBytes != nil {
+		metrics["KernelStack_bytes"] = float64(*meminfo.KernelStackBytes)
+	}
+	if meminfo.MappedBytes != nil {
+		metrics["Mapped_bytes"] = float64(*meminfo.MappedBytes)
+	}
+	if meminfo.MemAvailableBytes != nil {
+		metrics["MemAvailable_bytes"] = float64(*meminfo.MemAvailableBytes)
+	}
+	if meminfo.MemFreeBytes != nil {
+		metrics["MemFree_bytes"] = float64(*meminfo.MemFreeBytes)
+	}
+	if meminfo.MemTotalBytes != nil {
+		metrics["MemTotal_bytes"] = float64(*meminfo.MemTotalBytes)
+	}
+	if meminfo.MlockedBytes != nil {
+		metrics["Mlocked_bytes"] = float64(*meminfo.MlockedBytes)
+	}
+	if meminfo.NFSUnstableBytes != nil {
+		metrics["NFS_Unstable_bytes"] = float64(*meminfo.NFSUnstableBytes)
+	}
+	if meminfo.PageTablesBytes != nil {
+		metrics["PageTables_bytes"] = float64(*meminfo.PageTablesBytes)
+	}
+	if meminfo.PercpuBytes != nil {
+		metrics["Percpu_bytes"] = float64(*meminfo.PercpuBytes)
+	}
+	if meminfo.SReclaimableBytes != nil {
+		metrics["SReclaimable_bytes"] = float64(*meminfo.SReclaimableBytes)
+	}
+	if meminfo.SUnreclaimBytes != nil {
+		metrics["SUnreclaim_bytes"] = float64(*meminfo.SUnreclaimBytes)
+	}
+	if meminfo.ShmemBytes != nil {
+		metrics["Shmem_bytes"] = float64(*meminfo.ShmemBytes)
+	}
+	if meminfo.ShmemHugePagesBytes != nil {
+		metrics["ShmemHugePages_bytes"] = float64(*meminfo.ShmemHugePagesBytes)
+	}
+	if meminfo.ShmemPmdMappedBytes != nil {
+		metrics["ShmemPmdMapped_bytes"] = float64(*meminfo.ShmemPmdMappedBytes)
+	}
+	if meminfo.SlabBytes != nil {
+		metrics["Slab_bytes"] = float64(*meminfo.SlabBytes)
+	}
+	if meminfo.SwapCachedBytes != nil {
+		metrics["SwapCached_bytes"] = float64(*meminfo.SwapCachedBytes)
+	}
+	if meminfo.SwapFreeBytes != nil {
+		metrics["SwapFree_bytes"] = float64(*meminfo.SwapFreeBytes)
+	}
+	if meminfo.SwapTotalBytes != nil {
+		metrics["SwapTotal_bytes"] = float64(*meminfo.SwapTotalBytes)
+	}
+	if meminfo.UnevictableBytes != nil {
+		metrics["Unevictable_bytes"] = float64(*meminfo.UnevictableBytes)
+	}
+	if meminfo.VmallocChunkBytes != nil {
+		metrics["VmallocChunk_bytes"] = float64(*meminfo.VmallocChunkBytes)
+	}
+	if meminfo.VmallocTotalBytes != nil {
+		metrics["VmallocTotal_bytes"] = float64(*meminfo.VmallocTotalBytes)
+	}
+	if meminfo.VmallocUsedBytes != nil {
+		metrics["VmallocUsed_bytes"] = float64(*meminfo.VmallocUsedBytes)
+	}
+	if meminfo.WritebackBytes != nil {
+		metrics["Writeback_bytes"] = float64(*meminfo.WritebackBytes)
+	}
+	if meminfo.WritebackTmpBytes != nil {
+		metrics["WritebackTmp_bytes"] = float64(*meminfo.WritebackTmpBytes)
+	}
+
+	// These fields are always in bytes and do not have `Bytes`
+	// suffixed counterparts in the procfs.Meminfo struct, nor do
+	// they have `_bytes` suffix on the metric names.
+	if meminfo.HugePagesFree != nil {
+		metrics["HugePages_Free"] = float64(*meminfo.HugePagesFree)
+	}
+	if meminfo.HugePagesRsvd != nil {
+		metrics["HugePages_Rsvd"] = float64(*meminfo.HugePagesRsvd)
+	}
+	if meminfo.HugePagesSurp != nil {
+		metrics["HugePages_Surp"] = float64(*meminfo.HugePagesSurp)
+	}
+	if meminfo.HugePagesTotal != nil {
+		metrics["HugePages_Total"] = float64(*meminfo.HugePagesTotal)
 	}
 
-	return memInfo, scanner.Err()
+	return metrics, nil
 }

collector/meminfo_linux_test.go

@@ -19,18 +19,22 @@ package collector
 import (
 	"os"
 	"testing"
+
+	"github.com/go-kit/log"
 )
 
 func TestMemInfo(t *testing.T) {
-	file, err := os.Open("fixtures/proc/meminfo")
+	*procPath = "fixtures/proc"
+	logger := log.NewLogfmtLogger(os.Stderr)
+
+	collector, err := NewMeminfoCollector(logger)
 	if err != nil {
-		t.Fatal(err)
+		panic(err)
 	}
-	defer file.Close()
 
-	memInfo, err := parseMemInfo(file)
+	memInfo, err := collector.(*meminfoCollector).getMemInfo()
 	if err != nil {
-		t.Fatal(err)
+		panic(err)
 	}
 
 	if want, got := 3831959552.0, memInfo["MemTotal_bytes"]; want != got {

collector/meminfo_netbsd.go

@@ -17,9 +17,21 @@
 package collector
 
 import (
+	"github.com/go-kit/log"
 	"golang.org/x/sys/unix"
 )
 
+type meminfoCollector struct {
+	logger log.Logger
+}
+
+// NewMeminfoCollector returns a new Collector exposing memory stats.
+func NewMeminfoCollector(logger log.Logger) (Collector, error) {
+	return &meminfoCollector{
+		logger: logger,
+	}, nil
+}
+
 func (c *meminfoCollector) getMemInfo() (map[string]float64, error) {
 	uvmexp, err := unix.SysctlUvmexp("vm.uvmexp2")
 	if err != nil {

collector/meminfo_openbsd.go

@@ -18,6 +18,8 @@ package collector
 
 import (
 	"fmt"
+
+	"github.com/go-kit/log"
 )
 
 /*
@@ -53,6 +55,17 @@ sysctl_bcstats(struct bcachestats *bcstats)
 */
 import "C"
 
+type meminfoCollector struct {
+	logger log.Logger
+}
+
+// NewMeminfoCollector returns a new Collector exposing memory stats.
+func NewMeminfoCollector(logger log.Logger) (Collector, error) {
+	return &meminfoCollector{
+		logger: logger,
+	}, nil
+}
+
 func (c *meminfoCollector) getMemInfo() (map[string]float64, error) {
 	var uvmexp C.struct_uvmexp
 	var bcstats C.struct_bcachestats

collector/meminfo_openbsd_amd64.go

@@ -17,8 +17,10 @@
 package collector
 
 import (
-	"golang.org/x/sys/unix"
 	"unsafe"
+
+	"github.com/go-kit/log"
+	"golang.org/x/sys/unix"
 )
 
 const (
@@ -48,6 +50,17 @@ type bcachestats struct {
 	Dmaflips       int64
 }
 
+type meminfoCollector struct {
+	logger log.Logger
+}
+
+// NewMeminfoCollector returns a new Collector exposing memory stats.
+func NewMeminfoCollector(logger log.Logger) (Collector, error) {
+	return &meminfoCollector{
+		logger: logger,
+	}, nil
+}
+
 func (c *meminfoCollector) getMemInfo() (map[string]float64, error) {
 	uvmexpb, err := unix.SysctlRaw("vm.uvmexp")
 	if err != nil {