harmonized event filtering by type/tags in filter and output + correct logging

Fixes #2339

lib/logstash/filters/base.rb

@@ -13,17 +13,16 @@ class LogStash::Filters::Base < LogStash::Plugin
   # `exclude_fields`) must be met in order for the event to be handled by the filter.
 
   # The type to act on. If a type is given, then this filter will only
-  # act on messages with the same type. See any input plugin's "type"
+  # act on messages with the same type. See any input plugin's `type`
   # attribute for more.
   # Optional.
   config :type, :validate => :string, :default => "", :deprecated => "You can achieve this same behavior with the new conditionals, like: `if [type] == \"sometype\" { %PLUGIN% { ... } }`."
 
-  # Only handle events with all/any (controlled by `include_any` config option) of these tags.
+  # Only handle events with all of these tags.
   # Optional.
   config :tags, :validate => :array, :default => [], :deprecated => "You can achieve similar behavior with the new conditionals, like: `if \"sometag\" in [tags] { %PLUGIN% { ... } }`"
 
-  # Only handle events without all/any (controlled by `exclude_any` config
-  # option) of these tags.
+  # Only handle events without any of these tags.
   # Optional.
   config :exclude_tags, :validate => :array, :default => [], :deprecated => "You can achieve similar behavior with the new conditionals, like: `if !(\"sometag\" in [tags]) { %PLUGIN% { ... } }`"
 
@@ -203,7 +202,8 @@ class LogStash::Filters::Base < LogStash::Plugin
   def filter?(event)
     if !@type.empty?
       if event["type"] != @type
-        @logger.debug? and @logger.debug(["filters/#{self.class.name}: Skipping event because type doesn't match #{@type}", event])
+        @logger.debug? and @logger.debug("filters/#{self.class.name}: Skipping event because type doesn't match",
+                                         :type=> @type, :event => event)
         return false
       end
     end
@@ -215,14 +215,16 @@ class LogStash::Filters::Base < LogStash::Plugin
 
       # Is @tags a subset of the event's tags? If not, skip it.
       if (event["tags"] & @tags).size != @tags.size
-        @logger.debug(["filters/#{self.class.name}: Skipping event because tags don't match #{@tags.inspect}", event])
+        @logger.debug? and @logger.debug("filters/#{self.class.name}: Skipping event because tags don't match",
+                                         :tags => tags, :event => event)
         return false
       end
     end
 
     if !@exclude_tags.empty? && event["tags"]
       if (diff_tags = (event["tags"] & @exclude_tags)).size != 0
-        @logger.debug(["filters/#{self.class.name}: Skipping event because tags contains excluded tags: #{diff_tags.inspect}", event])
+        @logger.debug("filters/#{self.class.name}: Skipping event because tags contains excluded tags:",
+                      :diff_tags => diff_tags, :exclude_tags => @exclude_tags, :event => event)
         return false
       end
     end

lib/logstash/outputs/base.rb

@@ -18,12 +18,12 @@ class LogStash::Outputs::Base < LogStash::Plugin
   # Optional.
   config :type, :validate => :string, :default => "", :deprecated => "You can achieve this same behavior with the new conditionals, like: `if [type] == \"sometype\" { %PLUGIN% { ... } }`."
 
-  # Only handle events with all of these tags.  Note that if you specify
-  # a type, the event must also match that type.
+  # Only handle events with all of these tags.
   # Optional.
   config :tags, :validate => :array, :default => [], :deprecated => "You can achieve similar behavior with the new conditionals, like: `if \"sometag\" in [tags] { %PLUGIN% { ... } }`"
 
-  # Only handle events without any of these tags. Note this check is additional to type and tags.
+  # Only handle events without any of these tags.
+  # Optional.
   config :exclude_tags, :validate => :array, :default => [], :deprecated => "You can achieve similar behavior with the new conditionals, like: `if !(\"sometag\" in [tags]) { %PLUGIN% { ... } }`"
 
   # The codec used for output data. Output codecs are a convenient method for encoding your data before it leaves the output, without needing a separate filter in your Logstash pipeline.
@@ -87,7 +87,7 @@ class LogStash::Outputs::Base < LogStash::Plugin
   def handle(event)
     receive(event)
   end # def handle
-  
+
   def handle_worker(event)
     @worker_queue.push(event)
   end
@@ -96,23 +96,25 @@ class LogStash::Outputs::Base < LogStash::Plugin
   def output?(event)
     if !@type.empty?
       if event["type"] != @type
-        @logger.debug? and @logger.debug(["outputs/#{self.class.name}: Dropping event because type doesn't match #{@type}", event])
+        @logger.debug? and @logger.debug("outputs/#{self.class.name}: Dropping event because type doesn't match",
+                                         :type => @type, :event => event)
         return false
       end
     end
 
     if !@tags.empty?
       return false if !event["tags"]
-      @include_method = :any?
-      if !@tags.send(@include_method) {|tag| event["tags"].include?(tag)}
-        @logger.debug? and @logger.debug("outputs/#{self.class.name}: Dropping event because tags don't match #{@tags.inspect}", event)
+      if (event["tags"] & @tags).size != @tags.size
+        @logger.debug? and @logger.debug("outputs/#{self.class.name}: Dropping event because tags don't match",
+                                         :tags => @tags, :event => event)
         return false
       end
     end
 
     if !@exclude_tags.empty? && event["tags"]
-      if @exclude_tags.send(@exclude_method) {|tag| event["tags"].include?(tag)}
-        @logger.debug? and @logger.debug("outputs/#{self.class.name}: Dropping event because tags contains excluded tags: #{@exclude_tags.inspect}", event)
+      if (diff_tags = (event["tags"] & @exclude_tags)).size != 0
+        @logger.debug? and @logger.debug("outputs/#{self.class.name}: Dropping event because tags contains excluded tags",
+                                         :diff_tags => diff_tags, :exclude_tags => @exclude_tags, :event => event)
         return false
       end
     end

spec/outputs/base_spec.rb

@@ -0,0 +1,37 @@
+# encoding: utf-8
+require "logstash/devutils/rspec/spec_helper"
+require "logstash/outputs/base"
+require "logstash/namespace"
+
+# use a dummy NOOP output to test Outputs::Base
+class LogStash::Outputs::NOOP < LogStash::Outputs::Base
+  config_name "noop"
+  milestone 2
+
+  def register; end
+
+  def receive(event)
+    return output?(event)
+  end
+end
+
+describe "LogStash::Outputs::Base#output?" do
+  it "should filter by type" do
+    output = LogStash::Outputs::NOOP.new("type" => "noop")
+    expect(output.receive(LogStash::Event.new({"type" => "noop"}))).to eq(true)
+    expect(output.receive(LogStash::Event.new({"type" => "not_noop"}))).to eq(false)
+  end
+  
+  it "should filter by tags" do
+    output = LogStash::Outputs::NOOP.new("tags" => ["value", "value2"])
+    expect(output.receive(LogStash::Event.new({"tags" => ["value","value2"]}))).to eq(true)
+    expect(output.receive(LogStash::Event.new({"tags" => ["notvalue"]}))).to eq(false)
+    expect(output.receive(LogStash::Event.new({"tags" => ["value"]}))).to eq(false)
+  end
+
+  it "should exclude by tags" do
+    output = LogStash::Outputs::NOOP.new("exclude_tags" => ["value"])
+    expect(output.receive(LogStash::Event.new({"tags" => ["value"]}))).to eq(false)
+    expect(output.receive(LogStash::Event.new({"tags" => ["notvalue"]}))).to eq(true)
+  end
+end