From b6bb5065101b04dbc3e25971f5438de5d344fc86 Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Tue, 12 Dec 2023 15:23:58 +0100 Subject: [PATCH] Fix `config.log_level` being ignored when using a Broadcast Logger: - ### Problem If an application has configured a Broadcast Logger, setting the `config.log_level` to any value has no effect: ```ruby # config/application.rb config.logger = ActiveSupport::BroadcastLogger.new(Logger.new(STDOUT)) config.log_level = :warn puts Rails.logger.broadcasts.map(&:level) # => [Logger::DEBUG] ``` This is a side effect of #49621 which tried to fix the `log_level` default value overriding the whole broadcasts. ### Context The `log_level` default's value shouldn't apply to a Broadcast Logger, as otherwise it overrides whatever the application previously configured. While this behaviour is the same with a vanilla logger, at least we can workaround it: ```ruby # When using a vanilla logger config.logger = Logger.new(STDOUT, level: LOGGER::WARN) # Once the app boots, the level is overriden to DEBUG. We need to add the following line. config.log_level = :warn # When using a broadcast logger stdout = Logger.new(STDOUT, level: Logger::INFO) stderr = Logger.new(STDERR, level: Logger::ERROR) config.logger = ActiveSupport::BroadcastLogger(stdout, stderr) # Once the app boots the whole broadcast level is overriden to DEBUG. # There is no way to workaround this as you can't fine grain the level of each # loggers with `config.log_level=`. ``` ### Solution This PR ignores the default `log_level` value when using a Broadcast Logger, but ensure it gets used if an application manually sets it. Fix #50324 --- railties/lib/rails/application/bootstrap.rb | 6 ++++- .../lib/rails/application/configuration.rb | 13 ++++++++-- .../test/application/configuration_test.rb | 25 +++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/railties/lib/rails/application/bootstrap.rb b/railties/lib/rails/application/bootstrap.rb index 83a8da92988f6..af488c459e353 100644 --- a/railties/lib/rails/application/bootstrap.rb +++ b/railties/lib/rails/application/bootstrap.rb @@ -55,7 +55,11 @@ module Bootstrap logger end - unless Rails.logger.is_a?(ActiveSupport::BroadcastLogger) + if Rails.logger.is_a?(ActiveSupport::BroadcastLogger) + if config.broadcast_log_level + Rails.logger.level = ActiveSupport::Logger.const_get(config.broadcast_log_level.to_s.upcase) + end + else Rails.logger.level = ActiveSupport::Logger.const_get(config.log_level.to_s.upcase) broadcast_logger = ActiveSupport::BroadcastLogger.new(Rails.logger) broadcast_logger.formatter = Rails.logger.formatter diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index aa279063849e5..913f3000a4097 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -19,13 +19,13 @@ class Configuration < ::Rails::Engine::Configuration :ssl_options, :public_file_server, :session_options, :time_zone, :reload_classes_only_on_change, :beginning_of_week, :filter_redirect, :x, - :read_encrypted_secrets, :log_level, :content_security_policy_report_only, + :read_encrypted_secrets, :content_security_policy_report_only, :content_security_policy_nonce_generator, :content_security_policy_nonce_directives, :require_master_key, :credentials, :disable_sandbox, :sandbox_by_default, :add_autoload_paths_to_load_path, :rake_eager_load, :server_timing, :log_file_size, :dom_testing_default_html_version - attr_reader :encoding, :api_only, :loaded_config_version + attr_reader :encoding, :api_only, :loaded_config_version, :log_level def initialize(*) super @@ -373,6 +373,15 @@ def api_only=(value) @debug_exception_response_format ||= :api end + def log_level=(level) + @log_level = level + @broadcast_log_level = level + end + + def broadcast_log_level # :nodoc: + defined?(@broadcast_log_level) ? @broadcast_log_level : nil + end + def debug_exception_response_format @debug_exception_response_format || :default end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 9ca64ebe75cdd..5380dbdb55427 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1949,6 +1949,31 @@ def index assert_equal Logger::DEBUG, Rails.logger.level end + test "config.log_level does not override the level of the broadcast with the default value" do + add_to_config <<-RUBY + stdout = Logger.new(STDOUT, level: Logger::INFO) + stderr = Logger.new(STDERR, level: Logger::ERROR) + config.logger = ActiveSupport::BroadcastLogger.new(stdout, stderr) + RUBY + + app "development" + + assert_equal([Logger::INFO, Logger::ERROR], Rails.logger.broadcasts.map(&:level)) + end + + test "config.log_level overrides the level of the broadcast when a custom value is set" do + add_to_config <<-RUBY + stdout = Logger.new(STDOUT) + stderr = Logger.new(STDERR) + config.logger = ActiveSupport::BroadcastLogger.new(stdout, stderr) + config.log_level = :warn + RUBY + + app "development" + + assert_equal([Logger::WARN, Logger::WARN], Rails.logger.broadcasts.map(&:level)) + end + test "config.logger when logger is already a Broadcast Logger" do logger = ActiveSupport::BroadcastLogger.new