From 3bc3ccca658b15c6012f7d5d7858a81229edd2c2 Mon Sep 17 00:00:00 2001 From: kruskall <99559985+kruskall@users.noreply.github.com> Date: Thu, 26 Oct 2023 18:32:55 +0200 Subject: [PATCH] refactor: replace deprecated ioutil calls (#101) This PR replace deprecated ioutil calls, deprecated in go 1.16: https://go.dev/doc/go1.16#ioutil Main reason for this is to use the more efficient replacement for `ReadDir`: https://pkg.go.dev/io/ioutil#ReadDir While benchmarking APM Server locally (remote es + local apm-server) I noticed ~32% of cpu time was spent retrieving the metrics from the debugs var handler. The methods are from this library and I think there are a few quick/easy performance improvements. --- metric/memory/memory_helpers.go | 4 ++-- metric/system/cgroup/cgcommon/util.go | 3 +-- metric/system/cgroup/cgv1/cpuacct.go | 3 +-- metric/system/cgroup/cgv2/memory.go | 5 ++--- metric/system/cgroup/reader.go | 6 +++--- metric/system/cgroup/util.go | 3 +-- metric/system/filesystem/filesystem_test.go | 5 +---- metric/system/numcpu/cpu_linux.go | 3 +-- metric/system/process/process_linux_common.go | 17 ++++++++--------- 9 files changed, 20 insertions(+), 29 deletions(-) diff --git a/metric/memory/memory_helpers.go b/metric/memory/memory_helpers.go index e273de481..a2cf1e54b 100644 --- a/metric/memory/memory_helpers.go +++ b/metric/memory/memory_helpers.go @@ -22,7 +22,7 @@ import ( "bytes" "fmt" "io" - "io/ioutil" + "os" "strconv" "strings" @@ -58,7 +58,7 @@ func ParseMeminfo(rootfs resolve.Resolver) (map[string]uint64, error) { } func readFile(file string, handler func(string) bool) error { - contents, err := ioutil.ReadFile(file) + contents, err := os.ReadFile(file) if err != nil { return fmt.Errorf("error reading file %s: %w", file, err) } diff --git a/metric/system/cgroup/cgcommon/util.go b/metric/system/cgroup/cgcommon/util.go index 3ad1f87ed..a33159222 100644 --- a/metric/system/cgroup/cgcommon/util.go +++ b/metric/system/cgroup/cgcommon/util.go @@ -21,7 +21,6 @@ import ( "bytes" "errors" "fmt" - "io/ioutil" "os" "path/filepath" "strconv" @@ -36,7 +35,7 @@ var ( // ParseUintFromFile reads a single uint value from a file. func ParseUintFromFile(path ...string) (uint64, error) { - value, err := ioutil.ReadFile(filepath.Join(path...)) + value, err := os.ReadFile(filepath.Join(path...)) if err != nil { // Not all features are implemented/enabled by each OS. if os.IsNotExist(err) { diff --git a/metric/system/cgroup/cgv1/cpuacct.go b/metric/system/cgroup/cgv1/cpuacct.go index f3e49139b..0d742ec79 100644 --- a/metric/system/cgroup/cgv1/cpuacct.go +++ b/metric/system/cgroup/cgv1/cpuacct.go @@ -21,7 +21,6 @@ import ( "bufio" "bytes" "fmt" - "io/ioutil" "os" "path/filepath" "time" @@ -106,7 +105,7 @@ func cpuacctUsage(path string, cpuacct *CPUAccountingSubsystem) error { } func cpuacctUsagePerCPU(path string, cpuacct *CPUAccountingSubsystem) error { - contents, err := ioutil.ReadFile(filepath.Join(path, "cpuacct.usage_percpu")) + contents, err := os.ReadFile(filepath.Join(path, "cpuacct.usage_percpu")) if err != nil { if os.IsNotExist(err) { return nil diff --git a/metric/system/cgroup/cgv2/memory.go b/metric/system/cgroup/cgv2/memory.go index dcd4faf4d..4b3d32745 100644 --- a/metric/system/cgroup/cgv2/memory.go +++ b/metric/system/cgroup/cgv2/memory.go @@ -22,7 +22,6 @@ import ( "bytes" "errors" "fmt" - "io/ioutil" "os" "path/filepath" "reflect" @@ -249,7 +248,7 @@ func fetchEventsFile(path, file string) (Events, error) { // Some values, such as mem.max and mem.high, can be set to "max," which disables the metric. func maxOrValue(path, file string) (opt.Uint, error) { var finalMetric opt.Uint - highRaw, err := ioutil.ReadFile(filepath.Join(path, file)) + highRaw, err := os.ReadFile(filepath.Join(path, file)) if err != nil { return finalMetric, fmt.Errorf("error reading %s.high file: %w", path, err) } @@ -272,7 +271,7 @@ func maxOrValue(path, file string) (opt.Uint, error) { // Note that this assumes all the values in the struct are either `uint64`, `opt.Bytes` or `opt.BytesOpt` func fillStatStruct(path string) (MemoryStat, error) { statPath := filepath.Join(path, "memory.stat") - raw, err := ioutil.ReadFile(statPath) + raw, err := os.ReadFile(statPath) if err != nil { return MemoryStat{}, fmt.Errorf("error reading memory.stat: %w", err) } diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go index f9aae99fe..1debc009b 100644 --- a/metric/system/cgroup/reader.go +++ b/metric/system/cgroup/reader.go @@ -19,7 +19,7 @@ package cgroup import ( "fmt" - "io/ioutil" + "os" "path/filepath" "strconv" "strings" @@ -142,7 +142,7 @@ func NewReaderOptions(opts ReaderOptions) (*Reader, error) { func (r *Reader) CgroupsVersion(pid int) (CgroupsVersion, error) { cgPath := filepath.Join("/proc/", strconv.Itoa(pid), "cgroup") cgPath = r.rootfsMountpoint.ResolveHostFS(cgPath) - cgraw, err := ioutil.ReadFile(cgPath) + cgraw, err := os.ReadFile(cgPath) if err != nil { return CgroupsV1, fmt.Errorf("error reading %s: %w", cgPath, err) } @@ -358,7 +358,7 @@ func readControllerList(cgroupsFile string, v2path string) ([]string, error) { return []string{}, nil } file := filepath.Join(v2path, cgpath, "cgroup.controllers") - controllersRaw, err := ioutil.ReadFile(file) + controllersRaw, err := os.ReadFile(file) if err != nil { return nil, fmt.Errorf("error reading %s: %w", file, err) } diff --git a/metric/system/cgroup/util.go b/metric/system/cgroup/util.go index e7167a022..bf43828a2 100644 --- a/metric/system/cgroup/util.go +++ b/metric/system/cgroup/util.go @@ -21,7 +21,6 @@ import ( "bufio" "errors" "fmt" - "io/ioutil" "os" "path/filepath" "strconv" @@ -276,7 +275,7 @@ 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)) } - cgpaths, err := ioutil.ReadDir(controllerPath) + 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) } diff --git a/metric/system/filesystem/filesystem_test.go b/metric/system/filesystem/filesystem_test.go index 4f62d6826..3e5e1b8b1 100644 --- a/metric/system/filesystem/filesystem_test.go +++ b/metric/system/filesystem/filesystem_test.go @@ -18,7 +18,6 @@ package filesystem import ( - "io/ioutil" "os" "runtime" "testing" @@ -56,9 +55,7 @@ func TestFileSystemListFiltering(t *testing.T) { t.Skip("These cases don't need to work on Windows") } _ = logp.DevelopmentSetup() - fakeDevDir, err := ioutil.TempDir(os.TempDir(), "dir") - assert.Empty(t, err) - defer os.RemoveAll(fakeDevDir) + fakeDevDir := t.TempDir() cases := []struct { description string diff --git a/metric/system/numcpu/cpu_linux.go b/metric/system/numcpu/cpu_linux.go index 44f9c2b64..daba0acab 100644 --- a/metric/system/numcpu/cpu_linux.go +++ b/metric/system/numcpu/cpu_linux.go @@ -20,7 +20,6 @@ package numcpu import ( "errors" "fmt" - "io/ioutil" "os" "strings" ) @@ -42,7 +41,7 @@ func getCPU() (int, bool, error) { cpuPath = "/sys/devices/system/cpu/present" } - rawFile, err := ioutil.ReadFile(cpuPath) + rawFile, err := os.ReadFile(cpuPath) // if the file doesn't exist, assume it's a support issue and not a bug if errors.Is(err, os.ErrNotExist) { return -1, false, nil diff --git a/metric/system/process/process_linux_common.go b/metric/system/process/process_linux_common.go index bc47adb49..86e9a7db5 100644 --- a/metric/system/process/process_linux_common.go +++ b/metric/system/process/process_linux_common.go @@ -24,7 +24,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "os" "os/user" "strconv" @@ -246,7 +245,7 @@ func getUser(hostfs resolve.Resolver, pid int) (string, error) { func getEnvData(hostfs resolve.Resolver, pid int, filter func(string) bool) (mapstr.M, error) { path := hostfs.Join("proc", strconv.Itoa(pid), "environ") - data, err := ioutil.ReadFile(path) + data, err := os.ReadFile(path) if errors.Is(err, os.ErrPermission) { // pass through permission errors return nil, err } else if err != nil { @@ -277,7 +276,7 @@ func getMemData(hostfs resolve.Resolver, pid int) (ProcMemInfo, error) { // Memory data state := ProcMemInfo{} path := hostfs.Join("proc", strconv.Itoa(pid), "statm") - data, err := ioutil.ReadFile(path) + data, err := os.ReadFile(path) if err != nil { return state, fmt.Errorf("error opening file %s: %w", path, err) } @@ -306,7 +305,7 @@ func getCPUTime(hostfs resolve.Resolver, pid int) (ProcCPUInfo, error) { state := ProcCPUInfo{} pathCPU := hostfs.Join("proc", strconv.Itoa(pid), "stat") - data, err := ioutil.ReadFile(pathCPU) + data, err := os.ReadFile(pathCPU) if err != nil { return state, fmt.Errorf("error opening file %s: %w", pathCPU, err) } @@ -347,7 +346,7 @@ func getCPUTime(hostfs resolve.Resolver, pid int) (ProcCPUInfo, error) { func getArgs(hostfs resolve.Resolver, pid int) ([]string, error) { path := hostfs.Join("proc", strconv.Itoa(pid), "cmdline") - data, err := ioutil.ReadFile(path) + data, err := os.ReadFile(path) if err != nil { return nil, fmt.Errorf("error opening file %s: %w", path, err) } @@ -371,7 +370,7 @@ func getFDStats(hostfs resolve.Resolver, pid int) (ProcFDInfo, error) { state := ProcFDInfo{} path := hostfs.Join("proc", strconv.Itoa(pid), "limits") - data, err := ioutil.ReadFile(path) + data, err := os.ReadFile(path) if err != nil { return state, fmt.Errorf("error opening file %s: %w", path, err) } @@ -398,7 +397,7 @@ func getFDStats(hostfs resolve.Resolver, pid int) (ProcFDInfo, error) { } pathFD := hostfs.Join("proc", strconv.Itoa(pid), "fd") - fds, err := ioutil.ReadDir(pathFD) + fds, err := os.ReadDir(pathFD) if errors.Is(err, os.ErrPermission) { // ignore permission errors, passthrough other data return state, nil } else if err != nil { @@ -416,7 +415,7 @@ func getLinuxBootTime(hostfs resolve.Resolver) (uint64, error) { path := hostfs.Join("proc", "stat") // grab system boot time - data, err := ioutil.ReadFile(path) + data, err := os.ReadFile(path) if err != nil { return 0, fmt.Errorf("error opening file %s: %w", path, err) } @@ -440,7 +439,7 @@ func getLinuxBootTime(hostfs resolve.Resolver) (uint64, error) { func getProcStatus(hostfs resolve.Resolver, pid int) (map[string]string, error) { status := make(map[string]string, 42) path := hostfs.Join("proc", strconv.Itoa(pid), "status") - data, err := ioutil.ReadFile(path) + data, err := os.ReadFile(path) if err != nil { return nil, fmt.Errorf("error opening file %s: %w", path, err) }