Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix MDC handling in Slf4jLogger #850

Merged
merged 14 commits into from
Feb 13, 2025
Merged

Fix MDC handling in Slf4jLogger #850

merged 14 commits into from
Feb 13, 2025

Conversation

morgen-peschke
Copy link
Contributor

@morgen-peschke morgen-peschke commented Jun 14, 2024 •
edited
Loading

Updates the handling of the MDC in Slf4jLogger to resolve questions raised in #851

Changes:

  • Always clear the MDC before populating when logging with context
  • Don't skip the MDC massaging when logging without context

Checks:

  • laziness of msg parameter
  • dispatch to log4j Logger messages
  • MDC setup/restoration
  • MDC recovery when exceptions happen during message creation

@morgen-peschke
Expand test coverage of Slf4jLogger
30f79f3 
Checks:
- laziness of `msg` parameter
- dispatch to log4j Logger messages
- MDC setup
This was referenced Jun 14, 2024
Closed
Open
@morgen-peschke morgen-peschke changed the title Expand test coverage of Slf4jLogger Fix MDC handling in Slf4jLogger Jun 26, 2024
@morgen-peschke
Copy link
Contributor Author

Quick ping to help prevent this from disappearing into the ether 😉

@morgen-peschke
Copy link
Contributor Author

@rossabaker I'm not able to add reviewers, can you add a couple of people?

@rossabaker rossabaker requested a review from a team September 19, 2024 20:16
@bcarter97
Copy link

@rossabaker @danicheg any chance of this getting reviewed?

@r-glyde r-glyde mentioned this pull request Jan 31, 2025
Open
samspills
samspills approved these changes Feb 6, 2025
View reviewed changes
Copy link

@samspills samspills left a comment

Choose a reason for hiding this comment

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

I was a little surprised that the imports can be mixed between * and _ styles. Might be good to follow up to switch entirely to * for consistency, but absolutely not blocking this PR.

@rossabaker rossabaker merged commit fb7d847 into typelevel:main Feb 13, 2025
14 checks passed
@morgen-peschke morgen-peschke deleted the expand-testing-for-SlfjLogger branch February 13, 2025 17:44
@morgen-peschke morgen-peschke mentioned this pull request Feb 13, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samspills samspills samspills approved these changes

@Mike-Miller Mike-Miller Mike-Miller approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@morgen-peschke @bcarter97 @samspills @Mike-Miller @rossabaker