Add tests for issue to reproduce behavior

Fix break_on_match issue while evaluating multiple patterns

Closes elastic#1547

Make better exception message

Add more specs for inputs with arrays

Closes elastic#1547

lib/logstash/filters/grok.rb

@@ -257,16 +257,14 @@ def register
     @match.each do |field, patterns|
       patterns = [patterns] if patterns.is_a?(String)
 
-      if !@patterns.include?(field)
-        @patterns[field] = Grok::Pile.new
-        #@patterns[field].logger = @logger
-
-        add_patterns_from_files(@patternfiles, @patterns[field])
-      end
       @logger.info? and @logger.info("Grok compile", :field => field, :patterns => patterns)
       patterns.each do |pattern|
         @logger.debug? and @logger.debug("regexp: #{@type}/#{field}", :pattern => pattern)
-        @patterns[field].compile(pattern)
+        grok = Grok.new
+        grok.logger = @logger unless @logger.nil?
+        add_patterns_from_files(@patternfiles, grok)
+        grok.compile(pattern)
+        @patterns[field] << grok
       end
     end # @match.each
   end # def register
@@ -279,8 +277,8 @@ def filter(event)
     done = false
 
     @logger.debug? and @logger.debug("Running grok filter", :event => event);
-    @patterns.each do |field, grok|
-      if match(grok, field, event)
+    @patterns.each do |field, groks|
+      if match(groks, field, event)
         matched = true
         break if @break_on_match
       end
@@ -302,36 +300,38 @@ def filter(event)
   end # def filter
 
   private
-  def match(grok, field, event)
+  def match(groks, field, event)
     input = event[field]
     if input.is_a?(Array)
-      success = true
+      success = false
       input.each do |input|
-        grok, match = grok.match(input)
-        if match
-          match.each_capture do |capture, value|
-            handle(capture, value, event)
-          end
-        else
-          success = false
-        end
+        success |= match_against_groks(groks, input, event)
       end
       return success
-    #elsif input.is_a?(String)
     else
-      # Convert anything else to string (number, hash, etc)
-      grok, match = grok.match(input.to_s)
-      return false if !match
-
-      match.each_capture do |capture, value|
-        handle(capture, value, event)
-      end
-      return true
+      return match_against_groks(groks, input, event)
     end
   rescue StandardError => e
     @logger.warn("Grok regexp threw exception", :exception => e.message)
   end
 
+  private
+  def match_against_groks(groks, input, event)
+    matched = false
+    groks.each do |grok|
+      # Convert anything else to string (number, hash, etc)
+      match = grok.match(input.to_s)
+      if match
+        match.each_capture do |capture, value|
+          handle(capture, value, event)
+        end
+        matched = true
+        break if @break_on_match
+      end
+    end
+    return matched
+  end
+
   private
   def handle(capture, value, event)
     handler = @handlers[capture] ||= compile_capture_handler(capture)
@@ -392,12 +392,13 @@ def compile_capture_handler(capture)
   end # def compile_capture_handler
 
   private
-  def add_patterns_from_files(paths, pile)
-    paths.each { |path| add_patterns_from_file(path, pile) }
+  def add_patterns_from_files(paths, grok)
+    paths.each do |path|
+      if !File.exists?(path)
+        raise "Grok pattern file does not exist: #{path}"
+      end
+      grok.add_patterns_from_file(path)
+    end
   end # def add_patterns_from_files
 
-  private
-  def add_patterns_from_file(path, pile)
-    pile.add_patterns_from_file(path)
-  end # def add_patterns_from_file
 end # class LogStash::Filters::Grok

spec/filters/grok.rb

@@ -500,4 +500,133 @@
       insist { subject["foo"] }.is_a?(String)
     end
   end
+
+  describe "break_on_match default should be true and first match should exit filter" do
+    config <<-CONFIG
+      filter {
+        grok {
+          match => { "message" => "%{INT:foo}"
+                     "somefield" => "%{INT:bar}"}
+        }
+      }
+    CONFIG
+
+    sample("message" => "hello world 123", "somefield" => "testme abc 999") do
+      insist { subject["foo"] } == "123"
+      insist { subject["bar"] }.nil?
+    end
+  end
+
+  describe "break_on_match when set to false should try all patterns" do
+    config <<-CONFIG
+      filter {
+        grok {
+          match => { "message" => "%{INT:foo}"
+                     "somefield" => "%{INT:bar}"}
+          break_on_match => false
+        }
+      }
+    CONFIG
+
+    sample("message" => "hello world 123", "somefield" => "testme abc 999") do
+      insist { subject["foo"] } == "123"
+      insist { subject["bar"] } == "999"
+    end
+  end
+
+  describe "LOGSTASH-1547 - break_on_match should work on fields with multiple patterns" do
+    config <<-CONFIG
+      filter {
+        grok {
+          match => { "message" => ["%{GREEDYDATA:name1}beard", "tree%{GREEDYDATA:name2}"] }
+          break_on_match => false
+        }
+      }
+    CONFIG
+
+    sample "treebranch" do
+      insist { subject["name2"] } == "branch"
+    end
+
+    sample "bushbeard" do
+      insist { subject["name1"] } == "bush"
+    end
+
+    sample "treebeard" do
+      insist { subject["name1"] } == "tree"
+      insist { subject["name2"] } == "beard"
+    end
+  end
+
+  describe "break_on_match default for array input with single grok pattern" do
+    config <<-CONFIG
+      filter {
+        grok {
+          match => { "message" => "%{INT:foo}"}
+        }
+      }
+    CONFIG
+
+    # array input --
+    sample("message" => ["hello world 123", "line 23"]) do
+      insist { subject["foo"] } == ["123", "23"]
+      insist { subject["tags"] }.nil?
+    end
+
+    # array input, one of them matches
+    sample("message" => ["hello world 123", "abc"]) do
+      insist { subject["foo"] } == "123"
+      insist { subject["tags"] }.nil?
+    end
+  end
+
+  describe "break_on_match = true (default) for array input with multiple grok pattern" do
+    config <<-CONFIG
+      filter {
+        grok {
+          match => { "message" => ["%{INT:foo}", "%{WORD:bar}"] }
+        }
+      }
+    CONFIG
+
+    # array input --
+    sample("message" => ["hello world 123", "line 23"]) do
+      insist { subject["foo"] } == ["123", "23"]
+      insist { subject["bar"] }.nil?
+      insist { subject["tags"] }.nil?
+    end
+
+    # array input, one of them matches
+    sample("message" => ["hello world", "line 23"]) do
+      insist { subject["bar"] } == "hello"
+      insist { subject["foo"] } == "23"
+      insist { subject["tags"] }.nil?
+    end
+  end
+
+  describe "break_on_match = false for array input with multiple grok pattern" do
+    config <<-CONFIG
+      filter {
+        grok {
+          match => { "message" => ["%{INT:foo}", "%{WORD:bar}"] }
+          break_on_match => false
+        }
+      }
+    CONFIG
+
+    # array input --
+    sample("message" => ["hello world 123", "line 23"]) do
+      insist { subject["foo"] } == ["123", "23"]
+      insist { subject["bar"] } == ["hello", "line"]
+      insist { subject["tags"] }.nil?
+    end
+
+    # array input, one of them matches
+    sample("message" => ["hello world", "line 23"]) do
+      insist { subject["bar"] } == ["hello", "line"]
+      insist { subject["foo"] } == "23"
+      insist { subject["tags"] }.nil?
+    end
+  end
+
 end