appsignal-ruby icon indicating copy to clipboard operation
appsignal-ruby copied to clipboard

Logger improvements

Open unflxw opened this issue 5 months ago • 2 comments
trafficstars

Have #<< and #info use the same implementation

The #<< method is meant to use the INFO log level. Since we do some extra transformations on add_with_attributes when the #info method is called, we can use the same code path and make it easier to understand that it's just an alias.

Log default attributes when #add is used

When the #add method is used instead of the level-specific helper methods, log the default attributes as well.

Call #to_s on the message

Before refusing to log a message because it is not a string, attempt to call #to_s on the message.

Fixes https://github.com/appsignal/appsignal-ruby/issues/1418.

Broadcast log messages regardless of level

When broadcasting log messages, do so regardless of the log level of the AppSignal broadcaster. This allows loggers with less strict levels to log messages according to their own level thresholds.

To avoid executing the block passed as a message several times, which would run into the same issues as ActiveSupport::BroadcastLogger, wrap the given message block with an object that can be cast into a proc and be called more than once, but that only call the underlying block once, with the same return value each time. This also ensures that we don't call the underlying block if the message is not logged by any of the broadcasted loggers.

When the logger is silenced, however, do not broadcast any logs over the silenced log threshold. This makes sense because silencing is about the application code that performs the logging, not about its destination.

Move the Exception formatting to the #add method so that all error levels, not just #error, can format exceptions.

Fixes https://github.com/appsignal/appsignal-ruby/issues/1422.

Improve readability of logger method tests

Name the members of the permutations.

unflxw avatar Jun 18 '25 08:06 unflxw

Hi @unflxw,

We've found some issues with your Pull Request.

  • This Pull Request does not include a changeset. Add a changeset if the change impacts users and should be included in the changelog upon release. Read more about changesets.
    Ignore this rule by adding [skip changeset] to your Pull Request body. - (More info)

New issue guide | Backlog management | Rules | Feedback

backlog-helper[bot] avatar Jun 18 '25 08:06 backlog-helper[bot]

In addition to the comments above, address https://github.com/appsignal/appsignal-ruby/issues/1418#issuecomment-2984420543 before merging.

unflxw avatar Jun 18 '25 19:06 unflxw

This should now do the trick -- @andreaswachowski feel free to take a look! 🙇

unflxw avatar Jun 27 '25 15:06 unflxw

  • This Pull Request needs more reviews. @tombruijn - (More info)

This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

backlog-helper[bot] avatar Jun 30 '25 07:06 backlog-helper[bot]

Sorry for the belated reply @unflxw , didn't get to it earlier - I can confirm that this PR fixes #1418 👍

andreaswachowski avatar Jul 01 '25 06:07 andreaswachowski

Thank you @andreaswachowski!

unflxw avatar Jul 01 '25 08:07 unflxw