semantic_logger icon indicating copy to clipboard operation
semantic_logger copied to clipboard

Log level signal trap omits :fatal and :error levels

Open keithrbennett opened this issue 3 years ago • 1 comments
trafficstars

Environment

  • Ruby Version: 3.1.2
  • Semantic Logger Version: 4.11.0

Expected Behavior

Adding the signal handler and then sending the log level signal should cycle one level down per signal, then wrap around from trace to fatal.

Actual Behavior

This mostly works, but the fatal and error levels are missing from this cycle, as illustrated by this script (also available at https://github.com/reidmorrison/semantic_logger/issues/231):

#!/usr/bin/env ruby

require 'semantic_logger'

# Redefine add_signal_handler to output to STDERR (omit TTIN behavior for brevity):
module SemanticLogger
  def self.add_signal_handler(log_level_signal = "USR2", thread_dump_signal = "TTIN", gc_log_microseconds = 100_000)
    if log_level_signal
      Signal.trap(log_level_signal) do

        # This line added:
        STDERR.print "Original level: #{default_level.to_s.ljust(5)}. "

        index     = default_level == :trace ? LEVELS.find_index(:error) : LEVELS.find_index(default_level)
        new_level = LEVELS[index - 1]

        # This line disabled:
        #self["SemanticLogger"].warn "Changed global default log level to #{new_level.inspect}"
        
        self.default_level = new_level
        
        # This line added:
        STDERR.puts "New level is #{default_level}. "
      end
    end
  end
end

SemanticLogger.add_signal_handler('USR1')

7.times do
  print 'Sending signal...'
  Process.kill('USR1', Process.pid)
  sleep 0.01
end

=begin
Output is:

Sending signal...Original level: info . New level is debug. 
Sending signal...Original level: debug. New level is trace. 
Sending signal...Original level: trace. New level is warn. 
Sending signal...Original level: warn . New level is info. 
Sending signal...Original level: info . New level is debug. 
Sending signal...Original level: debug. New level is trace. 
Sending signal...Original level: trace. New level is warn. 

=end

Pull Request

I have submitted a PR (https://github.com/reidmorrison/semantic_logger/pull/229) that fixes this problem.

keithrbennett avatar Sep 11 '22 07:09 keithrbennett

A colleague pointed out that preventing the suppression of fatal and error messages may be a virtue and not a bug. However, if this were the case, then :error should be included in the cycle, whereas currently it is not.

In addition, I balk at prohibiting a feature that a library was deliberately designed to support.

In addition, it is especially awkward when the desired default level is fatal or error, and once the signal is used to cycle through the levels, both now become unavailable, making it impossible to return to the desired default.

keithrbennett avatar Sep 22 '22 15:09 keithrbennett

Thank you for the pull request, merged into master. Will publish a new release shortly with the fix.

reidmorrison avatar Oct 14 '22 19:10 reidmorrison