Fixes #27509 - logging reopen USR1 handler fixed

lib/proxy/log_buffer/decorator.rb

@@ -7,26 +7,15 @@ def self.instance
       @@instance ||= new(::Proxy::LoggerFactory.logger, ::Proxy::LoggerFactory.log_file)
     end
 
-    attr_accessor :formatter
+    attr_accessor :formatter, :roll_log
+    alias_method :roll_log?, :roll_log
 
     def initialize(logger, log_file, buffer = Proxy::LogBuffer::Buffer.instance)
       @logger = logger
       @buffer = buffer
       @log_file = log_file
       @mutex = Mutex.new
-      @roll_log = false
-    end
-
-    # due to synchronization can't re-open the log from the signal trap
-    def roll_log
-      @roll_log = true
-    end
-
-    def handle_log_rolling
-      return if @log_file.casecmp('STDOUT') == 0 || @log_file.casecmp('SYSLOG') == 0
-      @roll_log = false
-      @logger.close rescue nil
-      @logger = ::Proxy::LoggerFactory.logger
+      self.roll_log = false
     end
 
     def add(severity, message = nil, progname = nil, backtrace = nil)
@@ -40,8 +29,14 @@ def add(severity, message = nil, progname = nil, backtrace = nil)
       end
       message = formatter.call(severity, Time.now.utc, progname, message) if formatter
       return if message == ''
+      reopened = false
       @mutex.synchronize do
-        handle_log_rolling if @roll_log
+        if self.roll_log?
+          # decorator is in-memory only, reopen underlaying logging appenders
+          ::Logging.reopen
+          self.roll_log = false
+          reopened = true
+        end
         # add to the logger first
         @logger.add(severity, message)
         # add add to the buffer
@@ -53,6 +48,7 @@ def add(severity, message = nil, progname = nil, backtrace = nil)
           @buffer.push(rec)
         end
       end
+      info("Logging file reopened via USR1 signal") if reopened
       # exceptions are also sent to structured log if available
       self.exception("Error details", backtrace) if backtrace && backtrace.is_a?(Exception)
     end

lib/proxy/signal_handler.rb

@@ -35,7 +35,7 @@ def install_term_trap
 
   def install_usr1_trap
     trap(:USR1) do
-      ::Proxy::LogBuffer::Decorator.instance.roll_log
+      ::Proxy::LogBuffer::Decorator.instance.roll_log = true
     end
   end
 end

test/log_buffer/decorator_test.rb

@@ -71,31 +71,12 @@ def test_should_keep_request_id_in_buffer_when_available
     ::Logging.mdc['request'] = nil
   end
 
-  def test_should_not_roll_log_if_stdout_is_used
-    ::Proxy::LoggerFactory.stubs(:logger).returns(::Logger.new("/dev/null"))
-    (d = DecoratorForTesting.new(@logger, "STDOUT", nil)).handle_log_rolling
-    assert_equal @logger, d.logger
-  end
-
-  def test_should_not_roll_log_if_syslog_is_used
-    ::Proxy::LoggerFactory.stubs(:logger).returns(::Logger.new("/dev/null"))
-    (d = DecoratorForTesting.new(@logger, "SYSLOG", nil)).handle_log_rolling
-    assert_equal @logger, d.logger
-  end
-
-  def test_should_roll_log_if_file_logger_is_used
-    ::Proxy::LoggerFactory.stubs(:logger).returns(::Logger.new("/dev/null"))
-    (d = DecoratorForTesting.new(@logger, "/dev/null", nil)).handle_log_rolling
-    assert_not_equal @logger, d.logger
-  end
-
   def test_should_roll_log_if_flag_is_set
     ::Proxy::LoggerFactory.stubs(:logger).returns(::Logger.new("/dev/null"))
     d = DecoratorForTesting.new(@logger, "/dev/null", @buffer)
 
-    d.roll_log
+    d.roll_log = true
     d.add(DEBUG)
-
-    assert_not_equal @logger, d.logger
+    refute d.roll_log
   end
 end