Skip to content

Commit fe87449

Browse files
committed
BroadcastLogger should support TaggedLogging correctly.
[Fixes #49745 #52876] [Related #51883 #49771] This commit extends BroadcastLogger when TaggedLogging is loaded in order to ensure that BroadcastLogger#tagged behaves as expected when there are multiple loggers being broadcast to. TaggedLogging provides two related, but distinct interfaces: 1. Calling `logger.tagged(*tags)` without a block returns a new Logger with the provided tags pushed to its tag stack. 2. Calling `logger.tagged(*tags) { |logger| ... }` with a block pushes the tags to the existing Logger and yields the logger to block. Previously, BroadcastLogger would only function as expected if there was just one Logger being broadcast to. When there were multiple loggers: when calling `tagged` with a block, it would yield a block multiple times, once for each logger; when called without a block, it would call `tagged` on each logger and return an Array of the results. This inconsistent behaviour has caused issues such as those referenced above. In development environments in particular, logger configuration can often result in a BroadcastLogger with two loggers, since the `bin/rails server` command will always broadcast to STDOUT, resulting in confusing behaviour. This commit provides a `BroadcastLogger#tagged` implementation that, for the non-block form returns a new BroadcastLogger broadcasting to the new tagged Loggers, and for the block-form, it 'wraps' the user-provided block within nested calls to `TaggedLogging#logged`. The user-provided block is only executed in the innermost call. For example: ```ruby # BroadcastLogger with two Loggers being broadcasted to broadcaster.tagged("FOO") { |logger| ... } ``` Would execute similarly to: ```ruby broadcasts[0].tagged("FOO") { broadcasts[1].tagged("FOO") { yield(broadcaster) } } ```
1 parent ba468db commit fe87449

File tree

2 files changed

+91
-0
lines changed

2 files changed

+91
-0
lines changed

activesupport/lib/active_support/tagged_logging.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,30 @@ module ActiveSupport
2727
# it easy to stamp log lines with subdomains, request ids, and anything else
2828
# to aid debugging of multi-user production applications.
2929
module TaggedLogging
30+
# This module is included in ActiveSupport::BroadcastLogger to enable
31+
# broadcasting to tagged loggers with equivalent semantics.
32+
module Broadcasting
33+
def tagged(*tags, &block)
34+
return super unless broadcasts.any? { |logger| logger.respond_to?(:tagged) }
35+
36+
if block_given?
37+
# `tagged(...) { |logger| ... }` yields itself to the block
38+
broadcasts.inject(block) do |block, logger|
39+
if logger.respond_to?(:tagged)
40+
proc { logger.tagged(*tags) { block.call(self) } }
41+
else
42+
block
43+
end
44+
end.call
45+
else
46+
# `tagged(...) returns a new logger with the tags pushed
47+
self.class.new(*broadcasts.map { |logger|
48+
logger.respond_to?(:tagged) ? logger.tagged(*tags) : logger
49+
})
50+
end
51+
end
52+
end
53+
3054
module Formatter # :nodoc:
3155
# This method is invoked when a log event occurs.
3256
def call(severity, timestamp, progname, msg)
@@ -155,3 +179,5 @@ def flush
155179
end
156180
end
157181
end
182+
183+
ActiveSupport::BroadcastLogger.include ActiveSupport::TaggedLogging::Broadcasting

activesupport/test/tagged_logging_test.rb

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,3 +259,68 @@ def crozz_method
259259
assert_equal "[tag] [1, 2, 3]\n", @output.string
260260
end
261261
end
262+
263+
class TaggedLoggingBroadcastingTest < ActiveSupport::TestCase
264+
setup do
265+
@output1 = StringIO.new
266+
@output2 = StringIO.new
267+
@logger1 = ActiveSupport::TaggedLogging.new(Logger.new(@output1))
268+
@logger2 = ActiveSupport::TaggedLogging.new(Logger.new(@output2))
269+
@logger = ActiveSupport::BroadcastLogger.new(@logger1, @logger2)
270+
end
271+
272+
test "tags with a block" do
273+
@logger.tagged("BCX") { @logger.info "Funky time" }
274+
assert_equal "[BCX] Funky time\n", @output1.string
275+
assert_equal "[BCX] Funky time\n", @output1.string
276+
end
277+
278+
test "yields the logger to the block" do
279+
@logger.tagged("BCX") do |logger|
280+
assert_equal @logger, logger
281+
end
282+
end
283+
284+
test "returns the return value of the block" do
285+
return_value = @logger.tagged("BCX") do
286+
"Cool story"
287+
end
288+
assert_equal "Cool story", return_value
289+
end
290+
291+
test "it returns a tagged logger without a block" do
292+
logger = @logger.tagged("BCX")
293+
logger.info "Funky time"
294+
assert_equal "[BCX] Funky time\n", @output1.string
295+
assert_equal "[BCX] Funky time\n", @output2.string
296+
end
297+
298+
test "it doesn't tag the original logger without a block" do
299+
@logger.tagged("BCX")
300+
@logger.info "Funky time"
301+
assert_equal "Funky time\n", @output1.string
302+
assert_equal "Funky time\n", @output2.string
303+
end
304+
305+
test "it ignores non-tagged loggers with a block" do
306+
plain_output = StringIO.new
307+
plain_logger = ActiveSupport::Logger.new(plain_output)
308+
@logger.broadcast_to(plain_logger)
309+
310+
@logger.tagged("BCX") { @logger.info "Block funky time" }
311+
@logger.tagged("BCX").info "Chain cool story"
312+
assert_equal(<<~STR, plain_output.string)
313+
Block funky time
314+
Chain cool story
315+
STR
316+
end
317+
318+
test "it ignores non-tagged loggers without a block" do
319+
plain_output = StringIO.new
320+
plain_logger = ActiveSupport::Logger.new(plain_output)
321+
@logger.broadcast_to(plain_logger)
322+
323+
@logger.tagged("BCX").info "Cool story"
324+
assert_equal "Cool story\n", plain_output.string
325+
end
326+
end

0 commit comments

Comments
 (0)