Skip to content

fix: handler should inline group with empty key#33

Merged
samber merged 2 commits into
samber:mainfrom
AVM-Martin:fix/empty-group
Jan 30, 2026
Merged

fix: handler should inline group with empty key#33
samber merged 2 commits into
samber:mainfrom
AVM-Martin:fix/empty-group

Conversation

@AVM-Martin
Copy link
Copy Markdown
Contributor

It is true that we need to remove attributes with empty key, but we need to flatten nonempty groups with empty key

Refs: https://cs.opensource.google/go/go/+/refs/tags/go1.25.6:src/testing/slogtest/slogtest.go;l=117-129

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.33%. Comparing base (beda76f) to head (8127c02).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
+ Coverage   52.98%   54.33%   +1.34%     
==========================================
  Files           4        4              
  Lines         251      254       +3     
==========================================
+ Hits          133      138       +5     
+ Misses        115      113       -2     
  Partials        3        3              
Flag Coverage Δ
unittests 54.33% <100.00%> (+1.34%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@samber
Copy link
Copy Markdown
Owner

samber commented Jan 30, 2026

I agree

Let me update the 30+ pkg 😅

@samber samber merged commit d9bc3e1 into samber:main Jan 30, 2026
5 checks passed
@AVM-Martin
Copy link
Copy Markdown
Contributor Author

@samber thanks for maintaining all of these slog-* packages

There is 1 more issue with log attributes that implement slog.KindLogValuer, but I’m still looking into it.
I'll open another PR (either in this repository or in https://github.com/samber/slog-zerolog )

@AVM-Martin AVM-Martin deleted the fix/empty-group branch January 30, 2026 17:13
@samber
Copy link
Copy Markdown
Owner

samber commented Feb 1, 2026

Thanks @AVM-Martin for both fixes.

Did you find more bugs ? Can I upgrade every slog-xxx packages ?

@AVM-Martin
Copy link
Copy Markdown
Contributor Author

For now, I don't see another bugs.
This is the sample unit test that I use (with samber/slog-zerolog)
Thanks for your help.

package main_test

import (
	"bytes"
	"encoding/json"
	"log/slog"
	"testing"
	"testing/slogtest"

	"github.com/rs/zerolog"
	slogzerolog "github.com/samber/slog-zerolog/v2"
)

func TestNewHandler(t *testing.T) {
	var buf bytes.Buffer

	newHandler := func(t *testing.T) slog.Handler {
		logger := zerolog.New(&buf)

		return slogzerolog.Option{
			Logger: &logger,
			// Level:  slog.LevelInfo,
		}.NewZerologHandler()
	}

	result := func(t *testing.T) map[string]any {
		defer buf.Reset()

		var m map[string]any
		if err := json.Unmarshal(buf.Bytes(), &m); err != nil {
			t.Fatal(err)
		}

		if val, ok := m["message"]; ok {
			// zerolog use "message" as the key, key replacement is needed
			m[slog.MessageKey] = val
			delete(m, "message")
		}

		if val, ok := m["time"]; ok && val == "0001-01-01T00:00:00Z" {
			// zerolog does not ignore zero time, manual removal is needed
			delete(m, "time")
		}

		return m
	}

	slogtest.Run(t, newHandler, result)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants