From 8cfe5bbba399def9d0cd263c7a0f8c6795157632 Mon Sep 17 00:00:00 2001 From: Parampreet Singh <50599809+Paramadon@users.noreply.github.com> Date: Tue, 1 Oct 2024 11:04:26 -0400 Subject: [PATCH] Fix memory_swap Metric and Add Config Option in procstat (#159) --- patches/gopsutil/v3/process/process_bsd.go | 4 +- plugins/inputs/procstat/README.md | 3 ++ plugins/inputs/procstat/memmap.go | 20 +++++++ plugins/inputs/procstat/memmap_notlinux.go | 5 ++ plugins/inputs/procstat/process.go | 2 +- plugins/inputs/procstat/procstat.go | 27 ++++++++-- plugins/inputs/procstat/procstat_test.go | 63 +++++++++++++++++++++- 7 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 plugins/inputs/procstat/memmap.go create mode 100644 plugins/inputs/procstat/memmap_notlinux.go diff --git a/patches/gopsutil/v3/process/process_bsd.go b/patches/gopsutil/v3/process/process_bsd.go index 263829ffae5b8..cfc44009fa948 100644 --- a/patches/gopsutil/v3/process/process_bsd.go +++ b/patches/gopsutil/v3/process/process_bsd.go @@ -14,7 +14,9 @@ import ( type MemoryInfoExStat struct{} -type MemoryMapsStat struct{} +type MemoryMapsStat struct { + Swap uint64 +} func (p *Process) TgidWithContext(ctx context.Context) (int32, error) { return 0, common.ErrNotImplementedError diff --git a/plugins/inputs/procstat/README.md b/plugins/inputs/procstat/README.md index 60d213cd0c50d..0c93868e97df7 100644 --- a/plugins/inputs/procstat/README.md +++ b/plugins/inputs/procstat/README.md @@ -57,6 +57,9 @@ Processes can be selected for monitoring using one of several methods: ## when processes have a short lifetime. # pid_tag = false + ## Properties to collect + ## Available option "mmap" + # properties = ["mmap"] ## Method to use when finding process IDs. Can be one of 'pgrep', or ## 'native'. The pgrep finder calls the pgrep executable in the PATH while ## the native finder performs the search directly in a manor dependent on the diff --git a/plugins/inputs/procstat/memmap.go b/plugins/inputs/procstat/memmap.go new file mode 100644 index 0000000000000..c20513a5ad63c --- /dev/null +++ b/plugins/inputs/procstat/memmap.go @@ -0,0 +1,20 @@ +//go:build linux || darwin + +package procstat + +// pulled this from this commit https://github.com/influxdata/telegraf/pull/13779 +func collectMemmap(proc Process, prefix string, fields map[string]any) { + memMapStats, err := proc.MemoryMaps(true) + if err == nil && len(*memMapStats) == 1 { + memMap := (*memMapStats)[0] + fields[prefix+"memory_size"] = memMap.Size + fields[prefix+"memory_pss"] = memMap.Pss + fields[prefix+"memory_shared_clean"] = memMap.SharedClean + fields[prefix+"memory_shared_dirty"] = memMap.SharedDirty + fields[prefix+"memory_private_clean"] = memMap.PrivateClean + fields[prefix+"memory_private_dirty"] = memMap.PrivateDirty + fields[prefix+"memory_referenced"] = memMap.Referenced + fields[prefix+"memory_anonymous"] = memMap.Anonymous + fields[prefix+"memory_swap"] = memMap.Swap + } +} diff --git a/plugins/inputs/procstat/memmap_notlinux.go b/plugins/inputs/procstat/memmap_notlinux.go new file mode 100644 index 0000000000000..6c4f7694ed7dd --- /dev/null +++ b/plugins/inputs/procstat/memmap_notlinux.go @@ -0,0 +1,5 @@ +//go:build windows + +package procstat + +func collectMemmap(Process, string, map[string]any) {} diff --git a/plugins/inputs/procstat/process.go b/plugins/inputs/procstat/process.go index f31cef4abe1c6..afc22d39b2e8e 100644 --- a/plugins/inputs/procstat/process.go +++ b/plugins/inputs/procstat/process.go @@ -11,11 +11,11 @@ import ( type Process interface { PID() PID Tags() map[string]string - PageFaults() (*process.PageFaultsStat, error) IOCounters() (*process.IOCountersStat, error) MemoryInfo() (*process.MemoryInfoStat, error) Name() (string, error) + MemoryMaps(bool) (*[]process.MemoryMapsStat, error) Cmdline() (string, error) NumCtxSwitches() (*process.NumCtxSwitchesStat, error) NumFDs() (int32, error) diff --git a/plugins/inputs/procstat/procstat.go b/plugins/inputs/procstat/procstat.go index 915a1b13f44b4..4d5828210a2d0 100644 --- a/plugins/inputs/procstat/procstat.go +++ b/plugins/inputs/procstat/procstat.go @@ -23,6 +23,12 @@ var ( type PID int32 +type collectionConfig struct { + solarisMode bool + tagging map[string]bool + features map[string]bool +} + type Procstat struct { PidFinder string `toml:"pid_finder"` PidFile string `toml:"pid_file"` @@ -38,14 +44,15 @@ type Procstat struct { PidTag bool WinService string `toml:"win_service"` Mode string + Properties []string `toml:"properties"` - solarisMode bool - - finder PIDFinder - + cfg collectionConfig + solarisMode bool + finder PIDFinder createPIDFinder func() (PIDFinder, error) procs map[PID]Process - createProcess func(PID) (Process, error) + + createProcess func(PID) (Process, error) } var sampleConfig = ` @@ -288,6 +295,9 @@ func (p *Procstat) addMetric(proc Process, acc telegraf.Accumulator, t time.Time fields[prefix+"memory_stack"] = mem.Stack fields[prefix+"memory_locked"] = mem.Locked } + if p.cfg.features["mmap"] { + collectMemmap(proc, prefix, fields) + } memPerc, err := proc.MemoryPercent() if err == nil { @@ -564,6 +574,13 @@ func (p *Procstat) Init() error { if strings.ToLower(p.Mode) == "solaris" { p.solarisMode = true } + // Convert collection properties + p.cfg.features = make(map[string]bool, len(p.Properties)) + for _, prop := range p.Properties { + if prop == "mmap" { + p.cfg.features[prop] = true + } + } return nil } diff --git a/plugins/inputs/procstat/procstat_test.go b/plugins/inputs/procstat/procstat_test.go index 5b67232156bc1..87e3df091ec74 100644 --- a/plugins/inputs/procstat/procstat_test.go +++ b/plugins/inputs/procstat/procstat_test.go @@ -100,8 +100,9 @@ type testProc struct { tags map[string]string } -func newTestProc(_ PID) (Process, error) { +func newTestProc(pid PID) (Process, error) { proc := &testProc{ + pid: pid, tags: make(map[string]string), } return proc, nil @@ -131,6 +132,14 @@ func (p *testProc) MemoryInfo() (*process.MemoryInfoStat, error) { return &process.MemoryInfoStat{}, nil } +func (p *testProc) MemoryMaps(_ bool) (*[]process.MemoryMapsStat, error) { + return &[]process.MemoryMapsStat{ + { + Swap: 1024, + }, + }, nil +} + func (p *testProc) Name() (string, error) { return "test_proc", nil } @@ -179,6 +188,7 @@ func TestGather_CreateProcessErrorOk(t *testing.T) { p := Procstat{ Exe: exe, + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: func(PID) (Process, error) { return nil, fmt.Errorf("createProcess error") @@ -205,6 +215,7 @@ func TestGather_ProcessName(t *testing.T) { p := Procstat{ Exe: exe, ProcessName: "custom_name", + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: newTestProc, } @@ -219,6 +230,7 @@ func TestGather_NoProcessNameUsesReal(t *testing.T) { p := Procstat{ Exe: exe, + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: newTestProc, } @@ -232,6 +244,7 @@ func TestGather_NoPidTag(t *testing.T) { p := Procstat{ Exe: exe, + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: newTestProc, } @@ -246,6 +259,7 @@ func TestGather_PidTag(t *testing.T) { p := Procstat{ Exe: exe, PidTag: true, + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: newTestProc, } @@ -260,6 +274,7 @@ func TestGather_Prefix(t *testing.T) { p := Procstat{ Exe: exe, Prefix: "custom_prefix", + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: newTestProc, } @@ -272,6 +287,7 @@ func TestGather_Exe(t *testing.T) { p := Procstat{ Exe: exe, + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: newTestProc, } @@ -286,6 +302,7 @@ func TestGather_User(t *testing.T) { p := Procstat{ User: user, + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: newTestProc, } @@ -300,6 +317,7 @@ func TestGather_Pattern(t *testing.T) { p := Procstat{ Pattern: pattern, + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: newTestProc, } @@ -324,6 +342,7 @@ func TestGather_PidFile(t *testing.T) { p := Procstat{ PidFile: pidfile, + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: newTestProc, } @@ -339,6 +358,7 @@ func TestGather_PercentFirstPass(t *testing.T) { p := Procstat{ Pattern: "foo", PidTag: true, + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: NewProc, } @@ -355,6 +375,7 @@ func TestGather_PercentSecondPass(t *testing.T) { p := Procstat{ Pattern: "foo", PidTag: true, + Properties: []string{"mmap"}, createPIDFinder: pidFinder([]PID{pid}), createProcess: NewProc, } @@ -434,3 +455,43 @@ func TestGather_SameTimestamps(t *testing.T) { require.Equal(t, procstat.Time, procstatLookup.Time) } + +func TestGather_MemorySwap(t *testing.T) { + var acc testutil.Accumulator + pid := PID(os.Getpid()) + + p := Procstat{ + Exe: exe, + Properties: []string{"mmap"}, + createPIDFinder: pidFinder([]PID{pid}), + createProcess: newTestProc, + } + + require.NoError(t, acc.GatherError(p.Gather)) + + require.True(t, acc.HasIntField("procstat", "memory_swap")) + + fields := acc.Metrics[0].Fields + require.Equal(t, int64(1024), fields["memory_swap"]) +} + +func TestGather_NoMemorySwap(t *testing.T) { + var acc testutil.Accumulator + + p := Procstat{ + Exe: exe, + createPIDFinder: pidFinder([]PID{pid}), + createProcess: func(PID) (Process, error) { + proc := &testProc{ + pid: pid, + tags: map[string]string{ + "exe": exe, + }, + } + return proc, nil + }, + } + + require.NoError(t, acc.GatherError(p.Gather)) + require.False(t, acc.HasField("procstat", "memory_swap")) +}