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.

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

Support and/or section #1897

Merged
merged 15 commits into from
Apr 5, 2018
139 changes: 117 additions & 22 deletions lib/fluent/plugin/filter_grep.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,14 @@ class GrepFilter < Filter
def initialize
super

@_regexps = {}
@_excludes = {}
@_regexp_and_conditions = {}
@_exclude_and_conditions = {}
@_regexp_or_conditions = {}
@_exclude_or_conditions = {}
end

attr_reader :_regexp_and_conditions, :_exluce_and_conditions, :_regexp_or_conditions, :_exclude_or_conditions

Choose a reason for hiding this comment

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

small spelling mistake: _exluce_and_conditions -> _exclude_and_conditions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I've fixed.


helpers :record_accessor

REGEXP_MAX_NUM = 20
Expand All @@ -54,65 +58,143 @@ def initialize
config_param :key, :string
desc "The regular expression."
config_param :pattern do |value|
if value.start_with?("/") and value.end_with("/")
if value.start_with?("/") and value.end_with?("/")
Regexp.compile(value[1..-2])
else
Regexp.compile(value)
end
end
end

config_section :and, param_name: :and_conditions, multi: true do
config_section :regexp, param_name: :regexps, multi: true do
desc "The field name to which the regular expression is applied."
config_param :key, :string
desc "The regular expression."
config_param :pattern do |value|
if value.start_with?("/") and value.end_with?("/")
Copy link
Member

@repeatedly repeatedly Apr 3, 2018

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will write a patch for this.

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've created #1927

Regexp.compile(value[1..-2])
else
Regexp.compile(value)
end
end
end
config_section :exclude, param_name: :excludes, multi: true do
desc "The field name to which the regular expression is applied."
config_param :key, :string
desc "The regular expression."
config_param :pattern do |value|
if value.start_with?("/") and value.end_with?("/")
Regexp.compile(value[1..-2])
else
Regexp.compile(value)
end
end
end
end

config_section :or, param_name: :or_conditions, multi: true do
config_section :regexp, param_name: :regexps, multi: true do
desc "The field name to which the regular expression is applied."
config_param :key, :string
desc "The regular expression."
config_param :pattern do |value|
if value.start_with?("/") and value.end_with?("/")
Regexp.compile(value[1..-2])
else
Regexp.compile(value)
end
end
end
config_section :exclude, param_name: :excludes, multi: true do
desc "The field name to which the regular expression is applied."
config_param :key, :string
desc "The regular expression."
config_param :pattern do |value|
if value.start_with?("/") and value.end_with?("/")
Regexp.compile(value[1..-2])
else
Regexp.compile(value)
end
end
end
end

# for test
attr_reader :_regexps
attr_reader :_excludes

def configure(conf)
super

rs = {}
(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]
Copy link
Member

Choose a reason for hiding this comment

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

How about logging info or warning message for top level setting?
Top level multiple <regexp> is intepreted as 'and' condition.

@_regexp_and_conditions[key] = Expression.new(record_accessor_create(key), Regexp.compile(regexp))
end

es = {}
(1..REGEXP_MAX_NUM).each do |i|
next unless conf["exclude#{i}"]
key, exclude = conf["exclude#{i}"].split(/ /, 2)
raise Fluent::ConfigError, "exclude#{i} does not contain 2 parameters" unless exclude
raise Fluent::ConfigError, "exclude#{i} contains a duplicated key, #{key}" if es[key]
es[key] = Regexp.compile(exclude)
raise Fluent::ConfigError, "exclude#{i} contains a duplicated key, #{key}" if @_exclude_or_conditions[key]
@_exclude_or_conditions[key] = Expression.new(record_accessor_create(key), Regexp.compile(exclude))
end

@regexps.each do |e|
raise Fluent::ConfigError, "Duplicate key: #{e.key}" if rs.key?(e.key)
rs[e.key] = e.pattern
raise Fluent::ConfigError, "Duplicate key: #{e.key}" if @_regexp_and_conditions.key?(e.key)
@_regexp_and_conditions[e.key] = Expression.new(record_accessor_create(e.key), e.pattern)
end
@excludes.each do |e|
raise Fluent::ConfigError, "Duplicate key: #{e.key}" if es.key?(e.key)
es[e.key] = e.pattern
raise Fluent::ConfigError, "Duplicate key: #{e.key}" if @_exclude_or_conditions.key?(e.key)
@_exclude_or_conditions[e.key] = Expression.new(record_accessor_create(e.key), e.pattern)
end

rs.each_pair do |k, v|
@_regexps[record_accessor_create(k)] = v
@and_conditions.each do |and_condition|
if !and_condition.regexps.empty? && !and_condition.excludes.empty?
raise Fluent::ConfigError, "Do not specify both <regexp> and <exclude> in <and>"
end
and_condition.regexps.each do |e|
raise Fluent::ConfigError, "Duplicate key in <and>: #{e.key}" if @_regexp_and_conditions.key?(e.key)
@_regexp_and_conditions[e.key] = Expression.new(record_accessor_create(e.key), e.pattern)
end
and_condition.excludes.each do |e|
raise Fluent::ConfigError, "Duplicate key in <and>: #{e.key}" if @_exclude_and_conditions.key?(e.key)
@_exclude_and_conditions[e.key] = Expression.new(record_accessor_create(e.key), e.pattern)
end
end
es.each_pair do |k, v|
@_excludes[record_accessor_create(k)] = v

@or_conditions.each do |or_condition|
if !or_condition.regexps.empty? && !or_condition.excludes.empty?
raise Fluent::ConfigError, "Do not specify both <regexp> and <exclude> in <or>"
end
or_condition.regexps.each do |e|
raise Fluent::ConfigError, "Duplicate key in <or>: #{e.key}" if @_regexp_or_conditions.key?(e.key)
@_regexp_or_conditions[e.key] = Expression.new(record_accessor_create(e.key), e.pattern)
end
or_condition.excludes.each do |e|
raise Fluent::ConfigError, "Duplicate key in <or>: #{e.key}" if @_exclude_or_conditions.key?(e.key)
@_exclude_or_conditions[e.key] = Expression.new(record_accessor_create(e.key), e.pattern)
end
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|
Copy link
Member

Choose a reason for hiding this comment

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

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.

throw :break_loop unless expression.match?(record)
end
@_excludes.each do |key, exclude|
throw :break_loop if ::Fluent::StringUtil.match_regexp(exclude, key.call(record).to_s)
if !@_regexp_or_conditions.empty? && @_regexp_or_conditions.none? {|key, expression| expression.match?(record) }
throw :break_loop
end
if !@_exclude_and_conditions.empty? && @_exclude_and_conditions.all? {|key, expression| expression.match?(record) }
throw :break_loop
end
@_exclude_or_conditions.each do |key, expression|
throw :break_loop if expression.match?(record)
end
result = record
end
Expand All @@ -122,5 +204,18 @@ def filter(tag, time, record)
end
result
end

class Expression
Copy link
Member

Choose a reason for hiding this comment

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

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

attr_reader :key, :pattern

def initialize(key, pattern)
@key = key
@pattern = pattern
end

def match?(record)
::Fluent::StringUtil.match_regexp(@pattern, @key.call(record).to_s)
end
end
end
end
Loading