reidmorrison / rails_semantic_logger

Rails Semantic Logger replaces the Rails default logger with Semantic Logger
https://logger.rocketjob.io/rails
Apache License 2.0
329 stars 116 forks source link

Support Sidekiq v7 #169

Closed supairish closed 1 year ago

supairish commented 2 years ago

Hello!

Environment

Expected Behavior

I should be able to override the default Sidekiq logger with a semantic logger

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "rails",                  "~> 7.0.0"
  gem "amazing_print"
  gem 'rails_semantic_logger',  '~> 4.10.0'
  gem 'sidekiq',                '7.0'
end

require "rack/test"
require "action_controller/railtie"

class TestApp < Rails::Application
  config.root = __dir__
  config.hosts << "example.org"
  config.session_store :cookie_store, key: "cookie_store_key"
  secrets.secret_key_base = "secret_key_base"

  # Replace the Sidekiq logger
  Sidekiq.logger = SemanticLogger[Sidekiq] if defined?(Sidekiq)

  routes.draw do
    get "/" => "test#index"
  end
end

class TestController < ActionController::Base
  include Rails.application.routes.url_helpers

  def index
    render plain: "Home"
  end
end

require "minitest/autorun"

class BugTest < Minitest::Test
  include Rack::Test::Methods

  def test_returns_success
    get "/"
    assert last_response.ok?
  end

  private
    def app
      Rails.application
    end
end

Actual Behavior

If you upgrade from Sidekiq 6.x to 7.x. Attempting to boot app will raise exception due to Sidekiq removing the global module setter "logger= "

Not 100% sure but it appears options like this will now have to go through a config block setup

Comment found here https://github.com/mperham/sidekiq/compare/v6.5.7...v7.0.0#diff-6985e251e1b3fe0ce1f47369a1ede593aab9b27757ac7ff6e16591bc0318b034R86

Logger moved to Sidekiq::Config https://github.com/mperham/sidekiq/compare/v6.5.7...v7.0.0#diff-6605d43f981434252c6bd19fecf05464aef2436da1225630105f7ff2c0265ab1R245

NoMethodError:
  undefined method `logger=' for Sidekiq:Module

        Sidekiq.logger       = SemanticLogger[Sidekiq] if defined?(Sidekiq)
               ^^^^^^^^^^^^^^^
  Did you mean?  logger
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/rails_semantic_logger-4.10.0/lib/rails_semantic_logger/engine.rb:113:in `block in <class:Engine>'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.4/lib/active_support/lazy_load_hooks.rb:92:in `block in execute_hook'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.4/lib/active_support/lazy_load_hooks.rb:85:in `with_execution_control'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.4/lib/active_support/lazy_load_hooks.rb:90:in `execute_hook'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.4/lib/active_support/lazy_load_hooks.rb:76:in `block in run_load_hooks'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.4/lib/active_support/lazy_load_hooks.rb:75:in `each'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/activesupport-7.0.4/lib/active_support/lazy_load_hooks.rb:75:in `run_load_hooks'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/railties-7.0.4/lib/rails/application/bootstrap.rb:92:in `block in <module:Bootstrap>'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/railties-7.0.4/lib/rails/initializable.rb:32:in `instance_exec'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/railties-7.0.4/lib/rails/initializable.rb:32:in `run'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/railties-7.0.4/lib/rails/initializable.rb:61:in `block in run_initializers'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/railties-7.0.4/lib/rails/initializable.rb:60:in `run_initializers'
# /Users/supairish/.rvm/gems/ruby-3.1.2/gems/railties-7.0.4/lib/rails/application.rb:372:in `initialize!'
# ./config/environment.rb:7:in `<top (required)>'
reidmorrison commented 1 year ago

This commit will disable the automatic logger replacement, since Sidekiq 7 is not backward compatible. https://github.com/reidmorrison/rails_semantic_logger/commit/a64c48009f837ca875a451206e647ccba94cd0dd

Going to leave the issue open to determine we need to move replacing Sidekiq logging into a documentation change, or if there is some way to automatically replace the logger as it does today for Sidekiq v6.

reidmorrison commented 1 year ago

Published rails_semantic_logger (4.11.0) to check for Sidekiq upto v6.

This discussion covers how to configure Sidekiq logging with v7 now: https://github.com/reidmorrison/semantic_logger/discussions/246