Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

[Agent] Fix merge of config #17399

Merged
merged 3 commits into from
Apr 2, 2020
Merged

[Agent] Fix merge of config #17399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96d8b74
copying everything
michalpristas Apr 1, 2020
d6cce86
mage vendor
michalpristas Apr 1, 2020
09c4d44
changelog
michalpristas Apr 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x-pack/agent/CHANGELOG.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

- Fixed tests on windows {pull}16922[16922]
- Fixed installers for SNAPSHOTs and windows {pull}17077[17077]
- Fixed merge of config {pull}17399[17399]

==== New features

Expand Down
2 changes: 1 addition & 1 deletion x-pack/agent/pkg/agent/program/supported.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions x-pack/agent/pkg/agent/program/testdata/single_config-filebeat.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ filebeat:
- /var/log/hello1.log
- /var/log/hello2.log
index: logs-generic-default
vars:
var: value
output:
elasticsearch:
hosts:
Expand Down
2 changes: 2 additions & 0 deletions x-pack/agent/pkg/agent/program/testdata/single_config-metricbeat.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ metricbeat:
- module: docker
metricsets: [status]
index: metrics-docker.status-default
hosts: ["http://127.0.0.1:8080"]
- module: apache
metricsets: [info]
index: metrics-generic-testing
hosts: ["http://apache.remote"]
output:
elasticsearch:
hosts: [127.0.0.1:9200, 127.0.0.1:9300]
Expand Down
6 changes: 6 additions & 0 deletions x-pack/agent/pkg/agent/program/testdata/single_config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,24 @@ datasources:
streams:
- metricset: status
dataset: docker.status
hosts: ["http://127.0.0.1:8080"]
- type: logs
streams:
- paths:
- /var/log/hello1.log
- /var/log/hello2.log
vars:
var: value
- namespace: testing
use_output: default
inputs:
- type: apache/metrics
streams:
- enabled: true
metricset: info
hosts: ["http://apache.remote"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this is just a bad example? Because the same field should not exist in stream an input.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes i just needed to test override functionality somehow

Copy link
Contributor

Choose a reason for hiding this comment

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

Wasn't the conclusion we don't support override? @ph ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ruflin @michalpristas I think the conclusion was effectively we do not support overrides, I've looked at our email and this was the behavior he had in mind.

  • for keys defined in the input section we apply them in the streams.
  • for keys defined in the input section and present in the streams we fail hard.
  • If a key is not defined in the inputs but defined in the stream we take the value of the stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ruflin yes we don't this test is just to make sure nothing is replaced by anything defined at inputs level so we make sure it does not happen as we copy all the fields down the tree. where we need overrride are situation when we generate somethin in tree and we want to make sure it is not lost (e.g. processors) we generate on stream level (just like index) but then we merge it with top level processors (we cannot generate at top level as fields are stream specific)

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the comment from @ph , I would assume this test should fail hard? "for keys defined in the input section and present in the streams we fail hard."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think you're right, will make it fail entirely

hosts: ["http://apache.local"]
id: apache-metrics-id

settings.monitoring:
use_output: monitoring
Expand Down
72 changes: 67 additions & 5 deletions x-pack/agent/pkg/agent/transpiler/rules.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ func (r *RuleList) MarshalYAML() (interface{}, error) {
name = "copy"
case *CopyToListRule:
name = "copy_to_list"
case *CopyAllToListRule:
name = "copy_all_to_list"
case *RenameRule:
name = "rename"
case *TranslateRule:
Expand Down Expand Up @@ -123,6 +125,8 @@ func (r *RuleList) UnmarshalYAML(unmarshal func(interface{}) error) error {
r = &CopyRule{}
case "copy_to_list":
r = &CopyToListRule{}
case "copy_all_to_list":
r = &CopyAllToListRule{}
case "rename":
r = &RenameRule{}
case "translate":
Expand Down Expand Up @@ -231,8 +235,9 @@ func MakeArray(item Selector, to string) *MakeArrayRule {
// CopyToListRule is a rule which copies a specified
// node into every item in a provided list.
type CopyToListRule struct {
Item Selector
To string
Item Selector
To string
Overwrite bool
}

// Apply copies specified node into every item of the list.
Expand Down Expand Up @@ -260,17 +265,74 @@ func (r *CopyToListRule) Apply(ast *AST) error {
continue
}

if !r.Overwrite {
_, found := listItemMap.Find(r.Item)
if found {
continue
}
}

listItemMap.value = append(listItemMap.value, sourceNode.Clone())
}

return nil
}

// CopyToList creates a CopyToListRule
func CopyToList(item Selector, to string) *CopyToListRule {
func CopyToList(item Selector, to string, overwrite bool) *CopyToListRule {
return &CopyToListRule{
Item: item,
To: to,
Item: item,
To: to,
Overwrite: overwrite,
}
}

// CopyAllToListRule is a rule which copies a all nodes
// into every item in a provided list.
type CopyAllToListRule struct {
To string
Except []string
Overwrite bool
}

// Apply copies all nodes into every item of the list.
func (r *CopyAllToListRule) Apply(ast *AST) error {
// get list of nodes
astMap, err := ast.Map()
if err != nil {
return err
}

isFiltered := func(item string) bool {
for _, f := range r.Except {
if f == item {
return true
}
}

return false
}

// foreach node if not filtered out
for item := range astMap {
if isFiltered(item) {
continue
}

if err := CopyToList(item, r.To, r.Overwrite).Apply(ast); err != nil {
return err
}
}

return nil
}

// CopyAllToList creates a CopyAllToListRule
func CopyAllToList(to string, overwrite bool, except ...string) *CopyAllToListRule {
return &CopyAllToListRule{
To: to,
Except: except,
Overwrite: overwrite,
}
}

Expand Down
12 changes: 10 additions & 2 deletions x-pack/agent/pkg/agent/transpiler/rules_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ inputs:
`,
rule: &RuleList{
Rules: []Rule{
CopyToList("namespace", "inputs"),
CopyToList("namespace", "inputs", false),
},
},
},
Expand Down Expand Up @@ -561,7 +561,8 @@ func TestSerialization(t *testing.T) {
FilterValuesWithRegexp("inputs", "type", regexp.MustCompile("^metric/.*")),
ExtractListItem("path.p", "item", "target"),
InjectIndex("index-type"),
CopyToList("t1", "t2"),
CopyToList("t1", "t2", false),
CopyAllToList("t2", false, "a", "b"),
)

y := `- rename:
Expand Down Expand Up @@ -611,6 +612,13 @@ func TestSerialization(t *testing.T) {
- copy_to_list:
item: t1
to: t2
overwrite: false
- copy_all_to_list:
to: t2
except:
- a
- b
overwrite: false
`

t.Run("serialize_rules", func(t *testing.T) {
Expand Down
8 changes: 3 additions & 5 deletions x-pack/agent/spec/filebeat.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ rules:
- map:
path: inputsstreams
rules:
- copy_to_list:
item: type
to: streams
- copy_to_list:
item: processors
- copy_all_to_list:
to: streams
overwrite: false
except: ["streams", "id", "enabled", "title", "package", "namespace", "constraints", "use_output"]

- extract_list_items:
path: inputsstreams
Expand Down
8 changes: 3 additions & 5 deletions x-pack/agent/spec/metricbeat.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@ rules:
- map:
path: inputsstreams
rules:
- copy_to_list:
item: type
to: streams
- copy_to_list:
item: processors
- copy_all_to_list:
to: streams
overwrite: false
except: ["streams", "id", "enabled"]

- extract_list_items:
path: inputsstreams
Expand Down