Skip to content

v2: Fix log level inheritance#24

Merged
Roasbeef merged 3 commits intobtcsuite:masterfrom
ellemouton:fixLevelInheritance
Jul 28, 2025
Merged

v2: Fix log level inheritance#24
Roasbeef merged 3 commits intobtcsuite:masterfrom
ellemouton:fixLevelInheritance

Conversation

@ellemouton
Copy link
Copy Markdown
Contributor

In this PR, we fix log level inheritance of WithPrefix.

With this PR, we have the following behaviour:

  • if WithPrefix is used to create a child logger, the log level is inherited from the parent - meaning that if the parent's level is changed after the child is created, the child's level will also be updated.
  • if SubSystem is used to create a child logger, the log level is independent from the parent after creation. Meaning that after creation, a change in log level of the parent/child will not be reflected in the other.

Copy link
Copy Markdown

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, LGTM 🎉

This commit adds a unit test to assert the current, correct, behaviour
of SubSystem in that it will create a new logger that does not inherit
any log level changes from its parent after creation.
This commit adds a new `TestWithPrefixLevelInheritance` unit test which
demonstrates that log level inheritance between parent loggers and child
loggers created via "WithPrefix" does not work as expected.
In this commit, the bug demonstrated in the previous commit is fixed.
Any child logger derived via WithPrefix will inherit a pointer to the
same `level` variable of the parent meaning that any change inl log
level will be reflected in the child logger.
@ellemouton ellemouton force-pushed the fixLevelInheritance branch from 8ba0244 to f0b9cb6 Compare July 24, 2025 14:16
@ziggie1984
Copy link
Copy Markdown
Contributor

LGTM 🙏

@ellemouton
Copy link
Copy Markdown
Contributor Author

cc @Roasbeef for merge

Copy link
Copy Markdown
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎿

}

// Show that the buffer currently _does not_ contain the child log.
// NOTE: This is a bug and will be fixed in the next commit.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😆

@Roasbeef Roasbeef merged commit 6090e87 into btcsuite:master Jul 28, 2025
@ellemouton ellemouton deleted the fixLevelInheritance branch July 29, 2025 05:15
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.

4 participants