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

rafaelfranca · rafaelfranca · commit fad0c6b899ba · 2022-09-08T16:03:26.000Z
This reverts commit 5e770a3, reversing changes made to 989a04d. This was causing tags to leak to the broadcast logger when `tagged` without a block is used. Fix rails#45854.
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
@@ -227,64 +227,6 @@ class TaggedLoggingWithoutBlockTest < ActiveSupport::TestCase
     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
-
   test "keeps formatter singleton class methods" do
     plain_output = StringIO.new
     plain_logger = Logger.new(plain_output)
