-
Notifications
You must be signed in to change notification settings - Fork 70
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 | ||||||
|
Comment on lines
25
to
34
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. I agree with @jeremyevans that the default formatter should simplify by only allowing With those simplifications, I'd prefer to spend the default formatter's "complexity budget" elsewhere. 😉
def format_message_with_context(msg, ctx)
join_fields msg2str(msg), format_context(ctx)
end
def join_fields(*fields)
fields.reject {|f| f.nil? || f.empty? }.join(" ")
end
def format_context(context)
return unless context
context.filter_map {|k, v| format_pair(k,v) }.join(" ")
end
def format_pair(k, v)
"#{k}=#{format_value(v)}" unless v.nil?
end
def format_value(value)
# "redundant" interpolation to avoid crash if to_s doesn't return a string
# https://github.com/ruby/spec/blob/3affe1e54fcd11918a242ad5d4a7ba895ee30c4c/language/string_spec.rb#L130-L141
value = "#{value}"
value = value.dump if value.match?(/[[:space:][:cntrl]]/)
value
endThis enables subclasses like the following, that's kind of like ActiveSupport::TaggedLogger. class TaggedLogFormatter < Logger::Formatter
PRE_MSG_CTX = %i[user_id request_id controller action].freeze
# add brackets to each context pair, and treat key like a tag when value is true
def format_pair(k, v)
return format_tag(k) if v == true
(str = super) && "[#{str}]"
end
# print some context before the message and some after:
def format_message_context(msg, ctx)
return msg2str(msg) unless ctx
fields = Array(ctx[:tags]).map { format_tag(_1) }
fields << format_context(ctx.slice(*PRE_MSG_CTX))
fields << msg2str msg
fields << format_context ctx.except(:tags, *PRE_MSG_CTX)
join_fields(*fields)
end
def format_tag(tag) = "[#{format_value _1}]"
endOr, here's one that's kinda like logfmt for msg and context, but it uses the (more compact) default format for the other standard fields. class NotQuiteLogFmtFormatter < Logger::Formatter
# Use ISO8601
def format_datetime(time) = time.utc.iso8601(6)
# Formats as msg="my message"
# or error="error messege [MyError]\n/path/to/file.rb:123 in 'foo'\netc..."
def msg2str(msg)
name = msg.is_a?(Exception) ? "error" : "msg"
format_pair(name, super(msg))
end
# flip the order: prints msg="my message" before context
def format_message_context(msg, ctx)
join_fields msg2str(msg), format_context(ctx)
end
# Standardize formats for Float, Time, #to_ary
def format_value(value)
case value
in String then super
in Float then format("%.3f", value)
in Time then format_datetime(time)
in ->{ _1.respond_to?(:to_ary) then format_array(value)
else super
end
end
def format_array(array)
format_value "[#{Array(array).map {|v| format_value(v) }.join(", ")}]"
end
end(I haven't tested these at all... only typed them here in the github comment box, so... they probably don't work as-is.) |
||||||
|
|
||||||
| 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: