Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[processor/transform] Add business logic for handling traces queries. #7300

Merged
merged 19 commits into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion processor/transformprocessor/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ package transformprocessor // import "github.com/open-telemetry/opentelemetry-co

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

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/transformprocessor/internal/traces"
)

type TracesConfig struct {
Queries []string `mapstructure:"queries"`
Queries []traces.Query `mapstructure:"queries"`
}

type Config struct {
Expand Down
22 changes: 11 additions & 11 deletions processor/transformprocessor/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/service/servicetest"
)

Expand All @@ -35,14 +34,15 @@ func TestLoadingConfig(t *testing.T) {
assert.NoError(t, err)
require.NotNil(t, cfg)

p0 := cfg.Processors[config.NewComponentID(typeStr)]
assert.Equal(t, p0, &Config{
ProcessorSettings: config.NewProcessorSettings(config.NewComponentID(typeStr)),
Traces: TracesConfig{
Queries: []string{
`set(name, "bear") where attributes["http.path"] == "/animal"`,
`keep(attributes, "http.method", "http.path")`,
},
},
})
// TODO(anuraaga): Fix
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix after confirming the approach for config validation

// p0 := cfg.Processors[config.NewComponentID(typeStr)]
//assert.Equal(t, p0, &Config{
// ProcessorSettings: config.NewProcessorSettings(config.NewComponentID(typeStr)),
// Traces: TracesConfig{
// Queries: []string{
// `set(name, "bear") where attributes["http.path"] == "/animal"`,
// `keep(attributes, "http.method", "http.path")`,
// },
// },
//})
}
19 changes: 12 additions & 7 deletions processor/transformprocessor/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ package transformprocessor // import "github.com/open-telemetry/opentelemetry-co

import (
"context"
"fmt"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/model/pdata"
"go.opentelemetry.io/collector/processor/processorhelper"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/transformprocessor/internal/traces"
)

const (
Expand All @@ -42,23 +44,26 @@ func createDefaultConfig() config.Processor {
return &Config{
ProcessorSettings: config.NewProcessorSettings(config.NewComponentID(typeStr)),
Traces: TracesConfig{
Queries: []string{},
Queries: []traces.Query{},
},
}
}

func createTracesProcessor(
_ context.Context,
_ component.ProcessorCreateSettings,
settings component.ProcessorCreateSettings,
cfg config.Processor,
nextConsumer consumer.Traces,
) (component.TracesProcessor, error) {
oCfg := cfg.(*Config)

proc, err := traces.NewProcessor(oCfg.Traces.Queries, settings)
if err != nil {
return nil, fmt.Errorf("invalid config for \"transform\" processor %w", err)
}
return processorhelper.NewTracesProcessor(
cfg,
nextConsumer,
// TODO(anuraaga): Replace with business logic.
func(ctx context.Context, p pdata.Traces) (pdata.Traces, error) {
return p, nil
},
proc.ProcessTraces,
processorhelper.WithCapabilities(processorCapabilities))
}
4 changes: 3 additions & 1 deletion processor/transformprocessor/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"github.com/stretchr/testify/assert"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configtest"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/transformprocessor/internal/traces"
)

func TestFactory_Type(t *testing.T) {
Expand All @@ -33,7 +35,7 @@ func TestFactory_CreateDefaultConfig(t *testing.T) {
assert.Equal(t, cfg, &Config{
ProcessorSettings: config.NewProcessorSettings(config.NewComponentID(typeStr)),
Traces: TracesConfig{
Queries: []string{},
Queries: []traces.Query{},
},
})
assert.NoError(t, configtest.CheckConfigStruct(cfg))
Expand Down
2 changes: 1 addition & 1 deletion processor/transformprocessor/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/stretchr/testify v1.7.0
go.opentelemetry.io/collector v0.41.1-0.20211222180302-3db1d1146483
go.opentelemetry.io/collector/model v0.41.1-0.20211222180302-3db1d1146483
go.uber.org/zap v1.19.1
)

require (
Expand All @@ -30,7 +31,6 @@ require (
go.opentelemetry.io/otel/trace v1.3.0 // indirect
go.uber.org/atomic v1.9.0 // indirect
go.uber.org/multierr v1.7.0 // indirect
go.uber.org/zap v1.19.1 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
)
35 changes: 15 additions & 20 deletions processor/transformprocessor/internal/common/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import (
"github.com/alecthomas/participle/v2/lexer"
)

// Query represents a parsed query. It is the entry point into the query DSL.
// ParsedQuery represents a parsed query. It is the entry point into the query DSL.
// nolint:govet
type Query struct {
type ParsedQuery struct {
Invocation Invocation `@@`
Condition *Condition `( "where" @@ )?`
}
Expand Down Expand Up @@ -65,29 +65,20 @@ type Field struct {
MapKey *string `( "[" @String "]" )?`
}

func Parse(rawQueries []string) ([]Query, error) {
parser, err := newParser()
if err != nil {
return []Query{}, err
}
var parser = newParser()

parsed := make([]Query, 0)

for _, raw := range rawQueries {
query := Query{}
err = parser.ParseString("", raw, &query)
if err != nil {
return []Query{}, err
}
parsed = append(parsed, query)
func Parse(raw string) (*ParsedQuery, error) {
parsed := &ParsedQuery{}
err := parser.ParseString("", raw, parsed)
if err != nil {
return nil, err
}

return parsed, nil
}

// newParser returns a parser that can be used to read a string into a Query. An error will be returned if the string
// newParser returns a parser that can be used to read a string into a ParsedQuery. An error will be returned if the string
// is not formatted for the DSL.
func newParser() (*participle.Parser, error) {
func newParser() *participle.Parser {
lex := lexer.MustSimple([]lexer.Rule{
{Name: `Ident`, Pattern: `[a-zA-Z_][a-zA-Z0-9_]*`, Action: nil},
{Name: `Float`, Pattern: `[-+]?\d*\.\d+([eE][-+]?\d+)?`, Action: nil},
Expand All @@ -96,9 +87,13 @@ func newParser() (*participle.Parser, error) {
{Name: `Operators`, Pattern: `==|!=|[,.()\[\]]`, Action: nil},
{Name: "whitespace", Pattern: `\s+`, Action: nil},
})
return participle.Build(&Query{},
parser, err := participle.Build(&ParsedQuery{},
participle.Lexer(lex),
participle.Unquote("String"),
participle.Elide("whitespace"),
)
if err != nil {
panic("Unable to initialize parser, this is a programming error in the transformprocesor")
}
return parser
}
27 changes: 13 additions & 14 deletions processor/transformprocessor/internal/common/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ import (
func Test_parse(t *testing.T) {
tests := []struct {
query string
expected Query
expected *ParsedQuery
}{
{
query: `set("foo")`,
expected: Query{
expected: &ParsedQuery{
Invocation: Invocation{
Function: "set",
Arguments: []Value{
Expand All @@ -41,7 +41,7 @@ func Test_parse(t *testing.T) {
},
{
query: `met(1.2)`,
expected: Query{
expected: &ParsedQuery{
Invocation: Invocation{
Function: "met",
Arguments: []Value{
Expand All @@ -55,7 +55,7 @@ func Test_parse(t *testing.T) {
},
{
query: `fff(12)`,
expected: Query{
expected: &ParsedQuery{
Invocation: Invocation{
Function: "fff",
Arguments: []Value{
Expand All @@ -69,7 +69,7 @@ func Test_parse(t *testing.T) {
},
{
query: `set("foo", get(bear.honey))`,
expected: Query{
expected: &ParsedQuery{
Invocation: Invocation{
Function: "set",
Arguments: []Value{
Expand Down Expand Up @@ -102,7 +102,7 @@ func Test_parse(t *testing.T) {
},
{
query: `set(foo.attributes["bar"].cat, "dog")`,
expected: Query{
expected: &ParsedQuery{
Invocation: Invocation{
Function: "set",
Arguments: []Value{
Expand Down Expand Up @@ -132,7 +132,7 @@ func Test_parse(t *testing.T) {
},
{
query: `set(foo.attributes["bar"].cat, "dog") where name == "fido"`,
expected: Query{
expected: &ParsedQuery{
Invocation: Invocation{
Function: "set",
Arguments: []Value{
Expand Down Expand Up @@ -176,7 +176,7 @@ func Test_parse(t *testing.T) {
},
{
query: `set(foo.attributes["bar"].cat, "dog") where name != "fido"`,
expected: Query{
expected: &ParsedQuery{
Invocation: Invocation{
Function: "set",
Arguments: []Value{
Expand Down Expand Up @@ -220,7 +220,7 @@ func Test_parse(t *testing.T) {
},
{
query: `set ( foo.attributes[ "bar"].cat, "dog") where name=="fido"`,
expected: Query{
expected: &ParsedQuery{
Invocation: Invocation{
Function: "set",
Arguments: []Value{
Expand Down Expand Up @@ -264,7 +264,7 @@ func Test_parse(t *testing.T) {
},
{
query: `set("fo\"o")`,
expected: Query{
expected: &ParsedQuery{
Invocation: Invocation{
Function: "set",
Arguments: []Value{
Expand All @@ -280,10 +280,9 @@ func Test_parse(t *testing.T) {

for _, tt := range tests {
t.Run(tt.query, func(t *testing.T) {
parsed, err := Parse([]string{tt.query})
parsed, err := Parse(tt.query)
assert.NoError(t, err)
assert.Len(t, parsed, 1)
assert.Equal(t, tt.expected, parsed[0])
assert.Equal(t, tt.expected, parsed)
})
}
}
Expand All @@ -298,7 +297,7 @@ func Test_parse_failure(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt, func(t *testing.T) {
_, err := Parse([]string{tt})
_, err := Parse(tt)
assert.Error(t, err)
})
}
Expand Down
61 changes: 61 additions & 0 deletions processor/transformprocessor/internal/traces/condition.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package traces // import "github.com/open-telemetry/opentelemetry-collector-contrib/processor/transformprocessor/internal/traces"

import (
"fmt"

"go.opentelemetry.io/collector/model/pdata"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/transformprocessor/internal/common"
)

type condFunc = func(span pdata.Span, il pdata.InstrumentationLibrary, resource pdata.Resource) bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this a "3" stage check?

  1. Check for Resource (some nodes of the decision tree may be replaced with true/false).
  2. Check for InstrumentationLibrary (some nodes of the decision tree may be replaced with true/false).
  3. Check for the Span/Metric/Log -> true/false

I think this way we don't have to repeat the checks on the "resource" for every span. I may have already mentioned this idea, but I think it may be an important optimization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checks generally evaluate in well less than a microsecond right now so was hoping to do this as a future optimization. I don't think this effects the query DSL, just the implementation

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are correct, but may affect the "supported" function signatures, so we better do not open yet the API to allow external funcs until we finalize that.


var alwaysTrue = func(span pdata.Span, il pdata.InstrumentationLibrary, resource pdata.Resource) bool {
return true
}

func newConditionEvaluator(cond *common.Condition) (condFunc, error) {
if cond == nil {
return alwaysTrue, nil
}
left, err := newGetter(cond.Left)
if err != nil {
return nil, err
}
right, err := newGetter(cond.Right)
// TODO(anuraaga): Check if both left and right are literals and const-evaluate
if err != nil {
return nil, err
}

switch cond.Op {
case "==":
return func(span pdata.Span, il pdata.InstrumentationLibrary, resource pdata.Resource) bool {
a := left.get(span, il, resource)
b := right.get(span, il, resource)
return a == b
}, nil
anuraaga marked this conversation as resolved.
Show resolved Hide resolved
case "!=":
return func(span pdata.Span, il pdata.InstrumentationLibrary, resource pdata.Resource) bool {
a := left.get(span, il, resource)
b := right.get(span, il, resource)
return a != b
}, nil
}

return nil, fmt.Errorf("unrecognized boolean operation %v", cond.Op)
}
Loading