Skip to content

Commit

Permalink
Revert "Merge pull request #44695 from Edouard-chin/ec-tagger-logger-…
Browse files Browse the repository at this point in the history
…broadcast"

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 #45854.
  • Loading branch information
rafaelfranca committed Sep 8, 2022
1 parent e24fbf7 commit fad0c6b
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 77 deletions.
9 changes: 5 additions & 4 deletions activesupport/lib/active_support/logger.rb
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
15 changes: 0 additions & 15 deletions 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"
Expand Down Expand Up @@ -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 }
Expand Down
8 changes: 8 additions & 0 deletions activesupport/test/broadcast_logger_test.rb
Expand Up @@ -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
Expand Down
58 changes: 0 additions & 58 deletions activesupport/test/tagged_logging_test.rb
Expand Up @@ -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)
Expand Down

0 comments on commit fad0c6b

Please sign in to comment.