Make `Hanami::Logger` compatible with `Rack::CommonLogger`

[wuarmin]

Hey,

I want to improve the requests-logging of my hanami-api application and therefore I want to use Hanami::Logger, but Hanami::Logger needs the Hanami::CommonLogger-Rack-middleware, which is not around in pure hanami-api-applications.

Following works, but Rack::CommonLogger just calls the << method of Hanami::Logger, which results in std ruby logging behavior.

use Rack::CommonLogger, Hanami::Logger.new(formatter: Hanami::Logger::JSONFormatter.new)

I saw you are working on dry-logger. I guess it will be a replacement of Hanami::Logger, won't it?

Do you have a hint for me?

Thanks and best regards

[jodosha]

@wuarmin What does this means?

which results in std ruby logging

1. Does it logs to stdout?
2. Does it logs as regular formatting, ignoring the JSON formatter?

Could you please print here what the following setup does for you?

use Rack::CommonLogger, Hanami::Logger.new(formatter: Hanami::Logger::JSONFormatter.new)

1. Is it printing to stdout?
2. Is it formatting with JSON?

[wuarmin]

Hey @jodosha,

sorry, that was misleading.

Is it printing to stdout?
Is it formatting with JSON?

It logs to stdout, because Hanami::Logger uses the implementation of its parent (Logger), so of course the logging also ignores the JSON formatting.

Rack::CommonLogger calls the <<-method of Hanami::Logger, which is implemented by Logger https://github.com/rack/rack/blob/d0c6efc666ede26768f33935f00530629690369a/lib/rack/common_logger.rb#L66

In combination with Hanami::CommonLogger Hanami::Logger works, because of following code: https://github.com/hanami/hanami/blob/19db85ecb9dcaba7203f49cfdede6bf45e367875/lib/hanami/common_logger.rb#L75

We could alias Hanami::Logger's info method with write, to make it Rack::CommonLogger-compatible and change the implementation of Hanami::CommonLogger to somethink like this:

if logger.respond_to?(:info)
  logger.info(msg)
elsif logger.respond_to?(:write)
  logger.write(msg)
else
  logger << msg
end

Then Hanami::Logger will work with all common logger interfaces. WDYT?

[wuarmin]

Hi @jodosha, do you have an update? I could provide a PR. Thanks and best regards

[jodosha]

@wuarmin But given Hanami::CommonLogger is defined in hanami, does that mean that if we fix that class, in order to use that logger in your Hanami::API app, you'll need to bundle hanami?

[wuarmin]

@jodosha We should fix the Hanami::Logger, to make it compatible to Rack::CommonLogger. The Hanami::CommonLogger class remains as it is. We do not need bundle hanami.

[jodosha]

@wuarmin I see, thanks. So you would open a PR in hanami/utils then? Should I move this ticket to that repo?

[wuarmin]

@jodosha Great. Yes, I would open a PR. Yes, that would be appropriate. It belongs there. Thanks.

[jodosha]

@wuarmin Hey, did you had the chance to have a look at it? 😃

[wuarmin]

@jodosha hello, No, I was on parental leave because of my son, but will look at it at the end of the week. Best regards ;)

[jodosha]

@wuarmin Congrats! 👶

[wuarmin]

@jodosha Thanks!! ;).

Following code would make Hanami::Logger Rack::CommonLogger-compatible,

alias :write :info

but if we do that, we break following code of Hanami::CommonLogger, because info will never be called. https://github.com/hanami/hanami/blob/19db85ecb9dcaba7203f49cfdede6bf45e367875/lib/hanami/common_logger.rb#L75

If we do that, we also have to fix Hanami::CommonLogger with somethink like this:

if logger.respond_to?(:info)
  logger.info(msg)
elsif logger.respond_to?(:write)
  logger.write(msg)
else
  logger << msg
end

So we would have to make 2 changes. One in hanami/utils and the other in hanami (CommonLogger) itself. Do you have a better idea?

best regards

[jodosha]

Closing as obsolete. We now moved to dry-logger.