From 6bb2a777d731f526263e3308bf7e9aec0a5b799f Mon Sep 17 00:00:00 2001 From: Thomas Schickinger Date: Thu, 9 Mar 2017 10:37:56 +0100 Subject: [PATCH 1/4] Automatically add missing new lines when combining exception stacks. --- .rubocop.yml | 4 ++++ README.rdoc | 12 +++++++++--- fluent-plugin-detect-exceptions.gemspec | 5 +++-- lib/fluent/plugin/exception_detector.rb | 12 ++++++++++-- lib/fluent/plugin/out_detect_exceptions.rb | 2 +- test/plugin/test_exception_detector.rb | 12 ++++++++++-- test/plugin/test_out_detect_exceptions.rb | 4 ++-- 7 files changed, 39 insertions(+), 12 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index f88d786..7187ecf 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -26,3 +26,7 @@ Metrics/MethodLength: # Offense count: 3 Metrics/PerceivedComplexity: Max: 50 + +AllCops: + Exclude: + - '*.gemspec' diff --git a/README.rdoc b/README.rdoc index 0a4008c..bfd623a 100644 --- a/README.rdoc +++ b/README.rdoc @@ -2,8 +2,9 @@ fluent-plugin-detect-exceptions is an {output plugin for fluentd}[http://docs.fluentd.org/articles/output-plugin-overview] -which scans a log stream text messages or JSON records for multi-line exception -stack traces: If a consecutive sequence of log messages forms an exception stack +which scans a log stream text messages or JSON records containing single +output lines for multi-line exception stack traces: +If a consecutive sequence of single-line log messages forms an exception stack trace, the log messages are forwarded as a single, combined log message. Otherwise, the input log data is forwarded as is. @@ -22,6 +23,9 @@ cases where the content of log records that belong to a single exception stack are so similar (e.g. because they contain the timestamp of the log entry) that this loss of information is irrelevant. +When combining exception lines, it is ensured that they enter with a line +separator. If they don't, a line separator is added to the original log message. + This is NOT an official Google product. {Gem Version}[http://badge.fury.io/rb/fluent-plugin-detect-exceptions] @@ -53,7 +57,9 @@ The plugin supports the following parameters: a record. A prefix has to be a complete tag part. Example: If remove_tag_prefix is set to 'foo', the input tag foo.bar.baz is transformed to bar.baz and the input tag - 'foofoo.bar' is not modified. Default: empty string. + 'foofoo.bar' is not modified. + Removing a tag prefix is needed to avoid infinite + recursion in the fluentd config and is thus mandatory. [languages] A list of language for which exception stack traces shall be detected. The values in the list can be separated by commas or diff --git a/fluent-plugin-detect-exceptions.gemspec b/fluent-plugin-detect-exceptions.gemspec index dd9944b..045f8b6 100644 --- a/fluent-plugin-detect-exceptions.gemspec +++ b/fluent-plugin-detect-exceptions.gemspec @@ -11,12 +11,13 @@ eos gem.homepage = \ 'https://github.com/GoogleCloudPlatform/fluent-plugin-detect-exceptions' gem.license = 'Apache-2.0' - gem.version = '0.0.5' + gem.version = '0.0.6' gem.authors = ['Thomas Schickinger'] gem.email = ['schickin@google.com'] gem.required_ruby_version = Gem::Requirement.new('>= 2.0') - gem.files = Dir['**/*'].keep_if { |file| File.file?(file) } + gem.files = Dir['**/*'].keep_if{ |file| File.file?(file) && + !file.end_with?('gem') } gem.test_files = gem.files.grep(/^(test)/) gem.require_paths = ['lib'] diff --git a/lib/fluent/plugin/exception_detector.rb b/lib/fluent/plugin/exception_detector.rb index 0e9ba1f..e5b521a 100644 --- a/lib/fluent/plugin/exception_detector.rb +++ b/lib/fluent/plugin/exception_detector.rb @@ -223,6 +223,15 @@ def push(time_sec, record) force_flush if @max_lines > 0 && @messages.length == @max_lines end + def combined_message + combined = '' + @messages.each do |m| + combined << $RS if !combined.empty? && !combined.end_with?("\n") + combined << m + end + combined + end + def flush case @messages.length when 0 @@ -230,7 +239,6 @@ def flush when 1 @emit.call(@first_timestamp, @first_record) else - combined_message = @messages.join if @message_field.nil? output_record = combined_message else @@ -290,7 +298,7 @@ def update_buffer(detection_status, time_sec, record, message) def add(time_sec, record, message) if @messages.empty? - @first_record = record unless @message_field.nil? + @first_record = record @first_timestamp = time_sec @buffer_start_time = Time.now end diff --git a/lib/fluent/plugin/out_detect_exceptions.rb b/lib/fluent/plugin/out_detect_exceptions.rb index 3e4fe42..6f6c70a 100644 --- a/lib/fluent/plugin/out_detect_exceptions.rb +++ b/lib/fluent/plugin/out_detect_exceptions.rb @@ -25,7 +25,7 @@ class DetectExceptionsOutput < Output desc 'The field which contains the raw message text in the input JSON data.' config_param :message, :string, default: '' desc 'The prefix to be removed from the input tag when outputting a record.' - config_param :remove_tag_prefix, :string, default: '' + config_param :remove_tag_prefix, :string desc 'The interval of flushing the buffer for multiline format.' config_param :multiline_flush_interval, :time, default: nil desc 'Programming languages for which to detect exceptions. Default: all.' diff --git a/test/plugin/test_exception_detector.rb b/test/plugin/test_exception_detector.rb index 6642ea6..48a6d1b 100644 --- a/test/plugin/test_exception_detector.rb +++ b/test/plugin/test_exception_detector.rb @@ -277,8 +277,8 @@ def feed_lines(buffer, *messages) m.each_line do |line| buffer.push(0, line) end - buffer.flush end + buffer.flush end Struct.new('TestBufferScenario', :desc, :languages, :input, :expected) @@ -304,7 +304,15 @@ def test_buffer buffer_scenario('all exceptions from non-configured languages', [:ruby], [JAVA_EXC, PYTHON_EXC, GO_EXC], - JAVA_EXC.lines + PYTHON_EXC.lines + GO_EXC.lines) + JAVA_EXC.lines + PYTHON_EXC.lines + GO_EXC.lines), + buffer_scenario('exception lines with missing line ending', + [:all], + (JAVA_EXC.lines + + ARBITRARY_TEXT.lines + + [PYTHON_EXC]).collect(&:chomp), + [JAVA_EXC.chomp] + + ARBITRARY_TEXT.lines.collect(&:chomp) + + [PYTHON_EXC.chomp]) ].each do |s| out = [] buffer = Fluent::TraceAccumulator.new(nil, diff --git a/test/plugin/test_out_detect_exceptions.rb b/test/plugin/test_out_detect_exceptions.rb index 838de4f..b7d75fa 100644 --- a/test/plugin/test_out_detect_exceptions.rb +++ b/test/plugin/test_out_detect_exceptions.rb @@ -40,9 +40,9 @@ def setup Exception: ('spam', 'eggs') END - def create_driver(conf = CONFIG, tag = DEFAULT_TAG) + def create_driver(conf = '', tag = DEFAULT_TAG) d = Fluent::Test::OutputTestDriver.new(Fluent::DetectExceptionsOutput, tag) - d.configure(conf) + d.configure(CONFIG + conf) d end From 33bb9850c849ecfce84f7c72e3c8a3aa3278a356 Mon Sep 17 00:00:00 2001 From: Thomas Schickinger Date: Mon, 10 Apr 2017 09:19:00 +0200 Subject: [PATCH 2/4] Address review comments. --- README.rdoc | 23 ++++++++++++----------- fluent-plugin-detect-exceptions.gemspec | 4 ++-- lib/fluent/plugin/exception_detector.rb | 14 +++++--------- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/README.rdoc b/README.rdoc index bfd623a..9d2f91b 100644 --- a/README.rdoc +++ b/README.rdoc @@ -2,7 +2,7 @@ fluent-plugin-detect-exceptions is an {output plugin for fluentd}[http://docs.fluentd.org/articles/output-plugin-overview] -which scans a log stream text messages or JSON records containing single +which scans a log stream of text messages or JSON records containing single output lines for multi-line exception stack traces: If a consecutive sequence of single-line log messages forms an exception stack trace, the log messages are forwarded as a single, combined log message. @@ -23,8 +23,8 @@ cases where the content of log records that belong to a single exception stack are so similar (e.g. because they contain the timestamp of the log entry) that this loss of information is irrelevant. -When combining exception lines, it is ensured that they enter with a line -separator. If they don't, a line separator is added to the original log message. +When combining exception lines, it is ensured that they end with a line +separator. This is NOT an official Google product. @@ -46,6 +46,15 @@ will also install and configure the gem. The plugin supports the following parameters: +[remove_tag_prefix (required)] The prefix to remove from the input tag when + outputting a record. A prefix has to be a complete tag part. + Example: If remove_tag_prefix is set to 'foo', the input + tag foo.bar.baz is transformed to bar.baz and the input tag + 'foofoo.bar' is not modified. + This must be non-empty to avoid infinite recursion in + fluentd when processing the log entries that are re-emitted + by the plugin. + [message] Name of the field in the JSON record that contains the single-line log messages that shall be scanned for exceptions. If this is set to '', the plugin will try 'message' and 'log', @@ -53,14 +62,6 @@ The plugin supports the following parameters: This parameter is only applicable to structured (JSON) log streams. Default: ''. -[remove_tag_prefix] The prefix to remove from the input tag when outputting - a record. A prefix has to be a complete tag part. - Example: If remove_tag_prefix is set to 'foo', the input - tag foo.bar.baz is transformed to bar.baz and the input tag - 'foofoo.bar' is not modified. - Removing a tag prefix is needed to avoid infinite - recursion in the fluentd config and is thus mandatory. - [languages] A list of language for which exception stack traces shall be detected. The values in the list can be separated by commas or written as JSON list. diff --git a/fluent-plugin-detect-exceptions.gemspec b/fluent-plugin-detect-exceptions.gemspec index 045f8b6..dd88e75 100644 --- a/fluent-plugin-detect-exceptions.gemspec +++ b/fluent-plugin-detect-exceptions.gemspec @@ -16,8 +16,8 @@ eos gem.email = ['schickin@google.com'] gem.required_ruby_version = Gem::Requirement.new('>= 2.0') - gem.files = Dir['**/*'].keep_if{ |file| File.file?(file) && - !file.end_with?('gem') } + gem.files = Dir['**/*'].keep_if { |file| File.file?(file) && + !file.end_with?('.gem') } gem.test_files = gem.files.grep(/^(test)/) gem.require_paths = ['lib'] diff --git a/lib/fluent/plugin/exception_detector.rb b/lib/fluent/plugin/exception_detector.rb index e5b521a..5c79ba2 100644 --- a/lib/fluent/plugin/exception_detector.rb +++ b/lib/fluent/plugin/exception_detector.rb @@ -205,6 +205,7 @@ def initialize(message_field, languages, max_lines: 0, max_bytes: 0, @first_record = nil @first_timestamp = nil @emit = emit_callback + @line_separator = $RS || "\n" end def push(time_sec, record) @@ -223,15 +224,6 @@ def push(time_sec, record) force_flush if @max_lines > 0 && @messages.length == @max_lines end - def combined_message - combined = '' - @messages.each do |m| - combined << $RS if !combined.empty? && !combined.end_with?("\n") - combined << m - end - combined - end - def flush case @messages.length when 0 @@ -239,6 +231,10 @@ def flush when 1 @emit.call(@first_timestamp, @first_record) else + combined_message = @messages.each_with_object([]) do |line, memo| + memo << @line_separator unless memo.empty? || memo[-1].end_with?("\n") + memo << line + end.join if @message_field.nil? output_record = combined_message else From 31b8f31cba6e2e0053eb73fdd8e487938a29f875 Mon Sep 17 00:00:00 2001 From: Thomas Schickinger Date: Mon, 10 Apr 2017 09:21:34 +0200 Subject: [PATCH 3/4] Fix indentation in README.rdoc. --- README.rdoc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.rdoc b/README.rdoc index 9d2f91b..5e62721 100644 --- a/README.rdoc +++ b/README.rdoc @@ -48,12 +48,12 @@ The plugin supports the following parameters: [remove_tag_prefix (required)] The prefix to remove from the input tag when outputting a record. A prefix has to be a complete tag part. - Example: If remove_tag_prefix is set to 'foo', the input - tag foo.bar.baz is transformed to bar.baz and the input tag - 'foofoo.bar' is not modified. - This must be non-empty to avoid infinite recursion in - fluentd when processing the log entries that are re-emitted - by the plugin. + Example: If remove_tag_prefix is set to 'foo', the input + tag foo.bar.baz is transformed to bar.baz and the input tag + 'foofoo.bar' is not modified. + This must be non-empty to avoid infinite recursion in + fluentd when processing the log entries that are re-emitted + by the plugin. [message] Name of the field in the JSON record that contains the single-line log messages that shall be scanned for exceptions. From b2f4f31f1690a2e063a283a13dfc8f93e3d89963 Mon Sep 17 00:00:00 2001 From: Thomas Schickinger Date: Tue, 18 Apr 2017 20:56:01 +0200 Subject: [PATCH 4/4] Address second round of comments by igorpeshansky. --- lib/fluent/plugin/exception_detector.rb | 5 +++-- lib/fluent/plugin/out_detect_exceptions.rb | 7 +++++++ test/plugin/test_out_detect_exceptions.rb | 10 ++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/fluent/plugin/exception_detector.rb b/lib/fluent/plugin/exception_detector.rb index 5c79ba2..557e61e 100644 --- a/lib/fluent/plugin/exception_detector.rb +++ b/lib/fluent/plugin/exception_detector.rb @@ -183,6 +183,8 @@ def transition(line) class TraceAccumulator attr_reader :buffer_start_time + LINE_SEPARATOR = $RS || "\n" + # If message_field is nil, the instance is set up to accumulate # records that are plain strings (i.e. the whole record is concatenated). # Otherwise, the instance accepts records that are dictionaries (usually @@ -205,7 +207,6 @@ def initialize(message_field, languages, max_lines: 0, max_bytes: 0, @first_record = nil @first_timestamp = nil @emit = emit_callback - @line_separator = $RS || "\n" end def push(time_sec, record) @@ -232,7 +233,7 @@ def flush @emit.call(@first_timestamp, @first_record) else combined_message = @messages.each_with_object([]) do |line, memo| - memo << @line_separator unless memo.empty? || memo[-1].end_with?("\n") + memo << LINE_SEPARATOR unless memo.empty? || memo[-1].end_with?("\n") memo << line end.join if @message_field.nil? diff --git a/lib/fluent/plugin/out_detect_exceptions.rb b/lib/fluent/plugin/out_detect_exceptions.rb index 6f6c70a..114911e 100644 --- a/lib/fluent/plugin/out_detect_exceptions.rb +++ b/lib/fluent/plugin/out_detect_exceptions.rb @@ -39,6 +39,9 @@ class DetectExceptionsOutput < Output Fluent::Plugin.register_output('detect_exceptions', self) + ERROR_EMPTY_REMOVE_TAG_PREFIX = + 'remove_tag_prefix must not be empty.'.freeze + def configure(conf) super @@ -46,6 +49,10 @@ def configure(conf) @check_flush_interval = [multiline_flush_interval * 0.1, 1].max end + if remove_tag_prefix.empty? + raise ConfigError, ERROR_EMPTY_REMOVE_TAG_PREFIX + end + @languages = languages.map(&:to_sym) # Maps log stream tags to a corresponding TraceAccumulator. diff --git a/test/plugin/test_out_detect_exceptions.rb b/test/plugin/test_out_detect_exceptions.rb index b7d75fa..2f91c80 100644 --- a/test/plugin/test_out_detect_exceptions.rb +++ b/test/plugin/test_out_detect_exceptions.rb @@ -204,4 +204,14 @@ def test_separate_streams make_logs(t, 'something else', stream: 'java') assert_equal(expected, d.events) end + + def test_remove_tag_prefix_must_not_be_empty + d = Fluent::Test::OutputTestDriver.new(Fluent::DetectExceptionsOutput, + DEFAULT_TAG) + exc = assert_raise(Fluent::ConfigError) do + d.configure('remove_tag_prefix') + end + assert_equal(Fluent::DetectExceptionsOutput::ERROR_EMPTY_REMOVE_TAG_PREFIX, + exc.message) + end end