-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat(log): add slog-backed Logger type #22347
Changes from all commits
f7caf7b
cf20016
a078c39
c3b59df
8f1f86e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
# Log | ||
|
||
The `cosmossdk.io/log` provides a zerolog logging implementation for the Cosmos SDK and Cosmos SDK modules. | ||
|
||
To use a logger wrapping an instance of the standard library's `log/slog` package, use `cosmossdk.io/log/slog`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
// Package slog contains a Logger type that satisfies [cosmossdk.io/log.Logger], | ||
// backed by a standard library [*log/slog.Logger]. | ||
package slog | ||
|
||
import ( | ||
"log/slog" | ||
|
||
"cosmossdk.io/log" | ||
) | ||
|
||
var _ log.Logger = Logger{} | ||
|
||
// Logger satisfies [log.Logger] with logging backed by | ||
// an instance of [*slog.Logger]. | ||
type Logger struct { | ||
log *slog.Logger | ||
} | ||
|
||
// NewCustomLogger returns a Logger backed by an existing slog.Logger instance. | ||
// All logging methods are called directly on the *slog.Logger; | ||
// therefore it is the caller's responsibility to configure message filtering, | ||
// level filtering, output format, and so on. | ||
func NewCustomLogger(log *slog.Logger) Logger { | ||
return Logger{log: log} | ||
} | ||
|
||
func (l Logger) Info(msg string, keyVals ...any) { | ||
l.log.Info(msg, keyVals...) | ||
} | ||
|
||
func (l Logger) Warn(msg string, keyVals ...any) { | ||
l.log.Warn(msg, keyVals...) | ||
} | ||
|
||
func (l Logger) Error(msg string, keyVals ...any) { | ||
l.log.Error(msg, keyVals...) | ||
} | ||
|
||
func (l Logger) Debug(msg string, keyVals ...any) { | ||
l.log.Debug(msg, keyVals...) | ||
} | ||
|
||
func (l Logger) With(keyVals ...any) log.Logger { | ||
return Logger{log: l.log.With(keyVals...)} | ||
} | ||
Comment on lines
+43
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add validation in With method. The With method should also validate key-value pairs. func (l Logger) With(keyVals ...any) log.Logger {
+ validateKeyVals(keyVals...)
return Logger{log: l.log.With(keyVals...)}
}
|
||
|
||
// Impl returns l's underlying [*slog.Logger]. | ||
func (l Logger) Impl() any { | ||
return l.log | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
package slog_test | ||
|
||
import ( | ||
"bytes" | ||
"encoding/json" | ||
stdslog "log/slog" | ||
"testing" | ||
|
||
"cosmossdk.io/log/slog" | ||
) | ||
|
||
func TestSlog(t *testing.T) { | ||
var buf bytes.Buffer | ||
h := stdslog.NewJSONHandler(&buf, &stdslog.HandlerOptions{ | ||
Level: stdslog.LevelDebug, | ||
}) | ||
logger := slog.NewCustomLogger(stdslog.New(h)) | ||
Comment on lines
+12
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Verify interface compliance and enhance test setup. Consider adding interface compliance verification and extracting the logger setup into a helper function: func TestLoggerImplementsInterface(t *testing.T) {
var _ log.Logger = (*slog.Logger)(nil)
}
func setupTestLogger() (*bytes.Buffer, *slog.Logger) {
buf := &bytes.Buffer{}
h := stdslog.NewJSONHandler(buf, &stdslog.HandlerOptions{
Level: stdslog.LevelDebug,
})
return buf, slog.NewCustomLogger(stdslog.New(h))
} |
||
|
||
type logLine struct { | ||
Level string `json:"level"` | ||
Msg string `json:"msg"` | ||
Num int `json:"num"` | ||
} | ||
|
||
var line logLine | ||
|
||
logger.Debug("Message one", "num", 1) | ||
if err := json.Unmarshal(buf.Bytes(), &line); err != nil { | ||
t.Fatal(err) | ||
} | ||
if want := (logLine{ | ||
Level: stdslog.LevelDebug.String(), | ||
Msg: "Message one", | ||
Num: 1, | ||
}); want != line { | ||
t.Fatalf("unexpected log record: want %v, got %v", want, line) | ||
} | ||
|
||
buf.Reset() | ||
logger.Info("Message two", "num", 2) | ||
if err := json.Unmarshal(buf.Bytes(), &line); err != nil { | ||
t.Fatal(err) | ||
} | ||
if want := (logLine{ | ||
Level: stdslog.LevelInfo.String(), | ||
Msg: "Message two", | ||
Num: 2, | ||
}); want != line { | ||
t.Fatalf("unexpected log record: want %v, got %v", want, line) | ||
} | ||
|
||
buf.Reset() | ||
logger.Warn("Message three", "num", 3) | ||
if err := json.Unmarshal(buf.Bytes(), &line); err != nil { | ||
t.Fatal(err) | ||
} | ||
if want := (logLine{ | ||
Level: stdslog.LevelWarn.String(), | ||
Msg: "Message three", | ||
Num: 3, | ||
}); want != line { | ||
t.Fatalf("unexpected log record: want %v, got %v", want, line) | ||
} | ||
|
||
buf.Reset() | ||
logger.Error("Message four", "num", 4) | ||
if err := json.Unmarshal(buf.Bytes(), &line); err != nil { | ||
t.Fatal(err) | ||
} | ||
if want := (logLine{ | ||
Level: stdslog.LevelError.String(), | ||
Msg: "Message four", | ||
Num: 4, | ||
}); want != line { | ||
t.Fatalf("unexpected log record: want %v, got %v", want, line) | ||
} | ||
|
||
wLogger := logger.With("num", 5) | ||
buf.Reset() | ||
wLogger.Info("Using .With") | ||
|
||
if err := json.Unmarshal(buf.Bytes(), &line); err != nil { | ||
t.Fatal(err) | ||
} | ||
if want := (logLine{ | ||
Level: stdslog.LevelInfo.String(), | ||
Msg: "Using .With", | ||
Num: 5, | ||
}); want != line { | ||
t.Fatalf("unexpected log record: want %v, got %v", want, line) | ||
} | ||
} | ||
Comment on lines
+25
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add test cases for error conditions and edge cases. While basic logging functionality is covered, consider adding tests for:
Example test case for invalid arguments: func TestLoggerInvalidInput(t *testing.T) {
buf := &bytes.Buffer{}
logger := setupTestLogger(buf)
// Should not panic with odd number of kvs
logger.Info("test", "key") // missing value
// Should handle nil/empty values
logger.Info("", nil, "key", nil)
} |
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.
🛠️ Refactor suggestion
Consider validating key-value pairs in logging methods.
The logging methods accept variadic key-value pairs but don't validate that they come in pairs.
Add a helper function for validation:
Then use it in each method, for example:
func (l Logger) Info(msg string, keyVals ...any) { + validateKeyVals(keyVals...) l.log.Info(msg, keyVals...) }