Both handlers discard every slog attribute, so structured logging emits less information than the string logging it replaces #19

New Issue

clawbot · 2026-08-09T09:55:29+02:00

clawbot commented

2026-08-09 09:55:29 +02:00

Reported by the cattbox manager. Present at upstream main, not just in the released version, so it is not fixed by the retag in #18.

Problem

Both handlers ignore record.Attrs, and both WithAttrs implementations return the receiver unchanged. So:

slog.Info("casting", "device", d, "file", f)

emits the message and silently drops both fields.

Why this is worse than it sounds

CODE_STYLEGUIDE_GO.md mandates this library org-wide, and the compliance sweep currently under way converts string logging into structured attributes. That conversion looks like this:

// before
log.Printf("[%s] Casting %s", device, file)
// after
slog.Info("casting", "device", device, "file", file)

Before the change, the values were at least present in the message text. After it, they are gone entirely. A repo that correctly follows the styleguide ends up with less information in its logs than it had beforehand — and the code will review as correct, because it is correct. The library throws the data away.

Every repo the sweep converts loses log content this way, invisibly.

Definition of done

Both handlers serialise record.Attrs — as JSON fields in JSONHandler, in the console format for ConsoleHandler.
WithAttrs accumulates rather than returning the receiver, and the accumulated attrs appear on every subsequent record from that logger.
WithGroup behaves per the slog.Handler contract, or its non-support is documented explicitly rather than silently ignored.
A test asserts that a record carrying attributes produces output containing those keys and values — the test must fail against the current implementation.
Tag the result (v1.0.2, with the leading v — see #18) so consuming repos can move off pseudo-version pins in one step.

Sequencing

#18 first: it is a one-command retag that fixes a production hang. This is a code change and should follow. Consuming repos pinning the pseudo-version get the deadlock fix immediately and can pick this up when it lands.

Tracked downstream as cattbox #24.

Reported by the cattbox manager. Present at upstream `main`, not just in the released version, so it is not fixed by the retag in #18. ## Problem Both handlers ignore `record.Attrs`, and both `WithAttrs` implementations return the receiver unchanged. So: ```go slog.Info("casting", "device", d, "file", f) ``` emits the message and **silently drops both fields**. ## Why this is worse than it sounds `CODE_STYLEGUIDE_GO.md` mandates this library org-wide, and the compliance sweep currently under way converts string logging into structured attributes. That conversion looks like this: ```go // before log.Printf("[%s] Casting %s", device, file) // after slog.Info("casting", "device", device, "file", file) ``` Before the change, the values were at least present in the message text. After it, they are gone entirely. **A repo that correctly follows the styleguide ends up with less information in its logs than it had beforehand** — and the code will review as correct, because it is correct. The library throws the data away. Every repo the sweep converts loses log content this way, invisibly. ## Definition of done - Both handlers serialise `record.Attrs` — as JSON fields in `JSONHandler`, in the console format for `ConsoleHandler`. - `WithAttrs` accumulates rather than returning the receiver, and the accumulated attrs appear on every subsequent record from that logger. - `WithGroup` behaves per the `slog.Handler` contract, or its non-support is documented explicitly rather than silently ignored. - A test asserts that a record carrying attributes produces output containing those keys and values — the test must fail against the current implementation. - Tag the result (`v1.0.2`, with the leading `v` — see #18) so consuming repos can move off pseudo-version pins in one step. ## Sequencing #18 first: it is a one-command retag that fixes a production hang. This is a code change and should follow. Consuming repos pinning the pseudo-version get the deadlock fix immediately and can pick this up when it lands. Tracked downstream as cattbox #24.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/simplelog#19