refactor: check for sample type before applying matchers

internal/agent/agent.go

@@ -26,6 +26,7 @@ import (
 	"github.com/newrelic/infrastructure-agent/pkg/entity/host"
 	"github.com/newrelic/infrastructure-agent/pkg/helpers/metric"
 	"github.com/newrelic/infrastructure-agent/pkg/metrics/sampler"
+	process_sample_types "github.com/newrelic/infrastructure-agent/pkg/metrics/types"
 	"github.com/sirupsen/logrus"
 
 	"github.com/newrelic/infrastructure-agent/pkg/ctl"
@@ -159,8 +160,8 @@ type context struct {
 	resolver           hostname.ResolverChangeNotifier
 	EntityMap          entity.KnownIDs
 	idLookup           host.IDLookup
-	shouldIncludeEvent sampler.IncludeSampleMatchFn
-	shouldExcludeEvent sampler.ExcludeSampleMatchFn
+	shouldIncludeEvent sampler.IncludeProcessSampleMatchFn
+	shouldExcludeEvent sampler.ExcludeProcessSampleMatchFn
 }
 
 func (c *context) Context() context2.Context {
@@ -206,8 +207,8 @@ func NewContext(
 	buildVersion string,
 	resolver hostname.ResolverChangeNotifier,
 	lookup host.IDLookup,
-	sampleMatchFn sampler.IncludeSampleMatchFn,
-	sampleExcludeFn sampler.ExcludeSampleMatchFn,
+	sampleMatchFn sampler.IncludeProcessSampleMatchFn,
+	sampleExcludeFn sampler.ExcludeProcessSampleMatchFn,
 ) *context {
 	ctx, cancel := context2.WithCancel(context2.Background())
 
@@ -296,22 +297,22 @@ func NewAgent(
 	// 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.NewIncludeSampleMatchFn(cfg.EnableProcessMetrics, cfg.IncludeMetricsMatchers, ffRetriever)
+	processSampleMatchFn := sampler.NewIncludeProcessSampleMatchFn(cfg.EnableProcessMetrics, cfg.IncludeMetricsMatchers, ffRetriever)
 	// by default, do not apply exclude metrics matchers, only if no include ones are present
-	sampleExcludeFn := func(event any) bool {
+	processSampleExcludeFn := func(event any) bool {
 		return true
 	}
 	if len(cfg.IncludeMetricsMatchers) == 0 &&
 		cfg.EnableProcessMetrics != nil &&
 		*cfg.EnableProcessMetrics &&
 		len(cfg.ExcludeMetricsMatchers) > 0 {
-		sampleExcludeFn = sampler.NewExcludeSampleMatchFn(cfg.ExcludeMetricsMatchers)
+		processSampleExcludeFn = sampler.NewExcludeProcessSampleMatchFn(cfg.ExcludeMetricsMatchers)
 		// if there are not include matchers at all, we remove the matcher to exclude by default
-		sampleMatchFn = func(event any) bool {
+		processSampleMatchFn = func(event any) bool {
 			return false
 		}
 	}
-	ctx := NewContext(cfg, buildVersion, hostnameResolver, idLookupTable, sampler.IncludeSampleMatchFn(sampleMatchFn), sampleExcludeFn)
+	ctx := NewContext(cfg, buildVersion, hostnameResolver, idLookupTable, sampler.IncludeProcessSampleMatchFn(processSampleMatchFn), processSampleExcludeFn)
 
 	agentKey, err := idLookupTable.AgentKey()
 	if err != nil {
@@ -1210,10 +1211,17 @@ func (c *context) SendEvent(event sample.Event, entityKey entity.Key) {
 
 // Decides wether an event will be included or not.
 func (c *context) IncludeEvent(event any) bool {
-	shouldInclude := c.shouldIncludeEvent(event)
-	shouldExclude := c.shouldExcludeEvent(event)
-
-	return shouldInclude || !shouldExclude
+	switch event.(type) {
+	// rule is applied to process samples only
+	case *process_sample_types.ProcessSample, *process_sample_types.FlatProcessSample:
+		shouldInclude := c.shouldIncludeEvent(event)
+		shouldExclude := c.shouldExcludeEvent(event)
+
+		return shouldInclude || !shouldExclude
+	default:
+		// other samples are included
+		return true
+	}
 }
 
 func (c *context) Unregister(id ids.PluginID) {

pkg/metrics/sampler/matcher.go

@@ -30,13 +30,13 @@ var (
 // the metrics matcher (processor.MatcherChain) interface.
 type MatcherFn func(event any) bool
 
-// IncludeSampleMatchFn func that returns whether an event/sample should be included, it satisfies
+// IncludeProcessSampleMatchFn func that returns whether an event/sample should be included, it satisfies
 // the metrics matcher (processor.MatcherChain) interface.
-type IncludeSampleMatchFn MatcherFn
+type IncludeProcessSampleMatchFn MatcherFn
 
-// ExcludeSampleMatchFn func that returns whether an event/sample should be excluded, it satisfies
+// ExcludeProcessSampleMatchFn func that returns whether an event/sample should be excluded, it satisfies
 // the metrics matcher (processor.MatcherChain) interface.
-type ExcludeSampleMatchFn MatcherFn
+type ExcludeProcessSampleMatchFn MatcherFn
 
 // ExpressionMatcher is an interface every evaluator must implement
 type ExpressionMatcher interface {
@@ -262,17 +262,12 @@ func (ne constantMatcher) String() string {
 	return fmt.Sprint(ne.value)
 }
 
-// NewIncludeSampleMatchFn returns a function `func(sample) bool` that determinesif the sample
+// NewIncludeProcessSampleMatchFn returns a function `func(sample) bool` that determinesif the sample
 // should be included (true) as an event or not (false). Note that this is NOT the negation
-// of `NewExcludeSampleMatchFn`.
+// of `NewExcludeProcessSampleMatchFn`.
 // The include decision logic only applies to ProcessSamples. Other kinds of samples are always included.
-func NewIncludeSampleMatchFn(enableProcessMetrics *bool, metricsMatchers config.IncludeMetricsMap, ffRetriever feature_flags.Retriever) MatcherFn {
+func NewIncludeProcessSampleMatchFn(enableProcessMetrics *bool, metricsMatchers config.IncludeMetricsMap, ffRetriever feature_flags.Retriever) MatcherFn {
 	return func(sample any) bool {
-		// We return early if the sample is not a ProcessSample.
-		if !isProcessSample(sample) {
-			// Include this event
-			return true
-		}
 
 		// Continuing with the logic
 		// configuration option always takes precedence over FF and matchers configuration
@@ -305,34 +300,25 @@ func NewIncludeSampleMatchFn(enableProcessMetrics *bool, metricsMatchers config.
 
 		mlog.
 			WithField(config.TracesFieldName, config.FeatureTrace).
-			Trace("EnableProcessMetrics is TRUE and rules are NOT defined, ALL process metrics will be ENABLED")
+			Trace("EnableProcessMetrics is TRUE and rules are NOT defined, ALL process metrics will be ENABLED unless excluded_matching_metrics rules are defined")
 
 		return true
 	}
 }
 
-// NewExcludeSampleMatchFn returns a function `func(sample) bool` that determinesif the sample
+// NewExcludeProcessSampleMatchFn returns a function `func(sample) bool` that determinesif the sample
 // should be excluded (true) as an event or not (false). Note that this is NOT the negation
-// of `NewIncludeSampleMatchFn`. In particular, we don't check here for the `enableProcessMetrics`
+// of `NewIncludeProcessSampleMatchFn`. In particular, we don't check here for the `enableProcessMetrics`
 // being unset or disabled because it is checked before calling this function at `agent.NewAgent`.
 // The exclude decision logic only applies to ProcessSamples. Other kinds of samples are never excluded.
-func NewExcludeSampleMatchFn(metricsMatchers config.ExcludeMetricsMap) MatcherFn {
+func NewExcludeProcessSampleMatchFn(metricsMatchers config.ExcludeMetricsMap) MatcherFn {
 	return func(sample any) bool {
-		// We return early if the sample is not a ProcessSample.
-		if !isProcessSample(sample) {
-			// Do NOT exclude this event
-			return false
-		}
 
 		matcher := matcherFromMetricsMatchers(config.MetricsMap(metricsMatchers))
 		if matcher != nil {
 			return matcher(sample)
 		}
 
-		mlog.
-			WithField(config.TracesFieldName, config.FeatureTrace).
-			Trace("EnableProcessMetrics is TRUE and rules are NOT defined, ALL process metrics will be ENABLED")
-
 		return false
 	}
 }
@@ -350,12 +336,3 @@ func matcherFromMetricsMatchers(metricsMatchers config.MetricsMap) MatcherFn {
 
 	return nil
 }
-
-func isProcessSample(sample any) bool {
-	switch sample.(type) {
-	case *types.ProcessSample, *types.FlatProcessSample:
-		return true
-	default:
-		return false
-	}
-}

pkg/metrics/sampler/matcher_test.go

@@ -687,7 +687,7 @@ func TestNewSampleMatchFn(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			matchFn := sampler.NewIncludeSampleMatchFn(tt.args.enableProcessMetrics, tt.args.includeMetricsMatchers, tt.args.ffRetriever)
+			matchFn := sampler.NewIncludeProcessSampleMatchFn(tt.args.enableProcessMetrics, tt.args.includeMetricsMatchers, tt.args.ffRetriever)
 			assert.Equal(t, tt.include, matchFn(tt.args.sample))
 		})
 	}