Fix panic caused by providing a not valid regex pattern (#47)

* fix: fixed panic caused by providing a not valid regex pattern to process filter config

Co-authored-by: Carlos <croman@newrelic.com>

pkg/metrics/sampler/matcher.go

@@ -5,6 +5,7 @@ package sampler
 
 import (
 	"fmt"
+	"github.com/newrelic/infrastructure-agent/pkg/log"
 	"reflect"
 	"regexp"
 	"strings"
@@ -20,6 +21,8 @@ var (
 	typesToEvaluate = map[string]bool{"ProcessSample": true}
 )
 
+var mlog = log.WithComponent("SamplerMatcher")
+
 // IncludeSampleMatchFn func that returns whether an event/sample should be included, it satisfies
 // the metrics matcher (processor.MatcherChain) interface.
 type IncludeSampleMatchFn func(sample interface{}) bool
@@ -114,7 +117,7 @@ func build(dimensionName string, expr string) ExpressionMatcher {
 	// so this matcher basically get's ignored in the current implementation
 	mappedAttributeName, found := attrCache[dimensionName]
 	if !found {
-		return constantMatcher{false}
+		return constantMatcher{value: false}
 	}
 
 	eval := matcher{
@@ -123,7 +126,10 @@ func build(dimensionName string, expr string) ExpressionMatcher {
 
 	if strings.HasPrefix(expr, "regex") {
 		regex := strings.Trim(strings.TrimSpace(strings.TrimLeft(expr, "regex")), `"`)
-		cacheRegex(regex)
+		if err := cacheRegex(regex); err != nil {
+			mlog.WithError(err).Error(fmt.Sprintf("could not intitilize expression matcher for the provided configuration: '%s'", expr))
+			return constantMatcher{value: false}
+		}
 		eval.ExpectedValue = regex
 		eval.Evaluator = regularExpressionEvaluator
 	} else {
@@ -134,11 +140,16 @@ func build(dimensionName string, expr string) ExpressionMatcher {
 	return eval
 }
 
-func cacheRegex(regex string) {
+func cacheRegex(pattern string) error {
 	//if not cached yet, cache it
-	if _, ok := regexCache[regex]; !ok {
-		regexCache[regex] = regexp.MustCompile(regex)
+	if _, ok := regexCache[pattern]; !ok {
+		regex, err := regexp.Compile(pattern)
+		if err != nil {
+			return err
+		}
+		regexCache[pattern] = regex
 	}
+	return nil
 }
 
 // MatcherChain is a chain of evaluators
@@ -260,6 +271,6 @@ func NewSampleMatchFn(enableProcessMetrics *bool, includeMetricsMatchers config.
 		}
 
 		enabled, exists := ffRetriever.GetFeatureFlag(handler.FlagFullProcess)
-		return  exists && enabled
+		return exists && enabled
 	}
 }

pkg/metrics/sampler/matcher_test.go

@@ -26,8 +26,8 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-var metricDimensionProcessName string = "process.name"
-var metricDimensionProcessExecutable string = "process.executable"
+var metricDimensionProcessName = "process.name"
+var metricDimensionProcessExecutable = "process.executable"
 
 func Test_EvaluatorChain_WithSingleRule(t *testing.T) {
 
@@ -103,6 +103,14 @@ func Test_EvaluatorChain_WithSingleRule(t *testing.T) {
 			},
 			want: false,
 		},
+		{
+			name:  "ProcessCmdLine_NotValidRegex",
+			input: &types.ProcessSample{CmdLine: "/bin/java"},
+			rules: map[string][]string{
+				metricDimensionProcessExecutable: {"regex *"},
+			},
+			want: false,
+		},
 	}
 
 	for _, tc := range cases {
@@ -165,6 +173,15 @@ func Test_Evaluator_WithTwoLiteralRules(t *testing.T) {
 			},
 			want: true,
 		},
+		{
+			name:  "ProcessNameAndExecutableAreMatchNotValidRegex",
+			input: javaProcessSample,
+			rules: map[string][]string{
+				metricDimensionProcessExecutable: {"regex *"},
+				metricDimensionProcessName:       {"java"},
+			},
+			want: true,
+		},
 	}
 
 	for _, tc := range cases {
@@ -453,6 +470,27 @@ func Test_EvaluatorChain_WithMultipleRuleAttribute(t *testing.T) {
 			},
 			want: []bool{true, true, false, true, true, true, false},
 		},
+		{
+			/*
+				matchers:
+				  process.executable:
+				    - regex *					# bad regex
+				    - regex .*
+			*/
+			name: "ProcessExecutable_NotValidRegex",
+			input: []interface{}{
+				types.ProcessSample{CmdLine: "/bin/java"},
+				types.ProcessSample{CmdLine: "/bin/local/java"},
+			},
+			rules: map[string][]string{
+				metricDimensionProcessExecutable: {
+					"regex *",
+					"/bin/java",
+					"/bin/local/java",
+				},
+			},
+			want: []bool{true, true},
+		},
 	}
 
 	for _, tc := range cases {
@@ -509,11 +547,13 @@ func Test_EvaluatorChain_LogTraceMatcher(t *testing.T) {
 }
 
 type enabledFFRetriever struct{}
+
 func (e *enabledFFRetriever) GetFeatureFlag(name string) (enabled bool, exists bool) {
 	return true, true
 }
 
 type disabledFFRetriever struct{}
+
 func (e *disabledFFRetriever) GetFeatureFlag(name string) (enabled bool, exists bool) {
 	return false, true
 }
@@ -523,7 +563,6 @@ func TestNewSampleMatchFn(t *testing.T) {
 	falseVar := false
 	emptyMatchers := config.IncludeMetricsMap{}
 
-
 	type args struct {
 		enableProcessMetrics   *bool
 		includeMetricsMatchers config.IncludeMetricsMap
@@ -588,20 +627,20 @@ func TestNewSampleMatchFn(t *testing.T) {
 		{
 			name: "process samples matching rules are included",
 			args: args{
-				enableProcessMetrics: &trueVar,
+				enableProcessMetrics:   &trueVar,
 				includeMetricsMatchers: config.IncludeMetricsMap{"process.name": []string{"regex \"foo.*\""}},
-				ffRetriever: testFF.EmptyFFRetriever,
-				sample: &fixture.ProcessSample,
+				ffRetriever:            testFF.EmptyFFRetriever,
+				sample:                 &fixture.ProcessSample,
 			},
 			include: true,
 		},
 		{
 			name: "process samples not matching rules are not included",
 			args: args{
-				enableProcessMetrics: &trueVar,
+				enableProcessMetrics:   &trueVar,
 				includeMetricsMatchers: config.IncludeMetricsMap{"process.name": []string{"regex \"bar*\""}},
-				ffRetriever: testFF.EmptyFFRetriever,
-				sample: &fixture.ProcessSample,
+				ffRetriever:            testFF.EmptyFFRetriever,
+				sample:                 &fixture.ProcessSample,
 			},
 			include: false,
 		},