feat: deprecate NewReader and migrate to NewReaderOptions (#257)

## What does this PR do?

this is a nicer alternative to performing breaking change on NewReader

## Why is it important?

to avoid breaking changes and account for older branches having to
migrate

## Checklist

<!-- Mandatory
Add a checklist of things that are required to be reviewed in order to
have the PR approved

List here all the items you have verified BEFORE sending this PR. Please
DO NOT remove any item, striking through those that do not apply. (Just
in case, strikethrough uses two tildes. ~~Scratch this.~~)
-->

- [ ] My code follows the style guidelines of this project
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added an entry in `CHANGELOG.md`

## Author's Checklist

<!-- Recommended
Add a checklist of things that are required to be reviewed in order to
have the PR approved
-->
- [ ]

## Related issues

<!-- Recommended
Link related issues below. Insert the issue link or reference after the
word "Closes" if merging this should automatically close it.

- Closes #123
- Relates #123
- Requires #123
- Superseds #123
-->
-

Author: kruskall

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)
 	}

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)

metric/system/cgroup/util_test.go

@@ -157,7 +157,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 +184,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 +212,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 +242,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)