From db64467fc197c8758b0a66fe20a8eb34dd81ca09 Mon Sep 17 00:00:00 2001
From: Andrew Babichev <andrew.babichev@gmail.com>
Date: Mon, 11 Feb 2019 22:59:52 +0100
Subject: [PATCH] Pluggable Log Formatter (#4093)

* Rename log_format to log_formatter

Rails provides configuration options with this name.

* Pluggable log formatter instance
---
 6.0-Upgrade.md        | 16 +++++++++++-----
 lib/sidekiq.rb        | 13 +++++++++++--
 lib/sidekiq/cli.rb    |  4 +++-
 lib/sidekiq/logger.rb |  9 +--------
 test/test_logger.rb   | 22 +++++++++++++---------
 5 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/6.0-Upgrade.md b/6.0-Upgrade.md
index fcd85af3..fb4f4de1 100644
--- a/6.0-Upgrade.md
+++ b/6.0-Upgrade.md
@@ -1,24 +1,30 @@
 # Welcome to Sidekiq 6.0!
 
 Sidekiq 6.0 contains some breaking changes which streamline proper operation
-of Sidekiq.  It also drops support for EOL versions of Ruby and Rails.
+of Sidekiq. It also drops support for EOL versions of Ruby and Rails.
 
 ## What's New
 
 This release has major breaking changes.  Read and test carefully in production.
 
-- Logging has been redesigned to allow pluggable formats and several
+- Logging has been redesigned to allow pluggable formatters and several
   formats ship with Sidekiq:
   * default - your typical output on macOS
-  * json - a new format with key=value pairs for search indexing
   * heroku - a Heroku-specific formatter
+  * json - a new format with key=value pairs for search indexing
+  
 Sidekiq will enable the best formatter for the detected environment but
-you can override it by configuring the log format explicitly:
+you can override it by configuring the log formatter explicitly:
+
 ```
 Sidekiq.configure_server do |config|
-  config.log_format = :json # or nil for default
+  config.log_formatter = AcmeCorp::PlainLogFormatter.new
+  # Sidekiq::Logger::Formatters::Pretty.new (default)
+  # Sidekiq::Logger::Formatters::WhitoutTimestamp.new
+  # Sidekiq::Logger::Formatters::JSON.new
 end
 ```
+
 - **Remove the daemonization, logfile and pidfile command line arguments**.
   I've [noted for years](https://www.mikeperham.com/2014/09/22/dont-daemonize-your-daemons/)
  how modern services should be managed with a proper init system.
diff --git a/lib/sidekiq.rb b/lib/sidekiq.rb
index 6ef91cd8..3357de2e 100644
--- a/lib/sidekiq.rb
+++ b/lib/sidekiq.rb
@@ -174,12 +174,21 @@ module Sidekiq
   def self.load_json(string)
     JSON.parse(string)
   end
+
   def self.dump_json(object)
     JSON.generate(object)
   end
 
-  class << self
-    attr_accessor :log_format
+  def self.log_formatter
+    @log_formatter ||= if ENV['DYNO']
+      Sidekiq::Logger::Formatters::WithoutTimestamp.new
+    else
+      Sidekiq::Logger::Formatters::Pretty.new
+    end
+  end
+
+  def self.log_formatter=(log_formatter)
+    @log_formatter = log_formatter
   end
 
   def self.logger
diff --git a/lib/sidekiq/cli.rb b/lib/sidekiq/cli.rb
index a24a182c..5483b829 100644
--- a/lib/sidekiq/cli.rb
+++ b/lib/sidekiq/cli.rb
@@ -34,7 +34,9 @@ module Sidekiq
     # test coverage of Sidekiq::CLI are welcomed.
     def run
       boot_system
-      print_banner if environment == 'development' && $stdout.tty? && Sidekiq.log_format == nil
+      if environment == 'development' && $stdout.tty? && Sidekiq.log_formatter.is_a?(Sidekiq::Logger::Formatters::Pretty)
+        print_banner
+      end
 
       self_read, self_write = IO.pipe
       sigs = %w(INT TERM TTIN TSTP)
diff --git a/lib/sidekiq/logger.rb b/lib/sidekiq/logger.rb
index 3faa5dad..5180191b 100644
--- a/lib/sidekiq/logger.rb
+++ b/lib/sidekiq/logger.rb
@@ -9,14 +9,7 @@ module Sidekiq
     def initialize(*args)
       super
 
-      formatter_class = case Sidekiq.log_format
-      when :json
-        Formatters::JSON
-      else
-        ENV['DYNO'] ? Formatters::WithoutTimestamp : Formatters::Pretty
-      end
-
-      self.formatter = formatter_class.new
+      self.formatter = Sidekiq.log_formatter
     end
 
     def with_context(hash)
diff --git a/test/test_logger.rb b/test/test_logger.rb
index f5067d22..b6fca6d9 100644
--- a/test/test_logger.rb
+++ b/test/test_logger.rb
@@ -8,30 +8,34 @@ class TestLogger < Minitest::Test
     @output = StringIO.new
     @logger = Sidekiq::Logger.new(@output)
 
+    Sidekiq.log_formatter = nil
     Thread.current[:sidekiq_context] = nil
     Thread.current[:sidekiq_tid] = nil
   end
 
   def teardown
+    Sidekiq.log_formatter = nil
     Thread.current[:sidekiq_context] = nil
     Thread.current[:sidekiq_tid] = nil
   end
 
-  def test_format_selection
-    assert_kind_of Sidekiq::Logger::Formatters::Pretty, Sidekiq::Logger.new(STDOUT).formatter
+  def test_default_log_formatter
+    assert_kind_of Sidekiq::Logger::Formatters::Pretty, Sidekiq::Logger.new(@output).formatter
+  end
+
+  def test_heroku_log_formatter
     begin
       ENV['DYNO'] = 'dyno identifier'
-      assert_kind_of Sidekiq::Logger::Formatters::WithoutTimestamp, Sidekiq::Logger.new(STDOUT).formatter
+      assert_kind_of Sidekiq::Logger::Formatters::WithoutTimestamp, Sidekiq::Logger.new(@output).formatter
     ensure
       ENV['DYNO'] = nil
     end
+  end
 
-    begin
-      Sidekiq.log_format = :json
-      assert_kind_of Sidekiq::Logger::Formatters::JSON, Sidekiq::Logger.new(STDOUT).formatter
-    ensure
-      Sidekiq.log_format = nil
-    end
+  def test_json_log_formatter
+    Sidekiq.log_formatter = Sidekiq::Logger::Formatters::JSON.new
+
+    assert_kind_of Sidekiq::Logger::Formatters::JSON, Sidekiq::Logger.new(@output).formatter
   end
 
   def test_with_context