reidmorrison/rails_semantic_logger

Error when using customer logger in Rails with 7.1.0.alpha (main)

bmad opened this issue · 9 comments

bmad commented

Rails 7.1.0.alpha (main) branch now added a comparison to ActiveSupport::LogSubscriber expecting its logger’s level to always be an integer. This now causes an error using a custom semantic_logger logger because the logger uses a symbol to indicate log level.

Expected Behavior

A custom semantic_logger logger should work the same in Rails 7.1.0.alpha (main).

Actual Behavior

MANUAL STEPS TO REPRODUCE:

  • With Ruby 3.0.4, make a new rails repo with the Rails 7.1.0.alpha (main) branch
  • Add rspec-rails and semantic_logger gems to Gemfile
  • Run rails generate rspec:install
  • Add ActiveRecord::Base.logger = SemanticLogger["ActiveRecord"] to config/initializers/logging.rb
  • Create a test model with specs - rails generate model Article title:string
  • Run rspec

An ArgumentError: comparison of Symbol with 0 failed is raised.

Solution Thoughts

I filed an issue on Rails here rails/rails#47451 and filed a PR to fix the issue on Rails here rails/rails#47427. Unfortunately, after a discussion on the PR, I closed it out because it no longer seemed like a good idea.

To continue being able to use semantic_logger in Rails, I can only think of refactoring semantic_logger to use the standard ruby logger level integers. We could potentially keep the :trace level by having it's value be -1. This would unfortunately still change the interface of the gem.

I might be able to work on this, but I'd like to make sure this is an acceptable solution.

simi commented

I was looking into this problem as well. It is possible to bandaid this with following "shim". Anyway it makes sense to me to inline level method with Logger#level and return index instead of name. Indeed major bump would be needed.

module SemanticLoggerFix
  def silenced?(event)
    logger.nil? || logger.send(:level_index) > @event_levels.fetch(event, Float::INFINITY)
  end
end 

ActiveSupport::LogSubscriber.prepend(SemanticLoggerFix)
bmad commented

Ah, I would think that would work. Except we would need to do something like (logger.send(:level_index) - 1) because semantic_logger has trace at the 0 index, which bumps all of the other levels by one in comparison to ruby standard logging integers.

Going to move to Rails Semantic Logger to see if we can make it include the above patch on Rails 7.

There is no Rails 7.1.0.alpha gem available yet. Not able to add it to the tests, not only that minitest-rails does not support it yet either.

Rails Semantic Logger swaps out the stock Rails log subscribers. We can update them to ignore the silenced method since it may be duplicating what Semantic Logger already does (class level logging levels).

if config.rails_semantic_logger.semantic

With a full stack trace I can help diagnose further. Or, if you have the instructions on how to build a new rails app using rails main I could give it a try.

bmad commented

@reidmorrison - Thanks for looking at this, you should be able to reproduce this by:

  • Run rails new test_app and cd into the new app
  • Add rspec-rails and semantic_logger gems to the Gemfile
  • Update the rails gem to use the git main branch by updating the Gemfile reference to: gem 'rails', git: 'https://github.com/rails/rails.git', branch: 'main'
  • Run bundle install
  • Run rails generate rspec:install
  • Run echo 'ActiveRecord::Base.logger = SemanticLogger["ActiveRecord"]' >> config/initializers/logging.rb
  • Run bundle exec rails generate model Article title:string
  • Run bundle exec rspec

To be able to see the full stack trace including the rails code, you'll need to clone the rails repo locally and update the Gemfile rails reference in the app from the above steps to:
gem "rails", path: "<cloned_rails_path>"

I just started upgrading an application to gem 'rails', '7.1.0.beta1' and I am observing the same issue.

@reidmorrison facing the same issue. Is there any plan to fix the compatibility with rails 7.1?

rails_semantic_logger v 4.13.0 has been published and includes several community contributions to get it working with Rails 7.1. Please try the new version and open a new issue if the problem persists.

I have added a new issue here: #219