-
Notifications
You must be signed in to change notification settings - Fork 69
Context api #132
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
base: master
Are you sure you want to change the base?
Context api #132
Changes from all commits
ab4dc5a
60146b7
3910bf9
a011371
feeba96
71dba30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |||||
| class Logger | ||||||
| # Default formatter for log messages. | ||||||
| class Formatter | ||||||
| Format = "%.1s, [%s #%d] %5s -- %s: %s\n" | ||||||
| Format = "%.1s, %s[%s #%d] %5s -- %s: %s\n" | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, we should combine message and context and leave the format string unchanged:
Suggested change
Just for a quick visual scanning of logs: the more static fields don't vary much in length, so they should stay before context, which is going to vary in length much more. Also, although non-local context is often relatively static and worth putting before the message, which context fields should be prioritized over the message depends on the application. IMO, the best generic solution is to print all the context after the message. For example, that's what the go stdlib structured logging package does: with the default formatter, Ideally, custom formatter subclasses should be able to trivially print context before, after, or around the message. |
||||||
| DatetimeFormat = "%Y-%m-%dT%H:%M:%S.%6N" | ||||||
|
|
||||||
| attr_accessor :datetime_format | ||||||
|
|
@@ -12,16 +12,41 @@ def initialize | |||||
| @datetime_format = nil | ||||||
| end | ||||||
|
|
||||||
| def call(severity, time, progname, msg) | ||||||
| sprintf(Format, severity, format_datetime(time), Process.pid, severity, progname, msg2str(msg)) | ||||||
| def call(severity, time, progname, msg, context: nil) | ||||||
| context_str = format_context(context) | ||||||
|
|
||||||
| context_str << " " unless context_str.empty? | ||||||
|
|
||||||
| sprintf(Format, severity, context_str, format_datetime(time), Process.pid, severity, progname, msg2str(msg)) | ||||||
| end | ||||||
|
|
||||||
| private | ||||||
|
|
||||||
| def format_context(context) | ||||||
| case context | ||||||
| when Hash | ||||||
| filter_map_join(context) { |k, v| "[#{k}=#{v}]" unless v.nil? } | ||||||
| when Array | ||||||
| filter_map_join(context) { |v| "[#{v}]" unless v.nil? } | ||||||
| else | ||||||
| context.to_s.dup | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
| def format_datetime(time) | ||||||
| time.strftime(@datetime_format || DatetimeFormat) | ||||||
| end | ||||||
|
|
||||||
| if RUBY_VERSION >= "2.7.0" | ||||||
| def filter_map_join(context, &blk) | ||||||
| context.filter_map(&blk).join(" ") | ||||||
| end | ||||||
| else | ||||||
| def filter_map_join(context, &blk) | ||||||
| context = context.map(&blk).compact.join(" ") | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
| def msg2str(msg) | ||||||
| case msg | ||||||
| when ::String | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we assume that context is always a hash, this can be simplified to something like: