-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add if/then/else support to processors #10744
Add if/then/else support to processors #10744
Conversation
This allows for a condition to be used with a list of processors and for a list of processors to be executed if the condition does not match. This is a contrived example that adds fields and tags to the event if true, otherwise it drops the event. processors: [ { "if": { "and": [ { "equals.type": "login" }, { "range.uid.lt": 500 } ] }, "then": [ { "add_fields": { "target": "", "fields": { "uid_type": "reserved" } } }, { "add_tags": { "tags": [ "reserved_user_login" ] } } ], "else": { "drop_event": null } } ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewkroh Excellent works I have added a few comments, feel free to consider them or not, maybe the only things to look is the case of the NIL inside the THEN branch and the tests for multiple levels of IF.
The case for multiple if, It should work but it wont be pretty in the YAML :)
sb.WriteString(p.els.String()) | ||
} | ||
return sb.String() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this, I see that we do a nil check for the else branch, but looking at the code the then branch could be also nil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the config is Unpack
ed the then
field is is validated via validate:"required"
. This prevents it from ever being nil.
@@ -21,7 +21,8 @@ import ( | |||
"github.com/elastic/beats/libbeat/common" | |||
) | |||
|
|||
type PluginConfig []map[string]*common.Config | |||
// PluginConfig represents the list of processors. | |||
type PluginConfig []*common.Config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the comment maybe Processors
or List
might be a better name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't want to change it because it's public and used in multiple places in the code. But I agree the name isn't that great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah its public :(
if len(procs.List) > 0 { | ||
procs.log.Debugf("Generated new processors: %v", procs) | ||
} | ||
return procs, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only count the top level processors and not the processors defined inside a If/Else/branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should log information about all of the processors, even nested ones. But since there are multiple calls to New
you will see a few of these debug statements when using if/then/else. The last one being the complete processor tree.
=== RUN TestIfElseThenProcessor/if-then-else-if
2019-02-14T10:04:42.904-0500 DEBUG [conditions] conditions/conditions.go:95 New condition range: map[uid:{<nil> <nil> <nil> 0xc0000a6dc0}]
2019-02-14T10:04:42.904-0500 DEBUG [processors] processors/processor.go:88 Generated new processors: add_fields={"uid_type":"reserved"}
2019-02-14T10:04:42.904-0500 DEBUG [conditions] conditions/conditions.go:95 New condition equals: map[uid:{500 false}]
2019-02-14T10:04:42.904-0500 DEBUG [processors] processors/processor.go:88 Generated new processors: add_fields={"uid_type":"eq_500"}
2019-02-14T10:04:42.904-0500 DEBUG [processors] processors/processor.go:88 Generated new processors: add_fields={"uid_type":"gt_500"}
2019-02-14T10:04:42.904-0500 DEBUG [processors] processors/processor.go:88 Generated new processors: if equals: map[uid:{500 false}] then add_fields={"uid_type":"eq_500"} else add_fields={"uid_type":"gt_500"}
2019-02-14T10:04:42.904-0500 DEBUG [processors] processors/processor.go:88 Generated new processors: if range: map[uid:{<nil> <nil> <nil> 0xc0000a6dc0}] then add_fields={"uid_type":"reserved"} else if equals: map[uid:{500 false}] then add_fields={"uid_type":"eq_500"} else add_fields={"uid_type":"gt_500"}
@ph I updated the code with new test case for nesting and replied. This is ready for another look. |
@andrewkroh code LGTM, I will restart the jenkins job.. its a timeout for fetching golang. |
Jenkins test this please |
Jenkins is green. Travis is mostly green; just a metricbeat consul integration test failed. |
This allows for a condition to be used with a list of processors and for a list of
processors to be executed if the condition does not match.
This is a contrived example that adds fields and tags to the event if true, otherwise it drops the event.