From 046d2ea4e757e54ada8ed7a10db4e4e6f7fdc7a0 Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Mon, 6 Nov 2023 16:00:47 +0100 Subject: [PATCH 1/2] Fix CmdLine generation and caching When collecting process metrics the generation of `cmdLine` field and its caching were happening in the wrong place. This commit fixes it. --- CHANGELOG.md | 2 ++ metric/system/process/process.go | 10 ++++++---- metric/system/process/process_test.go | 2 ++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30a8f056b..4c95a06a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed + - Fix CmdLine generation and caching for system.process + ## [0.7.0] ### Added diff --git a/metric/system/process/process.go b/metric/system/process/process.go index 8ac76ac40..56b9e99de 100644 --- a/metric/system/process/process.go +++ b/metric/system/process/process.go @@ -222,7 +222,6 @@ func (procStats *Stats) pidFill(pid int, filter bool) (ProcState, bool, error) { if procStats.skipExtended { return status, true, nil } - status = procStats.cacheCmdLine(status) // Filter based on user-supplied func if filter { @@ -236,9 +235,7 @@ func (procStats *Stats) pidFill(pid int, filter bool) (ProcState, bool, error) { if err != nil { return status, true, fmt.Errorf("FillPidMetrics: %w", err) } - if len(status.Args) > 0 && status.Cmdline == "" { - status.Cmdline = strings.Join(status.Args, " ") - } + if status.CPU.Total.Ticks.Exists() { status.CPU.Total.Value = opt.FloatWith(metric.Round(float64(status.CPU.Total.Ticks.ValueOr(0)))) } @@ -266,6 +263,11 @@ func (procStats *Stats) pidFill(pid int, filter bool) (ProcState, bool, error) { return status, true, fmt.Errorf("FillMetricsRequiringMoreAccess: %w", err) } + if len(status.Args) > 0 && status.Cmdline == "" { + status.Cmdline = strings.Join(status.Args, " ") + } + status = procStats.cacheCmdLine(status) + // network data if procStats.EnableNetwork { procHandle, err := sysinfo.Process(pid) diff --git a/metric/system/process/process_test.go b/metric/system/process/process_test.go index 3f4344b04..330bcfda4 100644 --- a/metric/system/process/process_test.go +++ b/metric/system/process/process_test.go @@ -279,6 +279,8 @@ func TestGetProcess(t *testing.T) { case "linux": assert.True(t, (len(process.Cwd) > 0)) } + + assert.NotEmptyf(t, process.Cmdline, "cmdLine must be present") } func TestMatchProcs(t *testing.T) { From a12415f9f64d8e5fd516281eebe3cf4e53e8ef49 Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Tue, 7 Nov 2023 13:05:54 +0100 Subject: [PATCH 2/2] Fix cache read position --- metric/system/process/process.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/metric/system/process/process.go b/metric/system/process/process.go index 56b9e99de..0b1b80d75 100644 --- a/metric/system/process/process.go +++ b/metric/system/process/process.go @@ -223,6 +223,10 @@ func (procStats *Stats) pidFill(pid int, filter bool) (ProcState, bool, error) { return status, true, nil } + // Some OSes use the cache to avoid expensive system calls, + // cacheCmdLine reads from the cache. + status = procStats.cacheCmdLine(status) + // Filter based on user-supplied func if filter { if !procStats.matchProcess(status.Name) { @@ -263,10 +267,11 @@ func (procStats *Stats) pidFill(pid int, filter bool) (ProcState, bool, error) { return status, true, fmt.Errorf("FillMetricsRequiringMoreAccess: %w", err) } + // Generate `status.Cmdline` here for compatibility because on Windows + // `status.Args` is set by `FillMetricsRequiringMoreAccess`. if len(status.Args) > 0 && status.Cmdline == "" { status.Cmdline = strings.Join(status.Args, " ") } - status = procStats.cacheCmdLine(status) // network data if procStats.EnableNetwork {