[processor/tailsampling] Do not panic when creating a string_attribute with invalid filter (open-telemetry#43735)

csmarchbanks · web-flow · commit bdb46e1dba1c · 2025-10-29T11:56:11.000-04:00
#### Description
Avoids a panic, returning an error instead which can be handled
gracefully by the application.

&lt;!--Describe what testing was performed and which tests were added.--&gt;
#### Testing
Test case was added
diff --git a/.chloggen/do-not-panic-invalid-regex.yaml b/.chloggen/do-not-panic-invalid-regex.yaml
@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
+component: processor/tail_sampling
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fix panic when invalid regex was sent to string_attribute sampler
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [43735]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [user]
diff --git a/processor/tailsamplingprocessor/internal/sampling/and_test.go b/processor/tailsamplingprocessor/internal/sampling/and_test.go
@@ -16,7 +16,8 @@ import (
 )
 
 func TestAndEvaluatorNotSampled(t *testing.T) {
-	n1 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "name", []string{"value"}, false, 0, false)
+	n1, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "name", []string{"value"}, false, 0, false)
+	require.NoError(t, err)
 	n2, err := NewStatusCodeFilter(componenttest.NewNopTelemetrySettings(), []string{"ERROR"})
 	require.NoError(t, err)
 
@@ -40,7 +41,8 @@ func TestAndEvaluatorNotSampled(t *testing.T) {
 }
 
 func TestAndEvaluatorSampled(t *testing.T) {
-	n1 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"attribute_value"}, false, 0, false)
+	n1, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"attribute_value"}, false, 0, false)
+	require.NoError(t, err)
 	n2, err := NewStatusCodeFilter(componenttest.NewNopTelemetrySettings(), []string{"ERROR"})
 	require.NoError(t, err)
 
@@ -65,7 +67,8 @@ func TestAndEvaluatorSampled(t *testing.T) {
 }
 
 func TestAndEvaluatorStringInvertSampled(t *testing.T) {
-	n1 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"no_match"}, false, 0, true)
+	n1, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"no_match"}, false, 0, true)
+	require.NoError(t, err)
 	n2, err := NewStatusCodeFilter(componenttest.NewNopTelemetrySettings(), []string{"ERROR"})
 	require.NoError(t, err)
 
@@ -90,7 +93,8 @@ func TestAndEvaluatorStringInvertSampled(t *testing.T) {
 }
 
 func TestAndEvaluatorStringInvertNotSampled(t *testing.T) {
-	n1 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"attribute_value"}, false, 0, true)
+	n1, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"attribute_value"}, false, 0, true)
+	require.NoError(t, err)
 	n2, err := NewStatusCodeFilter(componenttest.NewNopTelemetrySettings(), []string{"ERROR"})
 	require.NoError(t, err)
 
diff --git a/processor/tailsamplingprocessor/internal/sampling/composite_test.go b/processor/tailsamplingprocessor/internal/sampling/composite_test.go
@@ -174,8 +174,10 @@ func TestCompositeEvaluatorSampled_AlwaysSampled(t *testing.T) {
 
 func TestCompositeEvaluatorInverseSampled_AlwaysSampled(t *testing.T) {
 	// The first policy does not match, the second matches through invert
-	n1 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", []string{"foo"}, false, 0, false)
-	n2 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", []string{"foo"}, false, 0, true)
+	n1, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", []string{"foo"}, false, 0, false)
+	require.NoError(t, err)
+	n2, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", []string{"foo"}, false, 0, true)
+	require.NoError(t, err)
 	c := NewComposite(zap.NewNop(), 10, []SubPolicyEvalParams{{n1, 20, "eval-1"}, {n2, 20, "eval-2"}}, FakeTimeProvider{}, false)
 
 	for i := 1; i <= 10; i++ {
@@ -192,8 +194,10 @@ func TestCompositeEvaluatorInverseSampled_AlwaysSampled(t *testing.T) {
 
 func TestCompositeEvaluatorInverseSampled_AlwaysSampled_RecordSubPolicy(t *testing.T) {
 	// The first policy does not match, the second matches through invert
-	n1 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", []string{"foo"}, false, 0, false)
-	n2 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", []string{"foo"}, false, 0, true)
+	n1, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", []string{"foo"}, false, 0, false)
+	require.NoError(t, err)
+	n2, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", []string{"foo"}, false, 0, true)
+	require.NoError(t, err)
 	c := NewComposite(zap.NewNop(), 10, []SubPolicyEvalParams{{n1, 20, "eval-1"}, {n2, 20, "eval-2"}}, FakeTimeProvider{}, true)
 
 	for i := 1; i <= 10; i++ {
diff --git a/processor/tailsamplingprocessor/internal/sampling/drop_test.go b/processor/tailsamplingprocessor/internal/sampling/drop_test.go
@@ -16,7 +16,8 @@ import (
 )
 
 func TestDropEvaluatorNotSampled(t *testing.T) {
-	n1 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "name", []string{"value"}, false, 0, false)
+	n1, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "name", []string{"value"}, false, 0, false)
+	require.NoError(t, err)
 	n2, err := NewStatusCodeFilter(componenttest.NewNopTelemetrySettings(), []string{"ERROR"})
 	require.NoError(t, err)
 
@@ -40,7 +41,8 @@ func TestDropEvaluatorNotSampled(t *testing.T) {
 }
 
 func TestDropEvaluatorSampled(t *testing.T) {
-	n1 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"attribute_value"}, false, 0, false)
+	n1, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"attribute_value"}, false, 0, false)
+	require.NoError(t, err)
 	n2, err := NewStatusCodeFilter(componenttest.NewNopTelemetrySettings(), []string{"ERROR"})
 	require.NoError(t, err)
 
@@ -65,7 +67,8 @@ func TestDropEvaluatorSampled(t *testing.T) {
 }
 
 func TestDropEvaluatorStringInvertMatch(t *testing.T) {
-	n1 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"no_match"}, false, 0, true)
+	n1, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"no_match"}, false, 0, true)
+	require.NoError(t, err)
 	n2, err := NewStatusCodeFilter(componenttest.NewNopTelemetrySettings(), []string{"ERROR"})
 	require.NoError(t, err)
 
