From 3217fe4da5641c47f42ca9f5e29fd2341fdff1e5 Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Thu, 8 Jul 2021 12:45:29 +0900 Subject: [PATCH] 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 --- lib/fluent/env.rb | 2 +- lib/fluent/oj_options.rb | 20 ++++++++++++++++++++ lib/fluent/plugin/formatter_json.rb | 16 +++++++++------- lib/fluent/plugin/parser_json.rb | 5 ++--- test/test_event_time.rb | 4 ++-- test/test_oj_options.rb | 26 +++++++++++++++++++++----- 6 files changed, 55 insertions(+), 18 deletions(-) diff --git a/lib/fluent/env.rb b/lib/fluent/env.rb index 8405ac8bfc..5ddf505bc2 100644 --- 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 index 4e17321720..f505fb432a 100644 --- 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 index 180d23eeca..8d850f28fb 100644 --- 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 index ea6fa657a3..840f6a963c 100644 --- 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 index 581a276e57..6ce82f5cc8 100644 --- 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 index 64b9a57c5b..cf91612d0c 100644 --- 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