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

More compatibility with ActiveSupport Logger broadcast usage #967

Closed
wants to merge 2 commits into from
Closed

More compatibility with ActiveSupport Logger broadcast usage #967

wants to merge 2 commits into from

Conversation

joergschiller
Copy link
Contributor

@joergschiller joergschiller commented May 31, 2023

#935 allows the usage of ActiveSupport::Logger.broadcast. There still seems to be an issue when using the convenience methods from logger. Logger.warn("Something happened") won't show up in AppSignal.

(There already is a detailed description what happens when adding a broadcast in #930.)

A new Rails App would create an ActiveSupport Logger in production if logging to the console is requested (which a Twelve-Factor App would do).

config/environments/production.rb:

if ENV["RAILS_LOG_TO_STDOUT"].present?
  logger = ActiveSupport::Logger.new($stdout)
  logger.formatter = config.log_formatter
  config.logger = ActiveSupport::TaggedLogging.new(logger)
end

I want to keep the console output and broadcast to an AppSignal logger:

appsignal_logger = ActiveSupport::TaggedLogging.new(Appsignal::Logger.new("rails"))
config.logger.extend(ActiveSupport::Logger.broadcast(appsignal_logger))

At first glance this seems to work. All requests are logged in AppSignal. But using the convenience methods would log to the console but not to AppSignal.

# Won't show up on appsignal.com
> Rails.logger.warn("Something happened")
Something happened
=> 19

# Will show up on appsignal.com
Rails.logger.add(Logger::WARN, "Something happened")
Something happened
=> 19

This is because the convenience methods are calling add with the log message in the third argument. But Appsignal::Logger#add won't take the message from group as a fallback if message is nil. It looks like you forgot to rename progname to group as well:

message = progname

After the fixes in this PR Rails.logger.warn("Something happened") works for me as expected.

This is how `Logger` calls `add` from the convenience
methods (e. g. `warn`).
The original `Logger#add` also won't return with a nil message:

```rb
> log = Logger.new(STDOUT)
> log.add(Logger::WARN, nil)
W, [2023-06-01T00:25:54.848371 #54035]  WARN -- : nil
=> true
```
@luismiramirez
Copy link
Member

Hey there @joergschiller!

Thank you so much for this contribution and for the extended information you provided with it. You're correct and this helps us a lot.

I've created a PR based on this and added you as a co author in #969

I'm closing this one in favor of #969

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants