[chore][pkg/stanza] Upgrade expr module to latest (#24648)

Fixes #24575 This upgrade involves a workaround for a grammar conflict introduced in [this PR](https://github.com/antonmedv/expr/pull/382/files). `pkg/stanza` previously added a custom function to the syntax called `env()`, which allows users to fetch values from OS environment variables. The latest version of the `expr` module introduced its own keyword `env`, but the meaning and usage were different. Still, the overlap was enough to cause failures in the compilation and rendering of expressions. The new syntax was introduced as a [Membership Operator](https://expr.medv.io/docs/Language-Definition#membership-operator), which means that the proper way to use it is with square brackets, `env[]`. This resulted in our custom `env()` syntax being flagged as invalid. More importantly, the meaning of the new `env[]` is not the same. While we use `env()` to lookup values from the OS environment, `env[]` is meant to pull values from an in-memory map. This map allows for customizations of the syntax, such as those that allow users to reference "body" or "attributes" within an expression, but it does not contain OS environment variables. Therefore, accessing it with the new syntax is of limited value to users. The solution used here is a [Patch](https://github.com/antonmedv/expr/blob/d2100ece96affe6766d97e2cf0e4ca4d145f0d30/expr.go#L98) option which allows us to inspect and modify elements of the grammar as an expression is being compiled. Fortunately, `env()` is understood to be a function call, while `env[]` is otherwise. This makes it possible to detect usage of our custom syntax and rename the function internally without any impact to the user. The PR also standardizes all uses of `expr.Compile` within `pkg/stanza` to ensure they are using the new patch option.
open-telemetry · Jul 31, 2023 · 44e1f67 · 44e1f67
1 parent 19f2a53
commit 44e1f67
Show file tree

Hide file tree

Showing 40 changed files with 94 additions and 64 deletions.
diff --git a/cmd/configschema/go.mod b/cmd/configschema/go.mod
@@ -270,7 +270,7 @@ require (
 	github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 // indirect
 	github.com/aliyun/aliyun-log-go-sdk v0.1.50 // indirect
 	github.com/andybalholm/brotli v1.0.5 // indirect
-	github.com/antonmedv/expr v1.12.5 // indirect
+	github.com/antonmedv/expr v1.12.7 // indirect
 	github.com/apache/arrow/go/arrow v0.0.0-20211112161151-bc219186db40 // indirect
 	github.com/apache/pulsar-client-go v0.8.1 // indirect
 	github.com/apache/pulsar-client-go/oauth2 v0.0.0-20220120090717-25e59572242e // 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
@@ -289,7 +289,7 @@ require (
 	github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 // indirect
 	github.com/aliyun/aliyun-log-go-sdk v0.1.50 // indirect
 	github.com/andybalholm/brotli v1.0.5 // indirect
-	github.com/antonmedv/expr v1.12.5 // indirect
+	github.com/antonmedv/expr v1.12.7 // indirect
 	github.com/apache/arrow/go/arrow v0.0.0-20211112161151-bc219186db40 // indirect
 	github.com/apache/pulsar-client-go v0.8.1 // indirect
 	github.com/apache/pulsar-client-go/oauth2 v0.0.0-20220120090717-25e59572242e // 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
@@ -65,7 +65,7 @@ require (
 	github.com/Microsoft/go-winio v0.6.0 // indirect
 	github.com/alecthomas/participle/v2 v2.0.0 // indirect
 	github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 // indirect
-	github.com/antonmedv/expr v1.12.5 // indirect
+	github.com/antonmedv/expr v1.12.7 // indirect
 	github.com/apache/thrift v0.18.1 // indirect
 	github.com/armon/go-metrics v0.4.1 // indirect
 	github.com/aws/aws-sdk-go v1.44.309 // indirect

diff --git a/cmd/oteltestbedcol/go.sum b/cmd/oteltestbedcol/go.sum
diff --git a/exporter/datadogexporter/go.mod b/exporter/datadogexporter/go.mod
@@ -65,7 +65,7 @@ require (
 	github.com/DataDog/zstd v1.5.2 // indirect
 	github.com/Microsoft/go-winio v0.6.1 // indirect
 	github.com/Showmax/go-fqdn v1.0.0 // indirect
-	github.com/antonmedv/expr v1.12.5 // indirect
+	github.com/antonmedv/expr v1.12.7 // indirect
 	github.com/armon/go-metrics v0.4.1 // indirect
 	github.com/benbjohnson/clock v1.3.0 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect

diff --git a/exporter/datadogexporter/go.sum b/exporter/datadogexporter/go.sum
diff --git a/go.mod b/go.mod
@@ -267,7 +267,7 @@ require (
 	github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 // indirect
 	github.com/aliyun/aliyun-log-go-sdk v0.1.50 // indirect
 	github.com/andybalholm/brotli v1.0.5 // indirect
-	github.com/antonmedv/expr v1.12.5 // indirect
+	github.com/antonmedv/expr v1.12.7 // indirect
 	github.com/apache/arrow/go/arrow v0.0.0-20211112161151-bc219186db40 // indirect
 	github.com/apache/pulsar-client-go v0.8.1 // indirect
 	github.com/apache/pulsar-client-go/oauth2 v0.0.0-20220120090717-25e59572242e // indirect

diff --git a/go.sum b/go.sum
diff --git a/pkg/stanza/go.mod b/pkg/stanza/go.mod
@@ -3,7 +3,7 @@ module github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza
 go 1.19
 
 require (
-	github.com/antonmedv/expr v1.12.5
+	github.com/antonmedv/expr v1.12.7
 	github.com/bmatcuk/doublestar/v4 v4.6.0
 	github.com/cespare/xxhash/v2 v2.2.0
 	github.com/influxdata/go-syslog/v3 v3.0.1-0.20210608084020-ac565dc76ba6

diff --git a/pkg/stanza/go.sum b/pkg/stanza/go.sum
diff --git a/pkg/stanza/operator/helper/expr_string.go b/pkg/stanza/operator/helper/expr_string.go
@@ -10,6 +10,7 @@ import (
 	"sync"
 
 	"github.com/antonmedv/expr"
+	"github.com/antonmedv/expr/ast"
 	"github.com/antonmedv/expr/vm"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/entry"
@@ -77,7 +78,7 @@ func (e ExprStringConfig) Build() (*ExprString, error) {
 
 	subExprs := make([]*vm.Program, 0, len(subExprStrings))
 	for _, subExprString := range subExprStrings {
-		program, err := expr.Compile(subExprString, expr.AllowUndefinedVariables())
+		program, err := expr.Compile(subExprString, expr.AllowUndefinedVariables(), expr.Patch(&patcher{}))
 		if err != nil {
 			return nil, errors.Wrap(err, "compile embedded expression")
 		}
@@ -90,6 +91,10 @@ func (e ExprStringConfig) Build() (*ExprString, error) {
 	}, nil
 }
 
+func ExprCompileBool(input string) (*vm.Program, error) {
+	return expr.Compile(input, expr.AllowUndefinedVariables(), expr.Patch(&patcher{}), expr.AsBool())
+}
+
 // An ExprString is made up of a list of string literals
 // interleaved with expressions. len(SubStrings) == len(SubExprs) + 1
 type ExprString struct {
@@ -113,14 +118,29 @@ func (e *ExprString) Render(env map[string]interface{}) (string, error) {
 		b.WriteString(outString)
 	}
 	b.WriteString(e.SubStrings[len(e.SubStrings)-1])
-
 	return b.String(), nil
 }
 
+type patcher struct{}
+
+func (p *patcher) Visit(node *ast.Node) {
+	n, ok := (*node).(*ast.CallNode)
+	if !ok {
+		return
+	}
+	c, ok := (n.Callee).(*ast.IdentifierNode)
+	if !ok {
+		return
+	}
+	if c.Value == "env" {
+		c.Value = "os_env_func"
+	}
+}
+
 var envPool = sync.Pool{
 	New: func() interface{} {
 		return map[string]interface{}{
-			"env": os.Getenv,
+			"os_env_func": os.Getenv,
 		}
 	},
 }

diff --git a/pkg/stanza/operator/helper/expr_string_test.go b/pkg/stanza/operator/helper/expr_string_test.go
@@ -13,7 +13,8 @@ import (
 )
 
 func TestExprString(t *testing.T) {
-	t.Setenv("TEST_EXPR_STRING_ENV", "foo")
+	t.Setenv("TEST_EXPR_STRING_ENV_FOO", "foo")
+	t.Setenv("TEST_EXPR_STRING_ENV_BAR", "bar")
 
 	exampleEntry := func() *entry.Entry {
 		e := entry.New()
@@ -74,14 +75,26 @@ func TestExprString(t *testing.T) {
 			"my EXPR(body.test )",
 			"my value",
 		},
+		{
+			"my EXPR( body.test)",
+			"my value",
+		},
 		{
 			"my EXPR(body.test)",
 			"my value",
 		},
 		{
-			"my EXPR(env('TEST_EXPR_STRING_ENV'))",
+			"my EXPR(env('TEST_EXPR_STRING_ENV_FOO'))",
+			"my foo",
+		},
+		{
+			"my EXPR( env('TEST_EXPR_STRING_ENV_FOO') )",
 			"my foo",
 		},
+		{
+			"my EXPR(env('TEST_EXPR_STRING_ENV_FOO')) EXPR(env('TEST_EXPR_STRING_ENV_BAR'))",
+			"my foo bar",
+		},
 		{
 			"EXPR( resource.id )",
 			"value",

diff --git a/pkg/stanza/operator/helper/transformer.go b/pkg/stanza/operator/helper/transformer.go
@@ -7,7 +7,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/antonmedv/expr"
 	"github.com/antonmedv/expr/vm"
 	"go.uber.org/zap"
 
@@ -53,7 +52,7 @@ func (c TransformerConfig) Build(logger *zap.SugaredLogger) (TransformerOperator
 	}
 
 	if c.IfExpr != "" {
-		compiled, err := expr.Compile(c.IfExpr, expr.AsBool(), expr.AllowUndefinedVariables())
+		compiled, err := ExprCompileBool(c.IfExpr)
 		if err != nil {
 			return TransformerOperator{}, fmt.Errorf("failed to compile expression '%s': %w", c.IfExpr, err)
 		}

diff --git a/pkg/stanza/operator/transformer/filter/filter.go b/pkg/stanza/operator/transformer/filter/filter.go
@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"math/big"
 
-	"github.com/antonmedv/expr"
 	"github.com/antonmedv/expr/vm"
 	"go.uber.org/zap"
 
@@ -56,7 +55,7 @@ func (c Config) Build(logger *zap.SugaredLogger) (operator.Operator, error) {
 		return nil, err
 	}
 
-	compiledExpression, err := expr.Compile(c.Expression, expr.AsBool(), expr.AllowUndefinedVariables())
+	compiledExpression, err := helper.ExprCompileBool(c.Expression)
 	if err != nil {
 		return nil, fmt.Errorf("failed to compile expression '%s': %w", c.Expression, err)
 	}

diff --git a/pkg/stanza/operator/transformer/recombine/recombine.go b/pkg/stanza/operator/transformer/recombine/recombine.go
@@ -80,13 +80,13 @@ func (c *Config) Build(logger *zap.SugaredLogger) (operator.Operator, error) {
 	var prog *vm.Program
 	if c.IsFirstEntry != "" {
 		matchesFirst = true
-		prog, err = expr.Compile(c.IsFirstEntry, expr.AsBool(), expr.AllowUndefinedVariables())
+		prog, err = helper.ExprCompileBool(c.IsFirstEntry)
 		if err != nil {
 			return nil, fmt.Errorf("failed to compile is_first_entry: %w", err)
 		}
 	} else {
 		matchesFirst = false
-		prog, err = expr.Compile(c.IsLastEntry, expr.AsBool(), expr.AllowUndefinedVariables())
+		prog, err = helper.ExprCompileBool(c.IsLastEntry)
 		if err != nil {
 			return nil, fmt.Errorf("failed to compile is_last_entry: %w", err)
 		}

diff --git a/pkg/stanza/operator/transformer/router/router.go b/pkg/stanza/operator/transformer/router/router.go
@@ -7,7 +7,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/antonmedv/expr"
 	"github.com/antonmedv/expr/vm"
 	"go.uber.org/zap"
 
@@ -65,7 +64,7 @@ func (c Config) Build(logger *zap.SugaredLogger) (operator.Operator, error) {
 
 	routes := make([]*Route, 0, len(c.Routes))
 	for _, routeConfig := range c.Routes {
-		compiled, err := expr.Compile(routeConfig.Expression, expr.AsBool(), expr.AllowUndefinedVariables())
+		compiled, err := helper.ExprCompileBool(routeConfig.Expression)
 		if err != nil {
 			return nil, fmt.Errorf("failed to compile expression '%s': %w", routeConfig.Expression, err)
 		}

diff --git a/processor/logstransformprocessor/go.mod b/processor/logstransformprocessor/go.mod
@@ -17,7 +17,7 @@ require (
 )
 
 require (
-	github.com/antonmedv/expr v1.12.5 // indirect
+	github.com/antonmedv/expr v1.12.7 // indirect
 	github.com/cenkalti/backoff/v4 v4.2.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect

diff --git a/processor/logstransformprocessor/go.sum b/processor/logstransformprocessor/go.sum
diff --git a/receiver/azureeventhubreceiver/go.mod b/receiver/azureeventhubreceiver/go.mod
@@ -33,7 +33,7 @@ require (
 	github.com/Azure/go-autorest/autorest/validation v0.3.1 // indirect
 	github.com/Azure/go-autorest/logger v0.2.1 // indirect
 	github.com/Azure/go-autorest/tracing v0.6.0 // indirect
-	github.com/antonmedv/expr v1.12.5 // indirect
+	github.com/antonmedv/expr v1.12.7 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cenkalti/backoff/v4 v4.2.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect

diff --git a/receiver/azureeventhubreceiver/go.sum b/receiver/azureeventhubreceiver/go.sum
diff --git a/receiver/filelogreceiver/go.mod b/receiver/filelogreceiver/go.mod
@@ -17,7 +17,7 @@ require (
 )
 
 require (
-	github.com/antonmedv/expr v1.12.5 // indirect
+	github.com/antonmedv/expr v1.12.7 // indirect
 	github.com/bmatcuk/doublestar/v4 v4.6.0 // indirect
 	github.com/cenkalti/backoff/v4 v4.2.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect

diff --git a/receiver/filelogreceiver/go.sum b/receiver/filelogreceiver/go.sum
diff --git a/receiver/journaldreceiver/go.mod b/receiver/journaldreceiver/go.mod
@@ -13,7 +13,7 @@ require (
 )
 
 require (
-	github.com/antonmedv/expr v1.12.5 // indirect
+	github.com/antonmedv/expr v1.12.7 // indirect
 	github.com/cenkalti/backoff/v4 v4.2.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect

diff --git a/receiver/journaldreceiver/go.sum b/receiver/journaldreceiver/go.sum
diff --git a/receiver/mongodbatlasreceiver/go.mod b/receiver/mongodbatlasreceiver/go.mod
@@ -26,7 +26,7 @@ require (
 )
 
 require (
-	github.com/antonmedv/expr v1.12.5 // indirect
+	github.com/antonmedv/expr v1.12.7 // indirect
 	github.com/benbjohnson/clock v1.3.0 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect