From 792e231a31065c46c57db7c9191135de88782cca Mon Sep 17 00:00:00 2001 From: Tiago Queiroz Date: Thu, 16 Nov 2023 12:54:20 +0100 Subject: [PATCH] Revert "reduce resource usage (#103)" This reverts commit 7e588f553c7233b115db4137d9598a4a9013d246. --- metric/system/cgroup/reader.go | 19 +------------ metric/system/cgroup/util.go | 27 +------------------ metric/system/process/process_darwin.go | 4 +-- metric/system/process/process_linux_common.go | 4 +-- metric/system/process/process_windows.go | 5 ++-- 5 files changed, 8 insertions(+), 51 deletions(-) diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go index f9bb6bb86c..1debc009b9 100644 --- a/metric/system/cgroup/reader.go +++ b/metric/system/cgroup/reader.go @@ -23,8 +23,6 @@ import ( "path/filepath" "strconv" "strings" - "sync" - "time" "github.com/elastic/elastic-agent-libs/logp" "github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/cgv1" @@ -70,7 +68,7 @@ const ( memoryStat = "memory" ) -// nolint: deadcode,structcheck,unused // needed by other platforms +//nolint: deadcode,structcheck,unused // needed by other platforms type mount struct { subsystem string // Subsystem name (e.g. cpuacct). mountpoint string // Mountpoint of the subsystem (e.g. /cgroup/cpuacct). @@ -79,17 +77,6 @@ type mount struct { fullPath string // Absolute path to the cgroup. It's the mountpoint joined with the path. } -// pathListWithTime combines PathList with a timestamp. -type pathListWithTime struct { - added time.Time - pathList PathList -} - -type pathCache struct { - sync.RWMutex - cache map[string]pathListWithTime -} - // Reader reads cgroup metrics and limits. type Reader struct { // Mountpoint of the root filesystem. Defaults to / if not set. This can be @@ -98,9 +85,6 @@ type Reader struct { ignoreRootCgroups bool // Ignore a cgroup when its path is "/". cgroupsHierarchyOverride string cgroupMountpoints Mountpoints // Mountpoints for each subsystem (e.g. cpu, cpuacct, memory, blkio). - - // Cache to map known v2 cgroup controllerPaths to pathListWithTime. - v2ControllerPathCache pathCache } // ReaderOptions holds options for NewReaderOptions. @@ -151,7 +135,6 @@ func NewReaderOptions(opts ReaderOptions) (*Reader, error) { ignoreRootCgroups: opts.IgnoreRootCgroups, cgroupsHierarchyOverride: opts.CgroupsHierarchyOverride, cgroupMountpoints: mountpoints, - v2ControllerPathCache: pathCache{cache: make(map[string]pathListWithTime)}, }, nil } diff --git a/metric/system/cgroup/util.go b/metric/system/cgroup/util.go index 95942b7a83..bf43828a2d 100644 --- a/metric/system/cgroup/util.go +++ b/metric/system/cgroup/util.go @@ -25,7 +25,6 @@ import ( "path/filepath" "strconv" "strings" - "time" "github.com/elastic/elastic-agent-libs/logp" "github.com/elastic/elastic-agent-system-metrics/metric/system/resolve" @@ -68,7 +67,7 @@ type PathList struct { // Flatten combines the V1 and V2 cgroups in cases where we don't need a map with keys func (pl PathList) Flatten() []ControllerPath { - list := make([]ControllerPath, 0, len(pl.V1)+len(pl.V2)) + list := []ControllerPath{} for _, v1 := range pl.V1 { list = append(list, v1) } @@ -256,7 +255,6 @@ func (r Reader) ProcessCgroupPaths(pid int) (PathList, error) { if r.cgroupsHierarchyOverride != "" { path = r.cgroupsHierarchyOverride } - // cgroup V2 // cgroup v2 controllers will always start with this string if strings.Contains(line, "0::/") { @@ -277,23 +275,6 @@ the container as /sys/fs/cgroup/unified and start the system module with the hos controllerPath = r.rootfsMountpoint.ResolveHostFS(filepath.Join("/sys/fs/cgroup/unified", path)) } - // Check if there is an entry for controllerPath already cached. - r.v2ControllerPathCache.Lock() - cacheEntry, ok := r.v2ControllerPathCache.cache[controllerPath] - if ok { - // If the cached entry for controllerPath is not older than 5 minutes, - // return the cached entry. - if time.Since(cacheEntry.added) < 5*time.Minute { - cPaths.V2 = cacheEntry.pathList.V2 - r.v2ControllerPathCache.Unlock() - continue - } - } - // Consider the existing entry for controllerPath invalid, as it is - // older than 5 minutes. - delete(r.v2ControllerPathCache.cache, controllerPath) - r.v2ControllerPathCache.Unlock() - cgpaths, err := os.ReadDir(controllerPath) if err != nil { return cPaths, fmt.Errorf("error fetching cgroupV2 controllers for cgroup location '%s' and path line '%s': %w", r.cgroupMountpoints.V2Loc, line, err) @@ -306,12 +287,6 @@ the container as /sys/fs/cgroup/unified and start the system module with the hos cPaths.V2[controllerName] = ControllerPath{ControllerPath: path, FullPath: controllerPath, IsV2: true} } } - r.v2ControllerPathCache.Lock() - r.v2ControllerPathCache.cache[controllerPath] = pathListWithTime{ - added: time.Now(), - pathList: cPaths, - } - r.v2ControllerPathCache.Unlock() // cgroup v1 } else { subsystems := strings.Split(fields[1], ",") diff --git a/metric/system/process/process_darwin.go b/metric/system/process/process_darwin.go index a68b854b89..950d9734b6 100644 --- a/metric/system/process/process_darwin.go +++ b/metric/system/process/process_darwin.go @@ -64,8 +64,8 @@ func (procStats *Stats) FetchPids() (ProcsMap, []ProcState, error) { bbuf := bytes.NewBuffer(buf) - procMap := make(ProcsMap, len(names)) - plist := make([]ProcState, 0, len(names)) + procMap := make(ProcsMap, 0) + var plist []ProcState for i := 0; i < num; i++ { if err := binary.Read(bbuf, binary.LittleEndian, &pid); err != nil { diff --git a/metric/system/process/process_linux_common.go b/metric/system/process/process_linux_common.go index 026cf45699..38427a85c8 100644 --- a/metric/system/process/process_linux_common.go +++ b/metric/system/process/process_linux_common.go @@ -58,8 +58,8 @@ func (procStats *Stats) FetchPids() (ProcsMap, []ProcState, error) { return nil, nil, fmt.Errorf("error reading directory names: %w", err) } - procMap := make(ProcsMap, len(names)) - plist := make([]ProcState, 0, len(names)) + procMap := make(ProcsMap) + var plist []ProcState // Iterate over the directory, fetch just enough info so we can filter based on user input. logger := logp.L() diff --git a/metric/system/process/process_windows.go b/metric/system/process/process_windows.go index 30a677fcb1..61882d6ece 100644 --- a/metric/system/process/process_windows.go +++ b/metric/system/process/process_windows.go @@ -38,9 +38,8 @@ func (procStats *Stats) FetchPids() (ProcsMap, []ProcState, error) { return nil, nil, fmt.Errorf("EnumProcesses failed: %w", err) } - procMap := make(ProcsMap, len(names)) - plist := make([]ProcState, 0, len(names)) - + procMap := make(ProcsMap, 0) + var plist []ProcState // This is probably the only implementation that doesn't benefit from our // little fillPid callback system. We'll need to iterate over everything // manually.