Replace invalid comparison print statement with a debug log entry (#1…

…4468)

Co-authored-by: Dmitrii Anoshin <[email protected]>

pkg/ottl/boolean_value.go

@@ -72,7 +72,7 @@ func (p *Parser) newComparisonEvaluator(comparison *Comparison) (boolExpressionE
 	return func(ctx TransformContext) bool {
 		a := left.Get(ctx)
 		b := right.Get(ctx)
-		return compare(a, b, comparison.Op)
+		return p.compare(a, b, comparison.Op)
 	}, nil
 
 }

pkg/ottl/boolean_value_test.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/component/componenttest"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest"
 )
@@ -81,7 +82,7 @@ func Test_newComparisonEvaluator(t *testing.T) {
 		defaultFunctionsForTests(),
 		testParsePath,
 		testParseEnum,
-		component.TelemetrySettings{},
+		componenttest.NewNopTelemetrySettings(),
 	)
 
 	tests := []struct {

pkg/ottl/compare.go

@@ -16,8 +16,8 @@ package ottl // import "github.com/open-telemetry/opentelemetry-collector-contri
 
 import (
 	"bytes"
-	"fmt"
 
+	"go.uber.org/zap"
 	"golang.org/x/exp/constraints"
 )
 
@@ -28,8 +28,8 @@ import (
 // invalidComparison returns false for everything except NE (where it returns true to indicate that the
 // objects were definitely not equivalent).
 // It also gives us an opportunity to log something.
-func invalidComparison(msg string, op CompareOp) bool {
-	fmt.Printf("%s with op %v\n", msg, op)
+func (p *Parser) invalidComparison(msg string, op CompareOp) bool {
+	p.telemetrySettings.Logger.Debug(msg, zap.Any("op", op))
 	return op == NE
 }
 
@@ -92,25 +92,25 @@ func compareBytes(a []byte, b []byte, op CompareOp) bool {
 	}
 }
 
-func compareBool(a bool, b any, op CompareOp) bool {
+func (p *Parser) compareBool(a bool, b any, op CompareOp) bool {
 	switch v := b.(type) {
 	case bool:
 		return compareBools(a, v, op)
 	default:
-		return invalidComparison("bool to non-bool", op)
+		return p.invalidComparison("bool to non-bool", op)
 	}
 }
 
-func compareString(a string, b any, op CompareOp) bool {
+func (p *Parser) compareString(a string, b any, op CompareOp) bool {
 	switch v := b.(type) {
 	case string:
 		return comparePrimitives(a, v, op)
 	default:
-		return invalidComparison("string to non-string", op)
+		return p.invalidComparison("string to non-string", op)
 	}
 }
 
-func compareByte(a []byte, b any, op CompareOp) bool {
+func (p *Parser) compareByte(a []byte, b any, op CompareOp) bool {
 	switch v := b.(type) {
 	case nil:
 		return op == NE
@@ -120,35 +120,35 @@ func compareByte(a []byte, b any, op CompareOp) bool {
 		}
 		return compareBytes(a, v, op)
 	default:
-		return invalidComparison("Bytes to non-Bytes", op)
+		return p.invalidComparison("Bytes to non-Bytes", op)
 	}
 }
 
-func compareInt64(a int64, b any, op CompareOp) bool {
+func (p *Parser) compareInt64(a int64, b any, op CompareOp) bool {
 	switch v := b.(type) {
 	case int64:
 		return comparePrimitives(a, v, op)
 	case float64:
 		return comparePrimitives(float64(a), v, op)
 	default:
-		return invalidComparison("int to non-numeric value", op)
+		return p.invalidComparison("int to non-numeric value", op)
 	}
 }
 
-func compareFloat64(a float64, b any, op CompareOp) bool {
+func (p *Parser) compareFloat64(a float64, b any, op CompareOp) bool {
 	switch v := b.(type) {
 	case int64:
 		return comparePrimitives(a, float64(v), op)
 	case float64:
 		return comparePrimitives(a, v, op)
 	default:
-		return invalidComparison("float to non-numeric value", op)
+		return p.invalidComparison("float to non-numeric value", op)
 	}
 }
 
 // a and b are the return values from a Getter; we try to compare them
 // according to the given operator.
-func compare(a any, b any, op CompareOp) bool {
+func (p *Parser) compare(a any, b any, op CompareOp) bool {
 	// nils are equal to each other and never equal to anything else,
 	// so if they're both nil, report equality.
 	if a == nil && b == nil {
@@ -159,20 +159,20 @@ func compare(a any, b any, op CompareOp) bool {
 	case nil:
 		// If a was nil, it means b wasn't and inequalities don't apply,
 		// so let's swap and give it the chance to get evaluated.
-		return compare(b, nil, op)
+		return p.compare(b, nil, op)
 	case bool:
-		return compareBool(v, b, op)
+		return p.compareBool(v, b, op)
 	case int64:
-		return compareInt64(v, b, op)
+		return p.compareInt64(v, b, op)
 	case float64:
-		return compareFloat64(v, b, op)
+		return p.compareFloat64(v, b, op)
 	case string:
-		return compareString(v, b, op)
+		return p.compareString(v, b, op)
 	case []byte:
 		if v == nil {
-			return compare(b, nil, op)
+			return p.compare(b, nil, op)
 		}
-		return compareByte(v, b, op)
+		return p.compareByte(v, b, op)
 	default:
 		// If we don't know what type it is, we can't do inequalities yet. So we can fall back to the old behavior where we just
 		// use Go's standard equality.
@@ -182,7 +182,7 @@ func compare(a any, b any, op CompareOp) bool {
 		case NE:
 			return a != b
 		default:
-			return invalidComparison("unsupported type for inequality on left", op)
+			return p.invalidComparison("unsupported type for inequality on left", op)
 		}
 	}
 }

pkg/ottl/compare_test.go

@@ -17,6 +17,8 @@ package ottl
 import (
 	"fmt"
 	"testing"
+
+	"go.opentelemetry.io/collector/component/componenttest"
 )
 
 // Our types are bool, int, float, string, Bytes, nil, so we compare all types in both directions.
@@ -114,78 +116,81 @@ func Test_compare(t *testing.T) {
 	for _, tt := range tests {
 		for _, op := range ops {
 			t.Run(fmt.Sprintf("%s %v", tt.name, op), func(t *testing.T) {
-				if got := compare(tt.a, tt.b, op); got != tt.want[op] {
+				p := NewParser(nil, nil, nil, componenttest.NewNopTelemetrySettings())
+				if got := p.compare(tt.a, tt.b, op); got != tt.want[op] {
 					t.Errorf("compare(%v, %v, %v) = %v, want %v", tt.a, tt.b, op, got, tt.want[op])
 				}
 			})
 		}
 	}
 }
 
+var testParser = NewParser(nil, nil, nil, componenttest.NewNopTelemetrySettings())
+
 // Benchmarks -- these benchmarks compare the performance of comparisons of a variety of data types.
 // It's not attempting to be exhaustive, but again, it hits most of the major types and combinations.
 // The summary is that they're pretty fast; all the calls to compare are 12 ns/op or less on a 2019 intel
 // mac pro laptop, and none of them have any allocations.
 func BenchmarkCompareEQInt64(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(i64a, i64b, EQ)
+		testParser.compare(i64a, i64b, EQ)
 	}
 }
 
 func BenchmarkCompareEQFloat(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(f64a, f64b, EQ)
+		testParser.compare(f64a, f64b, EQ)
 	}
 }
 func BenchmarkCompareEQString(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(sa, sb, EQ)
+		testParser.compare(sa, sb, EQ)
 	}
 }
 func BenchmarkCompareEQPString(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(&sa, &sb, EQ)
+		testParser.compare(&sa, &sb, EQ)
 	}
 }
 func BenchmarkCompareEQBytes(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(ba, bb, EQ)
+		testParser.compare(ba, bb, EQ)
 	}
 }
 func BenchmarkCompareEQNil(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(nil, nil, EQ)
+		testParser.compare(nil, nil, EQ)
 	}
 }
 func BenchmarkCompareNEInt(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(i64a, i64b, NE)
+		testParser.compare(i64a, i64b, NE)
 	}
 }
 
 func BenchmarkCompareNEFloat(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(f64a, f64b, NE)
+		testParser.compare(f64a, f64b, NE)
 	}
 }
 func BenchmarkCompareNEString(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(sa, sb, NE)
+		testParser.compare(sa, sb, NE)
 	}
 }
 func BenchmarkCompareLTFloat(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(f64a, f64b, LT)
+		testParser.compare(f64a, f64b, LT)
 	}
 }
 func BenchmarkCompareLTString(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(sa, sb, LT)
+		testParser.compare(sa, sb, LT)
 	}
 }
 func BenchmarkCompareLTNil(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		compare(nil, nil, LT)
+		testParser.compare(nil, nil, LT)
 	}
 }
 

