Automatically add missing new lines when combining exception stacks and make remove_tag_prefix mandatory

.rubocop.yml

@@ -26,3 +26,7 @@ Metrics/MethodLength:
 # Offense count: 3
 Metrics/PerceivedComplexity:
   Max: 50
+
+AllCops:
+  Exclude:
+  - '*.gemspec'

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 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.
 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 end with a line
+separator.
+
 This is NOT an official Google product.
 
 {<img src="https://badge.fury.io/rb/fluent-plugin-detect-exceptions.svg" alt="Gem Version" />}[http://badge.fury.io/rb/fluent-plugin-detect-exceptions]
@@ -42,19 +46,22 @@ 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',
            in that order.
            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. Default: empty string.
-
 [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.

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         = ['[email protected]']
   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']
 

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
@@ -230,7 +232,10 @@ def flush
       when 1
         @emit.call(@first_timestamp, @first_record)
       else
-        combined_message = @messages.join
+        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
@@ -290,7 +295,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

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.'
@@ -39,13 +39,20 @@ 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
 
       if multiline_flush_interval
         @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.

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,

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
 
@@ -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