Log Module Suggestions

paulcsmith · April 16, 2020, 7:44pm

Thanks for all the suggestions! I’ve got a couple thoughts as well:

Log::Context#empty?

I dig it!

Log::Severity::Verbose

Would it make more sense if this was below Debug ?

I would love to have this. There are definitely some spots where I basically want “super verbose debug”. Usually not useful but if you really need the info it is there.

This would be especially useful in shards because you provide verbose log info in all kinds of places that would normally overwhelm the user, but is helpful when you get into really tricky stuff.

Log::Context#to_json

I’d love to have this for the same reason as @Blacksmoke16. I like the idea of log/json. Right now I have just extended Log::Context in Dexter to allow this

Allow struct or something as the formatter

Could be cool. My workaround has been to provide a Proc that wraps the struct: dexter/src/dexter/base_formatter.cr at af4feded4d31698e2c6d8e602516d54925540706 · luckyframework/dexter · GitHub

That way you can do MyFormatter.proc. I also did something similar for the old logger module so that I could do more complex formatting. Would be great to have this kind of thing as a Log::Formatter abstract struct that accepts an initializer with the Log::Entry and IO. Similar to what BaseFormatter does that I linked above. Then add that to the Log::Formatter alias so it or a Proc can be used

Here is an example of using the BaseFormatter

github.com

luckyframework/dexter/blob/master/src/dexter/json_log_formatter.cr

require "json"
require "log/json"

module Dexter
  struct JSONLogFormatter < BaseFormatter
    def call
      context_data = entry.context.to_h
      local_data = context_data.delete("local").try(&.as_h)
      data = default_data

      if local_data
        data = data.merge(local_data)
      end

      data = data.merge(context_data)

      exception_data.try do |exception_data_|
        data = data.merge(exception_data_)
      end

This file has been truncated. show original

Yes the struct may be less performant but for stuff like Lucky’s PrettyLogFormatter we have to use one or the logic just gets too crazy

The other suggestions

I haven’t had a need for it so can’t really contribute to it. I haven nothing against it though

Some additional suggestions

Allow `Nil` as a type in `Log::Context`

Right now I’m dealing with a lot of issues trying to handle Nil types. I think Nil should be a context type because it can be valuable to know something is nil

For example, if I have a context.set user_id: current_user.try(&.id) then I would expect it to work. Right now this is not possible and is messing up some logging in Avram. I think I’ll need to monkey patch Log::Context to allow it but would prefer not to

Add `formatter` to `IOBackend#new`

Right now you have to do this:

backend = IOBackend.new
backend.formatter = MyFormatter
# Then bind it
Log.bind "*", :info, backend

Would be much nicer to do this:

Log.bind "*", :info, IOBackend.new(formatter: MyFormatter)

Proposal for different storage of log data

This will be a more complex proposal so will likely start a separate thread

Overall impressions

Aside from a few speed bumps like the Nil issue and and trying to get it working with a struct formatter it has been quite good. Dexter code is simplified tremendously now that Log has some kind of key/value storage

Happy to submit PRs

Some of my suggestions are simple. If they’d be accepted I’d happily open a PR