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.

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

Improve Logging to use static expressions #102

Open
Dragonsangel opened this issue Jan 31, 2023 · 1 comment
Open

Improve Logging to use static expressions #102

Dragonsangel opened this issue Jan 31, 2023 · 1 comment

Comments

@Dragonsangel
Copy link

The logs that are currently written by RedLock.net, are formatted at the time of writing the log entry.
Example from the current code base:

logger.LogWarning($"Expiry time {expiryTime.TotalMilliseconds}ms too low, setting to {MinimumExpiryTime.TotalMilliseconds}ms");

This would result in a log entry to be written as
Expiry time 5ms too low, setting to 10ms
where the message_template in the background would be
Expiry time 5ms too low, setting to 10ms.
This makes it difficult to search or filter on log entries in log aggregators, since the formatted string could potentially be any value.

From the Roslyn Analyser, there is warning "CA2254: Template should be a static expression", explicitly for these type of log entries, to allow for easier searching and filtering in log aggregators.
If the log writing example was to be changed to this

logger.LogWarning("Expiry time {ExpiryTotalMilliseconds}ms too low, setting to {MinimumExpiryTotalMilliseconds}ms", expiryTime.TotalMilliseconds, MinimumExpiryTime.TotalMilliseconds);

the resulting log entry would still be written as
Expiry time 5ms too low, setting to 10ms
but the message_template in the background would be
Expiry time {ExpiryTotalMilliseconds}ms too low, setting to {MinimumExpiryTotalMilliseconds}ms
which can be used to filter on all occurrences of this log entry, regardless of the values of expiryTime.TotalMilliseconds or MinimumExpiryTime.TotalMilliseconds

Here is the usage of Log message template explained further, with examples of formatting.
One additional suggestion I would like to make, is that the placeholder names be consistently case formatted, so either always upper CamelCase or lower camelCase.
If you can tell me which formatting you would prefer, I could make a PR with all the logging calls converted.

@Dragonsangel
Copy link
Author

I just saw that all the mentioned changes are already in PR #98

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

No branches or pull requests

1 participant