Abstract Logging
parsonsmatt opened this issue · 5 comments
Right now, yesod hardcodes logging to LoggerSet, which only operates on filepaths. This makes it difficult to integrate with any other means of logging.
Specifically, I want yesod to log via katip.
This would be a breaking change, since Yesod's makeLogger :: app -> IO LoggerSet is a wired in part of the application.
My ultimate goal here is really to move Yesod over to rio's logging system, and rio more generally.
That said, even without a breaking change, I think it's possible to extend Yesod to be more accepting of other logging functions. The MonadLogger instance (https://www.stackage.org/haddock/nightly-2021-08-05/yesod-core-1.6.21.0/src/Yesod.Core.Types.html#line-510) uses the rheLog field, so any underlying log system that can accept those four parameters should work.
I've written a library for hooking Yesod up to Katip scribes - but it's forced to hijack messageLoggerSource, which is not ideal. First class support would be best, especially to be able to easily use the same logger when creating the site as when using it.
Can you clarify what you mean by hijacking messageLoggerSource? I'd like to understand what the current workaround is, since I'm not hugely in favor of an associated type family approach if we can avoid it (referencing #1736).
I mean that logging to Katip scribes required manually calling out to (e.g.) logItem in messageLoggerSource, basically ignoring the Logger argument and the flow around makeLogger.
It was also problematic because I found that I had to remember to manually respect shouldLogIO, which is normally only called by the default messageLoggerSource implementation.
(The code for the instance is here.)
There's definitely some legacy cruft in the Yesod typeclass around logging. I'm trying to separate out the "relevant to other loggers" stuff from "would be nice to clean up" stuff. messageLoggerSource and shouldLogIO fall into the latter category.
If I'm reading the code in question, and the comments above, it seems this is less about "Katip isn't supported" and more "Katip isn't the default supported type and we'd like that configurable." Which I understand, but I'm hesitant to do because of the potential for more complex error messages and overall maintenance burden.
I am in favor of moving away from the current logging approach to something built around rio, and I think if we did that the right solution here would be having some kind of a HasLogFunc instance for Katip that makes everything tie together correctly. Yesod is currently too tied to a specific implementation. I'm just not sure I want to take the current type instance approach.
Does that make sense?