Skip to content

Commit

Permalink
[internal/filter] filtermetric to filterottl bridge (#23141)
Browse files Browse the repository at this point in the history
**Description:**
This PR adds a bridge between `filtermetric.NewSkipExpr` and
`filterottl.NewBoolExprForMetric` behind a feature gate. With the
feature gate enabled, any component using `filtermetric.NewSkipExpr`
will start using OTTL behind the scenes.

In addition, the filterprocessor's implementation of `newSkipResExpr`,
which is a skip expression for resources, is bridged to
`filterottl.NewBoolExprForResource`. Since this implementation exists
only for filtering metrics, the same feature gate is used for both.

While investigating the existing `internal/filtermetric` uses with
filterprocessor and attributesprocessor I found that:
- The attributes processor does not support Expressions or Resource
Attributes. The readme claims that it does, but the implementation does
not support it (and there are no tests). This bridge DOES NOT rectify
that.
- The filterprocessor allows filtering by resource attributes,
expressions, and metric name. Unlike it's implementation for filtering
spans and logs, the filterprocessor handles filtering spans at the
Resource level loop. This is the most performant solution, and how OTTL
likes to think about the problem, but it results in a different pattern
than filtering spans and logs. This bridge DOES NOT attempt to move the
resource implementation into `internal/filtermetric`.

**Link to tracking Issue:**
Related to
#18643
Related to
#18642

Depends on:
- [x]
#23142

**Testing:**
Added tests comparing the output of the existing config and the bridge.
  • Loading branch information
TylerHelmuth authored Jun 30, 2023
1 parent f1068be commit 46d03a0
Show file tree
Hide file tree
Showing 10 changed files with 554 additions and 61 deletions.
13 changes: 13 additions & 0 deletions internal/filter/filtermetric/filtermetric.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,28 @@
package filtermetric // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter/filtermetric"

import (
"go.opentelemetry.io/collector/featuregate"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter/expr"
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter/filterconfig"
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter/filterottl"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/ottlmetric"
)

var UseOTTLBridge = featuregate.GlobalRegistry().MustRegister(
"filter.filtermetric.useOTTLBridge",
featuregate.StageAlpha,
featuregate.WithRegisterDescription("When enabled, filtermetric will convert filtermetric configuration to OTTL and use filterottl evaluation"),
featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/18642"),
)

// NewSkipExpr creates a BoolExpr that on evaluation returns true if a metric should NOT be processed or kept.
// The logic determining if a metric should be processed is based on include and exclude settings.
// Include properties are checked before exclude settings are checked.
func NewSkipExpr(include *filterconfig.MetricMatchProperties, exclude *filterconfig.MetricMatchProperties) (expr.BoolExpr[ottlmetric.TransformContext], error) {
if UseOTTLBridge.IsEnabled() {
return filterottl.NewMetricSkipExprBridge(include, exclude)
}
var matchers []expr.BoolExpr[ottlmetric.TransformContext]
inclExpr, err := newExpr(include)
if err != nil {
Expand Down
123 changes: 123 additions & 0 deletions internal/filter/filtermetric/filtermetric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ package filtermetric

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/pmetric"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter/filterconfig"
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter/filterottl"
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter/filterset"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/ottlmetric"
)
Expand Down Expand Up @@ -86,3 +89,123 @@ func TestMatcherMatches(t *testing.T) {
})
}
}

