Revert "Merge pull request #44695 from Edouard-chin/ec-tagger-logger-…

…broadcast" This reverts commit 31925f5. This was causing tags to leak to the broadcast logger when `tagged` without a block is used. Fix #45854.
rails · Sep 8, 2022 · ff27758 · ff27758
1 parent 4d25c64
commit ff27758
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 77 deletions.
diff --git a/activesupport/lib/active_support/logger.rb b/activesupport/lib/active_support/logger.rb
@@ -22,10 +22,6 @@ def self.logger_outputs_to?(logger, *sources)
     # Broadcasts logs to multiple loggers.
     def self.broadcast(logger) # :nodoc:
       Module.new do
-        define_singleton_method(:extended) do |base|
-          base.public_send(:broadcast_to, logger) if base.respond_to?(:broadcast_to)
-        end
-
         define_method(:add) do |*args, &block|
           logger.add(*args, &block)
           super(*args, &block)
@@ -46,6 +42,11 @@ def self.broadcast(logger) # :nodoc:
           super(name)
         end
 
+        define_method(:formatter=) do |formatter|
+          logger.formatter = formatter
+          super(formatter)
+        end
+
         define_method(:level=) do |level|
           logger.level = level
           super(level)

diff --git a/activesupport/lib/active_support/tagged_logging.rb b/activesupport/lib/active_support/tagged_logging.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require "active_support/core_ext/module/delegation"
-require "active_support/core_ext/module/redefine_method"
 require "active_support/core_ext/object/blank"
 require "logger"
 require "active_support/logger"
@@ -95,20 +94,6 @@ def self.new(logger)
 
     delegate :push_tags, :pop_tags, :clear_tags!, to: :formatter
 
-    def broadcast_to(other_logger) # :nodoc:
-      define_singleton_method(:formatter=) do |formatter|
-        other_logger.formatter ||= formatter
-
-        other_logger.formatter.singleton_class.redefine_method(:current_tags) do
-          formatter.current_tags
-        end
-
-        super(formatter)
-      end
-
-      self.formatter = self.formatter.clone
-    end
-
     def tagged(*tags)
       if block_given?
         formatter.tagged(*tags) { yield self }

diff --git a/activesupport/test/broadcast_logger_test.rb b/activesupport/test/broadcast_logger_test.rb
@@ -55,6 +55,14 @@ class BroadcastLoggerTest < TestCase
       assert_equal ::Logger::FATAL, log2.progname
     end
 
+    test "#formatter= assigns to all the loggers" do
+      assert_nil logger.formatter
+      logger.formatter = ::Logger::FATAL
+
+      assert_equal ::Logger::FATAL, log1.formatter
+      assert_equal ::Logger::FATAL, log2.formatter
+    end
+
     test "#local_level= assigns the local_level to all loggers" do
       assert_equal ::Logger::DEBUG, logger.local_level
       logger.local_level = ::Logger::FATAL

diff --git a/activesupport/test/tagged_logging_test.rb b/activesupport/test/tagged_logging_test.rb
@@ -226,62 +226,4 @@ class TaggedLoggingWithoutBlockTest < ActiveSupport::TestCase
     assert_equal "[OMG] Broadcasting...\n", @output.string
     assert_equal "[OMG] Broadcasting...\n", broadcast_output.string
   end
-
-  test "keeps broadcasting functionality when passed a block" do
-    broadcast_output = StringIO.new
-    broadcast_logger = ActiveSupport::TaggedLogging.new(Logger.new(broadcast_output))
-    @logger.extend(ActiveSupport::Logger.broadcast(broadcast_logger))
-
-    @logger.tagged("OMG") { |logger| logger.info "Broadcasting..." }
-
-    assert_equal "[OMG] Broadcasting...\n", @output.string
-    assert_equal "[OMG] Broadcasting...\n", broadcast_output.string
-  end
-
-  test "broadcasting when passing a block works and  keeps formatter untouched" do
-    broadcast_output = StringIO.new
-    broadcast_logger = ActiveSupport::TaggedLogging.new(Logger.new(broadcast_output))
-    my_formatter = Class.new do
-      def call(_, _, _, msg)
-        ActiveSupport::JSON.encode(message: msg, tags: current_tags)
-      end
-    end
-    broadcast_logger.formatter = my_formatter.new
-
-    @logger.extend(ActiveSupport::Logger.broadcast(broadcast_logger))
-    @logger.tagged("OMG") { |logger| logger.info "Broadcasting..." }
-
-    assert_equal "[OMG] Broadcasting...\n", @output.string
-    assert_equal "{\"message\":\"Broadcasting...\",\"tags\":[\"OMG\"]}", broadcast_output.string
-  end
-
-  test "broadcasting without passing a block works and keeps formatter untouched" do
-    broadcast_output = StringIO.new
-    broadcast_logger = ActiveSupport::TaggedLogging.new(Logger.new(broadcast_output))
-    my_formatter = Class.new do
-      def call(_, _, _, msg)
-        ActiveSupport::JSON.encode(message: msg, tags: current_tags)
-      end
-    end
-    broadcast_logger.formatter = my_formatter.new
-
-    @logger.extend(ActiveSupport::Logger.broadcast(broadcast_logger))
-    tagger_logger1 = @logger.tagged("OMG")
-    tagger_logger2 = tagger_logger1.tagged("FOO")
-    tagger_logger2.info("Broadcasting...")
-
-    assert_equal "[OMG] [FOO] Broadcasting...\n", @output.string
-    assert_equal "{\"message\":\"Broadcasting...\",\"tags\":[\"OMG\",\"FOO\"]}", broadcast_output.string
-  end
-
-  test "broadcasting on a non tagged logger" do
-    broadcast_output = StringIO.new
-    broadcast_logger = ActiveSupport::Logger.new(broadcast_output)
-
-    @logger.extend(ActiveSupport::Logger.broadcast(broadcast_logger))
-    @logger.tagged("OMG") { |logger| logger.info "Broadcasting..." }
-
-    assert_equal "[OMG] Broadcasting...\n", @output.string
-    assert_equal "Broadcasting...\n", broadcast_output.string
-  end
 end