NR-288387 feat: config option to disable container decoration (#1888)

* NR-288387 feat: config option to disable container decoration * container sampler enabled if no config * remove deprecated macos GH runner * fix test in darwin
newrelic · Jul 9, 2024 · 1673648 · 1673648
1 parent dc896c4
commit 1673648
Show file tree

Hide file tree

Showing 11 changed files with 243 additions and 13 deletions.
diff --git a/.github/workflows/component_macos_harvest_test.yml b/.github/workflows/component_macos_harvest_test.yml
@@ -9,7 +9,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        os: [ macos-11, macos-12 ]
+        os: [ macos-12, macos-13, macos-14 ]
     steps:
       - uses: actions/checkout@v2
 

diff --git a/.github/workflows/component_macos_linter.yml b/.github/workflows/component_macos_linter.yml
@@ -7,7 +7,7 @@ jobs:
 
   run-lint:
     name: Lint tests
-    runs-on: macos-11
+    runs-on: macos-latest
     continue-on-error: true
 
     steps:

diff --git a/.github/workflows/component_macos_unit_test.yml b/.github/workflows/component_macos_unit_test.yml
@@ -9,7 +9,7 @@ env:
 jobs:
   unit-test-macos:
     name: unit tests
-    runs-on: macos-11
+    runs-on: macos-latest
 
     steps:
       - uses: actions/checkout@v2

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -1272,6 +1272,11 @@ type Config struct {
 	// Default (Windows): C:\ProgramData\New Relic\newrelic-infra\tmp
 	// Public: No
 	AgentTempDir string `envconfig:"agent_temp_dir" yaml:"agent_temp_dir"`
+
+	// ProcessContainerDecoration controls if the ProcessSample gets decorated with Container Information
+	// Default: true
+	// Public: Yes
+	ProcessContainerDecoration bool `envconfig:"process_container_decoration" yaml:"process_container_decoration"`
 }
 
 // KeyValMap is used whenever a key value pair configuration is required.
@@ -1896,6 +1901,7 @@ func NewConfig() *Config {
 		NtpMetrics:                  NewNtpConfig(),
 		Http:                        NewHttpConfig(),
 		AgentTempDir:                defaultAgentTempDir,
+		ProcessContainerDecoration:  defaultProcessContainerDecoration,
 	}
 }
 

diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go
@@ -113,6 +113,7 @@ var (
 	defaultNtpEnabled                    = false
 	defaultNtpInterval                   = uint(15) // minutes
 	defaultNtpTimeout                    = uint(5)  // seconds
+	defaultProcessContainerDecoration    = true
 )
 
 // Default internal values