pkg/ottl/go.mod

@@ -10,6 +10,7 @@ require (
 	go.opentelemetry.io/collector/pdata v0.60.1-0.20220928061250-1c217b366fbd
 	go.opentelemetry.io/otel/trace v1.10.0
 	go.uber.org/multierr v1.8.0
+	go.uber.org/zap v1.23.0
 	golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e
 )
 
@@ -29,7 +30,6 @@ require (
 	go.opentelemetry.io/otel v1.10.0 // indirect
 	go.opentelemetry.io/otel/metric v0.32.1 // indirect
 	go.uber.org/atomic v1.10.0 // indirect
-	go.uber.org/zap v1.23.0 // indirect
 	golang.org/x/net v0.0.0-20220225172249-27dd8689420f // indirect
 	golang.org/x/sys v0.0.0-20220808155132-1c4a2a72c664 // indirect
 	golang.org/x/text v0.3.7 // indirect

processor/transformprocessor/config.go

@@ -18,6 +18,7 @@ import (
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config"
 	"go.uber.org/multierr"
+	"go.uber.org/zap"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl"
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/ottldatapoints"
@@ -44,7 +45,7 @@ func (c *Config) Validate() error {
 		traces.Functions(),
 		ottltraces.ParsePath,
 		ottltraces.ParseEnum,
-		component.TelemetrySettings{},
+		component.TelemetrySettings{Logger: zap.NewNop()},
 	)
 	_, err := ottlp.ParseStatements(c.Traces.Queries)
 	if err != nil {
@@ -55,7 +56,7 @@ func (c *Config) Validate() error {
 		metrics.Functions(),
 		ottldatapoints.ParsePath,
 		ottldatapoints.ParseEnum,
-		component.TelemetrySettings{},
+		component.TelemetrySettings{Logger: zap.NewNop()},
 	)
 	_, err = ottlp.ParseStatements(c.Metrics.Queries)
 	if err != nil {
@@ -66,7 +67,7 @@ func (c *Config) Validate() error {
 		logs.Functions(),
 		ottllogs.ParsePath,
 		ottllogs.ParseEnum,
-		component.TelemetrySettings{},
+		component.TelemetrySettings{Logger: zap.NewNop()},
 	)
 	_, err = ottlp.ParseStatements(c.Logs.Queries)
 	if err != nil {

processor/transformprocessor/internal/logs/processor_test.go

@@ -20,7 +20,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
-	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/component/componenttest"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/plog"
 )
@@ -183,7 +183,7 @@ func TestProcess(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.statement, func(t *testing.T) {
 			td := constructLogs()
-			processor, err := NewProcessor([]string{tt.statement}, Functions(), component.TelemetrySettings{})
+			processor, err := NewProcessor([]string{tt.statement}, Functions(), componenttest.NewNopTelemetrySettings())
 			assert.NoError(t, err)
 
 			_, err = processor.ProcessLogs(context.Background(), td)

processor/transformprocessor/internal/metrics/processor_test.go

@@ -20,7 +20,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
-	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/component/componenttest"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/pmetric"
 )
@@ -314,7 +314,7 @@ func TestProcess(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.statements[0], func(t *testing.T) {
 			td := constructMetrics()
-			processor, err := NewProcessor(tt.statements, Functions(), component.TelemetrySettings{})
+			processor, err := NewProcessor(tt.statements, Functions(), componenttest.NewNopTelemetrySettings())
 			assert.NoError(t, err)
 
 			_, err = processor.ProcessMetrics(context.Background(), td)

processor/transformprocessor/internal/traces/processor_test.go

@@ -20,7 +20,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
-	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/component/componenttest"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 )
@@ -223,7 +223,7 @@ func TestProcess(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.statement, func(t *testing.T) {
 			td := constructTraces()
-			processor, err := NewProcessor([]string{tt.statement}, Functions(), component.TelemetrySettings{})
+			processor, err := NewProcessor([]string{tt.statement}, Functions(), componenttest.NewNopTelemetrySettings())
 			assert.NoError(t, err)
 
 			_, err = processor.ProcessTraces(context.Background(), td)
@@ -273,7 +273,7 @@ func BenchmarkTwoSpans(b *testing.B) {
 
 	for _, tt := range tests {
 		b.Run(tt.name, func(b *testing.B) {
-			processor, err := NewProcessor(tt.queries, Functions(), component.TelemetrySettings{})
+			processor, err := NewProcessor(tt.queries, Functions(), componenttest.NewNopTelemetrySettings())
 			assert.NoError(b, err)
 			b.ResetTimer()
 			for n := 0; n < b.N; n++ {
@@ -315,7 +315,7 @@ func BenchmarkHundredSpans(b *testing.B) {
 	}
 	for _, tt := range tests {
 		b.Run(tt.name, func(b *testing.B) {
-			processor, err := NewProcessor(tt.queries, Functions(), component.TelemetrySettings{})
+			processor, err := NewProcessor(tt.queries, Functions(), componenttest.NewNopTelemetrySettings())
 			assert.NoError(b, err)
 			b.ResetTimer()
 			for n := 0; n < b.N; n++ {