From 8c5d8ca55df774afad8ad02297dad48321037b28 Mon Sep 17 00:00:00 2001 From: Kenji Okimoto Date: Mon, 3 Jul 2017 17:13:28 +0900 Subject: [PATCH 1/3] Use config_section for filter_grep Users can use both `regexpN` and `` section. But `regexpN` displays deprecation warnings on boot. --- lib/fluent/plugin/filter_grep.rb | 48 ++++++++++++++++++++++++-------- test/plugin/test_filter_grep.rb | 36 ++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 14 deletions(-) diff --git a/lib/fluent/plugin/filter_grep.rb b/lib/fluent/plugin/filter_grep.rb index 1e67a06d25..dc2b19298b 100644 --- a/lib/fluent/plugin/filter_grep.rb +++ b/lib/fluent/plugin/filter_grep.rb @@ -24,32 +24,50 @@ class GrepFilter < Filter REGEXP_MAX_NUM = 20 - (1..REGEXP_MAX_NUM).each {|i| config_param :"regexp#{i}", :string, default: nil } - (1..REGEXP_MAX_NUM).each {|i| config_param :"exclude#{i}", :string, default: nil } + (1..REGEXP_MAX_NUM).each {|i| config_param :"regexp#{i}", :string, default: nil, deprecated: "Use section" } + (1..REGEXP_MAX_NUM).each {|i| config_param :"exclude#{i}", :string, default: nil, deprecated: "Use section" } + + 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| + Regexp.compile(value) + 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| + Regexp.compile(value) + end + end # for test - attr_reader :regexps - attr_reader :excludes + attr_reader :_regexps + attr_reader :_excludes def configure(conf) super - @regexps = {} + @_regexps = {} (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 @regexps[key] - @regexps[key] = Regexp.compile(regexp) + raise Fluent::ConfigError, "regexp#{i} contains a duplicated key, #{key}" if @_regexps[key] + @_regexps[key] = Regexp.compile(regexp) end - @excludes = {} + @_excludes = {} (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 @excludes[key] - @excludes[key] = Regexp.compile(exclude) + raise Fluent::ConfigError, "exclude#{i} contains a duplicated key, #{key}" if @_excludes[key] + @_excludes[key] = Regexp.compile(exclude) end end @@ -57,10 +75,16 @@ def filter(tag, time, record) result = nil begin catch(:break_loop) do - @regexps.each do |key, regexp| + @regexps.each do |e| + throw :break_loop unless ::Fluent::StringUtil.match_regexp(e.pattern, record[e.key].to_s) + end + @excludes.each do |e| + throw :break_loop if ::Fluent::StringUtil.match_regexp(e.pattern, record[e.key].to_s) + end + @_regexps.each do |key, regexp| throw :break_loop unless ::Fluent::StringUtil.match_regexp(regexp, record[key].to_s) end - @excludes.each do |key, exclude| + @_excludes.each do |key, exclude| throw :break_loop if ::Fluent::StringUtil.match_regexp(exclude, record[key].to_s) end result = record diff --git a/test/plugin/test_filter_grep.rb b/test/plugin/test_filter_grep.rb index 9295cf0b7e..597d3afe26 100644 --- a/test/plugin/test_filter_grep.rb +++ b/test/plugin/test_filter_grep.rb @@ -23,12 +23,12 @@ def create_driver(conf = '') test "regexpN can contain a space" do d = create_driver(%[regexp1 message foo]) - assert_equal(Regexp.compile(/ foo/), d.instance.regexps['message']) + assert_equal(Regexp.compile(/ foo/), d.instance._regexps['message']) end test "excludeN can contain a space" do d = create_driver(%[exclude1 message foo]) - assert_equal(Regexp.compile(/ foo/), d.instance.excludes['message']) + assert_equal(Regexp.compile(/ foo/), d.instance._excludes['message']) end end @@ -77,6 +77,38 @@ def filter(config, msgs) end end + test 'regexps' do + conf = %[ + + key message + pattern WARN + + ] + filtered_records = filter(conf, messages) + assert_equal(3, filtered_records.size) + assert_block('only WARN logs') do + filtered_records.all? { |r| + !r['message'].include?('INFO') + } + end + end + + test 'excludes' do + conf = %[ + + key message + pattern favicon + + ] + filtered_records = filter(conf, messages) + assert_equal(3, filtered_records.size) + assert_block('remove favicon logs') do + filtered_records.all? { |r| + !r['message'].include?('favicon') + } + end + end + sub_test_case 'with invalid sequence' do def messages [ From c0095ce07d0a5168f0f3cab8f5a62451fbaa0b66 Mon Sep 17 00:00:00 2001 From: Kenji Okimoto Date: Thu, 6 Jul 2017 11:02:00 +0900 Subject: [PATCH 2/3] Improve performance In previous verson: Rehearsal ------------------------------------------- flat 0.080000 0.010000 0.090000 ( 0.091635) section 0.100000 0.000000 0.100000 ( 0.101266) ---------------------------------- total: 0.190000sec user system total real flat 0.080000 0.000000 0.080000 ( 0.077353) section 0.100000 0.000000 0.100000 ( 0.099207) In this version: Rehearsal ------------------------------------------- flat 0.080000 0.000000 0.080000 ( 0.076263) section 0.070000 0.000000 0.070000 ( 0.068805) ---------------------------------- total: 0.150000sec user system total real flat 0.060000 0.000000 0.060000 ( 0.066826) section 0.070000 0.000000 0.070000 ( 0.065660) --- lib/fluent/plugin/filter_grep.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/fluent/plugin/filter_grep.rb b/lib/fluent/plugin/filter_grep.rb index dc2b19298b..bb4743aa72 100644 --- a/lib/fluent/plugin/filter_grep.rb +++ b/lib/fluent/plugin/filter_grep.rb @@ -69,18 +69,21 @@ def configure(conf) raise Fluent::ConfigError, "exclude#{i} contains a duplicated key, #{key}" if @_excludes[key] @_excludes[key] = Regexp.compile(exclude) end + + @regexps.each do |e| + raise Fluent::ConfigError, "Duplicate key: #{e.key}" if @_regexps.key?(e.key) + @_regexps[e.key] = e.pattern + end + @excludes.each do |e| + raise Fluent::ConfigError, "Duplicate key: #{e.key}" if @_excludes.key?(e.key) + @_excludes[e.key] = e.pattern + end end def filter(tag, time, record) result = nil begin catch(:break_loop) do - @regexps.each do |e| - throw :break_loop unless ::Fluent::StringUtil.match_regexp(e.pattern, record[e.key].to_s) - end - @excludes.each do |e| - throw :break_loop if ::Fluent::StringUtil.match_regexp(e.pattern, record[e.key].to_s) - end @_regexps.each do |key, regexp| throw :break_loop unless ::Fluent::StringUtil.match_regexp(regexp, record[key].to_s) end From 824aebef6f81695e4e84fefd243e8521b6029d37 Mon Sep 17 00:00:00 2001 From: Kenji Okimoto Date: Thu, 6 Jul 2017 11:04:19 +0900 Subject: [PATCH 3/3] Add test case for duplicate key --- test/plugin/test_filter_grep.rb | 39 +++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/plugin/test_filter_grep.rb b/test/plugin/test_filter_grep.rb index 597d3afe26..60c533d549 100644 --- a/test/plugin/test_filter_grep.rb +++ b/test/plugin/test_filter_grep.rb @@ -30,6 +30,45 @@ def create_driver(conf = '') d = create_driver(%[exclude1 message foo]) assert_equal(Regexp.compile(/ foo/), d.instance._excludes['message']) end + + sub_test_case "duplicate key" do + test "flat" do + conf = %[ + regexp1 message test + regexp2 message test2 + ] + assert_raise(Fluent::ConfigError) do + create_driver(conf) + end + end + test "section" do + conf = %[ + + key message + pattern test + + + key message + pattern test2 + + ] + assert_raise(Fluent::ConfigError) do + create_driver(conf) + end + end + test "mix" do + conf = %[ + regexp1 message test + + key message + pattern test + + ] + assert_raise(Fluent::ConfigError) do + create_driver(conf) + end + end + end end sub_test_case 'filter_stream' do