func Test_NewSkipExpr_With_Bridge(t *testing.T) {
tests := []struct {
name string
include *filterconfig.MetricMatchProperties
exclude *filterconfig.MetricMatchProperties
err error
}{
// Metric Name
{
name: "single static metric name include",
include: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricStrict,
MetricNames: []string{"metricA"},
},
},
{
name: "multiple static service name include",
include: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricStrict,
MetricNames: []string{"metricB", "metricC"},
},
},
{
name: "single regex service name include",
include: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricRegexp,
MetricNames: []string{".*A"},
},
},
{
name: "multiple regex service name include",
include: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricRegexp,
MetricNames: []string{".*B", ".*C"},
},
},
{
name: "single static metric name exclude",
exclude: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricStrict,
MetricNames: []string{"metricA"},
},
},
{
name: "multiple static service name exclude",
exclude: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricStrict,
MetricNames: []string{"metricB", "metricC"},
},
},
{
name: "single regex service name exclude",
exclude: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricRegexp,
MetricNames: []string{".*A"},
},
},
{
name: "multiple regex service name exclude",
exclude: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricRegexp,
MetricNames: []string{".*B", ".*C"},
},
},

// Expression
{
name: "expression errors",
include: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricExpr,
Expressions: []string{"MetricName == metricA"},
},
err: fmt.Errorf("expressions configuration cannot be converted to OTTL - see https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/filterprocessor#configuration for OTTL configuration"),
},

// Complex
{
name: "complex",
include: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricStrict,
MetricNames: []string{"metricA"},
},
exclude: &filterconfig.MetricMatchProperties{
MatchType: filterconfig.MetricRegexp,
MetricNames: []string{".*B", ".*C"},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
metric := pmetric.NewMetric()
metric.SetName("metricA")

resource := pcommon.NewResource()

scope := pcommon.NewInstrumentationScope()

tCtx := ottlmetric.NewTransformContext(metric, scope, resource)

boolExpr, err := NewSkipExpr(tt.include, tt.exclude)
require.NoError(t, err)
expectedResult, err := boolExpr.Eval(context.Background(), tCtx)
assert.NoError(t, err)

ottlBoolExpr, err := filterottl.NewMetricSkipExprBridge(tt.include, tt.exclude)

if tt.err != nil {
assert.Equal(t, tt.err, err)
} else {
assert.NoError(t, err)
ottlResult, err := ottlBoolExpr.Eval(context.Background(), tCtx)
assert.NoError(t, err)

assert.Equal(t, expectedResult, ottlResult)
}
})
}
}
95 changes: 91 additions & 4 deletions internal/filter/filterottl/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ import (
"github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter/filterset"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/ottllog"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/ottlmetric"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/ottlresource"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/ottlspan"
)

