Skip to content

Commit

Permalink
Implement logger broadcasting
Browse files Browse the repository at this point in the history
Implement broadcasting logs to other loggers, following the same
interface as `#broadcast_to`. Unlike `#broadcast_to`, however,
instead of forwarding each method call one-for-one, we rely on the
`#add` method which is expected of the logger interface.

As such, we should not run into the kinds of bugs that affect
`ActiveSupport::BroadcastLogger` when used alongside additional
logger features such as tagged logging, such as rails/rails#49745.

This enables customers to use both broadcast logging and tagged
logging with the AppSignal logger without running into
application-breaking bugs.

As a trade-off, unlike `ActiveSupport::BroadcastLogger`, our
broadcast implementation is sensitive to the order in which those
additional features are applied, expecting them to be applied on
top of the broadcast logger, and not on the underlying loggers:

```ruby
 # Incorrect example
some_logger = ActiveSupport::TaggedLogging.new(
  Logger.new("/etc/app.log")
)

appsignal_logger = Appsignal::Logger.new("app")

 # Even if `some_logger` is a tagging-enabled logger,
 # this logger will not behave as a tagging-enabled logger.
Rails.logger = appsignal_logger.broadcast_to(some_logger)
```

```ruby
 # Correct example
some_logger = Logger.new("/etc/app.log")
appsignal_logger = Appsignal::Logger.new("app")

appsignal_logger.broadcast_to(some_logger)
 # As the tagged logging is applied on the broadcast logger,
 # both logs sent to AppSignal and to `some_logger` will include
 # given tags, and `Rails.logger` will be a tagging-enabled logger.
Rails.logger = ActiveSupport::TaggedLogging.new(appsignal_logger)
```
  • Loading branch information
unflxw committed Dec 19, 2024
1 parent c061aa4 commit 4a7adfb
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 0 deletions.
14 changes: 14 additions & 0 deletions .changesets/implement-logger-broadcasting.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
bump: patch
type: add
---

Add logger broadcasting. This change implements an alternative within `Appsignal::Logger` to `ActiveSupport::BroadcastLogger`, following the same interface. This enables a proper workaround to the issues with `ActiveSupport::BroadcastLogger` (([#49745](https://github.com/rails/rails/issues/49745), [#51883](https://github.com/rails/rails/issues/51883))) when used alongside tagged logging.

For example, to use tagged logging both in logs emitted by the default `Rails.logger` and in logs sent to AppSignal, replace the `Rails.logger` with an AppSignal logger that broadcasts to the default `Rails.logger`:

```ruby
appsignal_logger = Appsignal::Logger.new("app")
appsignal_logger.broadcast_to(Rails.logger)
Rails.logger = ActiveSupport::TaggedLogging.new(appsignal_logger)
```
11 changes: 11 additions & 0 deletions lib/appsignal/logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def initialize(group, level: INFO, format: PLAINTEXT, attributes: {})
@mutex = Mutex.new
@default_attributes = attributes
@appsignal_attributes = {}
@loggers = []
end

# We support the various methods in the Ruby
Expand All @@ -60,6 +61,12 @@ def add(severity, message = nil, group = nil)

message = formatter.call(severity, Time.now, group, message) if formatter

@loggers.each do |logger|
logger.add(severity, message, group)
rescue
nil
end

unless message.is_a?(String)
Appsignal.internal_logger.warn(
"Logger message was ignored, because it was not a String: #{message.inspect}"
Expand Down Expand Up @@ -158,6 +165,10 @@ def silence(_severity = ERROR, &block)
block.call
end

def broadcast_to(logger)
@loggers << logger
end

private

attr_reader :default_attributes, :appsignal_attributes
Expand Down
47 changes: 47 additions & 0 deletions spec/lib/appsignal/logger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,53 @@
end
end

describe "#broadcast_to" do
it "broadcasts the message to the given logger" do
other_device = StringIO.new
other_logger = ::Logger.new(other_device)

logger.broadcast_to(other_logger)

expect(Appsignal::Extension).to receive(:log)
.with("group", 3, 0, "Log message", instance_of(Appsignal::Extension::Data))

logger.info("Log message")

expect(other_device.string).to include("INFO -- group: Log message")
end

if DependencyHelper.rails_present?
describe "wrapped in ActiveSupport::TaggedLogging" do
let(:other_stream) { StringIO.new }
let(:other_logger) { ::Logger.new(other_stream) }

let(:logger) do
appsignal_logger = Appsignal::Logger.new("group")
appsignal_logger.broadcast_to(other_logger)
ActiveSupport::TaggedLogging.new(appsignal_logger)
end

it "broadcasts a tagged message to the given logger" do
expect(Appsignal::Extension).to receive(:log)
.with(
"group",
3,
0,
a_string_starting_with("[My tag] [My other tag] Some message"),
Appsignal::Utils::Data.generate({})
)

logger.tagged("My tag", "My other tag") do
logger.info("Some message")
end

expect(other_stream.string)
.to include("INFO -- group: [My tag] [My other tag] Some message")
end
end
end
end

[
["debug", 2, ::Logger::INFO],
["info", 3, ::Logger::WARN],
Expand Down

0 comments on commit 4a7adfb

Please sign in to comment.