TaggedLogging to return a new logger instance

3 files changed, 34 insertions, 10 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 0614cdaabd..d8d8e061cb 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Changed `ActiveSupport::TaggedLogging.new` to return a new logger instance instead of mutating the one received as parameter.
+
+    *Thierry Joyal*
+
 *   Add `ActiveSupport::Duration#before` and `#after` as aliases for `#until` and `#since`
 
     These read more like English and require less mental gymnastics to read and write.
diff --git a/activesupport/lib/active_support/tagged_logging.rb b/activesupport/lib/active_support/tagged_logging.rb
index ad134c49b6..387bd69afa 100644
--- a/activesupport/lib/active_support/tagged_logging.rb
+++ b/activesupport/lib/active_support/tagged_logging.rb
@@ -57,8 +57,15 @@ module ActiveSupport
     end
 
     def self.new(logger)
-      # Ensure we set a default formatter so we aren't extending nil!
-      logger.formatter ||= ActiveSupport::Logger::SimpleFormatter.new
+      logger = logger.dup
+
+      if logger.formatter
+        logger.formatter = logger.formatter.dup
+      else
+        # Ensure we set a default formatter so we aren't extending nil!
+        logger.formatter = ActiveSupport::Logger::SimpleFormatter.new
+      end
+
       logger.formatter.extend Formatter
       logger.extend(self)
     end
diff --git a/activesupport/test/tagged_logging_test.rb b/activesupport/test/tagged_logging_test.rb
index 2469e827d4..87940472b4 100644
--- a/activesupport/test/tagged_logging_test.rb
+++ b/activesupport/test/tagged_logging_test.rb
@@ -17,9 +17,10 @@ class TaggedLoggingTest < ActiveSupport::TestCase
   test "sets logger.formatter if missing and extends it with a tagging API" do
     logger = Logger.new(StringIO.new)
     assert_nil logger.formatter
-    ActiveSupport::TaggedLogging.new(logger)
-    assert_not_nil logger.formatter
-    assert logger.formatter.respond_to?(:tagged)
+
+    other_logger = ActiveSupport::TaggedLogging.new(logger)
+    assert_not_nil other_logger.formatter
+    assert other_logger.formatter.respond_to?(:tagged)
   end
 
   test "tagged once" do
@@ -80,16 +81,28 @@ class TaggedLoggingTest < ActiveSupport::TestCase
   end
 
   test "keeps each tag in their own instance" do
-    @other_output = StringIO.new
-    @other_logger = ActiveSupport::TaggedLogging.new(MyLogger.new(@other_output))
+    other_output = StringIO.new
+    other_logger = ActiveSupport::TaggedLogging.new(MyLogger.new(other_output))
     @logger.tagged("OMG") do
-      @other_logger.tagged("BCX") do
+      other_logger.tagged("BCX") do
         @logger.info "Cool story"
-        @other_logger.info "Funky time"
+        other_logger.info "Funky time"
       end
     end
     assert_equal "[OMG] Cool story\n", @output.string
-    assert_equal "[BCX] Funky time\n", @other_output.string
+    assert_equal "[BCX] Funky time\n", other_output.string
+  end
+
+  test "does not share the same formatter instance of the original logger" do
+    other_logger = ActiveSupport::TaggedLogging.new(@logger)
+
+    @logger.tagged("OMG") do
+      other_logger.tagged("BCX") do
+        @logger.info "Cool story"
+        other_logger.info "Funky time"
+      end
+    end
+    assert_equal "[OMG] Cool story\n[BCX] Funky time\n", @output.string
   end
 
   test "cleans up the taggings on flush" do