ActiveSupport::LogSubscriber restore compatibility with SemanticLogger

Fix: rails#49563

The semantic_logger gems doesn't behave exactly like stdlib logger
in that `SemanticLogger#level` returns a Symbol while stdlib `Logger#level`
returns an Integer.

Because of this we can't simply compare integers, we have to use the
various `#{level}?` methods.

Author: byroot

activesupport/CHANGELOG.md

@@ -1,3 +1,13 @@
+*   Fix compatibility with the `semantic_logger` gem.
+
+    The `semantic_logger` gem doesn't behave exactly like stdlib logger in that
+    `SemanticLogger#level` returns a Symbol while stdlib `Logger#level` returns an Integer.
+
+    This caused the various `LogSubscriber` classes in Rails to break when assigned a
+    `SemanticLogger` instance.
+
+    *Jean Boussier*, *ojab*
+
 *   Fix MemoryStore to prevent race conditions when incrementing or decrementing.
 
     *Pierre Jambet*

activesupport/lib/active_support/log_subscriber.rb

@@ -86,6 +86,12 @@ class LogSubscriber < Subscriber
     mattr_accessor :colorize_logging, default: true
     class_attribute :log_levels, instance_accessor: false, default: {} # :nodoc:
 
+    LEVEL_CHECKS = {
+      debug: -> (logger) { !logger.debug? },
+      info: -> (logger) { !logger.info? },
+      error: -> (logger) { !logger.error? },
+    }
+
     class << self
       def logger
         @logger ||= if defined?(Rails) && Rails.respond_to?(:logger)
@@ -122,7 +128,7 @@ def set_event_levels
         end
 
         def subscribe_log_level(method, level)
-          self.log_levels = log_levels.merge(method => ::Logger.const_get(level.upcase))
+          self.log_levels = log_levels.merge(method => LEVEL_CHECKS.fetch(level))
           set_event_levels
         end
     end
@@ -137,7 +143,7 @@ def logger
     end
 
     def silenced?(event)
-      logger.nil? || logger.level > @event_levels.fetch(event, Float::INFINITY)
+      logger.nil? || @event_levels[event]&.call(logger)
     end
 
     def call(event)

activesupport/test/log_subscriber_test.rb

@@ -3,39 +3,44 @@
 require_relative "abstract_unit"
 require "active_support/log_subscriber/test_helper"
 
-class MyLogSubscriber < ActiveSupport::LogSubscriber
-  attr_reader :event
+class SyncLogSubscriberTest < ActiveSupport::TestCase
+  include ActiveSupport::LogSubscriber::TestHelper
 
-  def some_event(event)
-    @event = event
-    info event.name
-  end
+  class MyLogSubscriber < ActiveSupport::LogSubscriber
+    attr_reader :event
 
-  def foo(event)
-    debug "debug"
-    info { "info" }
-    warn "warn"
-  end
+    def some_event(event)
+      @event = event
+      info event.name
+    end
 
-  def bar(event)
-    info "#{color("cool", :red)}, #{color("isn't it?", :blue, bold: true)}"
-  end
+    def foo(event)
+      debug "debug"
+      info { "info" }
+      warn "warn"
+    end
 
-  def baz(event)
-    info "#{color("rad", :green, bold: true, underline: true)}, #{color("isn't it?", :yellow, italic: true)}"
-  end
+    def bar(event)
+      info "#{color("cool", :red)}, #{color("isn't it?", :blue, bold: true)}"
+    end
 
-  def deprecated(event)
-    info "#{color("bogus", :red, true)}"
-  end
+    def baz(event)
+      info "#{color("rad", :green, bold: true, underline: true)}, #{color("isn't it?", :yellow, italic: true)}"
+    end
 
-  def puke(event)
-    raise "puke"
-  end
-end
+    def deprecated(event)
+      info "#{color("bogus", :red, true)}"
+    end
 
-class SyncLogSubscriberTest < ActiveSupport::TestCase
-  include ActiveSupport::LogSubscriber::TestHelper
+    def puke(event)
+      raise "puke"
+    end
+
+    def debug_only(event)
+      debug "debug logs are enabled"
+    end
+    subscribe_log_level :debug_only, :debug
+  end
 
   def setup
     super
@@ -47,10 +52,6 @@ def teardown
     ActiveSupport::LogSubscriber.log_subscribers.clear
   end
 
-  def instrument(*args, &block)
-    ActiveSupport::Notifications.instrument(*args, &block)
-  end
-
   def test_proxies_method_to_rails_logger
     @log_subscriber.foo(nil)
     assert_equal %w(debug), @logger.logged(:debug)
@@ -165,4 +166,51 @@ def test_logging_does_not_die_on_failures
     assert_equal 1, @logger.logged(:error).size
     assert_match 'Could not log "puke.my_log_subscriber" event. RuntimeError: puke', @logger.logged(:error).last
   end
+
+  def test_subscribe_log_level
+    MyLogSubscriber.logger = @logger
+    @logger.level = Logger::INFO
+    MyLogSubscriber.attach_to :my_log_subscriber, @log_subscriber
+    assert_empty @logger.logged(:debug)
+
+    instrument "debug_only.my_log_subscriber"
+    wait
+    assert_empty @logger.logged(:debug)
+
+    @logger.level = Logger::DEBUG
+    instrument "debug_only.my_log_subscriber"
+    wait
+    assert_not_empty @logger.logged(:debug)
+  end
+
+  class MockSemanticLogger < MockLogger
+    LEVELS = [:debug, :info]
+    def level
+      LEVELS[super]
+    end
+  end
+
+  def test_subscribe_log_level_with_non_numeric_levels
+    # The semantic_logger gem doesn't returns integers but symbols as levels
+    @logger = MockSemanticLogger.new
+    set_logger(@logger)
+    MyLogSubscriber.logger = @logger
+    @logger.level = Logger::INFO
+    MyLogSubscriber.attach_to :my_log_subscriber, @log_subscriber
+    assert_empty @logger.logged(:debug)
+
+    instrument "debug_only.my_log_subscriber"
+    wait
+    assert_empty @logger.logged(:debug)
+
+    @logger.level = Logger::DEBUG
+    instrument "debug_only.my_log_subscriber"
+    wait
+    assert_not_empty @logger.logged(:debug)
+  end
+
+  private
+    def instrument(*args, &block)
+      ActiveSupport::Notifications.instrument(*args, &block)
+    end
 end