luckyframework / dexter Goto Github PK

It would be nice if the LOG_LEVEL environment variable could set the level

If you have a model instance, and you want to log that, right now you might use pp. If you try and pass this to Lucky.logger.info, you'll find that nothing happens. No compile error, no output.

# This does nothing
Lucky.logger.info(UserQuery.first)

# This throws an error about undefined method to_json
Lucky.logger.info({record: UserQuery.first})

I'm thinking maybe the first way throws a compile-time error, and then we add a special method like inspect or whatever to models.

# error: If you're trying to log a model, be sure to call #inspect
Lucky.logger.info(UserQuery.first)

Lucky.logger.info(UserQuery.first.inspect)

Side related: luckyframework/lucky#950

I'm getting the following build error on Crystal 0.35:

In lib/dexter/src/dexter/log/context.cr:1:7

 1 | class Log::Context

           ^

Error: Context is not a class, it's a struct

The command "crystal spec" exited with 1.

https://travis-ci.org/github/tilishop/lucky_svg_sprite.cr/jobs/690646879

Should I ignore it?

My understanding of history is that this library was made in a time where Crystal's Log class was not very robust and this library added functionality that was missing. (From what I've heard) The Log class is now better so we need to figure out what Dexter should look like going into the future and if there is even a place for it.

Should Dexter just be a library of formatters? Lucky has some formatters so should they be moved over or is it even worth it?

Let's say in production you set the log level to info. Then a user reports a problem and you want to change the log level to debug just for you while you poke around.

I am not sure what the API looks like but I'd almost want to do something like

if params.get?(:enable_super_special_log_just_for_me)
  Log.current_fiber_level = :debug # Sets all logs to `:debug` *just* for the current Fiber
end

In a handler, before pipe, whatever. This would be super handy and I'd like to build something in for this at some point. So you can use Breeze in production but just with a special token set in the query param or a header

Will remove backend and set level to none

Right now the bind options in Log are not type-safe without some work. They are extremely flexible, but for Lucky's needs (and probably others?) we'd prefer type-safe config with some sensible defaults to keep things focused

# Configure all logs. Like bind *, etc.
Log.dexter.configure(:info, backend) 

# Can leave off the backend. Will configure Avram::Log and all children
Avram::Log.dexter.configure(:warn) 

# Can further customize child Logs
Avram::QueryLog.dexter.configure(:none) 
Avram::FailedQueryLog.dexter.configure(:info, SomeOtherBackend.new)

This I believe will cover the 90% use case with a simper API

The method formatter is causing other libraries to not compile:
https://github.com/luckyframework/dexter/blob/master/src/dexter/logger.cr#L53

# :nodoc:
  def formatter=(value) : Void
    {% raise "Use log_formatter= instead" %}
  end

So even when using ::Logger like:

    def initialize(@io : IO = STDOUT)
      @logger = ::Logger.new(@io)
      @logger.level = ::Logger::DEBUG
      @logger.progname = "crest"
      @logger.formatter = default_formatter
      @filters = [] of Array(String | Regex)
    end

    abstract def request(request : Crest::Request) : String
    abstract def response(response : Crest::Response) : String

    def default_formatter
      ::Logger::Formatter.new do |_, datetime, progname, message, io|
        io << progname
        io << " | " << datetime.to_s("%F %T")
        io << " " << message
      end
    end

It get's the following error:

  in lib/crest/src/crest/logger.cr:19: undefined method 'log_formatter=' for Logger (compile-time type is Logger+)

      @logger.log_formatter = Dexter::Formatters::JsonLogFormatter

For testing. Right now it is a pain. Would be easy with Habitat...hehe

Maybe we can make Habitat work for Crystal and get it in stdlib. We'll see though :)

Since Dexter just wraps the crystal logger, I'm just gonna drop this here to watch.

crystal-lang/crystal#8810 (comment)

Looks like some stuff in the base logger is gonna change. When that all goes down, we can look at revamping this as needed.

When you log a bunch of asynchronous code, it gets logged out of order which leads to some confusing logs. To go along with luckyframework/lucky#950 (comment), one thing we could do is queue up everything that needs to be logged, then log the queue all at the same time.

This could also be along the same lines as #38 where using dexter becomes a queue center for logging and special formatting. Then you can use the standard Crystal Log for lower level control.