Add logging stack with slog adapter #1574
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||||||||||
| package log | ||||||||||||||||
|
|
||||||||||||||||
| // Level represents the log level, from debug to fatal | ||||||||||||||||
| type Level struct { | ||||||||||||||||
| level string | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| var ( | ||||||||||||||||
| // DebugLevel causes all logs to be logged | ||||||||||||||||
| DebugLevel = Level{"debug"} | ||||||||||||||||
| // InfoLevel causes all logs of level info or more severe to be logged | ||||||||||||||||
| InfoLevel = Level{"info"} | ||||||||||||||||
| // WarnLevel causes all logs of level warn or more severe to be logged | ||||||||||||||||
| WarnLevel = Level{"warn"} | ||||||||||||||||
| // ErrorLevel causes all logs of level error or more severe to be logged | ||||||||||||||||
| ErrorLevel = Level{"error"} | ||||||||||||||||
| // FatalLevel causes only logs of level fatal to be logged | ||||||||||||||||
| FatalLevel = Level{"fatal"} | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| // String returns the string representation for Level | ||||||||||||||||
| // | ||||||||||||||||
| // This is useful when trying to get the string values for Level and mapping level in other external libraries. For example: | ||||||||||||||||
| // ``` | ||||||||||||||||
| // trace.SetLogLevel(kvp.String("loglevel", log.DebugLevel.String()) | ||||||||||||||||
|
There was a problem hiding this comment. The example code in the comment contains a syntax error. It shows
Suggested change
|
||||||||||||||||
| // ``` | ||||||||||||||||
|
Comment on lines
+24
to
+26
There was a problem hiding this comment. The code example in the documentation is incomplete and contains a syntax error. The closing parenthesis is missing from the String() method call, and "kvp" is not defined in the example context. Consider revising to a complete, self-contained example.
Suggested change
|
||||||||||||||||
| func (l Level) String() string { | ||||||||||||||||
| return l.level | ||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package log | ||
|
|
||
| import ( | ||
| "context" | ||
| "log/slog" | ||
| ) | ||
|
|
||
| type Logger interface { | ||
| Log(ctx context.Context, level Level, msg string, fields ...slog.Attr) | ||
| Debug(msg string, fields ...slog.Attr) | ||
| Info(msg string, fields ...slog.Attr) | ||
| Warn(msg string, fields ...slog.Attr) | ||
| Error(msg string, fields ...slog.Attr) | ||
| Fatal(msg string, fields ...slog.Attr) | ||
| WithFields(fields ...slog.Attr) Logger | ||
| WithError(err error) Logger | ||
| Named(name string) Logger | ||
| WithLevel(level Level) Logger | ||
| Sync() error | ||
| Level() Level | ||
| } | ||
|
Comment on lines
+8
to
+21
There was a problem hiding this comment. The Logger interface and Exporters struct lack documentation. As part of a public API that will be used as a library by other repositories, these types should have clear documentation explaining their purpose and usage. Add godoc comments to describe what the Logger interface represents and what the Exporters struct is used for. |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,62 @@ | ||||||||||||||||||||||||||||
| package log | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||||
| "log/slog" | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| type NoopLogger struct{} | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| var _ Logger = (*NoopLogger)(nil) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
Comment on lines
+7
to
+11
There was a problem hiding this comment. The NoopLogger struct and NewNoopLogger function lack documentation. As part of a public API, add godoc comments explaining that NoopLogger is a logger implementation that discards all log messages, useful for testing or when logging is disabled.
Suggested change
|
||||||||||||||||||||||||||||
| func NewNoopLogger() *NoopLogger { | ||||||||||||||||||||||||||||
| return &NoopLogger{} | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) Level() Level { | ||||||||||||||||||||||||||||
| return DebugLevel | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) Log(_ context.Context, _ Level, _ string, _ ...slog.Attr) { | ||||||||||||||||||||||||||||
| // No-op | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) Debug(_ string, _ ...slog.Attr) { | ||||||||||||||||||||||||||||
| // No-op | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) Info(_ string, _ ...slog.Attr) { | ||||||||||||||||||||||||||||
| // No-op | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) Warn(_ string, _ ...slog.Attr) { | ||||||||||||||||||||||||||||
| // No-op | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) Error(_ string, _ ...slog.Attr) { | ||||||||||||||||||||||||||||
| // No-op | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) Fatal(_ string, _ ...slog.Attr) { | ||||||||||||||||||||||||||||
| // No-op | ||||||||||||||||||||||||||||
|
Comment on lines
+40
to
+41
There was a problem hiding this comment. The NoopLogger's Fatal method should maintain consistent behavior with the SlogLogger implementation. Since SlogLogger panics after logging a fatal message, NoopLogger should also panic to ensure consistent Fatal semantics across implementations. Otherwise, code that relies on Fatal terminating execution could continue running unexpectedly when using NoopLogger.
Suggested change
|
||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) WithFields(_ ...slog.Attr) Logger { | ||||||||||||||||||||||||||||
| return l | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) WithError(_ error) Logger { | ||||||||||||||||||||||||||||
| return l | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) Named(_ string) Logger { | ||||||||||||||||||||||||||||
| return l | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) WithLevel(_ Level) Logger { | ||||||||||||||||||||||||||||
| return l | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (l *NoopLogger) Sync() error { | ||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+62
There was a problem hiding this comment. The NoopLogger implementation lacks test coverage. While it's a simple no-op implementation, tests should verify that it correctly implements the Logger interface and that all methods are safe to call without side effects. Similar packages in this repository have corresponding test files. |
||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment for FatalLevel states "causes only logs of level fatal to be logged" which is inconsistent with other level comments. This suggests filtering that would suppress all other logs, but FatalLevel should be described as "causes all logs of level fatal to be logged" or "causes only fatal severity logs to be logged" to maintain consistency with the pattern of the other levels.