enable periodic flushing #36
Conversation
| @@ -109,6 +109,10 @@ class LogStash::Filters::Elapsed < LogStash::Filters::Base | |||
| # to the "end event"; if it's set to `true` a new "match event" is created. | |||
| config :new_event_on_match, :validate => :boolean, :required => false, :default => false | |||
|
|
|||
| # This filter must have its flush function called periodically to be able to purge | |||
| # expired stored start events. | |||
| config :periodic_flush, :validate => :boolean, :default => true | |||
There was a problem hiding this comment.
Will this require a docs change to set the default value of this field to true?
There was a problem hiding this comment.
oh, I guess so. The tricky part here is that this doc part seems to be common for these common "base" options so I am not sure how we can override it in this filter doc.
Maybe @karenzone can help here?
There was a problem hiding this comment.
Not TOO tricky when we can consult @dedemorton about advanced magic such as this.
From DeDe: "We can conditionally code the common docs to show different defaults...just need to know conditions where the default is different. We will also need to create a new version of the file for the versioned plugin docs...but Colin and Rob don’t need to worry about that bit."
Sooooo, we just need to know conditions where the default is different.
There was a problem hiding this comment.
@karenzone as per this PR, the periodic_flush option is now defaulted to true in this plugin and I don't see why anyone would want to change it actually.
config :periodic_flush, :validate => :boolean, :default => true
There was a problem hiding this comment.
The common options are defined in a shared file here: https://github.com/elastic/logstash/blob/master/docs/include/filter.asciidoc#plugins-{type}s-{plugin}-periodic_flush. Looks like the default value in the shared file is false.
To set the default to true, we'll need to add some conditional coding to the common file.
Are there other filter plugins that set periodic_flush to true by default, or just this one? If there are multiple plugins, we might want to introduce an asciidoc attribute to use in the condition. You can see how we've done something similar with the codec option here:
https://raw.githubusercontent.com/elastic/logstash/master/docs/include/input.asciidoc
ifdef::default_codec[]
* Default value is +"{default_codec}"+
endif::[]
ifndef::default_codec[]
* Default value is `"plain"`
endif::[]
We introduced an attribute called default_codec that overrides the setting in the common file when the default_codec attribute is defined in a plugin's attributes. For example, here you'll see :default_codec: line: https://raw.githubusercontent.com/elastic/logstash-docs/master/docs/plugins/inputs/stdin.asciidoc.
Using an asciidoc attribute is a good approach because it doesn't affect the versioned plugin reference (meaning that we won't have to create a new version of the file to work with 6.3+).
There was a problem hiding this comment.
I'm good with that @colinsurprenant - ok with you too @karenzone?
There was a problem hiding this comment.
Works for me. I've added it to my list for tracking: elastic/logstash#6693
There was a problem hiding this comment.
Sounds like we also need conditional coding on the description to indicate when the option is not configurable. @karenzone I can help with that when you're ready to work on the doc changes.
There was a problem hiding this comment.
@karenzone I just looked at elastic/logstash#6693 and note that this also impacts the metrics, aggregate, multilines and throttle filters (per my quick search ^^)
There was a problem hiding this comment.
@colinsurprenant I have added the expanded list of impacted filters to my tracking list, and will do a thorough search for additional when I correct the issue. Thank you for keeping me in the loop
|
published version 4.0.5 |
This puts back the
periodic_flushoption to a default of true so that the filter periodic flusherflushmethod is called by the pipeline to support the events cache expiration.