Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BroadcastLogger should support TaggedLogging correctly #53105

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

armstrjare
Copy link
Contributor

[Fixes #49745 #52876]
[Related #51883 #49771 #53093]

NB: I have a related pull request in #53093 that refactors BroadcastLogger to ensure that
block forms of delegated methods also only execute the block once.

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:

# BroadcastLogger with two Loggers being broadcasted to
broadcaster.tagged("FOO") { |logger|
  ...
}

Would execute similarly to:

broadcasts[0].tagged("FOO") {
  broadcasts[1].tagged("FOO") {
    yield(broadcaster)
  }
}

[Fixes rails#49745 rails#52876]
[Related rails#51883 rails#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)
    }
  }
  ```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rails.logger.tagged will execute block as many times as the number of tagged loggers in the broadcast
1 participant