diff --git a/pkg/metrics/process/sampler_darwin.go b/pkg/metrics/process/sampler_darwin.go
@@ -30,6 +30,7 @@ type processSampler struct {
 var (
 	_                       sampler.Sampler = (*processSampler)(nil) // static interface assertion
 	containerNotRunningErrs                 = map[string]struct{}{}
+	containerSamplerGetter                  = metrics.GetContainerSamplers //nolint:gochecknoglobals
 )
 
 // NewProcessSampler creates and returns a new process Sampler, given an agent context.
@@ -38,17 +39,22 @@ func NewProcessSampler(ctx agent.AgentContext) sampler.Sampler {
 
 	ttlSecs := config.DefaultContainerCacheMetadataLimit
 	apiVersion := ""
-	interval := config.FREQ_INTERVAL_FLOOR_PROCESS_METRICS
 	dockerContainerdNamespace := ""
+	interval := config.FREQ_INTERVAL_FLOOR_PROCESS_METRICS
+	var containerSamplers []metrics.ContainerSampler
 	if hasConfig {
 		cfg := ctx.Config()
 		ttlSecs = cfg.ContainerMetadataCacheLimit
 		apiVersion = cfg.DockerApiVersion
 		dockerContainerdNamespace = cfg.DockerContainerdNamespace
 		interval = cfg.MetricsProcessSampleRate
 	}
+
+	if (hasConfig && ctx.Config().ProcessContainerDecoration) || !hasConfig {
+		containerSamplers = containerSamplerGetter(time.Duration(ttlSecs)*time.Second, apiVersion, dockerContainerdNamespace)
+	}
+
 	harvester := newHarvester(ctx)
-	containerSamplers := metrics.GetContainerSamplers(time.Duration(ttlSecs)*time.Second, apiVersion, dockerContainerdNamespace)
 
 	return &processSampler{
 		harvest:           harvester,

diff --git a/pkg/metrics/process/sampler_darwin_test.go b/pkg/metrics/process/sampler_darwin_test.go
@@ -4,9 +4,9 @@ package process
 
 import (
 	"errors"
-	"testing"
-
 	"github.com/newrelic/infrastructure-agent/pkg/metrics"
+	"testing"
+	"time"
 
 	"github.com/newrelic/infrastructure-agent/internal/agent/mocks"
 	"github.com/newrelic/infrastructure-agent/pkg/config"
@@ -18,7 +18,7 @@ import (
 func TestProcessSampler_Sample(t *testing.T) {
 	ctx := new(mocks.AgentContext)
 	cfg := &config.Config{RunMode: config.ModeRoot}
-	ctx.On("Config").Times(3).Return(cfg)
+	ctx.On("Config").Times(4).Return(cfg)
 
 	harvester := &HarvesterMock{}
 	sampler := NewProcessSampler(ctx).(*processSampler)
@@ -61,7 +61,7 @@ func TestProcessSampler_Sample(t *testing.T) {
 func TestProcessSampler_Sample_ErrorOnProcessShouldNotStop(t *testing.T) {
 	ctx := new(mocks.AgentContext)
 	cfg := &config.Config{RunMode: config.ModeRoot}
-	ctx.On("Config").Times(3).Return(cfg)
+	ctx.On("Config").Times(4).Return(cfg)
 
 	harvester := &HarvesterMock{}
 	sampler := NewProcessSampler(ctx).(*processSampler)
@@ -108,7 +108,7 @@ func TestProcessSampler_Sample_ErrorOnProcessShouldNotStop(t *testing.T) {
 func TestProcessSampler_Sample_DockerDecorator(t *testing.T) {
 	ctx := new(mocks.AgentContext)
 	cfg := &config.Config{RunMode: config.ModeRoot}
-	ctx.On("Config").Times(3).Return(cfg)
+	ctx.On("Config").Times(4).Return(cfg)
 
 	harvester := &HarvesterMock{}
 	sampler := NewProcessSampler(ctx).(*processSampler)
@@ -153,3 +153,63 @@ func TestProcessSampler_Sample_DockerDecorator(t *testing.T) {
 
 	mock.AssertExpectationsForObjects(t, ctx, harvester)
 }
+
+//nolint:paralleltest
+func TestProcessSampler_Sample_DisabledDockerDecorator(t *testing.T) {
+	ctx := new(mocks.AgentContext)
+	cfg := config.NewConfig()
+	cfg.ProcessContainerDecoration = false
+	ctx.On("Config").Times(4).Return(cfg)
+
+	// The container sampler getter should not be called
+	containerSamplerGetter = func(cacheTTL time.Duration, dockerAPIVersion, dockerContainerdNamespace string) []metrics.ContainerSampler {
+		t.Errorf("containerSamplerGetter should not be called")
+
+		return nil
+	}
+
+	defer func() {
+		containerSamplerGetter = metrics.GetContainerSamplers
+	}()
+
+	var expected []metrics.ContainerSampler
+	sampler := NewProcessSampler(ctx).(*processSampler) //nolint:forcetypeassert
+	assert.Equal(t, expected, sampler.containerSamplers)
+}
+
+//nolint:paralleltest
+func TestProcessSampler_Sample_DockerDecoratorEnabledByDefault(t *testing.T) {
+	ctx := new(mocks.AgentContext)
+	cfg := config.NewConfig()
+	ctx.On("Config").Times(4).Return(cfg)
+
+	containerSamplerGetter = func(cacheTTL time.Duration, dockerAPIVersion, dockerContainerdNamespace string) []metrics.ContainerSampler {
+		return []metrics.ContainerSampler{&fakeContainerSampler{}}
+	}
+
+	defer func() {
+		containerSamplerGetter = metrics.GetContainerSamplers
+	}()
+
+	expected := []metrics.ContainerSampler{&fakeContainerSampler{}}
+	sampler := NewProcessSampler(ctx).(*processSampler) //nolint:forcetypeassert
+	assert.Equal(t, expected, sampler.containerSamplers)
+}
+
+//nolint:paralleltest
+func TestProcessSampler_Sample_DockerDecoratorEnabledWithNoConfig(t *testing.T) {
+	ctx := new(mocks.AgentContext)
+	ctx.On("Config").Times(2).Return(nil)
+
+	containerSamplerGetter = func(cacheTTL time.Duration, dockerAPIVersion, dockerContainerdNamespace string) []metrics.ContainerSampler {
+		return []metrics.ContainerSampler{&fakeContainerSampler{}}
+	}
+
+	defer func() {
+		containerSamplerGetter = metrics.GetContainerSamplers
+	}()
+
+	expected := []metrics.ContainerSampler{&fakeContainerSampler{}}
+	sampler := NewProcessSampler(ctx).(*processSampler) //nolint:forcetypeassert
+	assert.Equal(t, expected, sampler.containerSamplers)
+}
diff --git a/pkg/metrics/process/sampler_linux.go b/pkg/metrics/process/sampler_linux.go
@@ -31,6 +31,7 @@ type processSampler struct {
 var (
 	_                       sampler.Sampler = (*processSampler)(nil) // static interface assertion
 	containerNotRunningErrs                 = map[string]struct{}{}
+	containerSamplerGetter                  = metrics.GetContainerSamplers //nolint:gochecknoglobals
 )
 
 // NewProcessSampler creates and returns a new process Sampler, given an agent context.
@@ -41,16 +42,21 @@ func NewProcessSampler(ctx agent.AgentContext) sampler.Sampler {
 	apiVersion := ""
 	dockerContainerdNamespace := ""
 	interval := config.FREQ_INTERVAL_FLOOR_PROCESS_METRICS
+	var containerSamplers []metrics.ContainerSampler
 	if hasConfig {
 		cfg := ctx.Config()
 		ttlSecs = cfg.ContainerMetadataCacheLimit
 		apiVersion = cfg.DockerApiVersion
 		dockerContainerdNamespace = cfg.DockerContainerdNamespace
 		interval = cfg.MetricsProcessSampleRate
 	}
+
+	if (hasConfig && ctx.Config().ProcessContainerDecoration) || !hasConfig {
+		containerSamplers = containerSamplerGetter(time.Duration(ttlSecs)*time.Second, apiVersion, dockerContainerdNamespace)
+	}
+
 	cache := newCache()
 	harvest := newHarvester(ctx, &cache)
-	containerSamplers := metrics.GetContainerSamplers(time.Duration(ttlSecs)*time.Second, apiVersion, dockerContainerdNamespace)
 
 	return &processSampler{
 		harvest:           harvest,

diff --git a/pkg/metrics/process/sampler_linux_test.go b/pkg/metrics/process/sampler_linux_test.go
@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -67,6 +68,66 @@ func TestProcessSampler_DockerDecorator(t *testing.T) {
 	}
 }
 
+//nolint:paralleltest
+func TestProcessSampler_Sample_DisabledDockerDecorator(t *testing.T) {
+	ctx := new(mocks.AgentContext)
+	cfg := config.NewConfig()
+	cfg.ProcessContainerDecoration = false
+	ctx.On("Config").Times(4).Return(cfg)
+
+	// The container sampler getter should not be called
+	containerSamplerGetter = func(cacheTTL time.Duration, dockerAPIVersion, dockerContainerdNamespace string) []metrics.ContainerSampler {
+		t.Errorf("containerSamplerGetter should not be called")
+
+		return nil
+	}
+
+	defer func() {
+		containerSamplerGetter = metrics.GetContainerSamplers
+	}()
+
+	var expected []metrics.ContainerSampler
+	sampler := NewProcessSampler(ctx).(*processSampler) //nolint:forcetypeassert
+	assert.Equal(t, expected, sampler.containerSamplers)
+}
+
+//nolint:paralleltest
+func TestProcessSampler_Sample_DockerDecoratorEnabledByDefault(t *testing.T) {
+	ctx := new(mocks.AgentContext)
+	cfg := config.NewConfig()
+	ctx.On("Config").Times(4).Return(cfg)
+
+	containerSamplerGetter = func(cacheTTL time.Duration, dockerAPIVersion, dockerContainerdNamespace string) []metrics.ContainerSampler {
+		return []metrics.ContainerSampler{&fakeContainerSampler{}}
+	}
+
+	defer func() {
+		containerSamplerGetter = metrics.GetContainerSamplers
+	}()
+
+	expected := []metrics.ContainerSampler{&fakeContainerSampler{}}
+	sampler := NewProcessSampler(ctx).(*processSampler) //nolint:forcetypeassert
+	assert.Equal(t, expected, sampler.containerSamplers)
+}
+
+//nolint:paralleltest
+func TestProcessSampler_Sample_DockerDecoratorEnabledWithNoConfig(t *testing.T) {
+	ctx := new(mocks.AgentContext)
+	ctx.On("Config").Times(2).Return(nil)
+
+	containerSamplerGetter = func(cacheTTL time.Duration, dockerAPIVersion, dockerContainerdNamespace string) []metrics.ContainerSampler {
+		return []metrics.ContainerSampler{&fakeContainerSampler{}}
+	}
+
+	defer func() {
+		containerSamplerGetter = metrics.GetContainerSamplers
+	}()
+
+	expected := []metrics.ContainerSampler{&fakeContainerSampler{}}
+	sampler := NewProcessSampler(ctx).(*processSampler) //nolint:forcetypeassert
+	assert.Equal(t, expected, sampler.containerSamplers)
+}
+
 type harvesterMock struct {
 	samples map[int32]*types.ProcessSample
 }

diff --git a/pkg/metrics/procs_windows.go b/pkg/metrics/procs_windows.go
@@ -40,6 +40,7 @@ var (
 	// https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-queryfullprocessimagenamew
 	queryFullProcessImageName = modkernel32.NewProc("QueryFullProcessImageNameW")
 	containerNotRunningErrs   = map[string]struct{}{}
+	containerSamplerGetter    = GetContainerSamplers //nolint:gochecknoglobals
 )
 
 const (
@@ -347,7 +348,10 @@ func NewProcsMonitor(context agent.AgentContext) *ProcsMonitor {
 	)
 	ttlSecs := config.DefaultContainerCacheMetadataLimit
 	getProcFunc := getWin32Proc
-	if context != nil && context.Config() != nil {
+	var containerSamplers []ContainerSampler
+	hasConfig := context != nil && context.Config() != nil
+
+	if hasConfig {
 		if len(context.Config().AllowedListProcessSample) > 0 {
 			allowedListProcessing = true
 			for _, processName := range context.Config().AllowedListProcessSample {
@@ -362,10 +366,15 @@ func NewProcsMonitor(context agent.AgentContext) *ProcsMonitor {
 			getProcFunc = getWin32ProcFromWMI
 		}
 	}
+
+	if (hasConfig && context.Config().ProcessContainerDecoration) || !hasConfig {
+		containerSamplers = containerSamplerGetter(time.Duration(ttlSecs)*time.Second, apiVersion, dockerContainerdNamespace)
+	}
+
 	return &ProcsMonitor{
 		context:              context,
 		procCache:            make(map[string]*ProcessCacheEntry),
-		containerSamplers:    GetContainerSamplers(time.Duration(ttlSecs)*time.Second, apiVersion, dockerContainerdNamespace),
+		containerSamplers:    containerSamplers,
 		previousProcessTimes: make(map[string]*SystemTimes),
 		processInterrogator:  NewInternalProcessInterrogator(true),
 		waitForCleanup:       &sync.WaitGroup{},