🔊 Improve Logging: move to slog and add trace level #276
Conversation
| _, err := fmt.Fprint(rw, bouncer.banTemplateString) | ||
| if err != nil { | ||
| bouncer.log.Error("handleBanServeHTTP could not write template to ResponseWriter") | ||
| bouncer.log.Warn("handleBanServeHTTP could not write template to ResponseWriter: " + err.Error()) |
There was a problem hiding this comment.
Here you are using the loglevel with "bouncer.log.LOGLEVEL",
Above for Trace, you are using bouncer.log.Log(context, logger.LEVEL)
Is it possible to have it written the same way ?
What's the difference except adding the "context"
There was a problem hiding this comment.
Yes... I did not like that either. The thing is that slog only has convenience methods for the out of the box log levels. I solved that by adding a wrapper so log calls are now consistent.
There was a problem hiding this comment.
Nice, with the wrapper, code looks more readable
|
First thank you for the contribution, I think adding JSON and common log format option is a great addition to the plugin. You have added log level=Trace, but every log level DEBUG has been replaced by TRACE. I think adding TRACE could be interesting for some really verbose actions, but not all. |
IMHO trace would be anything that happens on a per request basis and is expected. It can be discussed.
It is, but mostly for background tasks (stream, ticker) and not as part of the request execution pipeline. |
|
On my side this looks good, only thing missing would be to edit issue template and ask for trace logs by default to throubleshoot |
|
I remove the trace log level and use all the other to do the same result as you want. |
3 participants
Fixes #275