Set Oj.default_options only once when fluentd is started

It shouldn't be set by each plugins, because later loaded plugins will overwrites it. Signed-off-by: Takuro Ashie <[email protected]>
fluent · Jul 8, 2021 · 3217fe4 · cosmo0920 · Jul 9, 2021 · 3217fe4
1 parent beae9f7
commit 3217fe4
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 18 deletions.
diff --git a/lib/fluent/env.rb b/lib/fluent/env.rb
@@ -22,7 +22,7 @@ module Fluent
   DEFAULT_PLUGIN_DIR = ENV['FLUENT_PLUGIN'] || '/etc/fluent/plugin'
   DEFAULT_SOCKET_PATH = ENV['FLUENT_SOCKET'] || '/var/run/fluent/fluent.sock'
   DEFAULT_BACKUP_DIR = ENV['FLUENT_BACKUP_DIR'] || '/tmp/fluent'
-  DEFAULT_OJ_OPTIONS = Fluent::OjOptions.get_options
+  DEFAULT_OJ_OPTIONS = Fluent::OjOptions.load_env
   DEFAULT_DIR_PERMISSION = 0755
   DEFAULT_FILE_PERMISSION = 0644
 

diff --git a/lib/fluent/oj_options.rb b/lib/fluent/oj_options.rb
@@ -20,6 +20,26 @@ class OjOptions
       'use_to_json': true
     }
 
+    @@available = false
+
+    def self.available?
+      @@available
+    end
+
+    def self.load_env
+      options = self.get_options
+      begin
+        require 'oj'
+        Oj.default_options = options
+        @@available = true
+      rescue LoadError
+        @@available = false
+      end
+      options
+    end
+
+    private
+
     def self.get_options
       options = {}
       DEFAULTS.each { |key, value| options[key] = value }

diff --git a/lib/fluent/plugin/formatter_json.rb b/lib/fluent/plugin/formatter_json.rb
@@ -15,7 +15,7 @@
 #
 
 require 'fluent/plugin/formatter'
-require 'fluent/env'
+require 'fluent/oj_options'
 
 module Fluent
   module Plugin
@@ -30,12 +30,14 @@ class JSONFormatter < Formatter
       def configure(conf)
         super
 
-        begin
-          raise LoadError unless @json_parser == 'oj'
-          require 'oj'
-          Oj.default_options = Fluent::DEFAULT_OJ_OPTIONS
-          @dump_proc = Oj.method(:dump)
-        rescue LoadError
+        if @json_parser == 'oj'
+          if Fluent::OjOptions.available?
+            @dump_proc = Oj.method(:dump)
+          else
+            log.info "Oj isn't installed, fallback to Yajl as json parser"
+            @dump_proc = Yajl.method(:dump)
+          end
+        else
           @dump_proc = Yajl.method(:dump)
         end
 

diff --git a/lib/fluent/plugin/parser_json.rb b/lib/fluent/plugin/parser_json.rb
@@ -15,8 +15,8 @@
 #
 
 require 'fluent/plugin/parser'
-require 'fluent/env'
 require 'fluent/time'
+require 'fluent/oj_options'
 
 require 'yajl'
 require 'json'
@@ -50,8 +50,7 @@ def configure(conf)
       def configure_json_parser(name)
         case name
         when :oj
-          require 'oj'
-          Oj.default_options = Fluent::DEFAULT_OJ_OPTIONS
+          raise LoadError unless Fluent::OjOptions.available?
           [Oj.method(:load), Oj::ParseError]
         when :json then [JSON.method(:load), JSON::ParserError]
         when :yajl then [Yajl.method(:load), Yajl::ParseError]

diff --git a/test/test_event_time.rb b/test/test_event_time.rb
@@ -64,8 +64,8 @@ class EventTimeTest < Test::Unit::TestCase
 
   test 'Oj.dump' do
     time = Fluent::EventTime.new(100)
-    require 'fluent/env'
-    Oj.default_options = Fluent::DEFAULT_OJ_OPTIONS
+    require 'fluent/oj_options'
+    Fluent::OjOptions.load_env
     assert_equal('{"time":100}', Oj.dump({'time' => time}))
     assert_equal('["tag",100,{"key":"value"}]', Oj.dump(["tag", time, {"key" => "value"}], mode: :compat))
   end

diff --git a/test/test_oj_options.rb b/test/test_oj_options.rb
@@ -3,6 +3,13 @@
 require 'fluent/oj_options'
 
 class OjOptionsTest < ::Test::Unit::TestCase
+  begin
+    require 'oj'
+    @@oj_is_avaibale = true
+  rescue LoadError
+    @@oj_is_avaibale = false
+  end
+
   setup do
     @orig_env = {}
     ENV.each do |key, value|
@@ -15,25 +22,34 @@ class OjOptionsTest < ::Test::Unit::TestCase
     @orig_env.each { |key, value| ENV[key] = value }
   end
 
-  sub_test_case "OjOptions" do
+  test "available?" do
+    assert_equal(@@oj_is_avaibale, Fluent::OjOptions.available?)
+  end
+
+  sub_test_case "set by environment variable" do
     test "when no env vars set, returns default options" do
       ENV.delete_if { |key| key.start_with?("FLUENT_OJ_OPTION_") }
-      assert_equal Fluent::OjOptions::DEFAULTS, Fluent::OjOptions.get_options
+      defaults = Fluent::OjOptions::DEFAULTS
+      assert_equal(defaults, Fluent::OjOptions.load_env)
+      assert_equal(defaults, Oj.default_options.slice(*defaults.keys)) if @@oj_is_avaibale
     end
 
     test "valid env var passed with valid value, default is overridden" do
       ENV["FLUENT_OJ_OPTION_BIGDECIMAL_LOAD"] = ":bigdecimal"
-      assert_equal :bigdecimal, Fluent::OjOptions.get_options[:bigdecimal_load]
+      assert_equal(:bigdecimal, Fluent::OjOptions.load_env[:bigdecimal_load])
+      assert_equal(:bigdecimal, Oj.default_options[:bigdecimal_load]) if @@oj_is_avaibale
     end
 
     test "valid env var passed with invalid value, default is not overriden" do
       ENV["FLUENT_OJ_OPTION_BIGDECIMAL_LOAD"] = ":conor"
-      assert_equal :float, Fluent::OjOptions.get_options[:bigdecimal_load]
+      assert_equal(:float, Fluent::OjOptions.load_env[:bigdecimal_load])
+      assert_equal(:float, Oj.default_options[:bigdecimal_load]) if @@oj_is_avaibale
     end
 
     test "invalid env var passed, nothing done with it" do
       ENV["FLUENT_OJ_OPTION_CONOR"] = ":conor"
-      assert_equal nil, Fluent::OjOptions.get_options[:conor]
+      assert_equal(nil, Fluent::OjOptions.load_env[:conor])
+      assert_equal(nil, Oj.default_options[:conor]) if @@oj_is_avaibale
     end
   end
 end