Skip to content

Commit

Permalink
Fix config.log_level being ignored when using a Broadcast Logger:
Browse files Browse the repository at this point in the history
- ### 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
  • Loading branch information
Edouard-chin committed Dec 12, 2023
1 parent 89d8569 commit b6bb506
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 3 deletions.
6 changes: 5 additions & 1 deletion railties/lib/rails/application/bootstrap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 11 additions & 2 deletions railties/lib/rails/application/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions railties/test/application/configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit b6bb506

Please sign in to comment.