Support and/or section #1897
Support and/or section #1897
Conversation
This is the first step to support and/or.
lib/fluent/plugin/filter_grep.rb
Outdated
| end | ||
|
|
||
| attr_reader :_regexp_and_conditions, :_exluce_and_conditions, :_regexp_or_conditions, :_exclude_or_conditions |
There was a problem hiding this comment.
small spelling mistake: _exluce_and_conditions -> _exclude_and_conditions
lib/fluent/plugin/filter_grep.rb
Outdated
| @@ -122,5 +201,18 @@ def filter(tag, time, record) | |||
| end | |||
| result | |||
| end | |||
|
|
|||
| class Expression | |||
There was a problem hiding this comment.
How about using Struct for it?
Expression = Struct.new(:key, :pattern) do
def match?(record)
::Fluent::StringUtil.match_regexp(pattern, key.call(record).to_s)
end
end
lib/fluent/plugin/filter_grep.rb
Outdated
| end | ||
| end | ||
|
|
||
| def filter(tag, time, record) | ||
| result = nil | ||
| begin | ||
| catch(:break_loop) do | ||
| @_regexps.each do |key, regexp| | ||
| throw :break_loop unless ::Fluent::StringUtil.match_regexp(regexp, key.call(record).to_s) | ||
| @_regexp_and_conditions.each do |key, expression| |
There was a problem hiding this comment.
This is very small optimization but how about using array for conditions?
In filter, we don't use key and hash related operations so array is good structure.
lib/fluent/plugin/filter_grep.rb
Outdated
| (1..REGEXP_MAX_NUM).each do |i| | ||
| next unless conf["regexp#{i}"] | ||
| key, regexp = conf["regexp#{i}"].split(/ /, 2) | ||
| raise Fluent::ConfigError, "regexp#{i} does not contain 2 parameters" unless regexp | ||
| raise Fluent::ConfigError, "regexp#{i} contains a duplicated key, #{key}" if rs[key] | ||
| rs[key] = Regexp.compile(regexp) | ||
| raise Fluent::ConfigError, "regexp#{i} contains a duplicated key, #{key}" if @_regexp_and_conditions[key] |
There was a problem hiding this comment.
How about logging info or warning message for top level setting?
Top level multiple <regexp> is intepreted as 'and' condition.
Because it is unclear how interpret for top level <regexp> and <exclude>.
Because we don't use Hash key and Hash related operations.
very simple benchmark result is here:
user system total real
new 2.427028 0.000255 2.427283 ( 2.435108)
old 2.497610 0.004182 2.501792 ( 2.507315)
See also https://gist.github.com/okkez/2b33ee7a946d137478fb5a6454551dfa
|
This looks great. @repeatedly can this get merged? (The failing test in Travis looks unrelated to these changes) |
|
Sorry for the delay. Will check it again! |
| config_param :key, :string | ||
| desc "The regular expression." | ||
| config_param :pattern do |value| | ||
| if value.start_with?("/") and value.end_with?("/") |
There was a problem hiding this comment.
Hmm... I see lots of similar patterns. We need to :regexp type for config_param.
@okkez Could you write a patch for adding regexp type in other PR?
There was a problem hiding this comment.
Sure, I will write a patch for this.
lib/fluent/plugin/filter_grep.rb
Outdated
|
|
||
| def configure(conf) | ||
| super | ||
|
|
||
| rs = {} | ||
| _regexp_and_conditions = {} |
There was a problem hiding this comment.
This name is conflicts with attr_reader's definition. Yeah, I know local variable is preferred but using other name , e.g. _regexp_and_conds, is better. Others are same.
|
@okkez We enabled DCO for fluentd repository. Could you re-commit last commit with |
Because these names conflict with attr_reader's definition. Signed-off-by: Kenji Okimoto <okimoto@clear-code.com>
|
I've re-committed and pushed the last commit with |
|
Thanks! |
For example:
For more details, see test code
See #1858