Merge pull request #1153 from ganmacs/not-to-warn-about-secondary-plu…

…gin-type Add a check that primary and secondary use `custom_format` or not
fluent · Aug 19, 2016 · 7194cb9 · 7194cb9
2 parents 0be9a64 + 971eaff
commit 7194cb9
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 2 deletions.
diff --git a/lib/fluent/plugin/output.rb b/lib/fluent/plugin/output.rb
@@ -301,7 +301,7 @@ def configure(conf)
           @secondary.acts_as_secondary(self)
           @secondary.configure(secondary_conf)
           @secondary.router = router if @secondary.has_router?
-          if self.class != @secondary.class
+          if (self.class != @secondary.class) && (@custom_format || @secondary.implement?(:custom_format))
             log.warn "secondary type should be same with primary one", primary: self.class.to_s, secondary: @secondary.class.to_s
           end
         else

diff --git a/test/plugin/test_output.rb b/test/plugin/test_output.rb
@@ -98,6 +98,17 @@ def try_write(chunk)
 end
 
 class OutputTest < Test::Unit::TestCase
+    class << self
+      def startup
+        $LOAD_PATH.unshift File.expand_path(File.join(File.dirname(__FILE__), '../scripts'))
+        require 'fluent/plugin/out_test'
+      end
+
+      def shutdown
+        $LOAD_PATH.shift
+      end
+    end
+
   def create_output(type=:full)
     case type
     when :bare     then FluentPluginOutputTest::DummyBareOutput.new
@@ -477,6 +488,36 @@ def waiting(seconds)
 
       i.stop; i.before_shutdown; i.shutdown; i.after_shutdown; i.close; i.terminate
     end
+
+    test "Warn if primary type is different from secondary type and either primary or secondary has custom_format" do
+      o = create_output(:buffered)
+      mock(o.log).warn("secondary type should be same with primary one",
+                                { primary: o.class.to_s, secondary: "Fluent::Plugin::TestOutput" })
+
+      o.configure(config_element('ROOT','',{},[config_element('secondary','',{'@type'=>'test', 'name' => "cool"})]))
+      assert_not_nil o.instance_variable_get(:@secondary)
+    end
+
+    test "don't warn if primary type is the same as secondary type" do
+      o = Fluent::Plugin::TestOutput.new
+      mock(o.log).warn("secondary type should be same with primary one",
+                                { primary: o.class.to_s, secondary: "Fluent::Plugin::TestOutput" }).never
+
+      o.configure(config_element('ROOT','',{'name' => "cool2"},
+                                 [config_element('secondary','',{'@type'=>'test', 'name' => "cool"}),
+                                  config_element('buffer','',{'@type'=>'memory'})]
+                                ))
+      assert_not_nil o.instance_variable_get(:@secondary)
+    end
+
+    test "don't warn if primary type is different from secondary type and both don't have custom_format" do
+      o = create_output(:standard)
+      mock(o.log).warn("secondary type should be same with primary one",
+                                { primary: o.class.to_s, secondary: "Fluent::Plugin::TestOutput" }).never
+
+      o.configure(config_element('ROOT','',{},[config_element('secondary','',{'@type'=>'test', 'name' => "cool"})]))
+      assert_not_nil o.instance_variable_get(:@secondary)
+    end
   end
 
   sub_test_case 'sync output feature' do

diff --git a/test/test_output.rb b/test/test_output.rb
@@ -95,12 +95,37 @@ def write(chunk)
     end
 
     def test_secondary
-      d = create_driver(CONFIG + %[
+      d = Fluent::Test::BufferedOutputTestDriver.new(Fluent::BufferedOutput) do
+        def write(chunk)
+          chunk.read
+        end
+      end
+
+      mock(d.instance.log).warn("secondary type should be same with primary one",
+                                { primary: d.instance.class.to_s, secondary: "Fluent::Plugin::Test2Output" })
+      d.configure(CONFIG + %[
+        <secondary>
+          type test2
+          name c0
+        </secondary>
+      ])
+
+      assert_not_nil d.instance.instance_variable_get(:@secondary).router
+    end
+
+    def test_secondary_with_no_warn_log
+      # ObjectBufferedOutput doesn't implemnt `custom_filter`
+      d = Fluent::Test::BufferedOutputTestDriver.new(Fluent::ObjectBufferedOutput)
+
+      mock(d.instance.log).warn("secondary type should be same with primary one",
+                                { primary: d.instance.class.to_s, secondary: "Fluent::Plugin::Test2Output" }).never
+      d.configure(CONFIG + %[
         <secondary>
           type test2
           name c0
         </secondary>
       ])
+
       assert_not_nil d.instance.instance_variable_get(:@secondary).router
     end
   end