With the following code in a Rails configuration file:
Rails.logger = Logger.new(STDERR)
Causes duplicate log statements when running the app with rails s. It's caused by the following:
|
def log_to_stdout |
|
wrapped_app # touch the app so the logger is set up |
|
|
|
console = ActiveSupport::Logger.new(STDOUT) |
|
console.formatter = Rails.logger.formatter |
|
console.level = Rails.logger.level |
|
|
|
unless ActiveSupport::Logger.logger_outputs_to?(Rails.logger, STDOUT) |
|
Rails.logger.extend(ActiveSupport::Logger.broadcast(console)) |
|
end |
|
end |
The implementation of logger_outputs_to? uses private details of the logger instance to determine whether it's logging to STDOUT. In this specific case, adding STDERR as part of the check might also work. However, not all logger objects have the same private details causing this check to fail in those cases too.
|
def self.logger_outputs_to?(logger, *sources) |
|
logdev = logger.instance_variable_get(:@logdev) |
|
logger_source = logdev.dev if logdev.respond_to?(:dev) |
|
sources.any? { |source| source == logger_source } |
|
end |
I don't think we should be inspecting private object details and I believe that the implementation creates too many edge cases which are better handled by explicit configuration.
My advice would be:
- Default to
Rails.logger = Logger.new(STDERR) (& don't log to log files).
- Don't use
instance_variable_get and consider removing logger_outputs_to?.
- Remove monkey patching (and private implementation introspection) of
Rails.logger.
- Educate users on how to get previous behaviour if it's desired.
If we want to support output to multiple targets, this should be a feature of the log library not the interface for logging. I suggest some kind of tee logger should be appropriate, but I think it's better if we keep this simple in Rails and let logger gem solve this problem.
With the following code in a Rails configuration file:
Causes duplicate log statements when running the app with
rails s. It's caused by the following:rails/railties/lib/rails/commands/server/server_command.rb
Lines 75 to 85 in 346ae79
The implementation of
logger_outputs_to?uses private details of the logger instance to determine whether it's logging toSTDOUT. In this specific case, addingSTDERRas part of the check might also work. However, not all logger objects have the same private details causing this check to fail in those cases too.rails/activesupport/lib/active_support/logger.rb
Lines 16 to 20 in 346ae79
I don't think we should be inspecting private object details and I believe that the implementation creates too many edge cases which are better handled by explicit configuration.
My advice would be:
Rails.logger = Logger.new(STDERR)(& don't log to log files).instance_variable_getand consider removinglogger_outputs_to?.Rails.logger.If we want to support output to multiple targets, this should be a feature of the log library not the interface for logging. I suggest some kind of tee logger should be appropriate, but I think it's better if we keep this simple in Rails and let
loggergem solve this problem.