Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log level signal trap omits :fatal and :error levels #231

Closed
keithrbennett opened this issue Sep 11, 2022 · 2 comments
Closed

Log level signal trap omits :fatal and :error levels #231

keithrbennett opened this issue Sep 11, 2022 · 2 comments

Comments

@keithrbennett
Copy link
Contributor

keithrbennett commented Sep 11, 2022

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 #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 (#229) that fixes this problem.

@keithrbennett
Copy link
Contributor Author

keithrbennett commented Sep 22, 2022

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.

@reidmorrison
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants