Skip to content

Replaces inline logging with source-generated LoggerMessage #2903

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.

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

Merged

Conversation

Meir017
Copy link
Contributor

@Meir017 Meir017 commented Jun 29, 2025

related #2899

Adopts source-generated LoggerMessage for all logging calls, replacing direct string interpolation and formatting in log messages. Introduces strongly-typed helper structs for efficient log value formatting and factors out complex ToString() calls.

Improves logging performance, ensures consistency across log entries, and reduces unnecessary allocations by deferring message formatting until required. Lays groundwork for easier log message maintenance and better structured logging.

Adopts source-generated LoggerMessage for all logging calls, replacing direct string interpolation and formatting in log messages. Introduces strongly-typed helper structs for efficient log value formatting and factors out complex ToString() calls.

Improves logging performance, ensures consistency across log entries, and reduces unnecessary allocations by deferring message formatting until required. Lays groundwork for easier log message maintenance and better structured logging.
@Meir017
Copy link
Contributor Author

Meir017 commented Jun 29, 2025

a have a question here:

The ILogger in the ConnectionMultiplexer class is nullable, this doesn't play well with the logging source-generator, would it make sense to make it none-null (default to NullLogger<ConnectionMultiplexer>.Instance or modify the configuration.LoggerFactory to default to the null factory)

FYI @NickCraver

@mgravell
Copy link
Collaborator

merging, thanks; I'll follow up with a tweak to improve code layout, but: I can absorb that

@mgravell mgravell merged commit b4aaced into StackExchange:main Jul 21, 2025
2 of 6 checks passed
@Meir017 Meir017 deleted the feature/logger-message-attribute branch July 21, 2025 09:15
@mgravell
Copy link
Collaborator

Additional change / observation: the generated code is (for me) generating errors because of a missing null-check:

image

I'll work around that

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