diff --git a/dev-tools/systemtests/testResolve.go b/dev-tools/systemtests/testResolve.go index ce25c9769..22db15eb4 100644 --- a/dev-tools/systemtests/testResolve.go +++ b/dev-tools/systemtests/testResolve.go @@ -20,7 +20,6 @@ package systemtests import ( "os" - "github.com/elastic/elastic-agent-libs/logp" "github.com/elastic/elastic-agent-system-metrics/metric/system/resolve" ) @@ -28,7 +27,6 @@ import ( // The logic here is extremely simple: if USE_HOSTFS is set, return that for the resolver func DockerTestResolver() resolve.Resolver { if path, set := os.LookupEnv("HOSTFS"); set { - logp.L().Infof("Using /hostfs for container tests") return resolve.NewTestResolver(path) } return resolve.NewTestResolver("/") diff --git a/metric/system/cgroup/bytesutil/bytesutil.go b/metric/system/cgroup/bytesutil/bytesutil.go new file mode 100644 index 000000000..7305105ca --- /dev/null +++ b/metric/system/cgroup/bytesutil/bytesutil.go @@ -0,0 +1,70 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package bytesutil + +import ( + "iter" +) + +var asciiSpace = [256]bool{'\t': true, '\n': true, '\v': true, '\f': true, '\r': true, ' ': true} + +// Fields returns an iterator that yields the fields of byte array b split around each single +// ASCII white space character (space, tab, newline, vertical tab, form feed, carriage return). +// It can be used with range loops like this: +// +// for i, field := range stringutil.Fields(b) { +// fmt.Printf("Field %d: %v\n", i, field) +// } +func Fields(b []byte) iter.Seq2[int, []byte] { + return func(yield func(int, []byte) bool) { + for i, bi := 0, 0; bi < len(b); i++ { + fieldStart := bi + // Find the end of the field + for bi < len(b) && !asciiSpace[b[bi]] { + bi++ + } + if !yield(i, b[fieldStart:bi]) { + return + } + bi++ + } + } +} + +// Split returns an iterator that yields the fields of byte array b split around +// the given character. +// It can be used with range loops like this: +// +// for i, field := range stringutil.Split(b, ',') { +// fmt.Printf("Field %d: %v\n", i, field) +// } +func Split(b []byte, sep byte) iter.Seq2[int, []byte] { + return func(yield func(int, []byte) bool) { + for i, bi := 0, 0; bi < len(b); i++ { + fieldStart := bi + // Find the end of the field + for bi < len(b) && b[bi] != sep { + bi++ + } + if !yield(i, b[fieldStart:bi]) { + return + } + bi++ + } + } +} diff --git a/metric/system/cgroup/bytesutil/bytesutil_test.go b/metric/system/cgroup/bytesutil/bytesutil_test.go new file mode 100644 index 000000000..25e13d8fc --- /dev/null +++ b/metric/system/cgroup/bytesutil/bytesutil_test.go @@ -0,0 +1,96 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package bytesutil + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestFields(t *testing.T) { + tests := []struct { + name string + input []byte + want []string + }{ + { + name: "empty array", + input: []byte(""), + want: []string{}, + }, + { + name: "single white space", + input: []byte(" "), + want: []string{"", ""}, + }, + { + name: "single word", + input: []byte("hello"), + want: []string{"hello"}, + }, + { + name: "multiple words", + input: []byte("hello world this is a test"), + want: []string{"hello", "world", "this", "is", "a", "test"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for i, f := range Fields(tt.input) { + require.Equal(t, tt.want[i], string(f), "Fields() mismatch at index %d", i) + } + }) + } +} + +func TestSplit(t *testing.T) { + tests := []struct { + name string + input []byte + want []string + }{ + { + name: "empty array", + input: []byte(""), + want: []string{}, + }, + { + name: "single separator", + input: []byte(","), + want: []string{"", ""}, + }, + { + name: "single word", + input: []byte("hello"), + want: []string{"hello"}, + }, + { + name: "multiple words", + input: []byte("hello,world,this,is,a,test"), + want: []string{"hello", "world", "this", "is", "a", "test"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for i, f := range Split(tt.input, ',') { + require.Equal(t, tt.want[i], string(f), "Split() mismatch at index %d", i) + } + }) + } +} diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go index 6bacae319..2af6f3e7b 100644 --- a/metric/system/cgroup/reader.go +++ b/metric/system/cgroup/reader.go @@ -27,6 +27,7 @@ import ( "time" "github.com/elastic/elastic-agent-libs/logp" + "github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/cgv1" "github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/cgv2" "github.com/elastic/elastic-agent-system-metrics/metric/system/resolve" @@ -370,6 +371,7 @@ func (r *Reader) readControllerList(cgroupsFile string) ([]string, error) { if cgpath == "" { return []string{}, nil } + cgpath = filepath.Clean(cgpath) // The path may have a relative prefix like "/../..`, which effectively is "/". cgFilePath := filepath.Join(r.cgroupMountpoints.V2Loc, cgpath, "cgroup.controllers") if cgroupNSStateFetch() && r.rootfsMountpoint.IsSet() { cgFilePath = filepath.Join(r.cgroupMountpoints.V2Loc, r.cgroupMountpoints.ContainerizedRootMount, cgpath, "cgroup.controllers") diff --git a/metric/system/cgroup/util.go b/metric/system/cgroup/util.go index 0e31fd6ad..d0407b860 100644 --- a/metric/system/cgroup/util.go +++ b/metric/system/cgroup/util.go @@ -19,6 +19,7 @@ package cgroup import ( "bufio" + "bytes" "errors" "fmt" "io/fs" @@ -30,6 +31,8 @@ import ( "time" "github.com/elastic/elastic-agent-libs/logp" + + "github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/bytesutil" "github.com/elastic/elastic-agent-system-metrics/metric/system/resolve" ) @@ -71,9 +74,9 @@ var ( // mountinfo represents a subset of the fields containing /proc/[pid]/mountinfo. type mountinfo struct { - mountpoint string - filesystemType string - superOptions []string + mountpoint []byte + filesystemType []byte + superOptions []byte } // Mountpoints organizes info about V1 and V2 cgroup mountpoints @@ -117,38 +120,44 @@ func (pl PathList) Flatten() []ControllerPath { // parseMountinfoLine parses a line from the /proc/[pid]/mountinfo file on // Linux. The format of the line is specified in section 3.5 of // https://www.kernel.org/doc/Documentation/filesystems/proc.txt. -func parseMountinfoLine(line string) (mountinfo, error) { +func parseMountinfoLine(line []byte) (mountinfo, error) { mount := mountinfo{} - fields := strings.Fields(line) - if len(fields) < 10 { - return mount, fmt.Errorf("invalid mountinfo line, expected at least "+ - "10 fields but got %d from line='%s'", len(fields), line) + var fields [10 + 6][]byte // support up to 6 optional fields + nFields := 0 + for i, field := range bytesutil.Fields(line) { + fields[i] = field + nFields = i + 1 + if nFields >= len(fields) { + break // avoid out of bounds + } } - mount.mountpoint = fields[4] + if nFields < 10 { + return mount, fmt.Errorf("invalid mountinfo line, expected at least "+ + "10 fields but got %d from line='%s'", nFields, line) + } var separatorIndex int - for i, value := range fields { - if value == "-" { + for i := 6; i < nFields; i++ { + if len(fields[i]) == 1 && fields[i][0] == '-' { separatorIndex = i break } } - if fields[separatorIndex] != "-" { + if separatorIndex == 0 { return mount, fmt.Errorf("invalid mountinfo line, separator ('-') not "+ "found in line='%s'", line) } - - if len(fields)-separatorIndex-1 < 3 { + if nFields-separatorIndex-1 < 3 { return mount, fmt.Errorf("invalid mountinfo line, expected at least "+ "3 fields after separator but got %d from line='%s'", - len(fields)-separatorIndex-1, line) + nFields-separatorIndex-1, line) } - fields = fields[separatorIndex+1:] - mount.filesystemType = fields[0] - mount.superOptions = strings.Split(fields[2], ",") + mount.mountpoint = fields[4] + mount.filesystemType = fields[separatorIndex+1] + mount.superOptions = fields[separatorIndex+3] return mount, nil } @@ -210,16 +219,22 @@ func SubsystemMountpoints(rootfs resolve.Resolver, subsystems map[string]struct{ } defer mountinfo.Close() - mounts := map[string]string{} + hostFS := rootfs.ResolveHostFS("") + mounts := make(map[string]string, len(subsystems)) // preallocate map with expected size mountInfo := Mountpoints{} - sc := bufio.NewScanner(mountinfo) possibleV2Paths := []string{} + + sc := bufio.NewScanner(mountinfo) for sc.Scan() { // https://www.kernel.org/doc/Documentation/filesystems/proc.txt // Example: // 25 21 0:20 / /cgroup/cpu rw,relatime - cgroup cgroup rw,cpu - line := strings.TrimSpace(sc.Text()) - if line == "" { + line := bytes.TrimSpace(sc.Bytes()) + if len(line) == 0 { + continue + } + + if !bytes.Contains(line, []byte(" cgroup")) { continue } @@ -229,34 +244,33 @@ func SubsystemMountpoints(rootfs resolve.Resolver, subsystems map[string]struct{ } // if the mountpoint from the subsystem has a different root than ours, it probably belongs to something else. - if !strings.HasPrefix(mount.mountpoint, rootfs.ResolveHostFS("")) { + if !bytes.HasPrefix(mount.mountpoint, []byte(hostFS)) { continue } // cgroupv1 option - if mount.filesystemType == "cgroup" { - for _, opt := range mount.superOptions { + if bytes.Equal(mount.filesystemType, []byte("cgroup")) { + for _, opt := range bytesutil.Split(mount.superOptions, ',') { // Sometimes the subsystem name is written like "name=blkio". - fields := strings.SplitN(opt, "=", 2) - if len(fields) > 1 { - opt = fields[1] + if _, v, found := bytes.Cut(opt, []byte{'='}); found { + opt = v } // Test if option is a subsystem name. - if _, found := subsystems[opt]; found { + if _, found := subsystems[string(opt)]; found { // Add the subsystem mount if it does not already exist. - if _, exists := mounts[opt]; !exists { - mounts[opt] = mount.mountpoint + if _, exists := mounts[string(opt)]; !exists { + mounts[string(opt)] = string(mount.mountpoint) } } } + continue } // V2 option - if mount.filesystemType == "cgroup2" { - possibleV2Paths = append(possibleV2Paths, mount.mountpoint) + if bytes.Equal(mount.filesystemType, []byte("cgroup2")) { + possibleV2Paths = append(possibleV2Paths, string(mount.mountpoint)) } - } mountInfo.V2Loc = getProperV2Paths(rootfs, possibleV2Paths) @@ -277,7 +291,7 @@ func SubsystemMountpoints(rootfs resolve.Resolver, subsystems map[string]struct{ return mountInfo, sc.Err() } -// isCgroupNSHost returns true if we're running inside a container with a +// isCgroupNSPrivate returns true if we're running inside a container with a // private cgroup namespace. Will return true if we're in a public namespace, or there's an error // Note that this function only makes sense *inside* a container. Outside it will probably always return false. func isCgroupNSPrivate() bool { @@ -290,9 +304,16 @@ func isCgroupNSPrivate() bool { } // if we have a path of just "/" that means we're in our own private namespace // if it's something else, we're probably in a host namespace - segments := strings.Split(strings.TrimSpace(string(raw)), ":") - return segments[len(segments)-1] == "/" + return isCgroupPathSlash(raw) +} +func isCgroupPathSlash(raw []byte) bool { + for i, field := range bytesutil.Split(bytes.TrimSpace(raw), ':') { + if i == 2 { + return bytes.Equal(field, []byte{'/'}) + } + } + return false } // tries to find the cgroup path for the currently-running container, @@ -477,7 +498,6 @@ func (r *Reader) ProcessCgroupPaths(pid int) (PathList, error) { logp.L().Debugf("cgroup for process %d contains a relative cgroup path (%s), but we were not able to find a root cgroup. Cgroup monitoring for this PID may be incomplete", pid, path) } else { - logp.L().Debugf("using root mount %s and path %s", r.cgroupMountpoints.ContainerizedRootMount, path) path = filepath.Join(r.cgroupMountpoints.ContainerizedRootMount, path) } } diff --git a/metric/system/cgroup/util_test.go b/metric/system/cgroup/util_test.go index 4ed0adc17..0f1e36869 100644 --- a/metric/system/cgroup/util_test.go +++ b/metric/system/cgroup/util_test.go @@ -21,6 +21,7 @@ package cgroup import ( + "bytes" "errors" "fmt" "os" @@ -30,6 +31,7 @@ import ( "github.com/stretchr/testify/require" "github.com/elastic/elastic-agent-libs/logp" + "github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/testhelpers" "github.com/elastic/elastic-agent-system-metrics/metric/system/resolve" ) @@ -155,6 +157,32 @@ func TestSubsystemMountpoints(t *testing.T) { assert.Equal(t, "testdata/docker/sys/fs/cgroup/perf_event", mountpoints.V1Mounts["perf_event"]) } +func BenchmarkSubsystemMountpoints(b *testing.B) { + subsystems := map[string]struct{}{ + "blkio": {}, + "cpu": {}, + "cpuacct": {}, + "cpuset": {}, + "devices": {}, + "freezer": {}, + "hugetlb": {}, + "memory": {}, + "perf_event": {}, + } + + resolver := resolve.NewTestResolver("testdata/docker") + + b.ResetTimer() + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + _, err := SubsystemMountpoints(resolver, subsystems) + if err != nil { + b.Fatalf("error in SubsystemMountpoints: %s", err) + } + } +} + func TestProcessCgroupPaths(t *testing.T) { reader, err := NewReader(resolve.NewTestResolver("testdata/docker"), false) if err != nil { @@ -232,7 +260,7 @@ func TestMountpointsV2(t *testing.T) { []byte(pidFmt), 0o744) require.NoError(t, err) - _ = logp.DevelopmentSetup() + _ = logp.DevelopmentSetup() //nolint:staticcheck // Use logp.NewDevelopmentLogger reader, err := NewReader(resolve.NewTestResolver("testdata/docker2"), false) require.NoError(t, err) @@ -262,14 +290,14 @@ func TestParseMountinfoLine(t *testing.T) { } for _, line := range lines { - mount, err := parseMountinfoLine(line) + mount, err := parseMountinfoLine([]byte(line)) if err != nil { t.Fatal(err) } - assert.Equal(t, "/sys/fs/cgroup/blkio", mount.mountpoint) - assert.Equal(t, "cgroup", mount.filesystemType) - assert.Len(t, mount.superOptions, 2) + assert.Equal(t, "/sys/fs/cgroup/blkio", string(mount.mountpoint)) + assert.Equal(t, "cgroup", string(mount.filesystemType)) + assert.Equal(t, bytes.Count(mount.superOptions, []byte{','}), 1) } } @@ -335,3 +363,8 @@ func TestFetchV2Paths(t *testing.T) { }) } } + +func TestIsCgroupPathSlash(t *testing.T) { + require.False(t, isCgroupPathSlash([]byte("0::/user.slice/user-1000.slice/session-520.scope"))) + require.True(t, isCgroupPathSlash([]byte("0::/"))) +} diff --git a/metric/system/process/process_container_test.go b/metric/system/process/process_container_test.go index 39fc8baea..70572ec84 100644 --- a/metric/system/process/process_container_test.go +++ b/metric/system/process/process_container_test.go @@ -29,6 +29,7 @@ import ( "github.com/elastic/elastic-agent-libs/logp" "github.com/elastic/elastic-agent-libs/mapstr" + "github.com/elastic/elastic-agent-system-metrics/dev-tools/systemtests" "github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup" ) @@ -38,7 +39,7 @@ import ( // However, they are designed so that `go test` can run them normally as well func TestContainerMonitoringFromInsideContainer(t *testing.T) { - _ = logp.DevelopmentSetup() + _ = logp.DevelopmentSetup() //nolint:staticcheck // Use logp.NewDevelopmentLogger testStats := Stats{CPUTicks: true, EnableCgroups: true, @@ -64,7 +65,7 @@ func TestContainerMonitoringFromInsideContainer(t *testing.T) { } func TestSelfMonitoringFromInsideContainer(t *testing.T) { - _ = logp.DevelopmentSetup() + _ = logp.DevelopmentSetup() //nolint:staticcheck // Use logp.NewDevelopmentLogger testStats := Stats{CPUTicks: true, EnableCgroups: true, @@ -89,7 +90,7 @@ func TestSelfMonitoringFromInsideContainer(t *testing.T) { } func TestSystemHostFromContainer(t *testing.T) { - _ = logp.DevelopmentSetup() + _ = logp.DevelopmentSetup() //nolint:staticcheck // Use logp.NewDevelopmentLogger testStats := Stats{CPUTicks: true, EnableCgroups: true, @@ -157,6 +158,7 @@ func validateProcResult(t *testing.T, result mapstr.M) { if runtime.GOOS == "linux" { cgroups := result["cgroup"] require.NotNil(t, cgroups) + require.True(t, false, "stopping tests on purpose to get logs") } }