@@ -90,7 +93,8 @@ func TestDropEvaluatorStringInvertMatch(t *testing.T) {
 }
 
 func TestDropEvaluatorStringInvertNotMatch(t *testing.T) {
-	n1 := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"attribute_value"}, false, 0, true)
+	n1, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "attribute_name", []string{"attribute_value"}, false, 0, true)
+	require.NoError(t, err)
 	n2, err := NewStatusCodeFilter(componenttest.NewNopTelemetrySettings(), []string{"ERROR"})
 	require.NoError(t, err)
 
diff --git a/processor/tailsamplingprocessor/internal/sampling/string_tag_filter.go b/processor/tailsamplingprocessor/internal/sampling/string_tag_filter.go
@@ -5,6 +5,7 @@ package sampling // import "github.com/open-telemetry/opentelemetry-collector-co
 
 import (
 	"context"
+	"fmt"
 	"regexp"
 
 	"github.com/golang/groupcache/lru"
@@ -36,13 +37,16 @@ var _ samplingpolicy.Evaluator = (*stringAttributeFilter)(nil)
 
 // NewStringAttributeFilter creates a policy evaluator that samples all traces with
 // the given attribute in the given numeric range.
-func NewStringAttributeFilter(settings component.TelemetrySettings, key string, values []string, regexMatchEnabled bool, evictSize int, invertMatch bool) samplingpolicy.Evaluator {
+func NewStringAttributeFilter(settings component.TelemetrySettings, key string, values []string, regexMatchEnabled bool, evictSize int, invertMatch bool) (samplingpolicy.Evaluator, error) {
 	// initialize regex filter rules and LRU cache for matched results
 	if regexMatchEnabled {
 		if evictSize <= 0 {
 			evictSize = defaultCacheSize
 		}
-		filterList := addFilters(values)
+		filterList, err := addFilters(values)
+		if err != nil {
+			return nil, err
+		}
 		regexStrSetting := &regexStrSetting{
 			matchedAttrs: lru.New(evictSize),
 			filterList:   filterList,
@@ -68,7 +72,7 @@ func NewStringAttributeFilter(settings component.TelemetrySettings, key string,
 				return false
 			},
 			invertMatch: invertMatch,
-		}
+		}, nil
 	}
 
 	// initialize the exact value map
@@ -87,7 +91,7 @@ func NewStringAttributeFilter(settings component.TelemetrySettings, key string,
 			return matched
 		},
 		invertMatch: invertMatch,
-	}
+	}, nil
 }
 
 // Evaluate looks at the trace data and returns a corresponding SamplingDecision.
