marcelropos / HM-DiscordBot

A Discordbot by and for first semester students of HM.
8 stars 5 forks source link

✨ Add logger pipe command #196

Closed schitcrafter closed 7 months ago

schitcrafter commented 8 months ago

Implements #159

maxwai commented 7 months ago

After trying the code, the log messages are not send in Discord. I found that it doesn't work when used inside block_on (don't know why), but wrapping it in a spawn_blocking makes it work. @schitcrafter look at the changes and merge if the changes are fine for you

schitcrafter commented 7 months ago

Gonna take a guess that tracing didn't like blocking in it's layer (it's calling from an async context, so tokio, which also doesn't like blocking), that's probably why spawning is necessary. Probably should've been that from the beginning to be honest.

For the LOG_LEVEL thing, I was under the impression that fmt already comes with an EnvFilter (it only does when using its init() function) - but I still think that we should use EnvFilter. You can set the log level like you do manually here, but you also have much finer control about what to log (like switching to trace only for some module/crate/function/whatever). That's very nice for debugging, in my opinion.

schitcrafter commented 7 months ago

@maxwai Changes look good but I'd at least like to know if we want the EnvFilter or not, then we can merge this (plus #199 and after some testing #201 )

maxwai commented 7 months ago

The Env Filter is not necessary here. we can still switch to trace using the env for the console and file outputs

schitcrafter commented 7 months ago

Yeah okay but...that's not why I'd use EnvFilter. I'd use it because if, for example, I wanted to debug something with subject commands, and switched to trace globally for that, I'd have tons of unnecessary output. With EnvFilter I could cut down on that by only specifying trace for that specific module. But if you don't wanna merge it