From 2b013ec107270c94b723d504058fbe49699e5583 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Mon, 3 May 2021 13:03:08 -0400 Subject: [PATCH 01/26] Add auto generated IDs to operators --- operator/helper/operator.go | 5 +++++ operator/operator.go | 3 +++ pipeline/config.go | 16 ++++++++++++++++ testutil/mocks.go | 3 +++ testutil/operator.go | 5 +++++ 5 files changed, 32 insertions(+) diff --git a/operator/helper/operator.go b/operator/helper/operator.go index f86b0a02..7ce68c4c 100644 --- a/operator/helper/operator.go +++ b/operator/helper/operator.go @@ -92,6 +92,11 @@ func (p *BasicOperator) ID() string { return p.OperatorID } +// SetID sets a new ID on the operator. +func (p *BasicOperator) SetID(id string) { + p.OperatorID = id +} + // Type will return the operator type. func (p *BasicOperator) Type() string { return p.OperatorType diff --git a/operator/operator.go b/operator/operator.go index d4e28469..34e583f7 100644 --- a/operator/operator.go +++ b/operator/operator.go @@ -28,6 +28,9 @@ import ( type Operator interface { // ID returns the id of the operator. ID() string + // SetID sets the id of the operator. + SetID(string) + // Type returns the type of the operator. Type() string diff --git a/pipeline/config.go b/pipeline/config.go index cf43ef02..c68e0c3c 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -15,6 +15,8 @@ package pipeline import ( + "fmt" + "github.com/open-telemetry/opentelemetry-log-collection/operator" ) @@ -32,9 +34,23 @@ func (c Config) BuildOperators(bc operator.BuildContext) ([]operator.Operator, e } operators = append(operators, op...) } + dedeplucateIDs(operators) return operators, nil } +func dedeplucateIDs(ops []operator.Operator) error { + typeMap := make(map[string]int) + for _, op := range ops { + if op.ID() != op.Type() { + continue + } + + op.SetID(fmt.Sprintf("%s%d", op.Type(), typeMap[op.Type()])) + typeMap[op.Type()]++ + } + return nil +} + // BuildPipeline will build a pipeline from the config. func (c Config) BuildPipeline(bc operator.BuildContext, defaultOperator operator.Operator) (*DirectedPipeline, error) { if defaultOperator != nil { diff --git a/testutil/mocks.go b/testutil/mocks.go index 65af6835..3f3414e9 100644 --- a/testutil/mocks.go +++ b/testutil/mocks.go @@ -59,6 +59,9 @@ func (f *FakeOutput) CanProcess() bool { return true } // ID always returns `fake` as the ID of a fake output operator func (f *FakeOutput) ID() string { return "$.fake" } +// SetID returns nothing +func (f *FakeOutput) SetID(string) {} + // Logger returns the logger of a fake output func (f *FakeOutput) Logger() *zap.SugaredLogger { return f.SugaredLogger } diff --git a/testutil/operator.go b/testutil/operator.go index 20ad941b..a29f7067 100644 --- a/testutil/operator.go +++ b/testutil/operator.go @@ -106,6 +106,11 @@ func (_m *Operator) Process(_a0 context.Context, _a1 *entry.Entry) error { return r0 } +// SetID provides a mock function with given fields: _a0 +func (_m *Operator) SetID(_a0 string) { + _m.Called(_a0) +} + // SetOutputs provides a mock function with given fields: _a0 func (_m *Operator) SetOutputs(_a0 []operator.Operator) error { ret := _m.Called(_a0) From a993f0b662a6c9311910bb40c7e3e8335b766391 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Fri, 7 May 2021 15:19:00 -0400 Subject: [PATCH 02/26] Move generation spot --- pipeline/directed.go | 14 +++++++++----- pipeline/directed_test.go | 6 +++--- testutil/mocks.go | 2 ++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/pipeline/directed.go b/pipeline/directed.go index bba1939f..d63da41c 100644 --- a/pipeline/directed.go +++ b/pipeline/directed.go @@ -80,13 +80,17 @@ func (p *DirectedPipeline) Operators() []operator.Operator { // addNodes will add operators as nodes to the supplied graph. func addNodes(graph *simple.DirectedGraph, operators []operator.Operator) error { + typeMap := make(map[string]int) for _, operator := range operators { operatorNode := createOperatorNode(operator) - if graph.Node(operatorNode.ID()) != nil { - return errors.NewError( - fmt.Sprintf("operator with id '%s' already exists in pipeline", operatorNode.Operator().ID()), - "ensure that each operator has a unique `type` or `id`", - ) + for graph.Node(operatorNode.ID()) != nil { + typeMap[operator.Type()]++ + operator.SetID(fmt.Sprintf("%s%d", operator.Type(), typeMap[operator.Type()])) + operatorNode = createOperatorNode(operator) + // return errors.NewError( + // fmt.Sprintf("operator with id '%s' already exists in pipeline", operatorNode.Operator().ID()), + // "ensure that each operator has a unique `type` or `id`", + // ) } graph.AddNode(operatorNode) diff --git a/pipeline/directed_test.go b/pipeline/directed_test.go index 73bd7379..49f86b52 100644 --- a/pipeline/directed_test.go +++ b/pipeline/directed_test.go @@ -115,9 +115,9 @@ func TestPipeline(t *testing.T) { operator2.On("SetOutputs", mock.Anything).Return(nil) operator2.On("Outputs").Return(nil) - _, err := NewDirectedPipeline([]operator.Operator{operator1, operator2}) - require.Error(t, err) - require.Contains(t, err.Error(), "already exists") + _, _ = NewDirectedPipeline([]operator.Operator{operator1, operator2}) + // require.Error(t, err) + // require.Contains(t, err.Error(), "already exists") }) t.Run("OutputNotExist", func(t *testing.T) { diff --git a/testutil/mocks.go b/testutil/mocks.go index 3f3414e9..ae53baeb 100644 --- a/testutil/mocks.go +++ b/testutil/mocks.go @@ -33,6 +33,8 @@ func NewMockOperator(id string) *Operator { mockOutput.On("ID").Return(id) mockOutput.On("CanProcess").Return(true) mockOutput.On("CanOutput").Return(true) + mockOutput.On("Type").Return("fake_output") + mockOutput.On("SetID").Return(id) return mockOutput } From d0153773def291ec5655d917237224cd7326b0de Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Tue, 11 May 2021 11:01:02 -0400 Subject: [PATCH 03/26] Add Testing --- pipeline/config.go | 19 ++- pipeline/config_test.go | 235 ++++++++++++++++++++++++++++++++++++++ pipeline/directed.go | 12 +- pipeline/directed_test.go | 9 +- testutil/mocks.go | 1 - 5 files changed, 259 insertions(+), 17 deletions(-) create mode 100644 pipeline/config_test.go diff --git a/pipeline/config.go b/pipeline/config.go index c68e0c3c..18ceeff8 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -26,6 +26,7 @@ type Config []operator.Config // BuildOperators builds the operators from the list of configs into operators func (c Config) BuildOperators(bc operator.BuildContext) ([]operator.Operator, error) { operators := make([]operator.Operator, 0, len(c)) + // dedeplucateIDs(operators) for i, builder := range c { nbc := getBuildContextWithDefaultOutput(c, i, bc) op, err := builder.Build(nbc) @@ -34,7 +35,6 @@ func (c Config) BuildOperators(bc operator.BuildContext) ([]operator.Operator, e } operators = append(operators, op...) } - dedeplucateIDs(operators) return operators, nil } @@ -45,7 +45,21 @@ func dedeplucateIDs(ops []operator.Operator) error { continue } - op.SetID(fmt.Sprintf("%s%d", op.Type(), typeMap[op.Type()])) + if typeMap[op.Type()] == 0 { + typeMap[op.Type()]++ + continue + } + newID := fmt.Sprintf("%s%d", op.Type(), typeMap[op.Type()]) + for i := 0; i < len(ops); i++ { + if ops[i].ID() == newID { + typeMap[op.Type()]++ + newID = fmt.Sprintf("%s%d", op.Type(), typeMap[op.Type()]) + i = 0 + continue + } + } + + op.SetID(newID) typeMap[op.Type()]++ } return nil @@ -53,6 +67,7 @@ func dedeplucateIDs(ops []operator.Operator) error { // BuildPipeline will build a pipeline from the config. func (c Config) BuildPipeline(bc operator.BuildContext, defaultOperator operator.Operator) (*DirectedPipeline, error) { + if defaultOperator != nil { bc.DefaultOutputIDs = []string{defaultOperator.ID()} } diff --git a/pipeline/config_test.go b/pipeline/config_test.go new file mode 100644 index 00000000..5635ec93 --- /dev/null +++ b/pipeline/config_test.go @@ -0,0 +1,235 @@ +// 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 pipeline + +import ( + "context" + "fmt" + "testing" + + "github.com/open-telemetry/opentelemetry-log-collection/entry" + "github.com/open-telemetry/opentelemetry-log-collection/operator" + "github.com/open-telemetry/opentelemetry-log-collection/operator/helper" + "github.com/stretchr/testify/require" +) + +type dummyOp struct { + helper.BasicOperator + OutputIDs []string + OutputOps []operator.Operator +} + +func newDummyOp(dummyID string, dummyType string) *dummyOp { + return &dummyOp{ + BasicOperator: helper.BasicOperator{ + OperatorID: dummyID, + OperatorType: dummyType, + }, + } +} + +// SetOutputs will set the outputs of the operator. +func (w *dummyOp) SetOutputs(operators []operator.Operator) error { + outputOperators := make([]operator.Operator, 0) + + for _, operatorID := range w.OutputIDs { + operator, ok := w.findOperator(operators, operatorID) + if !ok { + return fmt.Errorf("operator '%s' does not exist", operatorID) + } + + if !operator.CanProcess() { + return fmt.Errorf("operator '%s' can not process entries", operatorID) + } + + outputOperators = append(outputOperators, operator) + } + + w.OutputOps = outputOperators + return nil +} + +// FindOperator will find an operator matching the supplied id. +func (w *dummyOp) findOperator(operators []operator.Operator, operatorID string) (operator.Operator, bool) { + for _, operator := range operators { + if operator.ID() == operatorID { + return operator, true + } + } + return nil, false +} + +func (d dummyOp) CanOutput() bool { + return true +} +func (d dummyOp) CanProcess() bool { + return true +} + +func (d dummyOp) Outputs() []operator.Operator { + return d.OutputOps +} + +func (d *dummyOp) Process(context.Context, *entry.Entry) error { + return nil +} + +type deduplicateTestCase struct { + name string + ops func() []operator.Operator + expectedOps []operator.Operator +} + +func TestDeduplicateIDs(t *testing.T) { + cases := []deduplicateTestCase{ + { + "one_op_rename", + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser1", "json_parser")) + return ops + }(), + }, + { + "multi_op_rename", + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser1", "json_parser")) + ops = append(ops, newDummyOp("json_parser2", "json_parser")) + ops = append(ops, newDummyOp("json_parser3", "json_parser")) + ops = append(ops, newDummyOp("json_parser4", "json_parser")) + return ops + }(), + }, + { + "different_ops", + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("copy", "copy")) + ops = append(ops, newDummyOp("copy", "copy")) + + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser1", "json_parser")) + ops = append(ops, newDummyOp("json_parser2", "json_parser")) + ops = append(ops, newDummyOp("copy", "copy")) + ops = append(ops, newDummyOp("copy1", "copy")) + return ops + }(), + }, + { + "unordered", + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("copy", "copy")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("copy", "copy")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("copy", "copy")) + ops = append(ops, newDummyOp("json_parser1", "json_parser")) + ops = append(ops, newDummyOp("copy1", "copy")) + ops = append(ops, newDummyOp("json_parser2", "json_parser")) + return ops + }(), + }, + { + "already_renamed", + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser3", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser1", "json_parser")) + ops = append(ops, newDummyOp("json_parser2", "json_parser")) + ops = append(ops, newDummyOp("json_parser3", "json_parser")) + ops = append(ops, newDummyOp("json_parser4", "json_parser")) + return ops + }(), + }, + } + + for _, tc := range cases { + t.Run("Deduplicate/"+tc.name, func(t *testing.T) { + ops := tc.ops() + dedeplucateIDs(ops) + require.Equal(t, ops, tc.expectedOps) + }) + } + + cases = []deduplicateTestCase{ + { + "one_op_rename", + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser", "json_parser")) + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + ops = append(ops, newDummyOp("json_parser", "json_parser")) + ops = append(ops, newDummyOp("json_parser1", "json_parser")) + return ops + }(), + }, + } + + for _, tc := range cases { + t.Run("DeduplicateAndFixOutputs/"+tc.name, func(t *testing.T) { + ops := tc.ops() + dedeplucateIDs(ops) + require.Equal(t, ops, tc.expectedOps) + }) + } +} diff --git a/pipeline/directed.go b/pipeline/directed.go index d63da41c..56fa9f40 100644 --- a/pipeline/directed.go +++ b/pipeline/directed.go @@ -80,17 +80,13 @@ func (p *DirectedPipeline) Operators() []operator.Operator { // addNodes will add operators as nodes to the supplied graph. func addNodes(graph *simple.DirectedGraph, operators []operator.Operator) error { - typeMap := make(map[string]int) for _, operator := range operators { operatorNode := createOperatorNode(operator) for graph.Node(operatorNode.ID()) != nil { - typeMap[operator.Type()]++ - operator.SetID(fmt.Sprintf("%s%d", operator.Type(), typeMap[operator.Type()])) - operatorNode = createOperatorNode(operator) - // return errors.NewError( - // fmt.Sprintf("operator with id '%s' already exists in pipeline", operatorNode.Operator().ID()), - // "ensure that each operator has a unique `type` or `id`", - // ) + return errors.NewError( + fmt.Sprintf("operator with id '%s' already exists in pipeline", operatorNode.Operator().ID()), + "ensure that each operator has a unique `type` or `id`", + ) } graph.AddNode(operatorNode) diff --git a/pipeline/directed_test.go b/pipeline/directed_test.go index 49f86b52..862e0eee 100644 --- a/pipeline/directed_test.go +++ b/pipeline/directed_test.go @@ -108,16 +108,13 @@ func TestPipeline(t *testing.T) { }) t.Run("DuplicateNodeIDs", func(t *testing.T) { - operator1 := testutil.NewMockOperator("operator1") + operator1 := testutil.NewMockOperator("fake_output") operator1.On("SetOutputs", mock.Anything).Return(nil) operator1.On("Outputs").Return(nil) - operator2 := testutil.NewMockOperator("operator1") + operator1.On("SetID", mock.Anything).Return(nil) + operator2 := testutil.NewMockOperator("fake_output") operator2.On("SetOutputs", mock.Anything).Return(nil) operator2.On("Outputs").Return(nil) - - _, _ = NewDirectedPipeline([]operator.Operator{operator1, operator2}) - // require.Error(t, err) - // require.Contains(t, err.Error(), "already exists") }) t.Run("OutputNotExist", func(t *testing.T) { diff --git a/testutil/mocks.go b/testutil/mocks.go index ae53baeb..7619a6ce 100644 --- a/testutil/mocks.go +++ b/testutil/mocks.go @@ -34,7 +34,6 @@ func NewMockOperator(id string) *Operator { mockOutput.On("CanProcess").Return(true) mockOutput.On("CanOutput").Return(true) mockOutput.On("Type").Return("fake_output") - mockOutput.On("SetID").Return(id) return mockOutput } From 7e4cacebf55e2ed57d830dc8ceb3f11982226a4d Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Wed, 12 May 2021 09:20:40 -0400 Subject: [PATCH 04/26] Update where IDs are changed --- operator/config.go | 1 + operator/config_test.go | 1 + operator/helper/operator.go | 6 + operator/operator.go | 2 - pipeline/config.go | 4 +- pipeline/config_test.go | 230 +++++++++++++----------------------- 6 files changed, 95 insertions(+), 149 deletions(-) diff --git a/operator/config.go b/operator/config.go index c48816b3..1eae47ad 100644 --- a/operator/config.go +++ b/operator/config.go @@ -31,6 +31,7 @@ type Builder interface { ID() string Type() string Build(BuildContext) ([]Operator, error) + SetID(string) error } // UnmarshalJSON will unmarshal a config from JSON. diff --git a/operator/config_test.go b/operator/config_test.go index 0c2260cd..34457b82 100644 --- a/operator/config_test.go +++ b/operator/config_test.go @@ -31,6 +31,7 @@ type FakeBuilder struct { func (f *FakeBuilder) Build(context BuildContext) ([]Operator, error) { return nil, nil } func (f *FakeBuilder) ID() string { return "plugin" } func (f *FakeBuilder) Type() string { return "plugin" } +func (f *FakeBuilder) SetID(s string) error { return nil } func TestUnmarshalJSONErrors(t *testing.T) { t.Cleanup(func() { diff --git a/operator/helper/operator.go b/operator/helper/operator.go index 7ce68c4c..68b12b12 100644 --- a/operator/helper/operator.go +++ b/operator/helper/operator.go @@ -43,6 +43,12 @@ func (c BasicConfig) ID() string { return c.OperatorID } +// ID will return the operator id. +func (c BasicConfig) SetID(id string) error { + c.OperatorID = id + return nil +} + // Type will return the operator type. func (c BasicConfig) Type() string { return c.OperatorType diff --git a/operator/operator.go b/operator/operator.go index 34e583f7..4228a2a3 100644 --- a/operator/operator.go +++ b/operator/operator.go @@ -28,8 +28,6 @@ import ( type Operator interface { // ID returns the id of the operator. ID() string - // SetID sets the id of the operator. - SetID(string) // Type returns the type of the operator. Type() string diff --git a/pipeline/config.go b/pipeline/config.go index 18ceeff8..40aa8926 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -26,7 +26,7 @@ type Config []operator.Config // BuildOperators builds the operators from the list of configs into operators func (c Config) BuildOperators(bc operator.BuildContext) ([]operator.Operator, error) { operators := make([]operator.Operator, 0, len(c)) - // dedeplucateIDs(operators) + dedeplucateIDs(c) for i, builder := range c { nbc := getBuildContextWithDefaultOutput(c, i, bc) op, err := builder.Build(nbc) @@ -38,7 +38,7 @@ func (c Config) BuildOperators(bc operator.BuildContext) ([]operator.Operator, e return operators, nil } -func dedeplucateIDs(ops []operator.Operator) error { +func dedeplucateIDs(ops []operator.Config) error { typeMap := make(map[string]int) for _, op := range ops { if op.ID() != op.Type() { diff --git a/pipeline/config_test.go b/pipeline/config_test.go index 5635ec93..1678bcea 100644 --- a/pipeline/config_test.go +++ b/pipeline/config_test.go @@ -15,185 +15,151 @@ package pipeline import ( - "context" - "fmt" "testing" - "github.com/open-telemetry/opentelemetry-log-collection/entry" "github.com/open-telemetry/opentelemetry-log-collection/operator" "github.com/open-telemetry/opentelemetry-log-collection/operator/helper" "github.com/stretchr/testify/require" ) -type dummyOp struct { - helper.BasicOperator - OutputIDs []string - OutputOps []operator.Operator -} - -func newDummyOp(dummyID string, dummyType string) *dummyOp { - return &dummyOp{ - BasicOperator: helper.BasicOperator{ - OperatorID: dummyID, - OperatorType: dummyType, - }, - } -} - -// SetOutputs will set the outputs of the operator. -func (w *dummyOp) SetOutputs(operators []operator.Operator) error { - outputOperators := make([]operator.Operator, 0) - - for _, operatorID := range w.OutputIDs { - operator, ok := w.findOperator(operators, operatorID) - if !ok { - return fmt.Errorf("operator '%s' does not exist", operatorID) - } +type dummyOp helper.BasicConfig - if !operator.CanProcess() { - return fmt.Errorf("operator '%s' can not process entries", operatorID) - } - - outputOperators = append(outputOperators, operator) - } - - w.OutputOps = outputOperators - return nil +func (d *dummyOp) ID() string { + return d.OperatorID } -// FindOperator will find an operator matching the supplied id. -func (w *dummyOp) findOperator(operators []operator.Operator, operatorID string) (operator.Operator, bool) { - for _, operator := range operators { - if operator.ID() == operatorID { - return operator, true - } - } - return nil, false +func (d *dummyOp) Type() string { + return d.OperatorType } -func (d dummyOp) CanOutput() bool { - return true -} -func (d dummyOp) CanProcess() bool { - return true +func (d *dummyOp) SetID(id string) error { + d.OperatorID = id + return nil } -func (d dummyOp) Outputs() []operator.Operator { - return d.OutputOps +func (d *dummyOp) Build(bc operator.BuildContext) ([]operator.Operator, error) { + return nil, nil } -func (d *dummyOp) Process(context.Context, *entry.Entry) error { - return nil +func newDummyOp(dummyID string, dummyType string) *dummyOp { + return &dummyOp{ + OperatorID: dummyID, + OperatorType: dummyType, + } } type deduplicateTestCase struct { name string - ops func() []operator.Operator - expectedOps []operator.Operator + ops func() []operator.Config + expectedOps []operator.Config } func TestDeduplicateIDs(t *testing.T) { cases := []deduplicateTestCase{ { "one_op_rename", - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) + func() []operator.Config { + var ops []operator.Config + op1 := operator.Config{Builder: newDummyOp("json_parser", "json_parser")} + ops = append(ops, op1) + op2 := operator.Config{Builder: newDummyOp("json_parser", "json_parser")} + ops = append(ops, op2) return ops }, - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser1", "json_parser")) + func() []operator.Config { + var ops []operator.Config + op1 := operator.Config{Builder: newDummyOp("json_parser", "json_parser")} + ops = append(ops, op1) + op2 := operator.Config{Builder: newDummyOp("json_parser1", "json_parser")} + ops = append(ops, op2) return ops }(), }, { "multi_op_rename", - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) + func() []operator.Config { + var ops []operator.Config + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) return ops }, - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser1", "json_parser")) - ops = append(ops, newDummyOp("json_parser2", "json_parser")) - ops = append(ops, newDummyOp("json_parser3", "json_parser")) - ops = append(ops, newDummyOp("json_parser4", "json_parser")) + func() []operator.Config { + var ops []operator.Config + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser1", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser2", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser3", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser4", "json_parser")}) return ops }(), }, { "different_ops", - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("copy", "copy")) - ops = append(ops, newDummyOp("copy", "copy")) + func() []operator.Config { + var ops []operator.Config + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) return ops }, - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser1", "json_parser")) - ops = append(ops, newDummyOp("json_parser2", "json_parser")) - ops = append(ops, newDummyOp("copy", "copy")) - ops = append(ops, newDummyOp("copy1", "copy")) + func() []operator.Config { + var ops []operator.Config + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser1", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser2", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOp("copy1", "copy")}) return ops }(), }, { "unordered", - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("copy", "copy")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("copy", "copy")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) + func() []operator.Config { + var ops []operator.Config + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) return ops }, - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("copy", "copy")) - ops = append(ops, newDummyOp("json_parser1", "json_parser")) - ops = append(ops, newDummyOp("copy1", "copy")) - ops = append(ops, newDummyOp("json_parser2", "json_parser")) + func() []operator.Config { + var ops []operator.Config + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser1", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("copy1", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser2", "json_parser")}) return ops }(), }, { "already_renamed", - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser3", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) + func() []operator.Config { + var ops []operator.Config + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser3", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) return ops }, - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser1", "json_parser")) - ops = append(ops, newDummyOp("json_parser2", "json_parser")) - ops = append(ops, newDummyOp("json_parser3", "json_parser")) - ops = append(ops, newDummyOp("json_parser4", "json_parser")) + func() []operator.Config { + var ops []operator.Config + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser1", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser2", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser3", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOp("json_parser4", "json_parser")}) return ops }(), }, @@ -206,30 +172,4 @@ func TestDeduplicateIDs(t *testing.T) { require.Equal(t, ops, tc.expectedOps) }) } - - cases = []deduplicateTestCase{ - { - "one_op_rename", - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser", "json_parser")) - return ops - }, - func() []operator.Operator { - var ops []operator.Operator - ops = append(ops, newDummyOp("json_parser", "json_parser")) - ops = append(ops, newDummyOp("json_parser1", "json_parser")) - return ops - }(), - }, - } - - for _, tc := range cases { - t.Run("DeduplicateAndFixOutputs/"+tc.name, func(t *testing.T) { - ops := tc.ops() - dedeplucateIDs(ops) - require.Equal(t, ops, tc.expectedOps) - }) - } } From 7358fff613465712f32e05dde369b0491df28183 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Wed, 12 May 2021 15:29:52 -0400 Subject: [PATCH 05/26] Add testing for OutputIDs --- operator/helper/operator.go | 7 +- pipeline/config_test.go | 407 +++++++++++++++++++++++++++++------- 2 files changed, 332 insertions(+), 82 deletions(-) diff --git a/operator/helper/operator.go b/operator/helper/operator.go index 68b12b12..f7bf6323 100644 --- a/operator/helper/operator.go +++ b/operator/helper/operator.go @@ -43,7 +43,7 @@ func (c BasicConfig) ID() string { return c.OperatorID } -// ID will return the operator id. +// SetID will Update the operator id. func (c BasicConfig) SetID(id string) error { c.OperatorID = id return nil @@ -98,11 +98,6 @@ func (p *BasicOperator) ID() string { return p.OperatorID } -// SetID sets a new ID on the operator. -func (p *BasicOperator) SetID(id string) { - p.OperatorID = id -} - // Type will return the operator type. func (p *BasicOperator) Type() string { return p.OperatorType diff --git a/pipeline/config_test.go b/pipeline/config_test.go index 1678bcea..124b05bd 100644 --- a/pipeline/config_test.go +++ b/pipeline/config_test.go @@ -15,151 +15,247 @@ package pipeline import ( + "context" + "fmt" "testing" + "github.com/open-telemetry/opentelemetry-log-collection/entry" "github.com/open-telemetry/opentelemetry-log-collection/operator" "github.com/open-telemetry/opentelemetry-log-collection/operator/helper" + "github.com/open-telemetry/opentelemetry-log-collection/testutil" "github.com/stretchr/testify/require" + "go.uber.org/zap" ) -type dummyOp helper.BasicConfig +type dummyOpConfig struct { + helper.WriterConfig +} -func (d *dummyOp) ID() string { +func (d *dummyOpConfig) ID() string { return d.OperatorID } -func (d *dummyOp) Type() string { +func (d *dummyOpConfig) Type() string { return d.OperatorType } -func (d *dummyOp) SetID(id string) error { +func (d *dummyOpConfig) SetID(id string) error { d.OperatorID = id return nil } -func (d *dummyOp) Build(bc operator.BuildContext) ([]operator.Operator, error) { - return nil, nil +func (c *dummyOpConfig) Build(bc operator.BuildContext) ([]operator.Operator, error) { + // Namespace all the output IDs + namespacedID := bc.PrependNamespace(c.ID()) + + namespacedIDs := c.OutputIDs.WithNamespace(bc) + if len(namespacedIDs) == 0 { + namespacedIDs = bc.DefaultOutputIDs + } + + dummy := dummyOperator{ + WriterOperator: helper.WriterOperator{ + OutputIDs: namespacedIDs, + BasicOperator: helper.BasicOperator{ + OperatorType: c.OperatorType, + OperatorID: namespacedID, + }, + }, + } + + return []operator.Operator{dummy}, nil +} + +type dummyOperator struct { + helper.WriterOperator +} + +// ID returns the id of the operator. +func (d dummyOperator) ID() string { return d.OperatorID } + +// Type returns the type of the operator. +func (d dummyOperator) Type() string { return d.OperatorType } + +func (d dummyOperator) CanOutput() bool { return true } + +func (d dummyOperator) CanProcess() bool { return true } + +func (d dummyOperator) SetOutputs(operators []operator.Operator) error { + outputOperators := make([]operator.Operator, 0) + + for _, operatorID := range d.OutputIDs { + operator, ok := d.findOperator(operators, operatorID) + if !ok { + return fmt.Errorf("operator '%s' does not exist", operatorID) + } + + if !operator.CanProcess() { + return fmt.Errorf("operator '%s' can not process entries", operatorID) + } + + outputOperators = append(outputOperators, operator) + } + + d.OutputOperators = outputOperators + return nil } -func newDummyOp(dummyID string, dummyType string) *dummyOp { - return &dummyOp{ - OperatorID: dummyID, - OperatorType: dummyType, +func (w *dummyOperator) findOperator(operators []operator.Operator, operatorID string) (operator.Operator, bool) { + for _, operator := range operators { + if operator.ID() == operatorID { + return operator, true + } + } + return nil, false +} + +func (d dummyOperator) Outputs() []operator.Operator { return nil } + +func (d dummyOperator) Start(operator.Persister) error { return nil } + +func (d dummyOperator) Stop() error { return nil } + +// Process will process an entry from an operator. +func (d dummyOperator) Process(context.Context, *entry.Entry) error { return nil } + +// Logger returns the operator's logger +func (d dummyOperator) Logger() *zap.SugaredLogger { return nil } + +func newDummyOpConfig(dummyID string, dummyType string) *dummyOpConfig { + return &dummyOpConfig{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: dummyID, + OperatorType: dummyType, + }, + }, + } +} + +func newDummyOp(dummyID string, dummyType string) dummyOperator { + return dummyOperator{ + WriterOperator: helper.WriterOperator{ + BasicOperator: helper.BasicOperator{ + OperatorID: dummyID, + OperatorType: dummyType, + SugaredLogger: nil, + }, + }, } } type deduplicateTestCase struct { name string - ops func() []operator.Config - expectedOps []operator.Config + ops func() Config + expectedOps Config } func TestDeduplicateIDs(t *testing.T) { cases := []deduplicateTestCase{ { "one_op_rename", - func() []operator.Config { - var ops []operator.Config - op1 := operator.Config{Builder: newDummyOp("json_parser", "json_parser")} + func() Config { + var ops Config + op1 := operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")} ops = append(ops, op1) - op2 := operator.Config{Builder: newDummyOp("json_parser", "json_parser")} + op2 := operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")} ops = append(ops, op2) return ops }, - func() []operator.Config { - var ops []operator.Config - op1 := operator.Config{Builder: newDummyOp("json_parser", "json_parser")} + func() Config { + var ops Config + op1 := operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")} ops = append(ops, op1) - op2 := operator.Config{Builder: newDummyOp("json_parser1", "json_parser")} + op2 := operator.Config{Builder: newDummyOpConfig("json_parser1", "json_parser")} ops = append(ops, op2) return ops }(), }, { "multi_op_rename", - func() []operator.Config { - var ops []operator.Config - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) return ops }, - func() []operator.Config { - var ops []operator.Config - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser1", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser2", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser3", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser4", "json_parser")}) + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser1", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser2", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser4", "json_parser")}) return ops }(), }, { "different_ops", - func() []operator.Config { - var ops []operator.Config - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) return ops }, - func() []operator.Config { - var ops []operator.Config - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser1", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser2", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOp("copy1", "copy")}) + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser1", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser2", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy1", "copy")}) return ops }(), }, { "unordered", - func() []operator.Config { - var ops []operator.Config - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) return ops }, - func() []operator.Config { - var ops []operator.Config - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser1", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("copy1", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser2", "json_parser")}) + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser1", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy1", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser2", "json_parser")}) return ops }(), }, { "already_renamed", - func() []operator.Config { - var ops []operator.Config - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser3", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) return ops }, - func() []operator.Config { - var ops []operator.Config - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser1", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser2", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser3", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOp("json_parser4", "json_parser")}) + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser1", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser2", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser4", "json_parser")}) return ops }(), }, @@ -173,3 +269,162 @@ func TestDeduplicateIDs(t *testing.T) { }) } } + +type outputTestCase struct { + name string + ops func() Config + expectedOps []operator.Operator +} + +func TestUpdateOutputIDs(t *testing.T) { + cases := []outputTestCase{ + { + "one_op_rename", + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + op1 := newDummyOp("$.json_parser", "json_parser") + op1.OutputIDs = []string{"$.json_parser1"} + ops = append(ops, op1) + ops = append(ops, newDummyOp("$.json_parser1", "json_parser")) + return ops + }(), + }, + { + "multi_op_rename", + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + op1 := newDummyOp("$.json_parser", "json_parser") + op1.OutputIDs = []string{"$.json_parser1"} + ops = append(ops, op1) + op2 := newDummyOp("$.json_parser1", "json_parser") + op2.OutputIDs = []string{"$.json_parser2"} + ops = append(ops, op2) + op3 := newDummyOp("$.json_parser2", "json_parser") + op3.OutputIDs = []string{"$.json_parser3"} + ops = append(ops, op3) + op4 := newDummyOp("$.json_parser3", "json_parser") + op4.OutputIDs = []string{"$.json_parser4"} + ops = append(ops, op4) + op5 := newDummyOp("$.json_parser4", "json_parser") + ops = append(ops, op5) + return ops + }(), + }, + { + "different_ops", + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + op1 := newDummyOp("$.json_parser", "json_parser") + op1.OutputIDs = []string{"$.json_parser1"} + ops = append(ops, op1) + op2 := newDummyOp("$.json_parser1", "json_parser") + op2.OutputIDs = []string{"$.json_parser2"} + ops = append(ops, op2) + op3 := newDummyOp("$.json_parser2", "json_parser") + op3.OutputIDs = []string{"$.copy"} + ops = append(ops, op3) + op4 := newDummyOp("$.copy", "copy") + op4.OutputIDs = []string{"$.copy1"} + ops = append(ops, op4) + op5 := newDummyOp("$.copy1", "copy") + ops = append(ops, op5) + return ops + }(), + }, + { + "unordered", + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + op1 := newDummyOp("$.json_parser", "json_parser") + op1.OutputIDs = []string{"$.copy"} + ops = append(ops, op1) + op2 := newDummyOp("$.copy", "copy") + op2.OutputIDs = []string{"$.json_parser1"} + ops = append(ops, op2) + op3 := newDummyOp("$.json_parser1", "json_parser") + op3.OutputIDs = []string{"$.copy1"} + ops = append(ops, op3) + op4 := newDummyOp("$.copy1", "copy") + op4.OutputIDs = []string{"$.json_parser2"} + ops = append(ops, op4) + op5 := newDummyOp("$.json_parser2", "json_parser") + ops = append(ops, op5) + return ops + }(), + }, + { + "already_renamed", + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + return ops + }, + func() []operator.Operator { + var ops []operator.Operator + op1 := newDummyOp("$.json_parser", "json_parser") + op1.OutputIDs = []string{"$.json_parser1"} + ops = append(ops, op1) + op2 := newDummyOp("$.json_parser1", "json_parser") + op2.OutputIDs = []string{"$.json_parser2"} + ops = append(ops, op2) + op3 := newDummyOp("$.json_parser2", "json_parser") + op3.OutputIDs = []string{"$.json_parser3"} + ops = append(ops, op3) + op4 := newDummyOp("$.json_parser3", "json_parser") + op4.OutputIDs = []string{"$.json_parser4"} + ops = append(ops, op4) + op5 := newDummyOp("$.json_parser4", "json_parser") + ops = append(ops, op5) + return ops + }(), + }, + } + + for _, tc := range cases { + t.Run("Deduplicate/"+tc.name, func(t *testing.T) { + opsConfig := tc.ops() + bc := testutil.NewBuildContext(t) + ops, err := opsConfig.BuildOperators(bc) + require.NoError(t, err) + require.Equal(t, tc.expectedOps, ops) + require.Equal(t, ops, tc.expectedOps) + }) + } +} From 93a93dd504bc77b1087afc57919613e82f771c58 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Wed, 12 May 2021 15:53:05 -0400 Subject: [PATCH 06/26] Clean up diff --- operator/operator.go | 1 - pipeline/config.go | 1 - pipeline/directed.go | 2 +- pipeline/directed_test.go | 9 ++++++--- testutil/mocks.go | 4 ---- 5 files changed, 7 insertions(+), 10 deletions(-) diff --git a/operator/operator.go b/operator/operator.go index 4228a2a3..d4e28469 100644 --- a/operator/operator.go +++ b/operator/operator.go @@ -28,7 +28,6 @@ import ( type Operator interface { // ID returns the id of the operator. ID() string - // Type returns the type of the operator. Type() string diff --git a/pipeline/config.go b/pipeline/config.go index 40aa8926..e111d74a 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -67,7 +67,6 @@ func dedeplucateIDs(ops []operator.Config) error { // BuildPipeline will build a pipeline from the config. func (c Config) BuildPipeline(bc operator.BuildContext, defaultOperator operator.Operator) (*DirectedPipeline, error) { - if defaultOperator != nil { bc.DefaultOutputIDs = []string{defaultOperator.ID()} } diff --git a/pipeline/directed.go b/pipeline/directed.go index 56fa9f40..bba1939f 100644 --- a/pipeline/directed.go +++ b/pipeline/directed.go @@ -82,7 +82,7 @@ func (p *DirectedPipeline) Operators() []operator.Operator { func addNodes(graph *simple.DirectedGraph, operators []operator.Operator) error { for _, operator := range operators { operatorNode := createOperatorNode(operator) - for graph.Node(operatorNode.ID()) != nil { + if graph.Node(operatorNode.ID()) != nil { return errors.NewError( fmt.Sprintf("operator with id '%s' already exists in pipeline", operatorNode.Operator().ID()), "ensure that each operator has a unique `type` or `id`", diff --git a/pipeline/directed_test.go b/pipeline/directed_test.go index 862e0eee..73bd7379 100644 --- a/pipeline/directed_test.go +++ b/pipeline/directed_test.go @@ -108,13 +108,16 @@ func TestPipeline(t *testing.T) { }) t.Run("DuplicateNodeIDs", func(t *testing.T) { - operator1 := testutil.NewMockOperator("fake_output") + operator1 := testutil.NewMockOperator("operator1") operator1.On("SetOutputs", mock.Anything).Return(nil) operator1.On("Outputs").Return(nil) - operator1.On("SetID", mock.Anything).Return(nil) - operator2 := testutil.NewMockOperator("fake_output") + operator2 := testutil.NewMockOperator("operator1") operator2.On("SetOutputs", mock.Anything).Return(nil) operator2.On("Outputs").Return(nil) + + _, err := NewDirectedPipeline([]operator.Operator{operator1, operator2}) + require.Error(t, err) + require.Contains(t, err.Error(), "already exists") }) t.Run("OutputNotExist", func(t *testing.T) { diff --git a/testutil/mocks.go b/testutil/mocks.go index 7619a6ce..65af6835 100644 --- a/testutil/mocks.go +++ b/testutil/mocks.go @@ -33,7 +33,6 @@ func NewMockOperator(id string) *Operator { mockOutput.On("ID").Return(id) mockOutput.On("CanProcess").Return(true) mockOutput.On("CanOutput").Return(true) - mockOutput.On("Type").Return("fake_output") return mockOutput } @@ -60,9 +59,6 @@ func (f *FakeOutput) CanProcess() bool { return true } // ID always returns `fake` as the ID of a fake output operator func (f *FakeOutput) ID() string { return "$.fake" } -// SetID returns nothing -func (f *FakeOutput) SetID(string) {} - // Logger returns the logger of a fake output func (f *FakeOutput) Logger() *zap.SugaredLogger { return f.SugaredLogger } From 3c268c59e0b6cba6ddd9f44806fce5bbf31ef266 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Wed, 12 May 2021 16:24:59 -0400 Subject: [PATCH 07/26] Add Error handling --- pipeline/config.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pipeline/config.go b/pipeline/config.go index e111d74a..4ca292f3 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -26,7 +26,12 @@ type Config []operator.Config // BuildOperators builds the operators from the list of configs into operators func (c Config) BuildOperators(bc operator.BuildContext) ([]operator.Operator, error) { operators := make([]operator.Operator, 0, len(c)) - dedeplucateIDs(c) + + err := dedeplucateIDs(c) + if err != nil { + return nil, err + } + for i, builder := range c { nbc := getBuildContextWithDefaultOutput(c, i, bc) op, err := builder.Build(nbc) @@ -59,7 +64,10 @@ func dedeplucateIDs(ops []operator.Config) error { } } - op.SetID(newID) + err := op.SetID(newID) + if err != nil { + return err + } typeMap[op.Type()]++ } return nil From 7db86f8f1b540a015939de7fc539b8b4527bf0c5 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Wed, 12 May 2021 16:49:07 -0400 Subject: [PATCH 08/26] Fix CI --- pipeline/config_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pipeline/config_test.go b/pipeline/config_test.go index 124b05bd..7e544dfc 100644 --- a/pipeline/config_test.go +++ b/pipeline/config_test.go @@ -19,12 +19,13 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "github.com/open-telemetry/opentelemetry-log-collection/entry" "github.com/open-telemetry/opentelemetry-log-collection/operator" "github.com/open-telemetry/opentelemetry-log-collection/operator/helper" "github.com/open-telemetry/opentelemetry-log-collection/testutil" - "github.com/stretchr/testify/require" - "go.uber.org/zap" ) type dummyOpConfig struct { From 3034fc07ec3c815f6b4b82b0694fa757de1690bc Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Fri, 14 May 2021 11:27:47 -0400 Subject: [PATCH 09/26] Add testing for IDs --- plugin/config_test.go | 55 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/plugin/config_test.go b/plugin/config_test.go index 70c0df49..80da81e8 100644 --- a/plugin/config_test.go +++ b/plugin/config_test.go @@ -133,3 +133,58 @@ pipeline: require.Error(t, err) require.Contains(t, err.Error(), "reached max plugin depth") } + +func TestPluginIDs(t *testing.T) { + pluginContent := []byte(` +parameters: +pipeline: + - type: noop + output: {{ .output }} + - type: noop + output: {{ .output }} +`) + + configContent := []byte(` +id: my_plugin +type: my_plugin +output: stdout +`) + + plugin, err := NewPlugin("my_plugin", pluginContent) + require.NoError(t, err) + + operator.RegisterPlugin(plugin.ID, plugin.NewBuilder) + + var cfg operator.Config + err = yaml.Unmarshal(configContent, &cfg) + require.NoError(t, err) + + expected := operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + OutputIDs: []string{"stdout"}, + BasicConfig: helper.BasicConfig{ + OperatorID: "my_plugin", + OperatorType: "my_plugin", + }, + }, + Parameters: map[string]interface{}{}, + Plugin: plugin, + }, + } + + require.Equal(t, expected, cfg) + + operators, err := cfg.Build(testutil.NewBuildContext(t)) + require.NoError(t, err) + require.Len(t, operators, 2) + noop1, ok1 := operators[0].(*noop.NoopOperator) + noop2, ok2 := operators[1].(*noop.NoopOperator) + require.True(t, ok1) + require.True(t, ok2) + require.Equal(t, "$.my_plugin.noop", noop1.OperatorID) + require.Equal(t, "$.my_plugin.noop", noop2.OperatorID) + require.Equal(t, "noop", noop1.OperatorType) + require.Equal(t, "noop", noop2.OperatorType) + +} From dcc2aa6c5ac28120ef507a8538441dc188d18fc7 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Fri, 14 May 2021 13:02:46 -0400 Subject: [PATCH 10/26] Test Ops is pipeline --- plugin/config_test.go | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/plugin/config_test.go b/plugin/config_test.go index 80da81e8..45405314 100644 --- a/plugin/config_test.go +++ b/plugin/config_test.go @@ -139,21 +139,18 @@ func TestPluginIDs(t *testing.T) { parameters: pipeline: - type: noop - output: {{ .output }} - type: noop - output: {{ .output }} `) configContent := []byte(` id: my_plugin type: my_plugin -output: stdout `) - plugin, err := NewPlugin("my_plugin", pluginContent) + pluginVar, err := NewPlugin("my_plugin", pluginContent) require.NoError(t, err) - operator.RegisterPlugin(plugin.ID, plugin.NewBuilder) + operator.RegisterPlugin(pluginVar.ID, pluginVar.NewBuilder) var cfg operator.Config err = yaml.Unmarshal(configContent, &cfg) @@ -162,29 +159,33 @@ output: stdout expected := operator.Config{ Builder: &Config{ WriterConfig: helper.WriterConfig{ - OutputIDs: []string{"stdout"}, BasicConfig: helper.BasicConfig{ OperatorID: "my_plugin", OperatorType: "my_plugin", }, }, Parameters: map[string]interface{}{}, - Plugin: plugin, + Plugin: pluginVar, }, } require.Equal(t, expected, cfg) - operators, err := cfg.Build(testutil.NewBuildContext(t)) - require.NoError(t, err) - require.Len(t, operators, 2) - noop1, ok1 := operators[0].(*noop.NoopOperator) - noop2, ok2 := operators[1].(*noop.NoopOperator) - require.True(t, ok1) - require.True(t, ok2) - require.Equal(t, "$.my_plugin.noop", noop1.OperatorID) - require.Equal(t, "$.my_plugin.noop", noop2.OperatorID) - require.Equal(t, "noop", noop1.OperatorType) - require.Equal(t, "noop", noop2.OperatorType) + cfgs := pipeline.Config{cfg} + + _, err = cfgs.BuildPipeline(testutil.NewBuildContext(t), nil) + require.Error(t, err) + // Below implements testing for the IDs once the AutoID generation is implemented. + + // operators := pipe.Operators() + // require.Len(t, operators, 2) + // noop1, ok1 := operators[0].(*noop.NoopOperator) + // noop2, ok2 := operators[1].(*noop.NoopOperator) + // require.True(t, ok1) + // require.True(t, ok2) + // require.Equal(t, "$.my_plugin.noop", noop1.OperatorID) + // require.Equal(t, "$.my_plugin.noop1", noop2.OperatorID) + // require.Equal(t, "noop", noop1.OperatorType) + // require.Equal(t, "noop", noop2.OperatorType) } From a09b457255dded0ba53c809ff55fcc62806ea7c7 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Fri, 14 May 2021 15:46:43 -0400 Subject: [PATCH 11/26] Add testing template --- plugin/config_test.go | 90 ++++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/plugin/config_test.go b/plugin/config_test.go index 45405314..aa0f0840 100644 --- a/plugin/config_test.go +++ b/plugin/config_test.go @@ -134,58 +134,62 @@ pipeline: require.Contains(t, err.Error(), "reached max plugin depth") } +type PluginIDTestCase struct { + name string + pluginConfig pipeline.Config + expectedOpIDs []string +} + func TestPluginIDs(t *testing.T) { - pluginContent := []byte(` + const pluginName = "my_plugin" + + cases := []PluginIDTestCase{ + { + name: "basic_plugin", + pluginConfig: func() []operator.Config { + // TODO: ids shouldn't need to be specified once autogen IDs are implemented + pluginContent := []byte(` parameters: pipeline: - type: noop + id: noop - type: noop + id: noop1 `) - configContent := []byte(` -id: my_plugin -type: my_plugin -`) - - pluginVar, err := NewPlugin("my_plugin", pluginContent) - require.NoError(t, err) - - operator.RegisterPlugin(pluginVar.ID, pluginVar.NewBuilder) - - var cfg operator.Config - err = yaml.Unmarshal(configContent, &cfg) - require.NoError(t, err) - - expected := operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: "my_plugin", - OperatorType: "my_plugin", - }, + pluginVar, err := NewPlugin(pluginName, pluginContent) + require.NoError(t, err) + + operator.RegisterPlugin(pluginVar.ID, pluginVar.NewBuilder) + + return pipeline.Config{ + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + }, + } + }(), + expectedOpIDs: []string{ + "$." + pluginName + ".noop", + "$." + pluginName + ".noop1", }, - Parameters: map[string]interface{}{}, - Plugin: pluginVar, }, } - require.Equal(t, expected, cfg) - - cfgs := pipeline.Config{cfg} - - _, err = cfgs.BuildPipeline(testutil.NewBuildContext(t), nil) - require.Error(t, err) - - // Below implements testing for the IDs once the AutoID generation is implemented. - - // operators := pipe.Operators() - // require.Len(t, operators, 2) - // noop1, ok1 := operators[0].(*noop.NoopOperator) - // noop2, ok2 := operators[1].(*noop.NoopOperator) - // require.True(t, ok1) - // require.True(t, ok2) - // require.Equal(t, "$.my_plugin.noop", noop1.OperatorID) - // require.Equal(t, "$.my_plugin.noop1", noop2.OperatorID) - // require.Equal(t, "noop", noop1.OperatorType) - // require.Equal(t, "noop", noop2.OperatorType) + for _, tc := range cases { + pipe, err := tc.pluginConfig.BuildPipeline(testutil.NewBuildContext(t), nil) + require.NoError(t, err) + operators := pipe.Operators() + for i, op := range operators { + require.Equal(t, tc.expectedOpIDs[i], op.ID()) + } + } } From fe0327226842a2728adad1fa2b024a684e482379 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Mon, 17 May 2021 14:18:38 -0400 Subject: [PATCH 12/26] Change how default outputs are set --- operator/builtin/transformer/router/router.go | 15 +++ operator/helper/output.go | 13 ++ operator/helper/writer.go | 14 +- operator/operator.go | 4 + pipeline/config.go | 11 +- plugin/config_test.go | 125 ++++++++++++++---- testutil/mocks.go | 6 + testutil/operator.go | 30 +++++ 8 files changed, 189 insertions(+), 29 deletions(-) diff --git a/operator/builtin/transformer/router/router.go b/operator/builtin/transformer/router/router.go index f5260f5a..05a3e423 100644 --- a/operator/builtin/transformer/router/router.go +++ b/operator/builtin/transformer/router/router.go @@ -157,6 +157,15 @@ func (p *RouterOperator) Outputs() []operator.Operator { return outputs } +// Outputs will return all connected operators. +func (p *RouterOperator) GetOutputIDs() []string { + outputs := make([]string, 0, len(p.routes)) + for _, route := range p.routes { + outputs = append(outputs, route.OutputIDs...) + } + return outputs +} + // SetOutputs will set the outputs of the router operator. func (p *RouterOperator) SetOutputs(operators []operator.Operator) error { for _, route := range p.routes { @@ -170,6 +179,12 @@ func (p *RouterOperator) SetOutputs(operators []operator.Operator) error { return nil } +// SetOutputs will set the outputs of the router operator. +func (p *RouterOperator) SetOutputIDs(opIDs []string) error { + // TODO: determine proper way to set the IDs + return nil +} + // findOperators will find a subset of operators from a collection. func (p *RouterOperator) findOperators(operators []operator.Operator, operatorIDs []string) ([]operator.Operator, error) { result := make([]operator.Operator, 0) diff --git a/operator/helper/output.go b/operator/helper/output.go index 05976245..e93a942e 100644 --- a/operator/helper/output.go +++ b/operator/helper/output.go @@ -65,6 +65,11 @@ func (o *OutputOperator) Outputs() []operator.Operator { return []operator.Operator{} } +// Outputs will always return an empty array for an output ID. +func (o *OutputOperator) GetOutputIDs() []string { + return []string{} +} + // SetOutputs will return an error if called. func (o *OutputOperator) SetOutputs(operators []operator.Operator) error { return errors.NewError( @@ -72,3 +77,11 @@ func (o *OutputOperator) SetOutputs(operators []operator.Operator) error { "This is an unexpected internal error. Please submit a bug/issue.", ) } + +// SetOutputs will return an error if called. +func (o *OutputOperator) SetOutputIDs(opIDs []string) error { + return errors.NewError( + "Operator can not output, but is attempting to set an output.", + "This is an unexpected internal error. Please submit a bug/issue.", + ) +} diff --git a/operator/helper/writer.go b/operator/helper/writer.go index c2f42baa..f9426e5d 100644 --- a/operator/helper/writer.go +++ b/operator/helper/writer.go @@ -45,9 +45,6 @@ func (c WriterConfig) Build(bc operator.BuildContext) (WriterOperator, error) { // Namespace all the output IDs namespacedIDs := c.OutputIDs.WithNamespace(bc) - if len(namespacedIDs) == 0 { - namespacedIDs = bc.DefaultOutputIDs - } writer := WriterOperator{ OutputIDs: namespacedIDs, @@ -84,6 +81,11 @@ func (w *WriterOperator) Outputs() []operator.Operator { return w.OutputOperators } +// Outputs returns the outputs of the writer operator. +func (w *WriterOperator) GetOutputIDs() []string { + return w.OutputIDs +} + // SetOutputs will set the outputs of the operator. func (w *WriterOperator) SetOutputs(operators []operator.Operator) error { outputOperators := make([]operator.Operator, 0) @@ -105,6 +107,12 @@ func (w *WriterOperator) SetOutputs(operators []operator.Operator) error { return nil } +// SetOutputs will set the outputs of the operator. +func (w *WriterOperator) SetOutputIDs(opIds []string) error { + w.OutputIDs = opIds + return nil +} + // FindOperator will find an operator matching the supplied id. func (w *WriterOperator) findOperator(operators []operator.Operator, operatorID string) (operator.Operator, bool) { for _, operator := range operators { diff --git a/operator/operator.go b/operator/operator.go index d4e28469..d359d71e 100644 --- a/operator/operator.go +++ b/operator/operator.go @@ -40,8 +40,12 @@ type Operator interface { CanOutput() bool // Outputs returns the list of connected outputs. Outputs() []Operator + // Outputs returns the list of connected outputs. + GetOutputIDs() []string // SetOutputs will set the connected outputs. SetOutputs([]Operator) error + // SetOutputIDs will set the connected outputs' IDs. + SetOutputIDs([]string) error // CanProcess indicates if the operator will process entries from other operators. CanProcess() bool diff --git a/pipeline/config.go b/pipeline/config.go index cf43ef02..758f2a8f 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -24,9 +24,8 @@ type Config []operator.Config // BuildOperators builds the operators from the list of configs into operators func (c Config) BuildOperators(bc operator.BuildContext) ([]operator.Operator, error) { operators := make([]operator.Operator, 0, len(c)) - for i, builder := range c { - nbc := getBuildContextWithDefaultOutput(c, i, bc) - op, err := builder.Build(nbc) + for _, builder := range c { + op, err := builder.Build(bc) if err != nil { return nil, err } @@ -46,6 +45,12 @@ func (c Config) BuildPipeline(bc operator.BuildContext, defaultOperator operator return nil, err } + for i, op := range operators { + if len(op.GetOutputIDs()) == 0 && i+1 < len(operators) { + op.SetOutputIDs([]string{operators[i+1].ID()}) + } + } + if defaultOperator != nil { operators = append(operators, defaultOperator) } diff --git a/plugin/config_test.go b/plugin/config_test.go index aa0f0840..802cbc60 100644 --- a/plugin/config_test.go +++ b/plugin/config_test.go @@ -135,33 +135,95 @@ pipeline: } type PluginIDTestCase struct { - name string - pluginConfig pipeline.Config - expectedOpIDs []string + Name string + PluginConfig pipeline.Config + ExpectedOpIDs []string } func TestPluginIDs(t *testing.T) { - const pluginName = "my_plugin" - - cases := []PluginIDTestCase{ - { - name: "basic_plugin", - pluginConfig: func() []operator.Config { - // TODO: ids shouldn't need to be specified once autogen IDs are implemented - pluginContent := []byte(` + // TODO: ids shouldn't need to be specified once autogen IDs are implemented + pluginContent := []byte(` parameters: pipeline: - type: noop - id: noop + id: noop - type: noop id: noop1 `) + pluginName := "my_plugin" + pluginVar, err := NewPlugin(pluginName, pluginContent) + require.NoError(t, err) + operator.RegisterPlugin(pluginVar.ID, pluginVar.NewBuilder) - pluginVar, err := NewPlugin(pluginName, pluginContent) - require.NoError(t, err) - - operator.RegisterPlugin(pluginVar.ID, pluginVar.NewBuilder) + // TODO: remove ID assignment + pluginContent2 := []byte(` +parameters: +pipeline: + - type: noop + id: noop3 + - type: noop + id: noop4 +`) + secondPlugin := "secondPlugin" + secondPluginVar, err := NewPlugin(secondPlugin, pluginContent2) + require.NoError(t, err) + operator.RegisterPlugin(secondPluginVar.ID, secondPluginVar.NewBuilder) + cases := []PluginIDTestCase{ + // { + // Name: "basic_plugin", + // PluginConfig: func() []operator.Config { + // return pipeline.Config{ + // operator.Config{ + // Builder: &Config{ + // WriterConfig: helper.WriterConfig{ + // BasicConfig: helper.BasicConfig{ + // OperatorID: pluginName, + // OperatorType: pluginName, + // }, + // }, + // Parameters: map[string]interface{}{}, + // Plugin: pluginVar, + // }, + // }, + // } + // }(), + // ExpectedOpIDs: []string{ + // "$." + pluginName + ".noop", + // "$." + pluginName + ".noop1", + // }, + // }, + // { + // Name: "same_op_outside_plugin", + // PluginConfig: func() []operator.Config { + // return pipeline.Config{ + // operator.Config{ + // Builder: &Config{ + // WriterConfig: helper.WriterConfig{ + // BasicConfig: helper.BasicConfig{ + // OperatorID: pluginName, + // OperatorType: pluginName, + // }, + // }, + // Parameters: map[string]interface{}{}, + // Plugin: pluginVar, + // }, + // }, + // operator.Config{ + // // TODO: ID should be noop to start then auto gened to noop2 + // Builder: noop.NewNoopOperatorConfig("noop2"), + // }, + // } + // }(), + // ExpectedOpIDs: []string{ + // "$." + pluginName + ".noop", + // "$." + pluginName + ".noop1", + // "$.noop2", + // }, + // }, + { + Name: "two_plugins_with_same_ops", + PluginConfig: func() []operator.Config { return pipeline.Config{ operator.Config{ Builder: &Config{ @@ -175,21 +237,38 @@ pipeline: Plugin: pluginVar, }, }, + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: secondPlugin, + OperatorType: secondPlugin, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: secondPluginVar, + }, + }, } }(), - expectedOpIDs: []string{ + ExpectedOpIDs: []string{ "$." + pluginName + ".noop", "$." + pluginName + ".noop1", + "$." + secondPlugin + ".noop3", + "$." + secondPlugin + ".noop4", }, }, } for _, tc := range cases { - pipe, err := tc.pluginConfig.BuildPipeline(testutil.NewBuildContext(t), nil) - require.NoError(t, err) - operators := pipe.Operators() - for i, op := range operators { - require.Equal(t, tc.expectedOpIDs[i], op.ID()) - } + t.Run(tc.Name, func(t *testing.T) { + pipe, err := tc.PluginConfig.BuildPipeline(testutil.NewBuildContext(t), nil) + require.NoError(t, err) + operators := pipe.Operators() + require.Len(t, operators, len(tc.ExpectedOpIDs)) + for i, op := range operators { + require.Equal(t, tc.ExpectedOpIDs[i], op.ID()) + } + }) } } diff --git a/testutil/mocks.go b/testutil/mocks.go index 65af6835..e0bcf1d3 100644 --- a/testutil/mocks.go +++ b/testutil/mocks.go @@ -65,9 +65,15 @@ func (f *FakeOutput) Logger() *zap.SugaredLogger { return f.SugaredLogger } // Outputs always returns nil for a fake output func (f *FakeOutput) Outputs() []operator.Operator { return nil } +// Outputs always returns nil for a fake output +func (f *FakeOutput) GetOutputIDs() []string { return nil } + // SetOutputs immediately returns nil for a fake output func (f *FakeOutput) SetOutputs(outputs []operator.Operator) error { return nil } +// SetOutputs immediately returns nil for a fake output +func (f *FakeOutput) SetOutputIDs(s []string) error { return nil } + // Start immediately returns nil for a fake output func (f *FakeOutput) Start(_ operator.Persister) error { return nil } diff --git a/testutil/operator.go b/testutil/operator.go index 20ad941b..730170ec 100644 --- a/testutil/operator.go +++ b/testutil/operator.go @@ -46,6 +46,22 @@ func (_m *Operator) CanProcess() bool { return r0 } +// GetOutputIDs provides a mock function with given fields: +func (_m *Operator) GetOutputIDs() []string { + ret := _m.Called() + + var r0 []string + if rf, ok := ret.Get(0).(func() []string); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + return r0 +} + // ID provides a mock function with given fields: func (_m *Operator) ID() string { ret := _m.Called() @@ -106,6 +122,20 @@ func (_m *Operator) Process(_a0 context.Context, _a1 *entry.Entry) error { return r0 } +// SetOutputIDs provides a mock function with given fields: _a0 +func (_m *Operator) SetOutputIDs(_a0 []string) error { + ret := _m.Called(_a0) + + var r0 error + if rf, ok := ret.Get(0).(func([]string) error); ok { + r0 = rf(_a0) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // SetOutputs provides a mock function with given fields: _a0 func (_m *Operator) SetOutputs(_a0 []operator.Operator) error { ret := _m.Called(_a0) From 733ae20cd58d5229cfb85fcec3938a883ab16606 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Tue, 18 May 2021 15:32:37 -0400 Subject: [PATCH 13/26] Add testing for outputs --- pipeline/config.go | 18 ++-- plugin/config_test.go | 204 +++++++++++++++++++++--------------------- 2 files changed, 117 insertions(+), 105 deletions(-) diff --git a/pipeline/config.go b/pipeline/config.go index 758f2a8f..750fdddf 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -45,11 +45,7 @@ func (c Config) BuildPipeline(bc operator.BuildContext, defaultOperator operator return nil, err } - for i, op := range operators { - if len(op.GetOutputIDs()) == 0 && i+1 < len(operators) { - op.SetOutputIDs([]string{operators[i+1].ID()}) - } - } + SetOutputIDs(operators) if defaultOperator != nil { operators = append(operators, defaultOperator) @@ -58,6 +54,18 @@ func (c Config) BuildPipeline(bc operator.BuildContext, defaultOperator operator return NewDirectedPipeline(operators) } +func SetOutputIDs(operators []operator.Operator) error { + for i, op := range operators { + if len(op.GetOutputIDs()) == 0 && i+1 < len(operators) { + err := op.SetOutputIDs([]string{operators[i+1].ID()}) + if err != nil { + return err + } + } + } + return nil +} + func getBuildContextWithDefaultOutput(configs []operator.Config, i int, bc operator.BuildContext) operator.BuildContext { if i+1 >= len(configs) { return bc diff --git a/plugin/config_test.go b/plugin/config_test.go index 802cbc60..b4432114 100644 --- a/plugin/config_test.go +++ b/plugin/config_test.go @@ -101,39 +101,6 @@ output: stdout require.Equal(t, "noop", noop.OperatorType) } -func TestBuildRecursiveFails(t *testing.T) { - pluginConfig1 := []byte(` -pipeline: - - type: plugin2 -`) - - pluginConfig2 := []byte(` -pipeline: - - type: plugin1 -`) - - plugin1, err := NewPlugin("plugin1", pluginConfig1) - require.NoError(t, err) - plugin2, err := NewPlugin("plugin2", pluginConfig2) - require.NoError(t, err) - - t.Cleanup(func() { operator.DefaultRegistry = operator.NewRegistry() }) - operator.RegisterPlugin("plugin1", plugin1.NewBuilder) - operator.RegisterPlugin("plugin2", plugin2.NewBuilder) - - pipelineConfig := []byte(` -- type: plugin1 -`) - - var pipeline pipeline.Config - err = yaml.Unmarshal(pipelineConfig, &pipeline) - require.NoError(t, err) - - _, err = pipeline.BuildOperators(operator.NewBuildContext(zaptest.NewLogger(t).Sugar())) - require.Error(t, err) - require.Contains(t, err.Error(), "reached max plugin depth") -} - type PluginIDTestCase struct { Name string PluginConfig pipeline.Config @@ -145,10 +112,10 @@ func TestPluginIDs(t *testing.T) { pluginContent := []byte(` parameters: pipeline: - - type: noop - id: noop - - type: noop - id: noop1 + - id: noop + type: noop + - id: noop1 + type: noop `) pluginName := "my_plugin" pluginVar, err := NewPlugin(pluginName, pluginContent) @@ -159,10 +126,10 @@ pipeline: pluginContent2 := []byte(` parameters: pipeline: - - type: noop - id: noop3 - - type: noop - id: noop4 + - id: noop3 + type: noop + - id: noop4 + type: noop `) secondPlugin := "secondPlugin" secondPluginVar, err := NewPlugin(secondPlugin, pluginContent2) @@ -170,57 +137,57 @@ pipeline: operator.RegisterPlugin(secondPluginVar.ID, secondPluginVar.NewBuilder) cases := []PluginIDTestCase{ - // { - // Name: "basic_plugin", - // PluginConfig: func() []operator.Config { - // return pipeline.Config{ - // operator.Config{ - // Builder: &Config{ - // WriterConfig: helper.WriterConfig{ - // BasicConfig: helper.BasicConfig{ - // OperatorID: pluginName, - // OperatorType: pluginName, - // }, - // }, - // Parameters: map[string]interface{}{}, - // Plugin: pluginVar, - // }, - // }, - // } - // }(), - // ExpectedOpIDs: []string{ - // "$." + pluginName + ".noop", - // "$." + pluginName + ".noop1", - // }, - // }, - // { - // Name: "same_op_outside_plugin", - // PluginConfig: func() []operator.Config { - // return pipeline.Config{ - // operator.Config{ - // Builder: &Config{ - // WriterConfig: helper.WriterConfig{ - // BasicConfig: helper.BasicConfig{ - // OperatorID: pluginName, - // OperatorType: pluginName, - // }, - // }, - // Parameters: map[string]interface{}{}, - // Plugin: pluginVar, - // }, - // }, - // operator.Config{ - // // TODO: ID should be noop to start then auto gened to noop2 - // Builder: noop.NewNoopOperatorConfig("noop2"), - // }, - // } - // }(), - // ExpectedOpIDs: []string{ - // "$." + pluginName + ".noop", - // "$." + pluginName + ".noop1", - // "$.noop2", - // }, - // }, + { + Name: "basic_plugin", + PluginConfig: func() []operator.Config { + return pipeline.Config{ + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + }, + } + }(), + ExpectedOpIDs: []string{ + "$." + pluginName + ".noop", + "$." + pluginName + ".noop1", + }, + }, + { + Name: "same_op_outside_plugin", + PluginConfig: func() []operator.Config { + return pipeline.Config{ + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + }, + operator.Config{ + // TODO: ID should be noop to start then auto gened to noop2 + Builder: noop.NewNoopOperatorConfig("noop2"), + }, + } + }(), + ExpectedOpIDs: []string{ + "$." + pluginName + ".noop", + "$." + pluginName + ".noop1", + "$.noop2", + }, + }, { Name: "two_plugins_with_same_ops", PluginConfig: func() []operator.Config { @@ -261,14 +228,51 @@ pipeline: } for _, tc := range cases { - t.Run(tc.Name, func(t *testing.T) { - pipe, err := tc.PluginConfig.BuildPipeline(testutil.NewBuildContext(t), nil) - require.NoError(t, err) - operators := pipe.Operators() - require.Len(t, operators, len(tc.ExpectedOpIDs)) - for i, op := range operators { - require.Equal(t, tc.ExpectedOpIDs[i], op.ID()) + ops, err := tc.PluginConfig.BuildOperators(testutil.NewBuildContext(t)) + require.NoError(t, err) + + pipeline.SetOutputIDs(ops) + + require.Len(t, ops, len(tc.ExpectedOpIDs)) + for i, op := range ops { + require.Equal(t, tc.ExpectedOpIDs[i], op.ID()) + if i+1 < len(ops) { + out := op.GetOutputIDs() + require.Equal(t, out[0], tc.ExpectedOpIDs[i+1]) } - }) + } } } + +func TestBuildRecursiveFails(t *testing.T) { + pluginConfig1 := []byte(` +pipeline: + - type: plugin2 +`) + + pluginConfig2 := []byte(` +pipeline: + - type: plugin1 +`) + + plugin1, err := NewPlugin("plugin1", pluginConfig1) + require.NoError(t, err) + plugin2, err := NewPlugin("plugin2", pluginConfig2) + require.NoError(t, err) + + t.Cleanup(func() { operator.DefaultRegistry = operator.NewRegistry() }) + operator.RegisterPlugin("plugin1", plugin1.NewBuilder) + operator.RegisterPlugin("plugin2", plugin2.NewBuilder) + + pipelineConfig := []byte(` +- type: plugin1 +`) + + var pipeline pipeline.Config + err = yaml.Unmarshal(pipelineConfig, &pipeline) + require.NoError(t, err) + + _, err = pipeline.BuildOperators(operator.NewBuildContext(zaptest.NewLogger(t).Sugar())) + require.Error(t, err) + require.Contains(t, err.Error(), "reached max plugin depth") +} From 1800ef9c1f14d682e3d883fc28c23d13950bd5ae Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Tue, 18 May 2021 15:40:24 -0400 Subject: [PATCH 14/26] Remove old default outputs func --- pipeline/config.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/pipeline/config.go b/pipeline/config.go index 750fdddf..6b010598 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -65,13 +65,3 @@ func SetOutputIDs(operators []operator.Operator) error { } return nil } - -func getBuildContextWithDefaultOutput(configs []operator.Config, i int, bc operator.BuildContext) operator.BuildContext { - if i+1 >= len(configs) { - return bc - } - - id := configs[i+1].ID() - id = bc.PrependNamespace(id) - return bc.WithDefaultOutputIDs([]string{id}) -} From cc92043291b0ed5730e6693f684dcf9ac7aa0af2 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Wed, 19 May 2021 10:16:11 -0400 Subject: [PATCH 15/26] Add ID testing --- plugin/config_test.go | 137 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/plugin/config_test.go b/plugin/config_test.go index 70c0df49..b3ffa6a6 100644 --- a/plugin/config_test.go +++ b/plugin/config_test.go @@ -101,6 +101,143 @@ output: stdout require.Equal(t, "noop", noop.OperatorType) } +type PluginIDTestCase struct { + Name string + PluginConfig pipeline.Config + ExpectedOpIDs []string +} + +func TestPluginIDs(t *testing.T) { + // TODO: ids shouldn't need to be specified once autogen IDs are implemented + pluginContent := []byte(` +parameters: +pipeline: + - id: noop + type: noop + - id: noop1 + type: noop +`) + pluginName := "my_plugin" + pluginVar, err := NewPlugin(pluginName, pluginContent) + require.NoError(t, err) + operator.RegisterPlugin(pluginVar.ID, pluginVar.NewBuilder) + + // TODO: remove ID assignment + pluginContent2 := []byte(` +parameters: +pipeline: + - id: noop3 + type: noop + - id: noop4 + type: noop +`) + secondPlugin := "secondPlugin" + secondPluginVar, err := NewPlugin(secondPlugin, pluginContent2) + require.NoError(t, err) + operator.RegisterPlugin(secondPluginVar.ID, secondPluginVar.NewBuilder) + + cases := []PluginIDTestCase{ + { + Name: "basic_plugin", + PluginConfig: func() []operator.Config { + return pipeline.Config{ + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + }, + } + }(), + ExpectedOpIDs: []string{ + "$." + pluginName + ".noop", + "$." + pluginName + ".noop1", + }, + }, + { + Name: "same_op_outside_plugin", + PluginConfig: func() []operator.Config { + return pipeline.Config{ + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + }, + operator.Config{ + // TODO: ID should be noop to start then auto gened to noop2 + Builder: noop.NewNoopOperatorConfig("noop2"), + }, + } + }(), + ExpectedOpIDs: []string{ + "$." + pluginName + ".noop", + "$." + pluginName + ".noop1", + "$.noop2", + }, + }, + { + Name: "two_plugins_with_same_ops", + PluginConfig: func() []operator.Config { + return pipeline.Config{ + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + }, + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: secondPlugin, + OperatorType: secondPlugin, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: secondPluginVar, + }, + }, + } + }(), + ExpectedOpIDs: []string{ + "$." + pluginName + ".noop", + "$." + pluginName + ".noop1", + "$." + secondPlugin + ".noop3", + "$." + secondPlugin + ".noop4", + }, + }, + } + + for _, tc := range cases { + ops, err := tc.PluginConfig.BuildOperators(testutil.NewBuildContext(t)) + require.NoError(t, err) + + require.Len(t, ops, len(tc.ExpectedOpIDs)) + for i, op := range ops { + require.Equal(t, tc.ExpectedOpIDs[i], op.ID()) + } + } +} + func TestBuildRecursiveFails(t *testing.T) { pluginConfig1 := []byte(` pipeline: From 43c5b9633262ce2ee8fab2f21f90ce8902bbcf0a Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Thu, 20 May 2021 08:48:06 -0400 Subject: [PATCH 16/26] Add testing for output handling --- pipeline/config.go | 20 ++++- plugin/config_test.go | 165 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 176 insertions(+), 9 deletions(-) diff --git a/pipeline/config.go b/pipeline/config.go index 6b010598..b807af88 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -24,13 +24,20 @@ type Config []operator.Config // BuildOperators builds the operators from the list of configs into operators func (c Config) BuildOperators(bc operator.BuildContext) ([]operator.Operator, error) { operators := make([]operator.Operator, 0, len(c)) + pluginMap := make(map[string]string) for _, builder := range c { op, err := builder.Build(bc) if err != nil { return nil, err } + + if len(op) > 1 { + pluginMap[bc.PrependNamespace(builder.ID())] = op[0].ID() + } operators = append(operators, op...) } + SetOutputIDs(operators, pluginMap) + return operators, nil } @@ -45,8 +52,6 @@ func (c Config) BuildPipeline(bc operator.BuildContext, defaultOperator operator return nil, err } - SetOutputIDs(operators) - if defaultOperator != nil { operators = append(operators, defaultOperator) } @@ -54,13 +59,22 @@ func (c Config) BuildPipeline(bc operator.BuildContext, defaultOperator operator return NewDirectedPipeline(operators) } -func SetOutputIDs(operators []operator.Operator) error { +func SetOutputIDs(operators []operator.Operator, pluginMap map[string]string) error { for i, op := range operators { if len(op.GetOutputIDs()) == 0 && i+1 < len(operators) { err := op.SetOutputIDs([]string{operators[i+1].ID()}) if err != nil { return err } + continue + } + allOutputs := []string{} + for _, id := range op.GetOutputIDs() { + if pluginMap[id] != "" { + allOutputs = append(allOutputs, pluginMap[id]) + continue + } + allOutputs = append(allOutputs, id) } } return nil diff --git a/plugin/config_test.go b/plugin/config_test.go index b4432114..89a5b4de 100644 --- a/plugin/config_test.go +++ b/plugin/config_test.go @@ -116,6 +116,7 @@ pipeline: type: noop - id: noop1 type: noop + output: {{ .output }} `) pluginName := "my_plugin" pluginVar, err := NewPlugin(pluginName, pluginContent) @@ -231,19 +232,171 @@ pipeline: ops, err := tc.PluginConfig.BuildOperators(testutil.NewBuildContext(t)) require.NoError(t, err) - pipeline.SetOutputIDs(ops) - require.Len(t, ops, len(tc.ExpectedOpIDs)) for i, op := range ops { require.Equal(t, tc.ExpectedOpIDs[i], op.ID()) - if i+1 < len(ops) { - out := op.GetOutputIDs() - require.Equal(t, out[0], tc.ExpectedOpIDs[i+1]) - } } } } +type PluginOutputIDTestCase struct { + Name string + PluginConfig pipeline.Config + ExpectedOpIDs map[string][]string +} + +func TestPluginOutputIDs(t *testing.T) { + // TODO: ids shouldn't need to be specified once autogen IDs are implemented + pluginContent := []byte(` +parameters: +pipeline: + - id: noop + type: noop + - id: noop1 + type: noop +`) + pluginName := "my_plugin" + pluginVar, err := NewPlugin(pluginName, pluginContent) + require.NoError(t, err) + operator.RegisterPlugin(pluginVar.ID, pluginVar.NewBuilder) + + // TODO: remove ID assignment + pluginContent2 := []byte(` +parameters: +pipeline: + - id: noop2 + type: noop + - id: noop3 + type: noop +`) + secondPlugin := "secondPlugin" + secondPluginVar, err := NewPlugin(secondPlugin, pluginContent2) + require.NoError(t, err) + operator.RegisterPlugin(secondPluginVar.ID, secondPluginVar.NewBuilder) + + cases := []PluginOutputIDTestCase{ + // { + // Name: "same_op_outside_plugin", + // PluginConfig: func() []operator.Config { + // return pipeline.Config{ + // operator.Config{ + // // TODO: ID should be noop to start then auto gened to noop2 + // Builder: noop.NewNoopOperatorConfig("noop2"), + // }, + // operator.Config{ + // Builder: &Config{ + // WriterConfig: helper.WriterConfig{ + // BasicConfig: helper.BasicConfig{ + // OperatorID: pluginName, + // OperatorType: pluginName, + // }, + // }, + // Parameters: map[string]interface{}{}, + // Plugin: pluginVar, + // }, + // }, + // } + // }(), + // ExpectedOpIDs: map[string][]string{ + // "$.noop2": {"$." + pluginName + ".noop"}, + // "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, + // }, + // }, + // { + // Name: "two_plugins_with_same_ops", + // PluginConfig: func() []operator.Config { + // return pipeline.Config{ + // operator.Config{ + // Builder: &Config{ + // WriterConfig: helper.WriterConfig{ + // BasicConfig: helper.BasicConfig{ + // OperatorID: pluginName, + // OperatorType: pluginName, + // }, + // }, + // Parameters: map[string]interface{}{}, + // Plugin: pluginVar, + // }, + // }, + // operator.Config{ + // Builder: &Config{ + // WriterConfig: helper.WriterConfig{ + // BasicConfig: helper.BasicConfig{ + // OperatorID: secondPlugin, + // OperatorType: secondPlugin, + // }, + // }, + // Parameters: map[string]interface{}{}, + // Plugin: secondPluginVar, + // }, + // }, + // } + // }(), + // ExpectedOpIDs: map[string][]string{ + // "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, + // "$." + pluginName + ".noop1": {"$." + secondPlugin + ".noop2"}, + // "$." + secondPlugin + ".noop2": {"$." + secondPlugin + ".noop3"}, + // }, + // }, + { + Name: "two_plugins_specified_output", + PluginConfig: func() []operator.Config { + return pipeline.Config{ + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, + }, + OutputIDs: []string{"$.noop5"}, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + }, + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: secondPlugin, + OperatorType: secondPlugin, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: secondPluginVar, + }, + }, + operator.Config{ + Builder: noop.NewNoopOperatorConfig("noop5"), + }, + } + }(), + ExpectedOpIDs: map[string][]string{ + "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, + "$." + pluginName + ".noop1": {"$.noop5"}, + "$." + secondPlugin + ".noop2": {"$." + pluginName + ".noop3"}, + "$." + secondPlugin + ".noop3": {"$.noop5"}, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.Name, func(t *testing.T) { + ops, err := tc.PluginConfig.BuildOperators(testutil.NewBuildContext(t)) + require.NoError(t, err) + + for i, op := range ops { + if i+1 < len(ops) { + out := op.GetOutputIDs() + t.Log("ID:" + op.ID()) + require.Equal(t, tc.ExpectedOpIDs[op.ID()], out) + } + } + }) + } +} + func TestBuildRecursiveFails(t *testing.T) { pluginConfig1 := []byte(` pipeline: From 97821185ae7f39b71263b6d722dda66380e0d7a1 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Thu, 20 May 2021 14:50:29 -0400 Subject: [PATCH 17/26] Update Tests --- pipeline/config.go | 4 + plugin/config_test.go | 178 ++++++++++++++++++++++++++---------------- 2 files changed, 114 insertions(+), 68 deletions(-) diff --git a/pipeline/config.go b/pipeline/config.go index b807af88..3668a2db 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -76,6 +76,10 @@ func SetOutputIDs(operators []operator.Operator, pluginMap map[string]string) er } allOutputs = append(allOutputs, id) } + err := op.SetOutputIDs(allOutputs) + if err != nil { + return err + } } return nil } diff --git a/plugin/config_test.go b/plugin/config_test.go index f10c792a..d95cc3e5 100644 --- a/plugin/config_test.go +++ b/plugin/config_test.go @@ -254,6 +254,7 @@ pipeline: type: noop - id: noop1 type: noop + output: {{ .output }} `) pluginName := "my_plugin" pluginVar, err := NewPlugin(pluginName, pluginContent) @@ -264,9 +265,9 @@ pipeline: pluginContent2 := []byte(` parameters: pipeline: - - id: noop2 + - id: noop type: noop - - id: noop3 + - id: noop1 type: noop `) secondPlugin := "secondPlugin" @@ -275,69 +276,69 @@ pipeline: operator.RegisterPlugin(secondPluginVar.ID, secondPluginVar.NewBuilder) cases := []PluginOutputIDTestCase{ - // { - // Name: "same_op_outside_plugin", - // PluginConfig: func() []operator.Config { - // return pipeline.Config{ - // operator.Config{ - // // TODO: ID should be noop to start then auto gened to noop2 - // Builder: noop.NewNoopOperatorConfig("noop2"), - // }, - // operator.Config{ - // Builder: &Config{ - // WriterConfig: helper.WriterConfig{ - // BasicConfig: helper.BasicConfig{ - // OperatorID: pluginName, - // OperatorType: pluginName, - // }, - // }, - // Parameters: map[string]interface{}{}, - // Plugin: pluginVar, - // }, - // }, - // } - // }(), - // ExpectedOpIDs: map[string][]string{ - // "$.noop2": {"$." + pluginName + ".noop"}, - // "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, - // }, - // }, - // { - // Name: "two_plugins_with_same_ops", - // PluginConfig: func() []operator.Config { - // return pipeline.Config{ - // operator.Config{ - // Builder: &Config{ - // WriterConfig: helper.WriterConfig{ - // BasicConfig: helper.BasicConfig{ - // OperatorID: pluginName, - // OperatorType: pluginName, - // }, - // }, - // Parameters: map[string]interface{}{}, - // Plugin: pluginVar, - // }, - // }, - // operator.Config{ - // Builder: &Config{ - // WriterConfig: helper.WriterConfig{ - // BasicConfig: helper.BasicConfig{ - // OperatorID: secondPlugin, - // OperatorType: secondPlugin, - // }, - // }, - // Parameters: map[string]interface{}{}, - // Plugin: secondPluginVar, - // }, - // }, - // } - // }(), - // ExpectedOpIDs: map[string][]string{ - // "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, - // "$." + pluginName + ".noop1": {"$." + secondPlugin + ".noop2"}, - // "$." + secondPlugin + ".noop2": {"$." + secondPlugin + ".noop3"}, - // }, - // }, + { + Name: "same_op_outside_plugin", + PluginConfig: func() []operator.Config { + return pipeline.Config{ + operator.Config{ + // TODO: ID should be noop to start then auto gened to noop2 + Builder: noop.NewNoopOperatorConfig("noop2"), + }, + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + }, + } + }(), + ExpectedOpIDs: map[string][]string{ + "$.noop2": {"$." + pluginName + ".noop"}, + "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, + }, + }, + { + Name: "two_plugins_with_same_ops", + PluginConfig: func() []operator.Config { + return pipeline.Config{ + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + }, + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: secondPlugin, + OperatorType: secondPlugin, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: secondPluginVar, + }, + }, + } + }(), + ExpectedOpIDs: map[string][]string{ + "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, + "$." + pluginName + ".noop1": {"$." + secondPlugin + ".noop"}, + "$." + secondPlugin + ".noop": {"$." + secondPlugin + ".noop1"}, + }, + }, { Name: "two_plugins_specified_output", PluginConfig: func() []operator.Config { @@ -349,7 +350,7 @@ pipeline: OperatorID: pluginName, OperatorType: pluginName, }, - OutputIDs: []string{"$.noop5"}, + OutputIDs: []string{"noop5"}, }, Parameters: map[string]interface{}{}, Plugin: pluginVar, @@ -375,8 +376,49 @@ pipeline: ExpectedOpIDs: map[string][]string{ "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, "$." + pluginName + ".noop1": {"$.noop5"}, - "$." + secondPlugin + ".noop2": {"$." + pluginName + ".noop3"}, - "$." + secondPlugin + ".noop3": {"$.noop5"}, + "$." + secondPlugin + ".noop": {"$." + secondPlugin + ".noop1"}, + "$." + secondPlugin + ".noop1": {"$.noop5"}, + }, + }, + { + Name: "two_plugins_output_to_non_sequential_plugin", + PluginConfig: func() []operator.Config { + return pipeline.Config{ + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, + }, + OutputIDs: []string{secondPlugin}, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + }, + operator.Config{ + Builder: noop.NewNoopOperatorConfig("noop5"), + }, + operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: secondPlugin, + OperatorType: secondPlugin, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: secondPluginVar, + }, + }, + } + }(), + ExpectedOpIDs: map[string][]string{ + "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, + "$." + pluginName + ".noop1": {"$." + secondPlugin + ".noop"}, + "$.noop5": {"$." + secondPlugin + ".noop"}, + "$." + secondPlugin + ".noop": {"$." + secondPlugin + ".noop1"}, }, }, } From d193899fa660cced557df95551df824ee085757f Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Thu, 20 May 2021 15:05:27 -0400 Subject: [PATCH 18/26] Update mock comment --- testutil/mocks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testutil/mocks.go b/testutil/mocks.go index e0bcf1d3..f2ec4867 100644 --- a/testutil/mocks.go +++ b/testutil/mocks.go @@ -71,7 +71,7 @@ func (f *FakeOutput) GetOutputIDs() []string { return nil } // SetOutputs immediately returns nil for a fake output func (f *FakeOutput) SetOutputs(outputs []operator.Operator) error { return nil } -// SetOutputs immediately returns nil for a fake output +// SetOutputIDs immediately returns nil for a fake output func (f *FakeOutput) SetOutputIDs(s []string) error { return nil } // Start immediately returns nil for a fake output From dc78c195ee23533d4ce7de93532c2afd0d27437d Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Fri, 21 May 2021 13:45:10 -0400 Subject: [PATCH 19/26] Implement PR feedback --- operator/builtin/transformer/router/router.go | 10 ++--- operator/config.go | 1 + operator/config_test.go | 1 + operator/helper/output.go | 13 +++--- operator/helper/writer.go | 10 +++-- operator/operator.go | 2 +- pipeline/config.go | 42 +++++++++++-------- plugin/config.go | 2 + testutil/mocks.go | 2 +- testutil/operator.go | 13 +----- testutil/operator_builder.go | 14 +++++++ 11 files changed, 65 insertions(+), 45 deletions(-) diff --git a/operator/builtin/transformer/router/router.go b/operator/builtin/transformer/router/router.go index 05a3e423..d213bacd 100644 --- a/operator/builtin/transformer/router/router.go +++ b/operator/builtin/transformer/router/router.go @@ -95,6 +95,9 @@ func (c RouterOperatorConfig) Build(bc operator.BuildContext) ([]operator.Operat return []operator.Operator{routerOperator}, nil } +// BuildsMultipleOps returns false +func (c RouterOperatorConfig) BuildsMultipleOps() bool { return false } + // RouterOperator is an operator that routes entries based on matching expressions type RouterOperator struct { helper.BasicOperator @@ -179,11 +182,8 @@ func (p *RouterOperator) SetOutputs(operators []operator.Operator) error { return nil } -// SetOutputs will set the outputs of the router operator. -func (p *RouterOperator) SetOutputIDs(opIDs []string) error { - // TODO: determine proper way to set the IDs - return nil -} +// SetOutputIDs will do nothing. +func (p *RouterOperator) SetOutputIDs(opIDs []string) {} // findOperators will find a subset of operators from a collection. func (p *RouterOperator) findOperators(operators []operator.Operator, operatorIDs []string) ([]operator.Operator, error) { diff --git a/operator/config.go b/operator/config.go index c48816b3..8ffb70cd 100644 --- a/operator/config.go +++ b/operator/config.go @@ -31,6 +31,7 @@ type Builder interface { ID() string Type() string Build(BuildContext) ([]Operator, error) + BuildsMultipleOps() bool } // UnmarshalJSON will unmarshal a config from JSON. diff --git a/operator/config_test.go b/operator/config_test.go index 0c2260cd..3fc0dd0f 100644 --- a/operator/config_test.go +++ b/operator/config_test.go @@ -31,6 +31,7 @@ type FakeBuilder struct { func (f *FakeBuilder) Build(context BuildContext) ([]Operator, error) { return nil, nil } func (f *FakeBuilder) ID() string { return "plugin" } func (f *FakeBuilder) Type() string { return "plugin" } +func (f *FakeBuilder) BuildsMultipleOps() bool { return false } func TestUnmarshalJSONErrors(t *testing.T) { t.Cleanup(func() { diff --git a/operator/helper/output.go b/operator/helper/output.go index e93a942e..dafc20a5 100644 --- a/operator/helper/output.go +++ b/operator/helper/output.go @@ -45,6 +45,9 @@ func (c OutputConfig) Build(context operator.BuildContext) (OutputOperator, erro return outputOperator, nil } +// BuildsMultipleOps Returns false +func (c OutputConfig) BuildsMultipleOps() bool { return false } + // OutputOperator provides a basic implementation of an output operator. type OutputOperator struct { BasicOperator @@ -65,7 +68,7 @@ func (o *OutputOperator) Outputs() []operator.Operator { return []operator.Operator{} } -// Outputs will always return an empty array for an output ID. +// GetOutputIDs will always return an empty array for an output ID. func (o *OutputOperator) GetOutputIDs() []string { return []string{} } @@ -78,10 +81,6 @@ func (o *OutputOperator) SetOutputs(operators []operator.Operator) error { ) } -// SetOutputs will return an error if called. -func (o *OutputOperator) SetOutputIDs(opIDs []string) error { - return errors.NewError( - "Operator can not output, but is attempting to set an output.", - "This is an unexpected internal error. Please submit a bug/issue.", - ) +// SetOutputIDs will return nothing and does nothing. +func (o *OutputOperator) SetOutputIDs(opIDs []string) { } diff --git a/operator/helper/writer.go b/operator/helper/writer.go index f9426e5d..ad1a7bd2 100644 --- a/operator/helper/writer.go +++ b/operator/helper/writer.go @@ -53,6 +53,11 @@ func (c WriterConfig) Build(bc operator.BuildContext) (WriterOperator, error) { return writer, nil } +// BuildsMultipleOps Returns false as a base line +func (c WriterConfig) BuildsMultipleOps() bool { + return false +} + // WriterOperator is an operator that can write to other operators. type WriterOperator struct { BasicOperator @@ -81,7 +86,7 @@ func (w *WriterOperator) Outputs() []operator.Operator { return w.OutputOperators } -// Outputs returns the outputs of the writer operator. +// GetOutputIDs returns the output IDs of the writer operator. func (w *WriterOperator) GetOutputIDs() []string { return w.OutputIDs } @@ -108,9 +113,8 @@ func (w *WriterOperator) SetOutputs(operators []operator.Operator) error { } // SetOutputs will set the outputs of the operator. -func (w *WriterOperator) SetOutputIDs(opIds []string) error { +func (w *WriterOperator) SetOutputIDs(opIds []string) { w.OutputIDs = opIds - return nil } // FindOperator will find an operator matching the supplied id. diff --git a/operator/operator.go b/operator/operator.go index d359d71e..a8c26e38 100644 --- a/operator/operator.go +++ b/operator/operator.go @@ -45,7 +45,7 @@ type Operator interface { // SetOutputs will set the connected outputs. SetOutputs([]Operator) error // SetOutputIDs will set the connected outputs' IDs. - SetOutputIDs([]string) error + SetOutputIDs([]string) // CanProcess indicates if the operator will process entries from other operators. CanProcess() bool diff --git a/pipeline/config.go b/pipeline/config.go index 3668a2db..4708ac1f 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -21,22 +21,24 @@ import ( // Config is the configuration of a pipeline. type Config []operator.Config -// BuildOperators builds the operators from the list of configs into operators +// BuildOperators builds the operators from the list of configs into operators. func (c Config) BuildOperators(bc operator.BuildContext) ([]operator.Operator, error) { operators := make([]operator.Operator, 0, len(c)) - pluginMap := make(map[string]string) + // pluginMap is used for storing the applicable plugins in the pipeline and points to the first operator of that plugin. + // The map is then used in SetOutputIDs to assign the output value of an ops pointing to the plugin to the first operator in said plugin. + buildsMulti := make(map[string]string) for _, builder := range c { op, err := builder.Build(bc) if err != nil { return nil, err } - if len(op) > 1 { - pluginMap[bc.PrependNamespace(builder.ID())] = op[0].ID() + if builder.BuildsMultipleOps() { + buildsMulti[bc.PrependNamespace(builder.ID())] = op[0].ID() } operators = append(operators, op...) } - SetOutputIDs(operators, pluginMap) + SetOutputIDs(operators, buildsMulti) return operators, nil } @@ -59,26 +61,32 @@ func (c Config) BuildPipeline(bc operator.BuildContext, defaultOperator operator return NewDirectedPipeline(operators) } -func SetOutputIDs(operators []operator.Operator, pluginMap map[string]string) error { +// SetOutputIDs Loops through all the operators and sets a default output to the next operator in the slice. +// Additionally, if the output is set to a plugin, it sets the output to the first operator in the plugins pipeline. +func SetOutputIDs(operators []operator.Operator, buildsMulti map[string]string) error { for i, op := range operators { - if len(op.GetOutputIDs()) == 0 && i+1 < len(operators) { - err := op.SetOutputIDs([]string{operators[i+1].ID()}) - if err != nil { - return err - } + if i+1 == len(operators) { + break + } + + if len(op.GetOutputIDs()) == 0 { + op.SetOutputIDs([]string{operators[i+1].ID()}) continue } + + // Check if there are any plugins within the outputIDs of the operator. If there is, change the output to be the first op in the plugin allOutputs := []string{} + pluginFound := false for _, id := range op.GetOutputIDs() { - if pluginMap[id] != "" { - allOutputs = append(allOutputs, pluginMap[id]) - continue + if pid, ok := buildsMulti[id]; ok { + id = pid + pluginFound = true } allOutputs = append(allOutputs, id) } - err := op.SetOutputIDs(allOutputs) - if err != nil { - return err + + if pluginFound { + op.SetOutputIDs(allOutputs) } } return nil diff --git a/plugin/config.go b/plugin/config.go index bd6728c9..d3c4751a 100644 --- a/plugin/config.go +++ b/plugin/config.go @@ -60,6 +60,8 @@ func (c *Config) Build(bc operator.BuildContext) ([]operator.Operator, error) { return pipelineConfig.Pipeline.BuildOperators(nbc) } +func (c *Config) BuildsMultipleOps() bool { return true } + func (c *Config) getRenderParams(bc operator.BuildContext) map[string]interface{} { // Copy the parameters to avoid mutating them params := map[string]interface{}{} diff --git a/testutil/mocks.go b/testutil/mocks.go index f2ec4867..8698167b 100644 --- a/testutil/mocks.go +++ b/testutil/mocks.go @@ -72,7 +72,7 @@ func (f *FakeOutput) GetOutputIDs() []string { return nil } func (f *FakeOutput) SetOutputs(outputs []operator.Operator) error { return nil } // SetOutputIDs immediately returns nil for a fake output -func (f *FakeOutput) SetOutputIDs(s []string) error { return nil } +func (f *FakeOutput) SetOutputIDs(s []string) {} // Start immediately returns nil for a fake output func (f *FakeOutput) Start(_ operator.Persister) error { return nil } diff --git a/testutil/operator.go b/testutil/operator.go index 730170ec..a255be13 100644 --- a/testutil/operator.go +++ b/testutil/operator.go @@ -123,17 +123,8 @@ func (_m *Operator) Process(_a0 context.Context, _a1 *entry.Entry) error { } // SetOutputIDs provides a mock function with given fields: _a0 -func (_m *Operator) SetOutputIDs(_a0 []string) error { - ret := _m.Called(_a0) - - var r0 error - if rf, ok := ret.Get(0).(func([]string) error); ok { - r0 = rf(_a0) - } else { - r0 = ret.Error(0) - } - - return r0 +func (_m *Operator) SetOutputIDs(_a0 []string) { + _m.Called(_a0) } // SetOutputs provides a mock function with given fields: _a0 diff --git a/testutil/operator_builder.go b/testutil/operator_builder.go index 653bd854..b14fe035 100644 --- a/testutil/operator_builder.go +++ b/testutil/operator_builder.go @@ -35,6 +35,20 @@ func (_m *OperatorBuilder) Build(_a0 operator.BuildContext) ([]operator.Operator return r0, r1 } +// BuildsMultipleOps provides a mock function with given fields: +func (_m *OperatorBuilder) BuildsMultipleOps() bool { + ret := _m.Called() + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + // ID provides a mock function with given fields: func (_m *OperatorBuilder) ID() string { ret := _m.Called() From 705cc0c034bf181b607f3fa2a931c94d58b4b436 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Fri, 21 May 2021 13:49:24 -0400 Subject: [PATCH 20/26] Update comment --- pipeline/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pipeline/config.go b/pipeline/config.go index 4708ac1f..94eb84c0 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -24,8 +24,8 @@ type Config []operator.Config // BuildOperators builds the operators from the list of configs into operators. func (c Config) BuildOperators(bc operator.BuildContext) ([]operator.Operator, error) { operators := make([]operator.Operator, 0, len(c)) - // pluginMap is used for storing the applicable plugins in the pipeline and points to the first operator of that plugin. - // The map is then used in SetOutputIDs to assign the output value of an ops pointing to the plugin to the first operator in said plugin. + // buildsMulti is used for storing a key of the Builder that builds multiple operators. + // The map is then used in SetOutputIDs to assign the output value of any ops pointing to the Builder as their output to the first operator in said Builder. buildsMulti := make(map[string]string) for _, builder := range c { op, err := builder.Build(bc) From 37f8e0da952138f01f374725230238cd2efaeacb Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Mon, 24 May 2021 10:21:36 -0400 Subject: [PATCH 21/26] Implement PR feedback --- operator/builtin/transformer/router/router.go | 2 +- operator/helper/writer.go | 2 +- operator/operator.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/operator/builtin/transformer/router/router.go b/operator/builtin/transformer/router/router.go index d213bacd..989a760a 100644 --- a/operator/builtin/transformer/router/router.go +++ b/operator/builtin/transformer/router/router.go @@ -160,7 +160,7 @@ func (p *RouterOperator) Outputs() []operator.Operator { return outputs } -// Outputs will return all connected operators. +// GetOutputIDs will return all connected operators. func (p *RouterOperator) GetOutputIDs() []string { outputs := make([]string, 0, len(p.routes)) for _, route := range p.routes { diff --git a/operator/helper/writer.go b/operator/helper/writer.go index ad1a7bd2..4157ce92 100644 --- a/operator/helper/writer.go +++ b/operator/helper/writer.go @@ -112,7 +112,7 @@ func (w *WriterOperator) SetOutputs(operators []operator.Operator) error { return nil } -// SetOutputs will set the outputs of the operator. +// SetOutputIDs will set the outputs of the operator. func (w *WriterOperator) SetOutputIDs(opIds []string) { w.OutputIDs = opIds } diff --git a/operator/operator.go b/operator/operator.go index a8c26e38..ba7fecfc 100644 --- a/operator/operator.go +++ b/operator/operator.go @@ -40,7 +40,7 @@ type Operator interface { CanOutput() bool // Outputs returns the list of connected outputs. Outputs() []Operator - // Outputs returns the list of connected outputs. + // GetOutputIDs returns the list of connected outputs. GetOutputIDs() []string // SetOutputs will set the connected outputs. SetOutputs([]Operator) error From fced5e3a9d12a5099d11ca92c855d4d15397f83f Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Wed, 25 Aug 2021 13:01:52 -0400 Subject: [PATCH 22/26] WIP --- pipeline/config.go | 15 +-- pipeline/config_test.go | 241 ++++++++++++++++++++-------------------- 2 files changed, 128 insertions(+), 128 deletions(-) diff --git a/pipeline/config.go b/pipeline/config.go index 6392af36..ffbaddf8 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -25,6 +25,7 @@ type Config []operator.Config // BuildOperators builds the operators from the list of configs into operators. func (c Config) BuildOperators(bc operator.BuildContext, defaultOperator operator.Operator) ([]operator.Operator, error) { + dedeplucateIDs(c) // buildsMulti's key represents an operator's ID that builds multiple operators, e.g. Plugins. // the value is the plugin's first operator's ID. buildsMulti := make(map[string]string) @@ -55,7 +56,7 @@ func (c Config) BuildOperators(bc operator.BuildContext, defaultOperator operato func dedeplucateIDs(ops []operator.Config) error { typeMap := make(map[string]int) for _, op := range ops { - if op.ID() != op.Type() { + if op.Type() != op.ID() { continue } @@ -64,20 +65,20 @@ func dedeplucateIDs(ops []operator.Config) error { continue } newID := fmt.Sprintf("%s%d", op.Type(), typeMap[op.Type()]) - for i := 0; i < len(ops); i++ { - if ops[i].ID() == newID { + + for j := 0; j < len(ops); j++ { + if newID == ops[j].ID() { + j = 0 typeMap[op.Type()]++ newID = fmt.Sprintf("%s%d", op.Type(), typeMap[op.Type()]) - i = 0 - continue } } + typeMap[op.Type()]++ err := op.SetID(newID) if err != nil { return err } - typeMap[op.Type()]++ } return nil } @@ -103,7 +104,7 @@ func SetOutputIDs(operators []operator.Operator, buildsMulti map[string]string) } if len(op.GetOutputIDs()) == 0 { - op.SetOutputIDs([]string{operators[i+1].ID()}) + operators[i].SetOutputIDs([]string{operators[i+1].ID()}) continue } diff --git a/pipeline/config_test.go b/pipeline/config_test.go index 7ddfc399..53141240 100644 --- a/pipeline/config_test.go +++ b/pipeline/config_test.go @@ -300,126 +300,126 @@ func TestUpdateOutputIDs(t *testing.T) { return ops }(), }, - { - "multi_op_rename", - func() Config { - var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - return ops - }, - func() []operator.Operator { - var ops []operator.Operator - op1 := newDummyOp("$.json_parser", "json_parser") - op1.OutputIDs = []string{"$.json_parser1"} - ops = append(ops, op1) - op2 := newDummyOp("$.json_parser1", "json_parser") - op2.OutputIDs = []string{"$.json_parser2"} - ops = append(ops, op2) - op3 := newDummyOp("$.json_parser2", "json_parser") - op3.OutputIDs = []string{"$.json_parser3"} - ops = append(ops, op3) - op4 := newDummyOp("$.json_parser3", "json_parser") - op4.OutputIDs = []string{"$.json_parser4"} - ops = append(ops, op4) - op5 := newDummyOp("$.json_parser4", "json_parser") - ops = append(ops, op5) - return ops - }(), - }, - { - "different_ops", - func() Config { - var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - return ops - }, - func() []operator.Operator { - var ops []operator.Operator - op1 := newDummyOp("$.json_parser", "json_parser") - op1.OutputIDs = []string{"$.json_parser1"} - ops = append(ops, op1) - op2 := newDummyOp("$.json_parser1", "json_parser") - op2.OutputIDs = []string{"$.json_parser2"} - ops = append(ops, op2) - op3 := newDummyOp("$.json_parser2", "json_parser") - op3.OutputIDs = []string{"$.copy"} - ops = append(ops, op3) - op4 := newDummyOp("$.copy", "copy") - op4.OutputIDs = []string{"$.copy1"} - ops = append(ops, op4) - op5 := newDummyOp("$.copy1", "copy") - ops = append(ops, op5) - return ops - }(), - }, - { - "unordered", - func() Config { - var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - return ops - }, - func() []operator.Operator { - var ops []operator.Operator - op1 := newDummyOp("$.json_parser", "json_parser") - op1.OutputIDs = []string{"$.copy"} - ops = append(ops, op1) - op2 := newDummyOp("$.copy", "copy") - op2.OutputIDs = []string{"$.json_parser1"} - ops = append(ops, op2) - op3 := newDummyOp("$.json_parser1", "json_parser") - op3.OutputIDs = []string{"$.copy1"} - ops = append(ops, op3) - op4 := newDummyOp("$.copy1", "copy") - op4.OutputIDs = []string{"$.json_parser2"} - ops = append(ops, op4) - op5 := newDummyOp("$.json_parser2", "json_parser") - ops = append(ops, op5) - return ops - }(), - }, - { - "already_renamed", - func() Config { - var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - return ops - }, - func() []operator.Operator { - var ops []operator.Operator - op1 := newDummyOp("$.json_parser", "json_parser") - op1.OutputIDs = []string{"$.json_parser1"} - ops = append(ops, op1) - op2 := newDummyOp("$.json_parser1", "json_parser") - op2.OutputIDs = []string{"$.json_parser2"} - ops = append(ops, op2) - op3 := newDummyOp("$.json_parser2", "json_parser") - op3.OutputIDs = []string{"$.json_parser3"} - ops = append(ops, op3) - op4 := newDummyOp("$.json_parser3", "json_parser") - op4.OutputIDs = []string{"$.json_parser4"} - ops = append(ops, op4) - op5 := newDummyOp("$.json_parser4", "json_parser") - ops = append(ops, op5) - return ops - }(), - }, + // { + // "multi_op_rename", + // func() Config { + // var ops Config + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // return ops + // }, + // func() []operator.Operator { + // var ops []operator.Operator + // op1 := newDummyOp("$.json_parser", "json_parser") + // op1.OutputIDs = []string{"$.json_parser1"} + // ops = append(ops, op1) + // op2 := newDummyOp("$.json_parser1", "json_parser") + // op2.OutputIDs = []string{"$.json_parser2"} + // ops = append(ops, op2) + // op3 := newDummyOp("$.json_parser2", "json_parser") + // op3.OutputIDs = []string{"$.json_parser3"} + // ops = append(ops, op3) + // op4 := newDummyOp("$.json_parser3", "json_parser") + // op4.OutputIDs = []string{"$.json_parser4"} + // ops = append(ops, op4) + // op5 := newDummyOp("$.json_parser4", "json_parser") + // ops = append(ops, op5) + // return ops + // }(), + // }, + // { + // "different_ops", + // func() Config { + // var ops Config + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + // return ops + // }, + // func() []operator.Operator { + // var ops []operator.Operator + // op1 := newDummyOp("$.json_parser", "json_parser") + // op1.OutputIDs = []string{"$.json_parser1"} + // ops = append(ops, op1) + // op2 := newDummyOp("$.json_parser1", "json_parser") + // op2.OutputIDs = []string{"$.json_parser2"} + // ops = append(ops, op2) + // op3 := newDummyOp("$.json_parser2", "json_parser") + // op3.OutputIDs = []string{"$.copy"} + // ops = append(ops, op3) + // op4 := newDummyOp("$.copy", "copy") + // op4.OutputIDs = []string{"$.copy1"} + // ops = append(ops, op4) + // op5 := newDummyOp("$.copy1", "copy") + // ops = append(ops, op5) + // return ops + // }(), + // }, + // { + // "unordered", + // func() Config { + // var ops Config + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // return ops + // }, + // func() []operator.Operator { + // var ops []operator.Operator + // op1 := newDummyOp("$.json_parser", "json_parser") + // op1.OutputIDs = []string{"$.copy"} + // ops = append(ops, op1) + // op2 := newDummyOp("$.copy", "copy") + // op2.OutputIDs = []string{"$.json_parser1"} + // ops = append(ops, op2) + // op3 := newDummyOp("$.json_parser1", "json_parser") + // op3.OutputIDs = []string{"$.copy1"} + // ops = append(ops, op3) + // op4 := newDummyOp("$.copy1", "copy") + // op4.OutputIDs = []string{"$.json_parser2"} + // ops = append(ops, op4) + // op5 := newDummyOp("$.json_parser2", "json_parser") + // ops = append(ops, op5) + // return ops + // }(), + // }, + // { + // "already_renamed", + // func() Config { + // var ops Config + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) + // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + // return ops + // }, + // func() []operator.Operator { + // var ops []operator.Operator + // op1 := newDummyOp("$.json_parser", "json_parser") + // op1.OutputIDs = []string{"$.json_parser1"} + // ops = append(ops, op1) + // op2 := newDummyOp("$.json_parser1", "json_parser") + // op2.OutputIDs = []string{"$.json_parser2"} + // ops = append(ops, op2) + // op3 := newDummyOp("$.json_parser2", "json_parser") + // op3.OutputIDs = []string{"$.json_parser3"} + // ops = append(ops, op3) + // op4 := newDummyOp("$.json_parser3", "json_parser") + // op4.OutputIDs = []string{"$.json_parser4"} + // ops = append(ops, op4) + // op5 := newDummyOp("$.json_parser4", "json_parser") + // ops = append(ops, op5) + // return ops + // }(), + // }, } for _, tc := range cases { @@ -429,7 +429,6 @@ func TestUpdateOutputIDs(t *testing.T) { ops, err := opsConfig.BuildOperators(bc, nil) require.NoError(t, err) require.Equal(t, tc.expectedOps, ops) - require.Equal(t, ops, tc.expectedOps) }) } } From cfc67f76c60647bced2d96414fef7d40c456dd80 Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Wed, 25 Aug 2021 16:38:57 -0400 Subject: [PATCH 23/26] Update test --- operator/config.go | 2 +- operator/config_test.go | 2 +- operator/helper/operator.go | 3 +- pipeline/config.go | 7 +- pipeline/config_test.go | 338 +++++++++--------------------------- 5 files changed, 89 insertions(+), 263 deletions(-) diff --git a/operator/config.go b/operator/config.go index 1e2d71c7..954772b7 100644 --- a/operator/config.go +++ b/operator/config.go @@ -31,7 +31,7 @@ type Builder interface { ID() string Type() string Build(BuildContext) ([]Operator, error) - SetID(string) error + SetID(string) BuildsMultipleOps() bool } diff --git a/operator/config_test.go b/operator/config_test.go index 445c1728..70671d19 100644 --- a/operator/config_test.go +++ b/operator/config_test.go @@ -31,7 +31,7 @@ type FakeBuilder struct { func (f *FakeBuilder) Build(context BuildContext) ([]Operator, error) { return nil, nil } func (f *FakeBuilder) ID() string { return "plugin" } func (f *FakeBuilder) Type() string { return "plugin" } -func (f *FakeBuilder) SetID(s string) error { return nil } +func (f *FakeBuilder) SetID(s string) {} func (f *FakeBuilder) BuildsMultipleOps() bool { return false } func TestUnmarshalJSONErrors(t *testing.T) { diff --git a/operator/helper/operator.go b/operator/helper/operator.go index f7bf6323..ff10f687 100644 --- a/operator/helper/operator.go +++ b/operator/helper/operator.go @@ -44,9 +44,8 @@ func (c BasicConfig) ID() string { } // SetID will Update the operator id. -func (c BasicConfig) SetID(id string) error { +func (c *BasicConfig) SetID(id string) { c.OperatorID = id - return nil } // Type will return the operator type. diff --git a/pipeline/config.go b/pipeline/config.go index ffbaddf8..061867ab 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -75,10 +75,7 @@ func dedeplucateIDs(ops []operator.Config) error { } typeMap[op.Type()]++ - err := op.SetID(newID) - if err != nil { - return err - } + op.SetID(newID) } return nil } @@ -104,7 +101,7 @@ func SetOutputIDs(operators []operator.Operator, buildsMulti map[string]string) } if len(op.GetOutputIDs()) == 0 { - operators[i].SetOutputIDs([]string{operators[i+1].ID()}) + op.SetOutputIDs([]string{operators[i+1].ID()}) continue } diff --git a/pipeline/config_test.go b/pipeline/config_test.go index 53141240..8ac445bc 100644 --- a/pipeline/config_test.go +++ b/pipeline/config_test.go @@ -15,138 +15,23 @@ package pipeline import ( - "context" - "fmt" "testing" "github.com/stretchr/testify/require" - "go.uber.org/zap" - "github.com/open-telemetry/opentelemetry-log-collection/entry" "github.com/open-telemetry/opentelemetry-log-collection/operator" + "github.com/open-telemetry/opentelemetry-log-collection/operator/builtin/transformer/noop" "github.com/open-telemetry/opentelemetry-log-collection/operator/helper" "github.com/open-telemetry/opentelemetry-log-collection/testutil" ) -type dummyOpConfig struct { - helper.WriterConfig -} - -func (d *dummyOpConfig) ID() string { - return d.OperatorID -} - -func (d *dummyOpConfig) Type() string { - return d.OperatorType -} - -func (d *dummyOpConfig) SetID(id string) error { - d.OperatorID = id - return nil -} - -func (c *dummyOpConfig) Build(bc operator.BuildContext) ([]operator.Operator, error) { - // Namespace all the output IDs - namespacedID := bc.PrependNamespace(c.ID()) - - namespacedIDs := c.OutputIDs.WithNamespace(bc) - if len(namespacedIDs) == 0 { - namespacedIDs = bc.DefaultOutputIDs - } - - dummy := dummyOperator{ - WriterOperator: helper.WriterOperator{ - OutputIDs: namespacedIDs, - BasicOperator: helper.BasicOperator{ - OperatorType: c.OperatorType, - OperatorID: namespacedID, - }, - }, - } - - return []operator.Operator{dummy}, nil -} - -type dummyOperator struct { - helper.WriterOperator -} - -// ID returns the id of the operator. -func (d dummyOperator) ID() string { return d.OperatorID } - -// Type returns the type of the operator. -func (d dummyOperator) Type() string { return d.OperatorType } - -func (d dummyOperator) CanOutput() bool { return true } - -func (d dummyOperator) CanProcess() bool { return true } - -func (d dummyOperator) SetOutputs(operators []operator.Operator) error { - outputOperators := make([]operator.Operator, 0) - - for _, operatorID := range d.OutputIDs { - operator, ok := d.findOperator(operators, operatorID) - if !ok { - return fmt.Errorf("operator '%s' does not exist", operatorID) - } - - if !operator.CanProcess() { - return fmt.Errorf("operator '%s' can not process entries", operatorID) - } - - outputOperators = append(outputOperators, operator) - } - - d.OutputOperators = outputOperators - return nil -} - -func (w *dummyOperator) findOperator(operators []operator.Operator, operatorID string) (operator.Operator, bool) { - for _, operator := range operators { - if operator.ID() == operatorID { - return operator, true - } - } - return nil, false -} - -func (d dummyOperator) Outputs() []operator.Operator { return nil } - -func (d dummyOperator) GetOutputIDs() []string { return d.WriterOperator.OutputIDs } - -func (d dummyOperator) SetOutputIDs(ids []string) { d.OutputIDs = ids } - -func (d dummyOperator) Start(operator.Persister) error { return nil } - -func (d dummyOperator) Stop() error { return nil } - -// Process will process an entry from an operator. -func (d dummyOperator) Process(context.Context, *entry.Entry) error { return nil } - -// Logger returns the operator's logger -func (d dummyOperator) Logger() *zap.SugaredLogger { return nil } - -func newDummyOpConfig(dummyID string, dummyType string) *dummyOpConfig { - return &dummyOpConfig{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: dummyID, - OperatorType: dummyType, - }, +func newDummyOpConfig(dummyID string, dummyType string) *operator.Config { + return &operator.Config{ + Builder: &noop.NoopOperatorConfig{ + TransformerConfig: helper.NewTransformerConfig(dummyID, dummyType), }, } -} -func newDummyOp(dummyID string, dummyType string) dummyOperator { - return dummyOperator{ - WriterOperator: helper.WriterOperator{ - BasicOperator: helper.BasicOperator{ - OperatorID: dummyID, - OperatorType: dummyType, - SugaredLogger: nil, - }, - }, - } } type deduplicateTestCase struct { @@ -276,9 +161,9 @@ func TestDeduplicateIDs(t *testing.T) { } type outputTestCase struct { - name string - ops func() Config - expectedOps []operator.Operator + name string + ops func() Config + expectedOutputs []string } func TestUpdateOutputIDs(t *testing.T) { @@ -291,144 +176,89 @@ func TestUpdateOutputIDs(t *testing.T) { ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) return ops }, - func() []operator.Operator { - var ops []operator.Operator - op1 := newDummyOp("$.json_parser", "json_parser") - op1.OutputIDs = []string{"$.json_parser1"} - ops = append(ops, op1) - ops = append(ops, newDummyOp("$.json_parser1", "json_parser")) + []string{ + "$.json_parser1", + }, + }, + { + "multi_op_rename", + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) return ops - }(), + }, + []string{ + "$.json_parser1", + "$.json_parser2", + "$.json_parser3", + }, + }, + { + "different_ops", + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + return ops + }, + []string{ + "$.json_parser1", + "$.json_parser2", + "$.copy", + "$.copy1", + }, + }, + { + "unordered", + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + return ops + }, + []string{ + "$.copy", + "$.json_parser1", + "$.copy1", + }, + }, + { + "already_renamed", + func() Config { + var ops Config + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) + ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + return ops + }, + []string{ + "$.json_parser1", + "$.json_parser2", + "$.json_parser3", + "$.json_parser4", + }, }, - // { - // "multi_op_rename", - // func() Config { - // var ops Config - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // return ops - // }, - // func() []operator.Operator { - // var ops []operator.Operator - // op1 := newDummyOp("$.json_parser", "json_parser") - // op1.OutputIDs = []string{"$.json_parser1"} - // ops = append(ops, op1) - // op2 := newDummyOp("$.json_parser1", "json_parser") - // op2.OutputIDs = []string{"$.json_parser2"} - // ops = append(ops, op2) - // op3 := newDummyOp("$.json_parser2", "json_parser") - // op3.OutputIDs = []string{"$.json_parser3"} - // ops = append(ops, op3) - // op4 := newDummyOp("$.json_parser3", "json_parser") - // op4.OutputIDs = []string{"$.json_parser4"} - // ops = append(ops, op4) - // op5 := newDummyOp("$.json_parser4", "json_parser") - // ops = append(ops, op5) - // return ops - // }(), - // }, - // { - // "different_ops", - // func() Config { - // var ops Config - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - // return ops - // }, - // func() []operator.Operator { - // var ops []operator.Operator - // op1 := newDummyOp("$.json_parser", "json_parser") - // op1.OutputIDs = []string{"$.json_parser1"} - // ops = append(ops, op1) - // op2 := newDummyOp("$.json_parser1", "json_parser") - // op2.OutputIDs = []string{"$.json_parser2"} - // ops = append(ops, op2) - // op3 := newDummyOp("$.json_parser2", "json_parser") - // op3.OutputIDs = []string{"$.copy"} - // ops = append(ops, op3) - // op4 := newDummyOp("$.copy", "copy") - // op4.OutputIDs = []string{"$.copy1"} - // ops = append(ops, op4) - // op5 := newDummyOp("$.copy1", "copy") - // ops = append(ops, op5) - // return ops - // }(), - // }, - // { - // "unordered", - // func() Config { - // var ops Config - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // return ops - // }, - // func() []operator.Operator { - // var ops []operator.Operator - // op1 := newDummyOp("$.json_parser", "json_parser") - // op1.OutputIDs = []string{"$.copy"} - // ops = append(ops, op1) - // op2 := newDummyOp("$.copy", "copy") - // op2.OutputIDs = []string{"$.json_parser1"} - // ops = append(ops, op2) - // op3 := newDummyOp("$.json_parser1", "json_parser") - // op3.OutputIDs = []string{"$.copy1"} - // ops = append(ops, op3) - // op4 := newDummyOp("$.copy1", "copy") - // op4.OutputIDs = []string{"$.json_parser2"} - // ops = append(ops, op4) - // op5 := newDummyOp("$.json_parser2", "json_parser") - // ops = append(ops, op5) - // return ops - // }(), - // }, - // { - // "already_renamed", - // func() Config { - // var ops Config - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) - // ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - // return ops - // }, - // func() []operator.Operator { - // var ops []operator.Operator - // op1 := newDummyOp("$.json_parser", "json_parser") - // op1.OutputIDs = []string{"$.json_parser1"} - // ops = append(ops, op1) - // op2 := newDummyOp("$.json_parser1", "json_parser") - // op2.OutputIDs = []string{"$.json_parser2"} - // ops = append(ops, op2) - // op3 := newDummyOp("$.json_parser2", "json_parser") - // op3.OutputIDs = []string{"$.json_parser3"} - // ops = append(ops, op3) - // op4 := newDummyOp("$.json_parser3", "json_parser") - // op4.OutputIDs = []string{"$.json_parser4"} - // ops = append(ops, op4) - // op5 := newDummyOp("$.json_parser4", "json_parser") - // ops = append(ops, op5) - // return ops - // }(), - // }, } for _, tc := range cases { - t.Run("Deduplicate/"+tc.name, func(t *testing.T) { - opsConfig := tc.ops() + t.Run("UpdateOutputIDs/"+tc.name, func(t *testing.T) { bc := testutil.NewBuildContext(t) - ops, err := opsConfig.BuildOperators(bc, nil) + ops, err := tc.ops().BuildOperators(bc, nil) require.NoError(t, err) - require.Equal(t, tc.expectedOps, ops) + require.Equal(t, len(tc.expectedOutputs), len(ops)-1) + for i := 0; i < len(ops)-1; i++ { + require.Equal(t, tc.expectedOutputs[i], ops[i].GetOutputIDs()[0]) + } }) } } From 709d89bde64a46422459aa6af5a9a7f321f2b54c Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Wed, 25 Aug 2021 17:01:07 -0400 Subject: [PATCH 24/26] Remove error return value --- pipeline/config.go | 3 +-- pipeline/config_test.go | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pipeline/config.go b/pipeline/config.go index 061867ab..52f81f64 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -53,7 +53,7 @@ func (c Config) BuildOperators(bc operator.BuildContext, defaultOperator operato return operators, nil } -func dedeplucateIDs(ops []operator.Config) error { +func dedeplucateIDs(ops []operator.Config) { typeMap := make(map[string]int) for _, op := range ops { if op.Type() != op.ID() { @@ -77,7 +77,6 @@ func dedeplucateIDs(ops []operator.Config) error { typeMap[op.Type()]++ op.SetID(newID) } - return nil } // BuildPipeline will build a pipeline from the config. diff --git a/pipeline/config_test.go b/pipeline/config_test.go index 8ac445bc..75bb6ea9 100644 --- a/pipeline/config_test.go +++ b/pipeline/config_test.go @@ -31,7 +31,6 @@ func newDummyOpConfig(dummyID string, dummyType string) *operator.Config { TransformerConfig: helper.NewTransformerConfig(dummyID, dummyType), }, } - } type deduplicateTestCase struct { From 68f6b1fac7ed630f816e7f88d7a1f82e4aaa354e Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Tue, 7 Sep 2021 09:13:59 -0400 Subject: [PATCH 25/26] PR Feedback --- pipeline/config.go | 10 +- pipeline/config_test.go | 173 ++++++++++--------- plugin/config_test.go | 369 +++++++++++----------------------------- 3 files changed, 201 insertions(+), 351 deletions(-) diff --git a/pipeline/config.go b/pipeline/config.go index 52f81f64..a987794c 100644 --- a/pipeline/config.go +++ b/pipeline/config.go @@ -25,7 +25,7 @@ type Config []operator.Config // BuildOperators builds the operators from the list of configs into operators. func (c Config) BuildOperators(bc operator.BuildContext, defaultOperator operator.Operator) ([]operator.Operator, error) { - dedeplucateIDs(c) + c.dedeplucateIDs() // buildsMulti's key represents an operator's ID that builds multiple operators, e.g. Plugins. // the value is the plugin's first operator's ID. buildsMulti := make(map[string]string) @@ -53,9 +53,9 @@ func (c Config) BuildOperators(bc operator.BuildContext, defaultOperator operato return operators, nil } -func dedeplucateIDs(ops []operator.Config) { +func (c Config) dedeplucateIDs() { typeMap := make(map[string]int) - for _, op := range ops { + for _, op := range c { if op.Type() != op.ID() { continue } @@ -66,8 +66,8 @@ func dedeplucateIDs(ops []operator.Config) { } newID := fmt.Sprintf("%s%d", op.Type(), typeMap[op.Type()]) - for j := 0; j < len(ops); j++ { - if newID == ops[j].ID() { + for j := 0; j < len(c); j++ { + if newID == c[j].ID() { j = 0 typeMap[op.Type()]++ newID = fmt.Sprintf("%s%d", op.Type(), typeMap[op.Type()]) diff --git a/pipeline/config_test.go b/pipeline/config_test.go index 75bb6ea9..f3c953f2 100644 --- a/pipeline/config_test.go +++ b/pipeline/config_test.go @@ -20,17 +20,17 @@ import ( "github.com/stretchr/testify/require" "github.com/open-telemetry/opentelemetry-log-collection/operator" - "github.com/open-telemetry/opentelemetry-log-collection/operator/builtin/transformer/noop" - "github.com/open-telemetry/opentelemetry-log-collection/operator/helper" + "github.com/open-telemetry/opentelemetry-log-collection/operator/builtin/parser/json" + "github.com/open-telemetry/opentelemetry-log-collection/operator/builtin/transformer/copy" "github.com/open-telemetry/opentelemetry-log-collection/testutil" ) -func newDummyOpConfig(dummyID string, dummyType string) *operator.Config { - return &operator.Config{ - Builder: &noop.NoopOperatorConfig{ - TransformerConfig: helper.NewTransformerConfig(dummyID, dummyType), - }, - } +func newDummyJSON(dummyID string) operator.Config { + return operator.Config{Builder: json.NewJSONParserConfig(dummyID)} +} + +func newDummyCopy(dummyID string) operator.Config { + return operator.Config{Builder: copy.NewCopyOperatorConfig(dummyID)} } type deduplicateTestCase struct { @@ -45,18 +45,14 @@ func TestDeduplicateIDs(t *testing.T) { "one_op_rename", func() Config { var ops Config - op1 := operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")} - ops = append(ops, op1) - op2 := operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")} - ops = append(ops, op2) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) return ops }, func() Config { var ops Config - op1 := operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")} - ops = append(ops, op1) - op2 := operator.Config{Builder: newDummyOpConfig("json_parser1", "json_parser")} - ops = append(ops, op2) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser1")) return ops }(), }, @@ -64,21 +60,21 @@ func TestDeduplicateIDs(t *testing.T) { "multi_op_rename", func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) return ops }, func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser1", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser2", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser4", "json_parser")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser1")) + ops = append(ops, newDummyJSON("json_parser2")) + ops = append(ops, newDummyJSON("json_parser3")) + ops = append(ops, newDummyJSON("json_parser4")) return ops }(), }, @@ -86,21 +82,21 @@ func TestDeduplicateIDs(t *testing.T) { "different_ops", func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyCopy("copy")) + ops = append(ops, newDummyCopy("copy")) return ops }, func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser1", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser2", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy1", "copy")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser1")) + ops = append(ops, newDummyJSON("json_parser2")) + ops = append(ops, newDummyCopy("copy")) + ops = append(ops, newDummyCopy("copy1")) return ops }(), }, @@ -108,21 +104,20 @@ func TestDeduplicateIDs(t *testing.T) { "unordered", func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyCopy("copy")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyCopy("copy")) + ops = append(ops, newDummyJSON("json_parser")) return ops }, func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser1", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy1", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser2", "json_parser")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyCopy("copy")) + ops = append(ops, newDummyJSON("json_parser1")) + ops = append(ops, newDummyCopy("copy1")) + ops = append(ops, newDummyJSON("json_parser2")) return ops }(), }, @@ -130,21 +125,41 @@ func TestDeduplicateIDs(t *testing.T) { "already_renamed", func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser3")) + ops = append(ops, newDummyJSON("json_parser")) + return ops + }, + func() Config { + var ops Config + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser1")) + ops = append(ops, newDummyJSON("json_parser2")) + ops = append(ops, newDummyJSON("json_parser3")) + ops = append(ops, newDummyJSON("json_parser4")) + return ops + }(), + }, + { + "iterate_twice", + func() Config { + var ops Config + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser3")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) return ops }, func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser1", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser2", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser4", "json_parser")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser1")) + ops = append(ops, newDummyJSON("json_parser3")) + ops = append(ops, newDummyJSON("json_parser2")) + ops = append(ops, newDummyJSON("json_parser4")) return ops }(), }, @@ -153,7 +168,7 @@ func TestDeduplicateIDs(t *testing.T) { for _, tc := range cases { t.Run("Deduplicate/"+tc.name, func(t *testing.T) { ops := tc.ops() - dedeplucateIDs(ops) + ops.dedeplucateIDs() require.Equal(t, ops, tc.expectedOps) }) } @@ -171,8 +186,8 @@ func TestUpdateOutputIDs(t *testing.T) { "one_op_rename", func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) return ops }, []string{ @@ -183,10 +198,10 @@ func TestUpdateOutputIDs(t *testing.T) { "multi_op_rename", func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) return ops }, []string{ @@ -199,11 +214,11 @@ func TestUpdateOutputIDs(t *testing.T) { "different_ops", func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyCopy("copy")) + ops = append(ops, newDummyCopy("copy")) return ops }, []string{ @@ -217,10 +232,10 @@ func TestUpdateOutputIDs(t *testing.T) { "unordered", func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("copy", "copy")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyCopy("copy")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyCopy("copy")) return ops }, []string{ @@ -233,11 +248,11 @@ func TestUpdateOutputIDs(t *testing.T) { "already_renamed", func() Config { var ops Config - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser3", "json_parser")}) - ops = append(ops, operator.Config{Builder: newDummyOpConfig("json_parser", "json_parser")}) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser")) + ops = append(ops, newDummyJSON("json_parser3")) + ops = append(ops, newDummyJSON("json_parser")) return ops }, []string{ diff --git a/plugin/config_test.go b/plugin/config_test.go index 29ac4f92..1901c60a 100644 --- a/plugin/config_test.go +++ b/plugin/config_test.go @@ -107,15 +107,28 @@ type PluginIDTestCase struct { ExpectedOpIDs []string } +func dummyPlugin(pluginType string, pluginVar *Plugin) operator.Config { + return operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginType, + OperatorType: pluginType, + }, + }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, + }, + } +} + func TestPluginIDs(t *testing.T) { // TODO: ids shouldn't need to be specified once autogen IDs are implemented pluginContent := []byte(` parameters: pipeline: - - id: noop - type: noop - - id: noop1 - type: noop + - type: noop + - type: noop output: {{ .output }} `) pluginName := "my_plugin" @@ -127,10 +140,8 @@ pipeline: pluginContent2 := []byte(` parameters: pipeline: - - id: noop - type: noop - - id: noop1 - type: noop + - type: noop + - type: noop `) secondPlugin := "secondPlugin" secondPluginVar, err := NewPlugin(secondPlugin, pluginContent2) @@ -140,21 +151,10 @@ pipeline: cases := []PluginIDTestCase{ { Name: "basic_plugin", - PluginConfig: func() []operator.Config { - return pipeline.Config{ - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: pluginName, - OperatorType: pluginName, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: pluginVar, - }, - }, - } + PluginConfig: func() pipeline.Config { + var ops pipeline.Config + ops = append(ops, dummyPlugin(pluginName, pluginVar)) + return ops }(), ExpectedOpIDs: []string{ "$." + pluginName + ".noop", @@ -163,25 +163,11 @@ pipeline: }, { Name: "same_op_outside_plugin", - PluginConfig: func() []operator.Config { - return pipeline.Config{ - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: pluginName, - OperatorType: pluginName, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: pluginVar, - }, - }, - operator.Config{ - // TODO: ID should be noop to start then auto gened to noop2 - Builder: noop.NewNoopOperatorConfig("noop2"), - }, - } + PluginConfig: func() pipeline.Config { + var ops pipeline.Config + ops = append(ops, dummyPlugin(pluginName, pluginVar)) + ops = append(ops, operator.Config{Builder: noop.NewNoopOperatorConfig("noop2")}) + return ops }(), ExpectedOpIDs: []string{ "$." + pluginName + ".noop", @@ -191,33 +177,11 @@ pipeline: }, { Name: "two_plugins_with_same_ops", - PluginConfig: func() []operator.Config { - return pipeline.Config{ - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: pluginName, - OperatorType: pluginName, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: pluginVar, - }, - }, - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: secondPlugin, - OperatorType: secondPlugin, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: secondPluginVar, - }, - }, - } + PluginConfig: func() pipeline.Config { + var ops pipeline.Config + ops = append(ops, dummyPlugin(pluginName, pluginVar)) + ops = append(ops, dummyPlugin(secondPlugin, secondPluginVar)) + return ops }(), ExpectedOpIDs: []string{ "$." + pluginName + ".noop", @@ -229,13 +193,15 @@ pipeline: } for _, tc := range cases { - ops, err := tc.PluginConfig.BuildOperators(testutil.NewBuildContext(t), nil) - require.NoError(t, err) + t.Run(tc.Name, func(t *testing.T) { + ops, err := tc.PluginConfig.BuildOperators(testutil.NewBuildContext(t), nil) + require.NoError(t, err) - require.Len(t, ops, len(tc.ExpectedOpIDs)) - for i, op := range ops { - require.Equal(t, tc.ExpectedOpIDs[i], op.ID()) - } + require.Len(t, ops, len(tc.ExpectedOpIDs)) + for i, op := range ops { + require.Equal(t, tc.ExpectedOpIDs[i], op.ID()) + } + }) } } @@ -246,13 +212,11 @@ type PluginOutputIDTestCase struct { } func TestPluginOutputIDs(t *testing.T) { - // TODO: ids shouldn't need to be specified once autogen IDs are implemented pluginContent := []byte(` parameters: pipeline: - type: noop - - id: noop1 - type: noop + - type: noop output: {{ .output }} `) pluginName := "my_plugin" @@ -260,13 +224,11 @@ pipeline: require.NoError(t, err) operator.RegisterPlugin(pluginVar.ID, pluginVar.NewBuilder) - // TODO: remove ID assignment pluginContent2 := []byte(` parameters: pipeline: - type: noop - - id: noop1 - type: noop + - type: noop output: {{ .output }} `) secondPlugin := "secondPlugin" @@ -289,28 +251,12 @@ pipeline: cases := []PluginOutputIDTestCase{ { Name: "same_op_outside_plugin", - PluginConfig: func() []operator.Config { - return pipeline.Config{ - operator.Config{ - Builder: noop.NewNoopOperatorConfig("noop"), - }, - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: pluginName, - OperatorType: pluginName, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: pluginVar, - }, - }, - operator.Config{ - // TODO: ID should be noop to start then auto gened to noop1 - Builder: noop.NewNoopOperatorConfig("noop1"), - }, - } + PluginConfig: func() pipeline.Config { + var ops pipeline.Config + ops = append(ops, operator.Config{Builder: noop.NewNoopOperatorConfig("noop")}) + ops = append(ops, dummyPlugin(pluginName, pluginVar)) + ops = append(ops, operator.Config{Builder: noop.NewNoopOperatorConfig("noop")}) + return ops }(), ExpectedOpIDs: map[string][]string{ "$.noop": {"$." + pluginName + ".noop"}, @@ -320,33 +266,11 @@ pipeline: }, { Name: "two_plugins_with_same_ops", - PluginConfig: func() []operator.Config { - return pipeline.Config{ - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: pluginName, - OperatorType: pluginName, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: pluginVar, - }, - }, - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: secondPlugin, - OperatorType: secondPlugin, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: secondPluginVar, - }, - }, - } + PluginConfig: func() pipeline.Config { + var ops pipeline.Config + ops = append(ops, dummyPlugin(pluginName, pluginVar)) + ops = append(ops, dummyPlugin(secondPlugin, secondPluginVar)) + return ops }(), ExpectedOpIDs: map[string][]string{ "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, @@ -356,37 +280,25 @@ pipeline: }, { Name: "two_plugins_specified_output", - PluginConfig: func() []operator.Config { - return pipeline.Config{ - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: pluginName, - OperatorType: pluginName, - }, - OutputIDs: []string{"noop"}, - }, - Parameters: map[string]interface{}{}, - Plugin: pluginVar, - }, - }, - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: secondPlugin, - OperatorType: secondPlugin, - }, + PluginConfig: func() pipeline.Config { + var ops pipeline.Config + plugin1 := operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, }, - Parameters: map[string]interface{}{}, - Plugin: secondPluginVar, + OutputIDs: []string{"$.noop"}, }, - }, - operator.Config{ - Builder: noop.NewNoopOperatorConfig("noop"), + Parameters: map[string]interface{}{}, + Plugin: pluginVar, }, } + ops = append(ops, plugin1) + ops = append(ops, dummyPlugin(secondPlugin, secondPluginVar)) + ops = append(ops, operator.Config{Builder: noop.NewNoopOperatorConfig("noop")}) + return ops }(), ExpectedOpIDs: map[string][]string{ "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, @@ -397,37 +309,25 @@ pipeline: }, { Name: "two_plugins_output_to_non_sequential_plugin", - PluginConfig: func() []operator.Config { - return pipeline.Config{ - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: pluginName, - OperatorType: pluginName, - }, - OutputIDs: []string{secondPlugin}, - }, - Parameters: map[string]interface{}{}, - Plugin: pluginVar, - }, - }, - operator.Config{ - Builder: noop.NewNoopOperatorConfig("noop"), - }, - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: secondPlugin, - OperatorType: secondPlugin, - }, + PluginConfig: func() pipeline.Config { + var ops pipeline.Config + plugin1 := operator.Config{ + Builder: &Config{ + WriterConfig: helper.WriterConfig{ + BasicConfig: helper.BasicConfig{ + OperatorID: pluginName, + OperatorType: pluginName, }, - Parameters: map[string]interface{}{}, - Plugin: secondPluginVar, + OutputIDs: []string{"$." + secondPlugin}, }, + Parameters: map[string]interface{}{}, + Plugin: pluginVar, }, } + ops = append(ops, plugin1) + ops = append(ops, operator.Config{Builder: noop.NewNoopOperatorConfig("noop")}) + ops = append(ops, dummyPlugin(secondPlugin, secondPluginVar)) + return ops }(), ExpectedOpIDs: map[string][]string{ "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, @@ -438,44 +338,14 @@ pipeline: }, { Name: "two_plugins_with_multiple_outside_ops", - PluginConfig: func() []operator.Config { - return pipeline.Config{ - operator.Config{ - Builder: noop.NewNoopOperatorConfig("noop"), - }, - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: pluginName, - OperatorType: pluginName, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: pluginVar, - }, - }, - operator.Config{ - // TODO: ID should be noop to start then auto gened to noop1 - Builder: noop.NewNoopOperatorConfig("noop1"), - }, - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: secondPlugin, - OperatorType: secondPlugin, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: secondPluginVar, - }, - }, - operator.Config{ - // TODO: ID should be noop to start then auto gened to noop2 - Builder: noop.NewNoopOperatorConfig("noop2"), - }, - } + PluginConfig: func() pipeline.Config { + var ops pipeline.Config + ops = append(ops, operator.Config{Builder: noop.NewNoopOperatorConfig("noop")}) + ops = append(ops, dummyPlugin(pluginName, pluginVar)) + ops = append(ops, operator.Config{Builder: noop.NewNoopOperatorConfig("noop1")}) + ops = append(ops, dummyPlugin(secondPlugin, secondPluginVar)) + ops = append(ops, operator.Config{Builder: noop.NewNoopOperatorConfig("noop2")}) + return ops }(), ExpectedOpIDs: map[string][]string{ "$.noop": {"$." + pluginName + ".noop"}, @@ -488,33 +358,11 @@ pipeline: }, { Name: "two_plugins_of_same_type", - PluginConfig: func() []operator.Config { - return pipeline.Config{ - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: pluginName, - OperatorType: pluginName, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: pluginVar, - }, - }, - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: pluginName + "1", - OperatorType: pluginName, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: secondPluginVar, - }, - }, - } + PluginConfig: func() pipeline.Config { + var ops pipeline.Config + ops = append(ops, dummyPlugin(pluginName, pluginVar)) + ops = append(ops, dummyPlugin(pluginName, pluginVar)) + return ops }(), ExpectedOpIDs: map[string][]string{ "$." + pluginName + ".noop": {"$." + pluginName + ".noop1"}, @@ -524,24 +372,11 @@ pipeline: }, { Name: "plugin_within_a_plugin", - PluginConfig: func() []operator.Config { - return pipeline.Config{ - operator.Config{ - Builder: &Config{ - WriterConfig: helper.WriterConfig{ - BasicConfig: helper.BasicConfig{ - OperatorID: layeredPlugin, - OperatorType: layeredPlugin, - }, - }, - Parameters: map[string]interface{}{}, - Plugin: layeredPluginVar, - }, - }, - operator.Config{ - Builder: noop.NewNoopOperatorConfig("noop"), - }, - } + PluginConfig: func() pipeline.Config { + var ops pipeline.Config + ops = append(ops, dummyPlugin(layeredPlugin, layeredPluginVar)) + ops = append(ops, operator.Config{Builder: noop.NewNoopOperatorConfig("noop")}) + return ops }(), ExpectedOpIDs: map[string][]string{ "$.layeredPlugin." + pluginName + ".noop": {"$.layeredPlugin." + pluginName + ".noop1"}, From 99125fbd69243d3af558d869ce84fe6bed06f07b Mon Sep 17 00:00:00 2001 From: Mrod1598 Date: Tue, 7 Sep 2021 15:31:03 -0400 Subject: [PATCH 26/26] Rename test helper file --- testutil/{operator_builder.go => operator_builder_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename testutil/{operator_builder.go => operator_builder_test.go} (100%) diff --git a/testutil/operator_builder.go b/testutil/operator_builder_test.go similarity index 100% rename from testutil/operator_builder.go rename to testutil/operator_builder_test.go