const (
serviceNameStaticStatement = `resource.attributes["service.name"] == "%v"`
spanNameStaticStatement = `name == "%v"`
nameStaticStatement = `name == "%v"`
spanKindStaticStatement = `kind.deprecated_string == "%v"`
scopeNameStaticStatement = `instrumentation_scope.name == "%v"`
scopeVersionStaticStatement = `instrumentation_scope.version == "%v"`
Expand All @@ -30,7 +32,7 @@ const (
severityTextStaticStatement = `severity_text == "%v"`

serviceNameRegexStatement = `IsMatch(resource.attributes["service.name"], "%v")`
spanNameRegexStatement = `IsMatch(name, "%v")`
nameRegexStatement = `IsMatch(name, "%v")`
spanKindRegexStatement = `IsMatch(kind.deprecated_string, "%v")`
scopeNameRegexStatement = `IsMatch(instrumentation_scope.name, "%v")`
scopeVersionRegexStatement = `IsMatch(instrumentation_scope.version, "%v")`
Expand Down Expand Up @@ -83,6 +85,35 @@ func NewLogSkipExprBridge(mc *filterconfig.MatchConfig) (expr.BoolExpr[ottllog.T
return NewBoolExprForLog(statements, StandardLogFuncs(), ottl.PropagateError, component.TelemetrySettings{Logger: zap.NewNop()})
}

func NewResourceSkipExprBridge(mc *filterconfig.MatchConfig) (expr.BoolExpr[ottlresource.TransformContext], error) {
statements := make([]string, 0, 2)
if mc.Include != nil {
// OTTL treats resource attributes as attributes for the resource context.
mc.Include.Attributes = mc.Include.Resources
mc.Include.Resources = nil

statement, err := createStatement(*mc.Include)
if err != nil {
return nil, err
}
statements = append(statements, fmt.Sprintf("not (%v)", statement))
}

if mc.Exclude != nil {
// OTTL treats resource attributes as attributes for the resource context.
mc.Exclude.Attributes = mc.Exclude.Resources
mc.Exclude.Resources = nil

statement, err := createStatement(*mc.Exclude)
if err != nil {
return nil, err
}
statements = append(statements, fmt.Sprintf("%v", statement))
}

return NewBoolExprForResource(statements, StandardResourceFuncs(), ottl.PropagateError, component.TelemetrySettings{Logger: zap.NewNop()})
}

func NewSpanSkipExprBridge(mc *filterconfig.MatchConfig) (expr.BoolExpr[ottlspan.TransformContext], error) {
statements := make([]string, 0, 2)
if mc.Include != nil {
Expand Down Expand Up @@ -239,7 +270,7 @@ func createStatementTemplates(matchType filterset.MatchType) (statementTemplates
case filterset.Strict:
return statementTemplates{
serviceNameStatement: serviceNameStaticStatement,
spanNameStatement: spanNameStaticStatement,
spanNameStatement: nameStaticStatement,
spanKindStatement: spanKindStaticStatement,
scopeNameStatement: scopeNameStaticStatement,
scopeVersionStatement: scopeVersionStaticStatement,
Expand All @@ -251,7 +282,7 @@ func createStatementTemplates(matchType filterset.MatchType) (statementTemplates
case filterset.Regexp:
return statementTemplates{
serviceNameStatement: serviceNameRegexStatement,
spanNameStatement: spanNameRegexStatement,
spanNameStatement: nameRegexStatement,
spanKindStatement: spanKindRegexStatement,
scopeNameStatement: scopeNameRegexStatement,
scopeVersionStatement: scopeVersionRegexStatement,
Expand Down Expand Up @@ -315,3 +346,59 @@ func createSeverityNumberConditions(severityNumberProperties *filterconfig.LogSe
severityNumberCondition := fmt.Sprintf(severityNumberStatement, severityNumberProperties.MatchUndefined, severityNumberProperties.Min)
return &severityNumberCondition
}

func NewMetricSkipExprBridge(include *filterconfig.MetricMatchProperties, exclude *filterconfig.MetricMatchProperties) (expr.BoolExpr[ottlmetric.TransformContext], error) {
statements := make([]string, 0, 2)
if include != nil {
statement, err := createMetricStatement(*include)
if err != nil {
return nil, err
}
if statement != nil {
statements = append(statements, fmt.Sprintf("not (%v)", *statement))
}
}

if exclude != nil {
statement, err := createMetricStatement(*exclude)
if err != nil {
return nil, err
}
if statement != nil {
statements = append(statements, fmt.Sprintf("%v", *statement))
}
}

if len(statements) == 0 {
return nil, nil
}

return NewBoolExprForMetric(statements, StandardMetricFuncs(), ottl.PropagateError, component.TelemetrySettings{Logger: zap.NewNop()})
}

func createMetricStatement(mp filterconfig.MetricMatchProperties) (*string, error) {
if mp.MatchType == filterconfig.MetricExpr {
if len(mp.Expressions) == 0 {
return nil, nil
}
return nil, fmt.Errorf("expressions configuration cannot be converted to OTTL - see https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/filterprocessor#configuration for OTTL configuration")
}

if len(mp.MetricNames) == 0 {
return nil, nil
}

metricNameStatement := nameStaticStatement
if mp.MatchType == filterconfig.MetricRegexp {
metricNameStatement = nameRegexStatement
}
metricNameConditions := createBasicConditions(metricNameStatement, mp.MetricNames)
var format string
if len(metricNameConditions) > 1 {
format = "(%v)"
} else {
format = "%v"
}
statement := fmt.Sprintf(format, strings.Join(metricNameConditions, " or "))
return &statement, nil
}
Loading

0 comments on commit 46d03a0

Please sign in to comment.