rails_semantic_logger icon indicating copy to clipboard operation
rails_semantic_logger copied to clipboard

Prefer ActiveRecord::Base.logger instead of a custom logger instance in RailsSemanticLogger::ActiveRecord::LogSubscriber

Open kpumuk opened this issue 1 year ago • 2 comments
trafficstars

Description of changes

In this change I propose to switch from using a custom logger instance SemanticLogger["ActiveRecord"] to ::ActiveRecord::Base.logger when logging events from the RailsSemanticLogger::ActiveRecord::LogSubscriber.

  • Current behaviour is inconsistent with what is expected from ActiveRecord. When a .logger class attribute is set on a component level (ActionController, ActionMailer, ActiveRecord), the log subscriber should respect it to allow debugging uses (for example, in the IRB console)
  • This is consistent with how logs are handled in ActiveJob, ActionMailer, etc in semantic logger already
  • Performance consideration for accessing the class variable (if there were any) have been addressed in Rails 7.0 (https://github.com/rails/rails/pull/42237)

What this solves

When in a Rails console, a simple way to turn on AR logging:

ActiveRecord::Base.logger = Logger.new(STDOUT)
# or temporary increase verbosity
ActiveRecord::Base.logger.level = Logger::DEBUG

Concerns

  • name changed from ActiveRecord to ActiveRecord::Base. To preserve old behaviour, one might create an initializer:
     ActiveSupport.on_load(:active_record) do
       ::ActiveRecord::Base.logger = SemanticLogger[::ActiveRecord]
     end
    
  • There might be a performance impact for Rails < 7.0, although Rails 6.1 official EoL is in a month (https://endoflife.date/rails)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

kpumuk avatar Aug 18 '24 18:08 kpumuk