diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 7fdbf27d6..fae974d21 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -283,9 +283,36 @@ func NewAgent( cloudHarvester.Initialize(cloud.WithProvider(cloud.Type(cfg.CloudProvider))) idLookupTable := NewIdLookup(hostnameResolver, cloudHarvester, cfg.DisplayName) + + // Matchers logic: + // * If enable_process_metrics is defined and false, no process will be sent + // * If enable_process_metrics is not defined ( == nil ) AND there are include_metrics_matchers, + // then only the metrics that match the include_metrics_matchers will be sent + // * If enable_process_metrics is not defined ( == nil ) AND there are exclude_metrics_matchers, + // exlude list is ignored and no process is sent + // * If enable_process_metrics == true AND there are include_metrics_matchers and exclude_metrics_matchers, + // exclude ones are ignored and only the ones that match the include_metrics_matchers are sent + // * If enable_process_metrics == true AND there are exlude_metrics_matchers , + // all the processes but excluded ones will be sent + // * If all the cases where include_metrics_matchers and exclude_metrics_matchers are present, + // exclude ones will be ignored + sampleMatchFn := sampler.NewSampleMatchFn(cfg.EnableProcessMetrics, config.MetricsMap(cfg.IncludeMetricsMatchers), ffRetriever) - sampleExcludeFn := sampler.NewSampleMatchFn(cfg.EnableProcessMetrics, config.MetricsMap(cfg.ExcludeMetricsMatchers), ffRetriever) - ctx := NewContext(cfg, buildVersion, hostnameResolver, idLookupTable, sampler.IncludeSampleMatchFn(sampleMatchFn), sampler.ExcludeSampleMatchFn(sampleExcludeFn)) + // by default, do not apply exclude metrics matchers, only if no include ones are present + sampleExcludeFn := func(event any) bool { + return true + } + if len(cfg.IncludeMetricsMatchers) == 0 && + cfg.EnableProcessMetrics != nil && + *cfg.EnableProcessMetrics && + len(cfg.ExcludeMetricsMatchers) > 0 { + sampleExcludeFn = sampler.NewSampleMatchFn(cfg.EnableProcessMetrics, config.MetricsMap(cfg.ExcludeMetricsMatchers), ffRetriever) + // if there are not include matchers at all, we remove the matcher to exclude by default + sampleMatchFn = func(event any) bool { + return false + } + } + ctx := NewContext(cfg, buildVersion, hostnameResolver, idLookupTable, sampler.IncludeSampleMatchFn(sampleMatchFn), sampleExcludeFn) agentKey, err := idLookupTable.AgentKey() if err != nil { @@ -1165,7 +1192,7 @@ func (c *context) SendEvent(event sample.Event, entityKey entity.Key) { // check if event should be included // include takes precedence, so the event will be included if // it IS NOT EXCLUDED or if it IS INCLUDED - includeSample := !c.shouldExcludeEvent(event) || c.shouldIncludeEvent(event) + includeSample := c.includeEvent(event) if !includeSample { aclog. WithField("entity_key", entityKey.String()). @@ -1182,6 +1209,13 @@ func (c *context) SendEvent(event sample.Event, entityKey entity.Key) { } } +func (c *context) includeEvent(event any) bool { + shouldInclude := c.shouldIncludeEvent(event) + shouldExclude := c.shouldExcludeEvent(event) + + return shouldInclude || !shouldExclude +} + func (c *context) Unregister(id ids.PluginID) { c.ch <- types.NewNotApplicableOutput(id) } diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index f3aa68f6e..b9401df13 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -922,42 +922,64 @@ func Test_ProcessSamplingExcludes(t *testing.T) { ProcessDisplayName: "some-process", } + boolAsPointer := func(val bool) *bool { + return &val + } + type testCase struct { - name string - c *config.Config - ff feature_flags.Retriever - want bool + name string + c *config.Config + ff feature_flags.Retriever + expectInclude bool } testCases := []testCase{ { - name: "Include not matching must not exclude", - c: &config.Config{IncludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, DisableCloudMetadata: true}, - ff: test.NewFFRetrieverReturning(false, false), - want: false, + name: "Include not matching must not include", + c: &config.Config{EnableProcessMetrics: boolAsPointer(true), IncludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, DisableCloudMetadata: true}, + ff: test.NewFFRetrieverReturning(false, false), + expectInclude: false, }, { - name: "Include matching should not exclude", - c: &config.Config{IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, DisableCloudMetadata: true}, - ff: test.NewFFRetrieverReturning(false, false), - want: false, + name: "Include matching should not exclude", + c: &config.Config{EnableProcessMetrics: boolAsPointer(true), IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, DisableCloudMetadata: true}, + ff: test.NewFFRetrieverReturning(false, false), + expectInclude: true, }, { - name: "Exclude matching should exclude", - c: &config.Config{ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, DisableCloudMetadata: true}, - ff: test.NewFFRetrieverReturning(false, false), - want: true, + name: "Exclude matching should exclude with process metrics enabled", + c: &config.Config{EnableProcessMetrics: boolAsPointer(true), ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, DisableCloudMetadata: true}, + ff: test.NewFFRetrieverReturning(false, false), + expectInclude: false, }, { - name: "Exclude not matching should not exclude", - c: &config.Config{ExcludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, DisableCloudMetadata: true}, - ff: test.NewFFRetrieverReturning(false, false), - want: false, + name: "Exclude matching should exclude", + c: &config.Config{ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, DisableCloudMetadata: true}, + ff: test.NewFFRetrieverReturning(false, false), + expectInclude: false, }, { - name: "Exclude matching should exclude even if include is configured", - c: &config.Config{IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, DisableCloudMetadata: true}, - ff: test.NewFFRetrieverReturning(false, false), - want: true, + name: "Exclude not matching should not exclude with process metrics enabled", + c: &config.Config{EnableProcessMetrics: boolAsPointer(true), ExcludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, DisableCloudMetadata: true}, + ff: test.NewFFRetrieverReturning(false, false), + expectInclude: true, + }, + { + name: "Exclude not matching should not exclude", + c: &config.Config{ExcludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, DisableCloudMetadata: true}, + ff: test.NewFFRetrieverReturning(false, false), + expectInclude: false, + }, + { + name: "Include matching should include even if exclude is configured with process metrics enabled", + c: &config.Config{EnableProcessMetrics: boolAsPointer(true), IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, DisableCloudMetadata: true}, + ff: test.NewFFRetrieverReturning(false, false), + expectInclude: true, + }, + { + name: "Include matching should be include even when enable_process_metrics is not defined (nil)", + c: &config.Config{IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, DisableCloudMetadata: true}, + ff: test.NewFFRetrieverReturning(false, false), + expectInclude: true, }, } @@ -968,8 +990,205 @@ func Test_ProcessSamplingExcludes(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { t.Parallel() - actual := a.Context.shouldExcludeEvent(someSample) - assert.Equal(t, testCase.want, actual) + assert.Equal(t, testCase.expectInclude, a.Context.includeEvent(someSample)) + }) + } +} + +func Test_ProcessSamplingExcludesAllCases(t *testing.T) { + t.Parallel() + + someSample := &types.ProcessSample{ + ProcessDisplayName: "some-process", + } + + boolAsPointer := func(val bool) *bool { + return &val + } + + type testCase struct { + name string + processMetricsEnabled *bool + IncludeMetricsMatchers map[string][]string + ExcludeMetricsMatchers map[string][]string + expectInclude bool + } + + //nolint:dupword + // Test cases + // EnableProcessMetrics | IncludeMetricsMatchers | ExcludeMetricsMatchers | Expected include + // nil | nil | nil | false + // nil | [process that matches ] | nil | true + // nil | [process that not matches ] | nil | false + // nil | [process that matches ] | [process that matches ] | true + // nil | [process that not matches ] | [process that not matche ] | false + // nil | nil | [process that matches ] | false + // nil | nil | [process that not matches ] | false + // false | nil | nil | false + // false | [process that matches ] | nil | false + // false | nil | [process that not matche ] | false + // false | [process that matches ] | [process that matches ] | false + // false | [process that not matches ] | [process that not matche ] | false + // true | nil | nil | true + // true | [process that matches ] | nil | true + // true | [process that not matches ] | nil | false + // true | [process that matches ] | [process that matches ] | true + // true | [process that not matches ] | [process that not matche ] | false + // true | nil | [process that matches ] | false + // true | nil | [process that not matches ] | true + + testCases := []testCase{ + { + name: "nil | nil | nil | false", + processMetricsEnabled: nil, + IncludeMetricsMatchers: nil, + ExcludeMetricsMatchers: nil, + expectInclude: false, + }, + { + name: "nil | [process that matches] | nil | true", + processMetricsEnabled: nil, + IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + ExcludeMetricsMatchers: nil, + expectInclude: true, + }, + { + name: "nil | [process that not matches] | nil | false", + processMetricsEnabled: nil, + IncludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + ExcludeMetricsMatchers: nil, + expectInclude: false, + }, + { + name: "nil | [process that matches] | [process that matches] | true", + processMetricsEnabled: nil, + IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + expectInclude: true, + }, + { + name: "nil | [process that not matches] | [process that not matches] | false", + processMetricsEnabled: nil, + IncludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + expectInclude: false, + }, + { + name: "nil | nil | [process that matches] | false", + processMetricsEnabled: nil, + IncludeMetricsMatchers: nil, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + expectInclude: false, + }, + { + name: "nil | nil | [process that not matches] | false", + processMetricsEnabled: nil, + IncludeMetricsMatchers: nil, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + expectInclude: false, + }, + { + name: "false | nil | nil | false", + processMetricsEnabled: boolAsPointer(false), + IncludeMetricsMatchers: nil, + ExcludeMetricsMatchers: nil, + expectInclude: false, + }, + { + name: "false | [process that matches] | nil | false", + processMetricsEnabled: boolAsPointer(false), + IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + ExcludeMetricsMatchers: nil, + expectInclude: false, + }, + { + name: "false | nil | [process that not matches] | false", + processMetricsEnabled: boolAsPointer(false), + IncludeMetricsMatchers: nil, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + expectInclude: false, + }, + { + name: "false | [process that matches] | [process that matches] | false", + processMetricsEnabled: boolAsPointer(false), + IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + expectInclude: false, + }, + { + name: "false | [process that not matches] | [process that not matches] | false", + processMetricsEnabled: boolAsPointer(false), + IncludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + expectInclude: false, + }, + { + name: "true | nil | nil | true", + processMetricsEnabled: boolAsPointer(true), + IncludeMetricsMatchers: nil, + ExcludeMetricsMatchers: nil, + expectInclude: true, + }, + { + name: "true | [process that matches] | nil | true", + processMetricsEnabled: boolAsPointer(true), + IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + ExcludeMetricsMatchers: nil, + expectInclude: true, + }, + { + name: "true | [process that not matches] | nil | false", + processMetricsEnabled: boolAsPointer(true), + IncludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + ExcludeMetricsMatchers: nil, + expectInclude: false, + }, + { + name: "true | [process that matches] | [process that matches] | true", + processMetricsEnabled: boolAsPointer(true), + IncludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + expectInclude: true, + }, + { + name: "true | [process that not matches] | [process that not matches] | false", + processMetricsEnabled: boolAsPointer(true), + IncludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + expectInclude: false, + }, + { + name: "true | nil | [process that matches] | false", + processMetricsEnabled: boolAsPointer(true), + IncludeMetricsMatchers: nil, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"some-process"}}, + expectInclude: false, + }, + { + name: "true | nil | [process that not matches] | true", + processMetricsEnabled: boolAsPointer(true), + IncludeMetricsMatchers: nil, + ExcludeMetricsMatchers: map[string][]string{"process.name": {"does-not-match"}}, + expectInclude: true, + }, + } + + for _, tc := range testCases { + testCase := tc + cnf := &config.Config{ + EnableProcessMetrics: testCase.processMetricsEnabled, + IncludeMetricsMatchers: testCase.IncludeMetricsMatchers, + ExcludeMetricsMatchers: testCase.ExcludeMetricsMatchers, + DisableCloudMetadata: true, + } + + ff := test.NewFFRetrieverReturning(false, false) + a, _ := NewAgent(cnf, "test", "userAgent", ff) + + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, testCase.expectInclude, a.Context.includeEvent(someSample)) }) } }