Skip to content

promslog: don't rename reserved keys inside groups - #1021

Open
vignesh2027 wants to merge 1 commit into
prometheus:mainfrom
vignesh2027:fix/promslog-grouped-reserved-keys
Open

vignesh2027 wants to merge 1 commit into
prometheus:mainfrom
vignesh2027:fix/promslog-grouped-reserved-keys

Conversation

@vignesh2027

@vignesh2027 vignesh2027 commented Oct 22, 2026 •

Copy link
Copy Markdown

Both ReplaceAttr functions in promslog rename an attribute whose key is a built-in key (time, level, source, plus ts and caller in the go-kit style) to logged_<key>, so it can't collide with the top-level attribute of the same name. They ignore the groups argument, so attributes nested in a group get renamed too, even though they can't collide with anything:

logger.Info("msg", slog.Group("req", "level", "high"))
// before: ... msg=msg req.logged_level=high
// after:  ... msg=msg req.level=high

The same happens with logger.WithGroup("sub").

slog passes the built-in attributes with an empty groups slice, so the fix skips the reserved key handling when groups is not empty. Top-level renaming and the time.Duration formatting are unchanged.

Tests: added TestReservedKeysInGroup, which covers both log styles with slog.Group and WithGroup, and checks that the built-in keys are still written. It fails on main with req.logged_level=grouped in the output and passes with the change. TestReservedKeys still passes.

The ReplaceAttr functions rename attributes whose key matches a built-in
key (time, level, source, and ts and caller for the go-kit style) to
logged_<key>, so they don't collide with the top-level built-in
attributes. They ignored the groups argument, so the same rename was
applied to attributes nested in a group, for example
slog.Group("req", "level", "high") was written as req.logged_level=high.

A grouped attribute can't collide with a top-level key, so leave it
untouched. Built-in attributes are always passed with no groups.

Signed-off-by: vigneshwar <applemacbook6sep2004@gmail.com>
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.

1 participant