-
-
Notifications
You must be signed in to change notification settings - Fork 541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add logger configuration for backends #1294
Comments
Necessary to provide colorized logging within the console for improved readablity while also logging to a file for historical context (if needed). I've logged an [issue](hanami/hanami#1294) for consideration that could make this less tedious.
Yeah I'd be in favor of doing this even though it'd make things a bit more complicated on the config side. We could even capture all method calls that are supported by the instance and eventually forward them once the instance is available. |
Yeah, this would be great because if the Hanami configuration for the logger is a simple proxy it means you could potentially do something like this where you chain the desired behavior like you can do with the native Dry Logger (well, sort of, I'm dreaming a bit here). Example: config.logger
.colorize
.formatter(:rack)
.add_backend(stream: Hanami.app.root.join("log/development.log") This would make configuring the logger rather elegant but probably the most powerful aspect of this is being able to add multiple backends easily with minimal effort. |
Right. My only concern is that config object methods may potentially clash with logger methods. I wonder now if maybe using a provider for logger to tune the instance would be better? Something like: class MyApp < Hanami::App
# This doesn't exist, but we could potentially have shortcuts like that
prepare(:logger) do
logger.add_backend(stream: ...)
end
end The reasoning here is that extending config with even more options just to translate them into method calls is actually more complex than exposing a convenient way to access components (logger in this case) in order to tweak them using their own API. |
FWIW, right now you can actually provide a complete replacement for the logger by creating your own provider at The logger is one of our more complex bits of config, in that we want to be able to allow users to tweak specific bits of logger config without having to replace it wholesale (so they can leverage the framework's defaults where possible), while at the same time allowing that logger to be wholesale replaced if the user so desires. The much more powerful and dry-logger integration also came into Hanami right before our 2.0 release, so we haven't really had much time to step back and think this through again. Your input here is helpful in this regard, @bkuhlmann. One thing I do know is that I'd rather not add new or special general purpose APIs (like you've suggested here with this App-level What I do think would be helpful here is to continue to take a "wait and see" approach. Will many others come to us with suggestions like @bkuhlmann's here? We don't know yet! And I'd like to make sure we're not putting extra work or complexity into areas that don't really need it. Because, let's be honest, this snippet of code from the original post isn't too bad at all, really: environment :development do
config.logger.options[:colorize] = true
config.logger = config.logger.instance.add_backend(
colorize: false,
stream: Hanami.app.root.join("log/development.log")
)
end It's still pretty understandable, and the fact that this kind of advanced arrangement is possible is already a win :) Granted, it is kind of awkward that the logger has to be reassigned like that, so if anything, perhaps we can just smooth over this part in particular, something like: config.logger.prepare do |logger|
logger.add_backend(
colorize: false,
stream: Hanami.app.root.join("log/development.log")
)
end This block would be called with the logger at the end of |
Peter/Tim:
Tim:
On final thought, Tim, because I think this is interesting (Peter hints at this as well in terms of simply exposing the logger instance for further tweaking):
I definitely don't want to make any of this more complicated but feels like a potential solution falls within the following criteria (maybe?):
|
Thanks Tim for chiming in ❤️
You can always use |
Yeah, I like the idea of an environment helper inside providers! It's not uncommon that they may need to do different things per env, and we should make this feel nice :) |
Implementation-wise, that will require more hooks into dry-system, however. Right now dry-system entirely "owns" the provider lifecycle-related code. We'd need to update dry-system to accept a custom provider source class in its config (or something like that, I haven't thought about it too deeply). This would allow Hanami to configure it with a custom subclass that provides this environment helper. In fact, there's one more related thing I've wanted to do in providers: right now, Honestly, however, I'd prioritize all of these much lower than finishing our view and persistence layer. These would be really nice bits of polish for when we do get to them though. Sometime this year! |
Peter: Thanks for the Tim: Yep, more persistence and view layer support first would be most welcome. 🙇 By the way, I've been struggling with the use of |
Thanks for bringing this up, but let's not have this convo here as it's not strictly related to the logger issue. Could you start a thread in the discussion forum? I'm also not a fan of |
This is more a dry-system naming issue than Hanami, FWIW. If we can have Hanami provide its own dry-system provider source implementation (per my thoughts above), then we can just make it named "slice" (and additionally app when we know the provider is being registered with the app) as an alias of target. (BTW, target itself is an alias of target_container, just as container in the same context is an alias of provider_container. Those names might be more self-explanatory, albeit longer) There's are good reasons these two different containers exist for each provider, and I just need to fine the time to write it down 😩. For example, we need them to support providers registered with a namespace. And they open up a range of interesting use cases where external provider sources are used and then the user of that provider source can more exactly control what eventually gets registered in the target container. FWIW, the multiple containers were not actually my invention! So this is definitely not me here trying to defend my "baby." In fact, during my big dry-system refactors of 2021/2022 I tried to remove the provider container, and only in doing that did I realise its usefulness. |
Peter/Tim: OK, I extracted this discussion to this discussion proposal to keep this issue clean. 😉 |
Overview
Hello. 👋 I ran into a situation where I needed to colorize logging in my development environment while also logging to a file and was wondering if it'd be possible to add
config.logger.add_backend
support instead ofconfig.logger.instance.add_backend
?I realize this is a minor quality of life improvement in terms of intuiting matching functionality as found in Dry Logger but would reduce a few characters of typing and feels a bit nicer. Also, having to mutate the instance feels wrong too.
Steps to Recreate
Here's how to make this work:
Environment
The text was updated successfully, but these errors were encountered: