Repository navigation
feat: host filesystem, load and swap metrics in standalone mode; fix meminfo KiB - #381
mayankpande88 wants to merge 3 commits into
Conversation
memoryInfo multiplied /proc/meminfo values by 1000, but the kernel's kB there are 1024 bytes, so node_resources_memory_* read about 2.3% low (upstream has the same bug). They now match node_exporter's node_memory_*_bytes.
Without Kubernetes there is usually no node_exporter, and the agent had
no filesystem space (only volumes a workload writes to), load or swap
metrics. In standalone mode the node collector now exports them under
node_exporter's names: node_filesystem_{size,free,avail}_bytes,
node_filesystem_files{,_free} and node_filesystem_readonly per mount
(pid 1's mounts, without node_exporter's default pseudo and container
filesystems), node_load{1,5,15} and node_memory_Swap{Total,Free}_bytes.
statfs runs with a timeout, and a mount that hangs is skipped for 5
minutes, so a dead network mount cannot block a scrape. On Kubernetes
these stay off: node_exporter usually runs there and sums would count
the same host twice.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces host metrics collection (load average, swap memory, and filesystem statistics) under node_exporter names for standalone/VM environments where node_exporter is not running. It also corrects the memory unit multiplier from 1000 to 1024 (KiB) in /proc/meminfo parsing. The review feedback focuses on improving the robustness of the statfs timeout mechanism by refactoring it into a method on hostCollector and tracking active statfs calls to prevent spawning duplicate goroutines and leaking them when network mounts are hung.
| statfsTimeout = 2 * time.Second | ||
| stuckMountRetry = 5 * time.Minute | ||
| errStatfsTimeout = errors.New("statfs timed out") | ||
| ) |
There was a problem hiding this comment.
Define a new error errStatfsRunning to represent when a statfs call is already in progress for a given mount point. This will be used to prevent spawning duplicate goroutines for hung mounts.
| statfsTimeout = 2 * time.Second | |
| stuckMountRetry = 5 * time.Minute | |
| errStatfsTimeout = errors.New("statfs timed out") | |
| ) | |
| statfsTimeout = 2 * time.Second | |
| stuckMountRetry = 5 * time.Minute | |
| errStatfsTimeout = errors.New("statfs timed out") | |
| errStatfsRunning = errors.New("statfs already running") | |
| ) |
There was a problem hiding this comment.
Done in fca1ae5 (see the statfs thread).
| type hostCollector struct { | ||
| procRoot string | ||
|
|
||
| lock sync.Mutex | ||
| stuck map[string]time.Time // mount points whose statfs timed out | ||
| } |
There was a problem hiding this comment.
Add a running map to hostCollector to track active statfs calls per mount point. This prevents spawning multiple goroutines for the same hung mount point, which would otherwise lead to a goroutine leak over time.
| type hostCollector struct { | |
| procRoot string | |
| lock sync.Mutex | |
| stuck map[string]time.Time // mount points whose statfs timed out | |
| } | |
| type hostCollector struct { | |
| procRoot string | |
| lock sync.Mutex | |
| stuck map[string]time.Time // mount points whose statfs timed out | |
| running map[string]bool // mount points with an active statfs call | |
| } |
There was a problem hiding this comment.
Done in fca1ae5 (see the statfs thread).
| func newHostCollector(procRoot string) *hostCollector { | ||
| return &hostCollector{procRoot: procRoot, stuck: map[string]time.Time{}} | ||
| } |
There was a problem hiding this comment.
Initialize the running map in newHostCollector.
func newHostCollector(procRoot string) *hostCollector {
return &hostCollector{
procRoot: procRoot,
stuck: map[string]time.Time{},
running: map[string]bool{},
}
}There was a problem hiding this comment.
Done in fca1ae5 (see the statfs thread).
| s, err := statfs(path.Join(h.procRoot, "1", "root", m.mountPoint)) | ||
| if err != nil { | ||
| if errors.Is(err, errStatfsTimeout) { | ||
| klog.Warningf("statfs of %s timed out, skipping it for %s", m.mountPoint, stuckMountRetry) | ||
| h.markStuck(m.mountPoint) | ||
| } | ||
| continue | ||
| } |
There was a problem hiding this comment.
Update the collect method to call the new h.statfs method instead of the package-level statfs function.
| s, err := statfs(path.Join(h.procRoot, "1", "root", m.mountPoint)) | |
| if err != nil { | |
| if errors.Is(err, errStatfsTimeout) { | |
| klog.Warningf("statfs of %s timed out, skipping it for %s", m.mountPoint, stuckMountRetry) | |
| h.markStuck(m.mountPoint) | |
| } | |
| continue | |
| } | |
| s, err := h.statfs(m.mountPoint) | |
| if err != nil { | |
| if errors.Is(err, errStatfsTimeout) { | |
| klog.Warningf("statfs of %s timed out, skipping it for %s", m.mountPoint, stuckMountRetry) | |
| h.markStuck(m.mountPoint) | |
| } | |
| continue | |
| } |
There was a problem hiding this comment.
Done in fca1ae5 (see the statfs thread).
| // statfs runs statfs(2) with a timeout: on a hung network mount it never | ||
| // returns, and a scrape must not hang with it. | ||
| func statfs(p string) (*unix.Statfs_t, error) { | ||
| type result struct { | ||
| s unix.Statfs_t | ||
| err error | ||
| } | ||
| ch := make(chan result, 1) | ||
| go func() { | ||
| var r result | ||
| r.err = unix.Statfs(p, &r.s) | ||
| ch <- r | ||
| }() | ||
| select { | ||
| case r := <-ch: | ||
| if r.err != nil { | ||
| return nil, r.err | ||
| } | ||
| return &r.s, nil | ||
| case <-time.After(statfsTimeout): | ||
| return nil, errStatfsTimeout | ||
| } | ||
| } |
There was a problem hiding this comment.
Refactor statfs into a method on hostCollector that tracks active calls. If a statfs call is already running for a mount point (e.g., because it is hung), it returns errStatfsRunning immediately to avoid leaking goroutines. Additionally, when a previously stuck mount successfully returns, it automatically clears its stuck status from the stuck map, enabling instant recovery.
// statfs runs statfs(2) with a timeout. If a previous statfs call for the same
// mount point is still running (e.g. hung on a network mount), it returns
// errStatfsRunning immediately to avoid leaking goroutines and blocking scrapes.
func (h *hostCollector) statfs(mountPoint string) (*unix.Statfs_t, error) {
h.lock.Lock()
if h.running[mountPoint] {
h.lock.Unlock()
return nil, errStatfsRunning
}
h.running[mountPoint] = true
h.lock.Unlock()
type result struct {
s unix.Statfs_t
err error
}
ch := make(chan result, 1)
p := path.Join(h.procRoot, "1", "root", mountPoint)
go func() {
var r result
r.err = unix.Statfs(p, &r.s)
ch <- r
h.lock.Lock()
delete(h.running, mountPoint)
h.lock.Unlock()
}()
select {
case r := <-ch:
if r.err != nil {
return nil, r.err
}
h.lock.Lock()
delete(h.stuck, mountPoint)
h.lock.Unlock()
return &r.s, nil
case <-time.After(statfsTimeout):
return nil, errStatfsTimeout
}
}There was a problem hiding this comment.
Fixed in fca1ae5, along these lines.
- In-flight tracking:
statfsis now a method that records an in-flight call per mount point. It returnserrStatfsRunninginstead of starting a second goroutine while the first is still blocked in the kernel. - Recovery: a successful call clears the stuck mark.
- Test:
TestStatfsHungMountSingleCalluses a hanging statfs and checks that it's called exactly once until it returns, then works again. It passes under-race.
A mount whose statfs hung was retried every 5 minutes, and each retry started another goroutine while the previous one was still blocked in the kernel, holding an OS thread: a dead network mount leaked one thread every 5 minutes. A second statfs is no longer started while one is in flight for that mount point, and a successful call clears the mount's stuck mark.
1 participant
Summary
Host metrics for hosts without node_exporter, plus a fix to the existing memory metrics.
Standalone mode (no Kubernetes): the node collector exports the host's filesystem space on every mount, load average and swap, under node_exporter's names:
node_filesystem_{size,free,avail}_bytes,node_filesystem_files,node_filesystem_files_freeandnode_filesystem_readonly(labelsdevice,fstype,mountpoint);node_load1,node_load5,node_load15;node_memory_SwapTotal_bytes,node_memory_SwapFree_bytes.Before this, the agent had no filesystem space except for volumes a workload writes to, and no load or swap.
Kubernetes: these stay off. node_exporter usually runs there, and sums by instance would count the same host twice.
/proc/meminfounits: the "kB" there are KiB, but values were multiplied by 1000, sonode_resources_memory_*read about 2.3% low (upstream has the same bug). They now report correct values, about 2.4% higher than before, matching node_exporter'snode_memory_*_bytes.Engineering detail
Mounts:
/proc/1/mounts, with node_exporter's default exclusions:/dev,/proc,/sys,/run/credentials/...,/var/lib/docker/...and/var/lib/containers/storage/....\040for a space) are decoded, and read-only mounts are reported.statfs:
/proc/1/root/<mountpoint>, with a 2s timeout.TestStatfsHungMountSingleCallcovers it.Where it's switched on: standalone mode is when the IP resolver is the VM resolver (no Kubernetes API).
Tests: load average parsing, mount filtering, escapes and read-only flags, and swap plus the KiB multiplier in the memory test.
CI: gofmt, goimports, vet, golangci-lint,
go test(excluding/containers) and the build all pass in a Linux container with Go 1.26.5.Local e2e: I built agent binaries from this branch and from main and ran both as systemd services on a local Debian 12 VM (kernel 6.1), with a 256 MiB swapfile.
df -B1exactly (12468432896 / 8699375616)./,/boot/efi,/run,/run/lock,/run/user/501and the VM's two host shares were reported;/dev/shmwas left out, as in node_exporter.node_load*matched/proc/loadavg, and swap matched/proc/meminfo.node_resources_memory_total_byteswas 2081710080 (= MemTotal × 1024) on this branch and 2032920000 on main.node_filesystem_*ornode_load*.