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

…en-telemetry#14468)

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++ {