harmonize add_field and add_tag behavior in inputs and filters

Fixes #2390

lib/logstash/filters/base.rb

@@ -4,6 +4,7 @@ require "logstash/event"
 require "logstash/logging"
 require "logstash/plugin"
 require "logstash/config/mixin"
+require "logstash/util/decorators"
 
 class LogStash::Filters::Base < LogStash::Plugin
   include LogStash::Config::Mixin
@@ -179,21 +180,7 @@ class LogStash::Filters::Base < LogStash::Plugin
   # matches the filter's conditions (right type, etc)
   protected
   def filter_matched(event)
-    @add_field.each do |field, value|
+    LogStash::Util::Decorators.add_fields(@add_field,event,"filters/#{self.class.name}")
-      field = event.sprintf(field)
-      value = [value] if !value.is_a?(Array)
-      value.each do |v|
-        v = event.sprintf(v)
-        if event.include?(field)
-          event[field] = [event[field]] if !event[field].is_a?(Array)
-          event[field] << v
-        else
-          event[field] = v
-        end
-        @logger.debug? and @logger.debug("filters/#{self.class.name}: adding value to field",
-                                       :field => field, :value => value)
-      end
-    end
 
     @remove_field.each do |field|
       field = event.sprintf(field)
@@ -202,12 +189,7 @@ class LogStash::Filters::Base < LogStash::Plugin
       event.remove(field)
     end
 
-    @add_tag.each do |tag|
+    LogStash::Util::Decorators.add_tags(@add_tag,event,"filters/#{self.class.name}")
-      tag = event.sprintf(tag)
-      @logger.debug? and @logger.debug("filters/#{self.class.name}: adding tag",
-                                       :tag => tag)
-      (event["tags"] ||= []) << tag
-    end
 
     @remove_tag.each do |tag|
       break if event["tags"].nil?

lib/logstash/inputs/base.rb

@@ -5,6 +5,7 @@ require "logstash/plugin"
 require "logstash/logging"
 require "logstash/config/mixin"
 require "logstash/codecs/base"
+require "logstash/util/decorators"
 
 # This is the base class for Logstash inputs.
 class LogStash::Inputs::Base < LogStash::Plugin
@@ -109,14 +110,8 @@ class LogStash::Inputs::Base < LogStash::Plugin
     # Only set 'type' if not already set. This is backwards-compatible behavior
     event["type"] = @type if @type && !event.include?("type")
 
-    if @tags.any?
+    LogStash::Util::Decorators.add_fields(@add_field,event,"inputs/#{self.class.name}")
-      event["tags"] ||= []
+    LogStash::Util::Decorators.add_tags(@tags,event,"inputs/#{self.class.name}")
-      event["tags"] += @tags
-    end
-
-    @add_field.each do |field, value|
-      event[field] = value
-    end
   end
 
   protected

lib/logstash/util/decorators.rb

@@ -0,0 +1,46 @@
+# encoding: utf-8
+
+require "logstash/namespace"
+require "logstash/util"
+
+module LogStash::Util
+
+  # Decorators provides common manipulation on the event data.
+  module Decorators
+    extend self
+    
+    @logger = Cabin::Channel.get(LogStash)
+
+    # fields is a hash of field => value
+    # where both `field` and `value` can use sprintf syntax.
+    def add_fields(fields,event, pluginname)
+      fields.each do |field, value|
+        field = event.sprintf(field)
+        value = Array(value)
+        value.each do |v|
+          v = event.sprintf(v)
+          if event.include?(field)
+            event[field] = Array(event[field])
+            event[field] << v
+          else
+            event[field] = v
+          end
+          @logger.debug? and @logger.debug("#{pluginname}: adding value to field",
+                                         :field => field, :value => value)
+        end
+      end
+    end
+
+    # tags is an array of string. sprintf syntax can be used.
+    def add_tags(tags, event, pluginname)
+      tags.each do |tag|
+        tag = event.sprintf(tag)
+        @logger.debug? and @logger.debug("#{pluginname}: adding tag",
+                                       :tag => tag)
+        (event["tags"] ||= []) << tag
+      end
+    end
+
+  end # module LogStash::Util::Decorators
+
+end # module LogStash::Util

spec/inputs/base_spec.rb

@@ -1,6 +1,67 @@
 # encoding: utf-8
 require "spec_helper"
 
+# use a dummy NOOP input to test Inputs::Base
+class LogStash::Inputs::NOOP < LogStash::Inputs::Base
+  config_name "noop"
+  milestone 2
+
+  def register; end
+
+end
+
+describe "LogStash::Inputs::Base#decorate" do
+  it "should add tag" do
+    input = LogStash::Inputs::NOOP.new("tags" => "value")
+    evt = LogStash::Event.new({"type" => "noop"})
+    input.instance_eval {decorate(evt)}
+    expect(evt["tags"]).to eq(["value"])
+  end
+
+  it "should add multiple tag" do
+    input = LogStash::Inputs::NOOP.new("tags" => ["value1","value2"])
+    evt = LogStash::Event.new({"type" => "noop"})
+    input.instance_eval {decorate(evt)}
+    expect(evt["tags"]).to eq(["value1","value2"])
+  end
+
+  it "should allow duplicates  tag" do
+    input = LogStash::Inputs::NOOP.new("tags" => ["value","value"])
+    evt = LogStash::Event.new({"type" => "noop"})
+    input.instance_eval {decorate(evt)}
+    expect(evt["tags"]).to eq(["value","value"])
+  end
+
+  it "should add tag with sprintf" do
+    input = LogStash::Inputs::NOOP.new("tags" => "%{type}")
+    evt = LogStash::Event.new({"type" => "noop"})
+    input.instance_eval {decorate(evt)}
+    expect(evt["tags"]).to eq(["noop"])
+  end
+
+  it "should add single field" do
+    input = LogStash::Inputs::NOOP.new("add_field" => {"field" => "value"})
+    evt = LogStash::Event.new({"type" => "noop"})
+    input.instance_eval {decorate(evt)}
+    expect(evt["field"]).to eq("value")
+  end
+
+  it "should add single field with sprintf" do
+    input = LogStash::Inputs::NOOP.new("add_field" => {"%{type}" => "%{type}"})
+    evt = LogStash::Event.new({"type" => "noop"})
+    input.instance_eval {decorate(evt)}
+    expect(evt["noop"]).to eq("noop")
+  end
+
+  it "should add multiple field" do
+    input = LogStash::Inputs::NOOP.new("add_field" => {"field" => ["value1", "value2"], "field2" => "value"})
+    evt = LogStash::Event.new({"type" => "noop"})
+    input.instance_eval {decorate(evt)}
+    expect(evt["field"]).to eq(["value1","value2"])
+    expect(evt["field2"]).to eq("value")
+  end
+end
+
 describe "LogStash::Inputs::Base#fix_streaming_codecs" do
   it "should carry the charset setting along when switching" do
     require "logstash/inputs/tcp"