filterset: fix concurrency issue when enabling caching. (open-telemet…

…ry#29869) Fixes open-telemetry#11829 Signed-off-by: Bogdan Drutu <[email protected]>
splunkericl · Dec 13, 2023 · 0b59407 · 0b59407
1 parent 6ec33f1
commit 0b59407
Show file tree

Hide file tree

Showing 33 changed files with 98 additions and 37 deletions.
diff --git a/.chloggen/fix-11829.yaml b/.chloggen/fix-11829.yaml
@@ -0,0 +1,22 @@
+# 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. filelogreceiver)
+component: "filterset"
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fix concurrency issue when enabling caching.
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [11829]
+
+# 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/cmd/configschema/go.mod b/cmd/configschema/go.mod
@@ -392,6 +392,7 @@ require (
 	github.com/hashicorp/go-uuid v1.0.3 // indirect
 	github.com/hashicorp/go-version v1.6.0 // indirect
 	github.com/hashicorp/golang-lru v1.0.2 // indirect
+	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
 	github.com/hashicorp/hcl v1.0.0 // indirect
 	github.com/hashicorp/nomad/api v0.0.0-20230721134942-515895c7690c // indirect
 	github.com/hashicorp/serf v0.10.1 // indirect

diff --git a/cmd/configschema/go.sum b/cmd/configschema/go.sum
diff --git a/cmd/otelcontribcol/go.mod b/cmd/otelcontribcol/go.mod
@@ -441,6 +441,7 @@ require (
 	github.com/hashicorp/go-uuid v1.0.3 // indirect
 	github.com/hashicorp/go-version v1.6.0 // indirect
 	github.com/hashicorp/golang-lru v1.0.2 // indirect
+	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
 	github.com/hashicorp/hcl v1.0.0 // indirect
 	github.com/hashicorp/nomad/api v0.0.0-20230721134942-515895c7690c // indirect
 	github.com/hashicorp/serf v0.10.1 // indirect

diff --git a/cmd/otelcontribcol/go.sum b/cmd/otelcontribcol/go.sum
diff --git a/cmd/oteltestbedcol/go.mod b/cmd/oteltestbedcol/go.mod
@@ -129,6 +129,7 @@ require (
 	github.com/hashicorp/go-rootcerts v1.0.2 // indirect
 	github.com/hashicorp/go-version v1.6.0 // indirect
 	github.com/hashicorp/golang-lru v1.0.2 // indirect
+	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
 	github.com/hashicorp/nomad/api v0.0.0-20230721134942-515895c7690c // indirect
 	github.com/hashicorp/serf v0.10.1 // indirect
 	github.com/hetznercloud/hcloud-go/v2 v2.4.0 // indirect

diff --git a/cmd/oteltestbedcol/go.sum b/cmd/oteltestbedcol/go.sum
diff --git a/connector/countconnector/go.mod b/connector/countconnector/go.mod
@@ -23,10 +23,10 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/gobwas/glob v0.2.3 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
-	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/uuid v1.5.0 // indirect
 	github.com/hashicorp/go-version v1.6.0 // indirect
+	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
 	github.com/iancoleman/strcase v0.3.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/knadh/koanf/maps v0.1.1 // indirect

diff --git a/connector/countconnector/go.sum b/connector/countconnector/go.sum
diff --git a/exporter/datadogexporter/go.mod b/exporter/datadogexporter/go.mod
@@ -144,6 +144,7 @@ require (
 	github.com/hashicorp/go-rootcerts v1.0.2 // indirect
 	github.com/hashicorp/go-version v1.6.0 // indirect
 	github.com/hashicorp/golang-lru v1.0.2 // indirect
+	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
 	github.com/hashicorp/nomad/api v0.0.0-20230721134942-515895c7690c // indirect
 	github.com/hashicorp/serf v0.10.1 // indirect
 	github.com/hetznercloud/hcloud-go/v2 v2.4.0 // indirect

diff --git a/exporter/datadogexporter/go.sum b/exporter/datadogexporter/go.sum
diff --git a/exporter/datadogexporter/integrationtest/go.mod b/exporter/datadogexporter/integrationtest/go.mod
@@ -88,6 +88,7 @@ require (
 	github.com/google/uuid v1.5.0 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.18.1 // indirect
 	github.com/hashicorp/go-version v1.6.0 // indirect
+	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
 	github.com/iancoleman/strcase v0.3.0 // indirect
 	github.com/imdario/mergo v0.3.16 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect

diff --git a/exporter/datadogexporter/integrationtest/go.sum b/exporter/datadogexporter/integrationtest/go.sum
diff --git a/exporter/honeycombmarkerexporter/go.mod b/exporter/honeycombmarkerexporter/go.mod
@@ -25,11 +25,11 @@ require (
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/gobwas/glob v0.2.3 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
-	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/golang/snappy v0.0.4 // indirect
 	github.com/google/uuid v1.5.0 // indirect
 	github.com/hashicorp/go-version v1.6.0 // indirect
+	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
 	github.com/iancoleman/strcase v0.3.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/klauspost/compress v1.17.4 // indirect

diff --git a/exporter/honeycombmarkerexporter/go.sum b/exporter/honeycombmarkerexporter/go.sum
diff --git a/go.mod b/go.mod
@@ -420,6 +420,7 @@ require (
 	github.com/hashicorp/go-uuid v1.0.3 // indirect
 	github.com/hashicorp/go-version v1.6.0 // indirect
 	github.com/hashicorp/golang-lru v1.0.2 // indirect
+	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
 	github.com/hashicorp/hcl v1.0.0 // indirect
 	github.com/hashicorp/nomad/api v0.0.0-20230721134942-515895c7690c // indirect
 	github.com/hashicorp/serf v0.10.1 // indirect

diff --git a/go.sum b/go.sum
diff --git a/internal/filter/filterset/regexp/regexpfilterset.go b/internal/filter/filterset/regexp/regexpfilterset.go
@@ -4,9 +4,10 @@
 package regexp // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter/filterset/regexp"
 
 import (
+	"math"
 	"regexp"
 
-	"github.com/golang/groupcache/lru"
+	lru "github.com/hashicorp/golang-lru/v2"
 )
 
 // FilterSet encapsulates a set of filters and caches match results.
@@ -16,9 +17,8 @@ import (
 // FilterSet satisfies the FilterSet interface from
 // "go.opentelemetry.io/collector/internal/processor/filterset"
 type FilterSet struct {
-	regexes      []*regexp.Regexp
-	cacheEnabled bool
-	cache        *lru.Cache
+	regexes []*regexp.Regexp
+	cache   *lru.Cache[string, bool]
 }
 
 // NewFilterSet constructs a FilterSet of re2 regex strings.
@@ -28,37 +28,45 @@ func NewFilterSet(filters []string, cfg *Config) (*FilterSet, error) {
 		regexes: make([]*regexp.Regexp, 0, len(filters)),
 	}
 
-	if cfg != nil && cfg.CacheEnabled {
-		fs.cacheEnabled = true
-		fs.cache = lru.New(cfg.CacheMaxNumEntries)
-	}
-
 	if err := fs.addFilters(filters); err != nil {
 		return nil, err
 	}
 
+	if cfg != nil && cfg.CacheEnabled {
+		// Because of legacy behavior, CacheMaxNumEntries == 0 means unbounded cache.
+		numEntries := cfg.CacheMaxNumEntries
+		if numEntries == 0 {
+			numEntries = math.MaxInt
+		}
+		var err error
+		fs.cache, err = lru.New[string, bool](numEntries)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	return fs, nil
 }
 
 // Matches returns true if the given string matches any of the FilterSet's filters.
 // The given string must be fully matched by at least one filter's re2 regex.
 func (rfs *FilterSet) Matches(toMatch string) bool {
-	if rfs.cacheEnabled {
+	if rfs.cache != nil {
 		if v, ok := rfs.cache.Get(toMatch); ok {
-			return v.(bool)
+			return v
 		}
 	}
 
 	for _, r := range rfs.regexes {
 		if r.MatchString(toMatch) {
-			if rfs.cacheEnabled {
+			if rfs.cache != nil {
 				rfs.cache.Add(toMatch, true)
 			}
 			return true
 		}
 	}
 
-	if rfs.cacheEnabled {
+	if rfs.cache != nil {
 		rfs.cache.Add(toMatch, false)
 	}
 	return false

diff --git a/internal/filter/filterset/regexp/regexpfilterset_test.go b/internal/filter/filterset/regexp/regexpfilterset_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var (
@@ -64,7 +65,7 @@ func TestRegexpMatches(t *testing.T) {
 	fs, err := NewFilterSet(validRegexpFilters, &Config{})
 	assert.NotNil(t, fs)
 	assert.NoError(t, err)
-	assert.False(t, fs.cacheEnabled)
+	assert.Nil(t, fs.cache)
 
 	matches := []string{
 		"full/name/match",
@@ -105,9 +106,9 @@ func TestRegexpDeDup(t *testing.T) {
 		"prefix/.*",
 	}
 	fs, err := NewFilterSet(dupRegexpFilters, &Config{})
+	require.NoError(t, err)
 	assert.NotNil(t, fs)
-	assert.NoError(t, err)
-	assert.False(t, fs.cacheEnabled)
+	assert.Nil(t, fs.cache)
 	assert.EqualValues(t, 1, len(fs.regexes))
 }
 
@@ -117,9 +118,9 @@ func TestRegexpMatchesCaches(t *testing.T) {
 		CacheEnabled:       true,
 		CacheMaxNumEntries: 0,
 	})
+	require.NoError(t, err)
 	assert.NotNil(t, fs)
-	assert.NoError(t, err)
-	assert.True(t, fs.cacheEnabled)
+	assert.NotNil(t, fs.cache)
 
 	matches := []string{
 		"full/name/match",
@@ -140,7 +141,7 @@ func TestRegexpMatchesCaches(t *testing.T) {
 			assert.True(t, fs.Matches(m))
 
 			matched, ok := fs.cache.Get(m)
-			assert.True(t, matched.(bool) && ok)
+			assert.True(t, matched && ok)
 		})
 	}
 
@@ -155,7 +156,7 @@ func TestRegexpMatchesCaches(t *testing.T) {
 			assert.False(t, fs.Matches(m))
 
 			matched, ok := fs.cache.Get(m)
-			assert.True(t, !matched.(bool) && ok)
+			assert.True(t, !matched && ok)
 		})
 	}
 }
@@ -166,8 +167,9 @@ func TestWithCacheSize(t *testing.T) {
 		CacheEnabled:       true,
 		CacheMaxNumEntries: size,
 	})
+	require.NoError(t, err)
 	assert.NotNil(t, fs)
-	assert.NoError(t, err)
+	assert.NotNil(t, fs.cache)
 
 	matches := []string{
 		"prefix/test/match",

diff --git a/internal/filter/go.mod b/internal/filter/go.mod
@@ -4,7 +4,7 @@ go 1.20
 
 require (
 	github.com/expr-lang/expr v1.15.7
-	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da
+	github.com/hashicorp/golang-lru/v2 v2.0.7
 	github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.91.0
 	github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl v0.91.0
 	github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil v0.91.0

diff --git a/internal/filter/go.sum b/internal/filter/go.sum
diff --git a/processor/attributesprocessor/go.mod b/processor/attributesprocessor/go.mod
@@ -24,10 +24,10 @@ require (
 	github.com/expr-lang/expr v1.15.7 // indirect
 	github.com/gobwas/glob v0.2.3 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
-	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/uuid v1.5.0 // indirect
 	github.com/hashicorp/go-version v1.6.0 // indirect
+	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
 	github.com/iancoleman/strcase v0.3.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/knadh/koanf/maps v0.1.1 // indirect