-
-
Notifications
You must be signed in to change notification settings - Fork 6
Fix ToSlogHandler duplicating time and level in output #30
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
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # Go-PKGZ/LGR Development Guidelines | ||
|
|
||
| ## Build & Test Commands | ||
| - Build: `go build -race` | ||
| - Test all: `go test -timeout=60s -race -covermode=atomic -coverprofile=profile.cov` | ||
| - Test single file: `go test -run TestName` | ||
| - Benchmark: `go test -bench=. -run=Bench` | ||
| - Lint: `golangci-lint run` | ||
|
|
||
| ## Code Style Guidelines | ||
| - Go 1.21 compatibility required | ||
| - Maximum line length: 140 characters | ||
| - No package names with underscores | ||
| - Use early returns (enforced by prealloc linter) | ||
| - Test files use testify for assertions: `require` for fatal assertions, `assert` for non-fatal ones | ||
| - Indent with tabs, not spaces | ||
|
|
||
| ## Error Handling | ||
| - FATAL logs to stderr and calls os.Exit(1) | ||
| - ERROR logs to both stdout and stderr | ||
| - PANIC logs stack trace and runtime info to stderr | ||
| - Stack traces for ERROR level can be enabled with StackTraceOnError option | ||
|
|
||
| ## Project Conventions | ||
| - Public API follows interface-based design (`lgr.L` interface) | ||
| - Avoid global loggers, prefer dependency injection | ||
| - Functional options pattern for logger configuration | ||
| - Secret logging sanitization with `lgr.Secret` option |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,7 @@ import ( | |||||||||||
| "io" | ||||||||||||
| "log/slog" | ||||||||||||
| "os" | ||||||||||||
| "regexp" | ||||||||||||
| "strings" | ||||||||||||
| "testing" | ||||||||||||
| "time" | ||||||||||||
|
|
@@ -38,11 +39,51 @@ func TestSlogHandlerBasic(t *testing.T) { | |||||||||||
| // verify output | ||||||||||||
| outStr := buff.String() | ||||||||||||
| assert.Contains(t, outStr, "DEBUG debug message") | ||||||||||||
| assert.Contains(t, outStr, "INFO info message") | ||||||||||||
| assert.Contains(t, outStr, "WARN warn message") | ||||||||||||
| assert.Contains(t, outStr, "INFO info message") | ||||||||||||
| assert.Contains(t, outStr, "WARN warn message") | ||||||||||||
| assert.Contains(t, outStr, "ERROR error message") | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| func TestSlogHandlerNoDuplication(t *testing.T) { | ||||||||||||
| buff := bytes.NewBuffer([]byte{}) | ||||||||||||
| logger := lgr.New(lgr.Out(buff), lgr.Err(buff), lgr.Debug, lgr.Msec) | ||||||||||||
|
|
||||||||||||
| handler := lgr.ToSlogHandler(logger) | ||||||||||||
| slogger := slog.New(handler) | ||||||||||||
|
|
||||||||||||
| t.Run("info level not duplicated", func(t *testing.T) { | ||||||||||||
| buff.Reset() | ||||||||||||
| slogger.Info("test message", "key1", "value1") | ||||||||||||
| line := buff.String() | ||||||||||||
| t.Logf("output: %s", line) | ||||||||||||
| // time and level must appear exactly once | ||||||||||||
| assert.Equal(t, 1, strings.Count(line, "INFO"), "INFO level should appear once, got: %s", line) | ||||||||||||
| assert.Contains(t, line, "test message") | ||||||||||||
| assert.Contains(t, line, `key1="value1"`) | ||||||||||||
| }) | ||||||||||||
|
|
||||||||||||
| t.Run("debug level not duplicated", func(t *testing.T) { | ||||||||||||
| buff.Reset() | ||||||||||||
| slogger.Debug("debug msg") | ||||||||||||
| line := buff.String() | ||||||||||||
| t.Logf("output: %s", line) | ||||||||||||
| assert.Equal(t, 1, strings.Count(line, "DEBUG"), "DEBUG level should appear once, got: %s", line) | ||||||||||||
| // must not contain INFO for a debug message | ||||||||||||
| assert.NotContains(t, line, "INFO") | ||||||||||||
|
Comment on lines
+71
to
+72
|
||||||||||||
| // must not contain INFO for a debug message | |
| assert.NotContains(t, line, "INFO") | |
| // ensure the structured prefix has DEBUG as the level (and not INFO) | |
| levelRe := regexp.MustCompile(`^\d{4}/\d{2}/\d{2}.*\bDEBUG\b`) | |
| assert.True(t, levelRe.MatchString(line), "expected DEBUG level in structured prefix, got: %s", line) |
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.
Handlepasses the fully formattedlogMsgas the format string toh.lgr.Logf(logMsg). Forlgr.Limplementations that delegate tofmt.Printf/log.Printf(e.g.lgr.Std), any%in the slog message or attribute values will be interpreted as formatting verbs, corrupting output (and can emit%!artifacts). Prefer callingLogfwith a constant format (e.g.,h.lgr.Logf("%s", logMsg)) so messages are treated as data, not a format string.