Switching to slog #11
Conversation
| } | ||
| func NewLogger(filename string) *Logger { | ||
| if filename == "" { | ||
| handler := slog.NewJSONHandler(os.Stderr, &slog.HandlerOptions{ |
There was a problem hiding this comment.
imho the default log format is better for human readability on stderr, while JSON makes sense for file output.
There was a problem hiding this comment.
I think it's a lot more convenient to use file based logger when working with the LS, but I made the switch in #12 to make that happen.
| } | ||
|
|
||
| // Debug logs a debug message | ||
| func (l *Logger) Debug(msg string, args ...interface{}) { |
There was a problem hiding this comment.
I'm a bit rusty but these methods look like noops since you are embedding *slog.Logger. Do you need them?
There was a problem hiding this comment.
No. Weekend brain. That was part of an incomplete process of setting up an interface, then explicitly implementing the interface for the logger so that it can be easily replaced in the future, etc.
I adjusted that in #12
| // Check if the current line is the one we're looking for | ||
| if scannedLines == int(position.Line) { | ||
| l.Debugf("Hit line %d: %s", position.Line, line) | ||
| l.Debug( |
There was a problem hiding this comment.
I see that you chose to use formatting instead of structured logging while using Zap and had to switch over for slog. The structured logging does add quite a bit of verbosity. Curious if you considered writing Debugf, Infof, etc functions on the log wrapper to do formatting instead of key/value pairs? Then I think most of the other changes would be unnecessary.
There was a problem hiding this comment.
Ya. The reason why structured logging was preferred here is that we're going to be logging JSON events between the server and client, so I figured it would be most convenient to log with the JSON logger.
2 participants
Closes #9