feature(logging): Ensure logged messages are formatted correctly #443
Conversation
Cawllec
commented
Mar 26, 2018
Cawllec
commented
- Ensures logged messages have the correct bugsnag formatting
- Ensures custom loggers receive the formatted message rather than the raw message
- Adds a new public method - 'format_message'
- Ensures logged messages have the correct bugsnag formatting - Ensures custom loggers receive the formatted message rather than the raw message - Adds a new public method - 'format_message'
martin308
changed the title
There was a problem hiding this comment.
This implementation works in some cases but not in others. I've broken down two common ones for comparison:
- ☑️ Bugsnag is configured to use the default logger. The formatting is the same as before, including
[Bugsnag]prefix and a timestamp.
** [Bugsnag] 2018-03-27 13:12:34 -0700: Warning No API key has been set
In this case, it clear where the message came from, what time it was sent, and what the content of the message was.
-
❌ Bugsnag is configured to use Rails' logger (the default in a Rails app). The timestamp is duplicated (or does not match exactly) because a formatted message is being reformatted, inserting a new timestamp along the way.
W, [2018-03-27T13:12:34.629375 #98093] WARN -- : ** [Bugsnag] 2018-03-27 13:12:34 -0700: Warning No API key has been set
The Logger class includes a built-in solution for these cases, allowing the program name which triggered the message to be specified. More details included inline.
lib/bugsnag/configuration.rb
Outdated
| @@ -88,9 +88,6 @@ def initialize | |||
| # Set up logging | |||
| self.logger = Logger.new(STDOUT) | |||
| self.logger.level = Logger::INFO | |||
| self.logger.formatter = proc do |severity, datetime, progname, msg| | |||
| "** [Bugsnag] #{datetime}: #{msg}\n" | |||
There was a problem hiding this comment.
Keep the formatter as before, only removing the [Bugsnag] component
lib/bugsnag/configuration.rb
Outdated
| @@ -169,19 +166,26 @@ def clear_request_data | |||
| ## | |||
| # Logs an info level message | |||
| def info(message) | |||
| logger.info(message) | |||
| logger.info(format_message(message)) | |||
There was a problem hiding this comment.
Use the block form of Logger logging methods to specify the program name which sent the message. This differentiates Bugsnag log messages from others sent using the same logger.
- logger.info(format_message(message))
+ logger.info('Bugsnag') { message }
lib/bugsnag/configuration.rb
Outdated
|
|
||
| ## | ||
| # Formats a message being logged by Bugsnag | ||
| def format_message(message) |
There was a problem hiding this comment.
While this is unneeded in a simpler implementation, it should be noted that this method should have been private as it has no role in the public interface.
- Removed 'format_message' method - Re-added formatter utilising progname for Bugsnag stamp - Added PROG_NAME const for logging program name
|
I've adjusted how this works based on your feedback - the formatters back in place and the |
kattrali
force-pushed
the
cawllec/improve-logger-clarity
branch
from
14235b7
to
19e5941
Compare
|
Looks good, I made one adjustment to make the |
