From 06b56a9ea9cb5a304e6142c0e6220299949164aa Mon Sep 17 00:00:00 2001 From: kruskall <99559985+kruskall@users.noreply.github.com> Date: Fri, 15 Aug 2025 03:58:38 +0200 Subject: [PATCH 1/2] feat: deprecate NewReader and migrate to NewReaderOptions this is a nicer alternative to performing breaking change on NewReader --- metric/system/cgroup/reader.go | 11 ++++++++--- metric/system/cgroup/reader_test.go | 24 ++++++++++++++++++++---- metric/system/cgroup/util_test.go | 21 +++++++++++++++++---- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go index 573b780c9..2684e98b8 100644 --- a/metric/system/cgroup/reader.go +++ b/metric/system/cgroup/reader.go @@ -119,11 +119,13 @@ type ReaderOptions struct { } // NewReader creates and returns a new Reader. -func NewReader(rootfsMountpoint resolve.Resolver, ignoreRootCgroups bool, logger *logp.Logger) (*Reader, error) { +// +// Deprecated: use NewReaderOptions +func NewReader(rootfsMountpoint resolve.Resolver, ignoreRootCgroups bool) (*Reader, error) { return NewReaderOptions(ReaderOptions{ RootfsMountpoint: rootfsMountpoint, IgnoreRootCgroups: ignoreRootCgroups, - Logger: logger, + Logger: logp.NewNopLogger(), }) } @@ -258,7 +260,10 @@ func (r *Reader) GetV2StatsForProcess(pid int) (*StatsV2, error) { //nolint: dup // ProcessCgroupPaths is a wrapper around Reader.ProcessCgroupPaths for libraries that only need the slimmer functionality from // the gosigar cgroups code. This does not have the same function signature, and consumers still need to distinguish between v1 and v2 cgroups. func ProcessCgroupPaths(hostfs resolve.Resolver, pid int, logger *logp.Logger) (PathList, error) { - reader, err := NewReader(hostfs, false, logger) + reader, err := NewReaderOptions(ReaderOptions{ + RootfsMountpoint: hostfs, + Logger: logger, + }) if err != nil { return PathList{}, fmt.Errorf("error creating cgroups reader: %w", err) } diff --git a/metric/system/cgroup/reader_test.go b/metric/system/cgroup/reader_test.go index 366f94be9..bcbf4fcec 100644 --- a/metric/system/cgroup/reader_test.go +++ b/metric/system/cgroup/reader_test.go @@ -53,7 +53,11 @@ func TestReaderOptsWithoutResolve(t *testing.T) { func TestV1EventDifferentPaths(t *testing.T) { pid := 3757 - reader, err := NewReader(resolve.NewTestResolver("testdata/ubuntu1804"), true, logptest.NewTestingLogger(t, "")) + reader, err := NewReaderOptions(ReaderOptions{ + RootfsMountpoint: resolve.NewTestResolver("testdata/ubuntu1804"), + IgnoreRootCgroups: true, + Logger: logptest.NewTestingLogger(t, ""), + }) require.NoError(t, err, "error in NewReader") stats, err := reader.GetV1StatsForProcess(pid) @@ -67,7 +71,11 @@ func TestV1EventDifferentPaths(t *testing.T) { } func TestReaderGetStatsV1(t *testing.T) { - reader, err := NewReader(resolve.NewTestResolver("testdata/docker"), true, logptest.NewTestingLogger(t, "")) + reader, err := NewReaderOptions(ReaderOptions{ + RootfsMountpoint: resolve.NewTestResolver("testdata/docker"), + IgnoreRootCgroups: true, + Logger: logptest.NewTestingLogger(t, ""), + }) require.NoError(t, err, "error in NewReader") stats, err := reader.GetV1StatsForProcess(985) @@ -97,7 +105,11 @@ func TestReaderGetStatsV1(t *testing.T) { // testcase for the situation where both cgroup v1 and v2 controllers exist but // /sys/fs/cgroup/unified is not mounted func TestReaderGetStatsV1MalformedHybrid(t *testing.T) { - reader, err := NewReader(resolve.NewTestResolver("testdata/amzn2"), true, logptest.NewTestingLogger(t, "")) + reader, err := NewReaderOptions(ReaderOptions{ + RootfsMountpoint: resolve.NewTestResolver("testdata/amzn2"), + IgnoreRootCgroups: true, + Logger: logptest.NewTestingLogger(t, ""), + }) require.NoError(t, err, "error in NewReader") stats, err := reader.GetV1StatsForProcess(493239) @@ -124,7 +136,11 @@ func TestReaderGetStatsV1MalformedHybrid(t *testing.T) { } func TestReaderGetStatsV2(t *testing.T) { - reader, err := NewReader(resolve.NewTestResolver("testdata/docker"), true, logptest.NewTestingLogger(t, "")) + reader, err := NewReaderOptions(ReaderOptions{ + RootfsMountpoint: resolve.NewTestResolver("testdata/docker"), + IgnoreRootCgroups: true, + Logger: logptest.NewTestingLogger(t, ""), + }) require.NoError(t, err, "error in NewReader") stats, err := reader.GetV2StatsForProcess(312) diff --git a/metric/system/cgroup/util_test.go b/metric/system/cgroup/util_test.go index ee4f5b021..8bddd88c1 100644 --- a/metric/system/cgroup/util_test.go +++ b/metric/system/cgroup/util_test.go @@ -23,6 +23,7 @@ package cgroup import ( "errors" "fmt" + "io/ioutil" "os" "testing" @@ -157,7 +158,10 @@ func TestSubsystemMountpoints(t *testing.T) { } func TestProcessCgroupPaths(t *testing.T) { - reader, err := NewReader(resolve.NewTestResolver("testdata/docker"), false, logptest.NewTestingLogger(t, "")) + reader, err := NewReaderOptions(ReaderOptions{ + RootfsMountpoint: resolve.NewTestResolver("testdata/docker"), + Logger: logptest.NewTestingLogger(t, ""), + }) if err != nil { t.Fatalf("error in NewReader: %s", err) } @@ -181,7 +185,10 @@ func TestProcessCgroupPaths(t *testing.T) { } func TestProcessCgroupHybridPaths(t *testing.T) { - reader, err := NewReader(resolve.NewTestResolver("testdata/amzn2"), false, logptest.NewTestingLogger(t, "")) + reader, err := NewReaderOptions(ReaderOptions{ + RootfsMountpoint: resolve.NewTestResolver("testdata/amzn2"), + Logger: logptest.NewTestingLogger(t, ""), + }) if err != nil { t.Fatalf("error in NewReader: %s", err) } @@ -206,7 +213,10 @@ func TestProcessCgroupHybridPaths(t *testing.T) { } func TestProcessCgroupPathsV2(t *testing.T) { - reader, err := NewReader(resolve.NewTestResolver("testdata/docker"), false, logptest.NewTestingLogger(t, "")) + reader, err := NewReaderOptions(ReaderOptions{ + RootfsMountpoint: resolve.NewTestResolver("testdata/docker"), + Logger: logptest.NewTestingLogger(t, ""), + }) if err != nil { t.Fatalf("error in NewReader: %s", err) } @@ -233,7 +243,10 @@ func TestMountpointsV2(t *testing.T) { []byte(pidFmt), 0o744) require.NoError(t, err) - reader, err := NewReader(resolve.NewTestResolver("testdata/docker2"), false, logptest.NewTestingLogger(t, "")) + reader, err := NewReaderOptions(ReaderOptions{ + RootfsMountpoint: resolve.NewTestResolver("testdata/docker2"), + Logger: logptest.NewTestingLogger(t, ""), + }) require.NoError(t, err) stats, err := reader.GetStatsForPid(2233801) From b755577a79ebfd539815e16c165eec1012d68066 Mon Sep 17 00:00:00 2001 From: kruskall <99559985+kruskall@users.noreply.github.com> Date: Fri, 15 Aug 2025 04:05:42 +0200 Subject: [PATCH 2/2] Update util_test.go --- metric/system/cgroup/util_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/metric/system/cgroup/util_test.go b/metric/system/cgroup/util_test.go index 8bddd88c1..4ba6a8e1e 100644 --- a/metric/system/cgroup/util_test.go +++ b/metric/system/cgroup/util_test.go @@ -23,7 +23,6 @@ package cgroup import ( "errors" "fmt" - "io/ioutil" "os" "testing"