@@ -151,11 +155,14 @@ func (saf *stringAttributeFilter) Evaluate(_ context.Context, _ pcommon.TraceID,
 
 // addFilters compiles all the given filters and stores them as regexes.
 // All regexes are automatically anchored to enforce full string matches.
-func addFilters(exprs []string) []*regexp.Regexp {
+func addFilters(exprs []string) ([]*regexp.Regexp, error) {
 	list := make([]*regexp.Regexp, 0, len(exprs))
 	for _, entry := range exprs {
-		rule := regexp.MustCompile(entry)
+		rule, err := regexp.Compile(entry)
+		if err != nil {
+			return nil, fmt.Errorf("invalid regex `%s`: %w", entry, err)
+		}
 		list = append(list, rule)
 	}
-	return list
+	return list, nil
 }
diff --git a/processor/tailsamplingprocessor/internal/sampling/string_tag_filter_test.go b/processor/tailsamplingprocessor/internal/sampling/string_tag_filter_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/component/componenttest"
 	"go.opentelemetry.io/collector/featuregate"
 	"go.opentelemetry.io/collector/pdata/pcommon"
@@ -226,17 +227,33 @@ func TestStringTagFilter(t *testing.T) {
 					assert.NoError(t, err)
 				}()
 			}
-			filter := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), c.filterCfg.Key, c.filterCfg.Values, c.filterCfg.EnabledRegexMatching, c.filterCfg.CacheMaxSize, c.filterCfg.InvertMatch)
+			filter, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), c.filterCfg.Key, c.filterCfg.Values, c.filterCfg.EnabledRegexMatching, c.filterCfg.CacheMaxSize, c.filterCfg.InvertMatch)
+			require.NoError(t, err)
 			decision, err := filter.Evaluate(t.Context(), pcommon.TraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}), c.Trace)
 			assert.NoError(t, err)
 			assert.Equal(t, decision, c.Decision)
 		})
 	}
 }
 
+func TestStringTagFilter_InvalidRegex(t *testing.T) {
+	t.Run("error when regex is enabled", func(t *testing.T) {
+		filter, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "key", []string{"*invalid-regex"}, true, defaultCacheSize, false)
+		assert.Error(t, err)
+		assert.Nil(t, filter)
+	})
+
+	t.Run("succeeds when regex is disabled", func(t *testing.T) {
+		filter, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "key", []string{"*invalid-regex"}, false, defaultCacheSize, false)
+		assert.NoError(t, err)
+		assert.NotNil(t, filter)
+	})
+}
+
 func BenchmarkStringTagFilterEvaluatePlainText(b *testing.B) {
 	trace := newTraceStringAttrs(map[string]any{"example": "value"}, "", "")
-	filter := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "example", []string{"value"}, false, 0, false)
+	filter, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "example", []string{"value"}, false, 0, false)
+	require.NoError(b, err)
 
 	for b.Loop() {
 		_, err := filter.Evaluate(b.Context(), pcommon.TraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}), trace)
@@ -246,7 +263,8 @@ func BenchmarkStringTagFilterEvaluatePlainText(b *testing.B) {
 
 func BenchmarkStringTagFilterEvaluateRegex(b *testing.B) {
 	trace := newTraceStringAttrs(map[string]any{"example": "grpc.health.v1.HealthCheck"}, "", "")
-	filter := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "example", []string{"v[0-9]+.HealthCheck$"}, true, 0, false)
+	filter, err := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), "example", []string{"v[0-9]+.HealthCheck$"}, true, 0, false)
+	require.NoError(b, err)
 
 	for b.Loop() {
 		_, err := filter.Evaluate(b.Context(), pcommon.TraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}), trace)
diff --git a/processor/tailsamplingprocessor/processor.go b/processor/tailsamplingprocessor/processor.go
@@ -230,7 +230,7 @@ func getSharedPolicyEvaluator(settings component.TelemetrySettings, cfg *sharedP
 		return sampling.NewProbabilisticSampler(settings, pCfg.HashSalt, pCfg.SamplingPercentage), nil
 	case StringAttribute:
 		safCfg := cfg.StringAttributeCfg
-		return sampling.NewStringAttributeFilter(settings, safCfg.Key, safCfg.Values, safCfg.EnabledRegexMatching, safCfg.CacheMaxSize, safCfg.InvertMatch), nil
+		return sampling.NewStringAttributeFilter(settings, safCfg.Key, safCfg.Values, safCfg.EnabledRegexMatching, safCfg.CacheMaxSize, safCfg.InvertMatch)
 	case StatusCode:
 		scfCfg := cfg.StatusCodeCfg
 		return sampling.NewStatusCodeFilter(settings, scfCfg.StatusCodes)
