-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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 support for pass/drop/tagpass/tagdrop for outputs #401
Add support for pass/drop/tagpass/tagdrop for outputs #401
Conversation
@sparrc Still need to write additional tests, but wanted to see if this is what you had in mind. This version is working to filter both tags and measurements by prefix. |
f1556db
to
363b1c7
Compare
@oldmantaiter sorry I should have warned you earlier that I had some large agent/config changes coming. This may be very difficult to rebase but give it your best shot and let me know if you have any questions. |
@sparrc No problem, I'll fix up |
def5f49
to
6aaa856
Compare
// Also lists the tags to filter | ||
type PluginConfig struct { | ||
type PrefixFilterConfig struct { |
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 don't like overloading this, I would prefer if we kept PluginConfig
and add another struct called just Filter
that can be part of PluginConfig. Then probably will need to add an OutputConfig struct as well
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.
Or if Name & Interval aren't being used from here (I'm not sure if they are?), remove them and just call this struct Filter
5a3a77b
to
ec206eb
Compare
Reuses same logic as the plugins for filtering points, should be only a marginal performance decrease to check all the points before writing to the output. Added examples to the README as well (for generic pass/drop as well as output pass/drop/tagpass/tagdrop). X-Github-Closes influxdata#398
ec206eb
to
63f35fe
Compare
if cp.Pass != nil { | ||
for _, pat := range cp.Pass { | ||
// based on the drop/pass filter parameters | ||
func (f Filter) ShouldPass(measurement string) bool { |
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.
Any reason you made these non-pointer receivers? (Filter
instead of *Filter
)
Thanks a bunch @oldmantaiter! This looks really good, just the one comment about non-pointer receivers and then I'll work on getting this merged |
@sparrc Thanks! The non-pointer in Filter is really just for the tests (they fail with a pointer because of inequality) EDIT: I misread your comments, it was basically to keep from reference/de-reference because the tests required the struct and not a pointer to it to pass.. if that makes sense? |
Drop []string | ||
Pass []string | ||
|
||
TagDrop []TagFilter | ||
TagPass []TagFilter | ||
|
||
IsActive bool |
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.
good idea 👍
@oldmantaiter, cool, that makes sense, do you have any more changes or is this PR ready to merge? |
thanks @oldmantaiter! This will be in release 0.2.4 |
@sparrc Didn't see the previous question, ran it for a couple hours on my local testing environment with the options enabled and did not see any issues so it is good to go. Thanks! |
Reuses a lot of the logic for the plugin configuration, and changed
the ConfiguredPlugin struct to be ConfiguredFilter and a generic
function to parse the filter (called parseFilter).
X-Github-Closes #398