aserafin / grape_logging

Request logging for Grape!
MIT License
147 stars 76 forks source link

Problems using formatters with rails 5 logger #65

Open kbaum opened 5 years ago

kbaum commented 5 years ago

I ran into #48 and from what I can tell, formatters would not work when passing in the Rails.logger. Suppose we have this:

use GrapeLogging::Middleware::RequestLogger, { logger: Rails.logger, formatter: GrapeLogging::Formatters::Json.new }

It looks like log reporter would attempt to set the Rails.logger formatter globally

  class LoggerReporter
    def initialize(logger, formatter, log_level)
      @logger = logger || Logger.new(STDOUT)
      @log_level = log_level || :info
      if @logger.respond_to?(:formatter=)
        @logger.formatter = formatter || @logger.formatter || GrapeLogging::Formatters::Default.new
      end
    end

This won't work because Rails assumes the formatter is of type ActiveSupport::TaggedLogging::Formatter and we get the error mentioned in #48.

undefined method `clear_tags!

Either way I am not sure we would actually want to set the GrapeLogging Formatter globally on the Rails.logger as this seems a bit heavy handed.

To get around this issue, I used the instrumentation_key feature and avoid using Rails.logger and GrapeLogging formatters altogether.

use GrapeLogging::Middleware::RequestLogger, {
  instrumentation_key: 'grape_logging',
  include: [GrapeLogging::Loggers::FilterParameters.new]
}

ActiveSupport::Notifications.subscribe('grape_logging') do |_name, _starts, _ends, _notification_id, payload|
  Rails.logger.info("API request #{payload.to_json}")
end

This is fine for my purposes but I am not sure of the right way to use grape log formatters and Rails.logger together.