-
Notifications
You must be signed in to change notification settings - Fork 769
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
Update logging to use generated logs #2531
Conversation
{ | ||
private static readonly Action<ILogger, Exception?> _unableToDisableMaxRequestBodySize = | ||
LoggerMessage.Define(LogLevel.Debug, new EventId(1, "UnableToDisableMaxRequestBodySizeLimit"), "Unable to disable the max request body size limit."); | ||
[LoggerMessage(Level = LogLevel.Trace, EventId = 27, EventName = "DeadlineStopped", Message = "Request deadline stopped.")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you preserve the previous order? I'd like to have them ordered by the event id. That way new log methods can be added to the end and it's easy to see the previous event id.
Also, keeping that order also makes code reviewing the changes easier.
Hi, thanks for the PR. FYI this is just one file of many with logs. See https://github.com/search?q=repo%3Agrpc%2Fgrpc-dotnet+LoggerMessage.Define&type=code You're welcome to update more if you'd like or stick with this one. I'll close the issue once all are updated. |
Hey, of course! I ordered the methods according to Event ID. |
One pr is fine |
Hey I updated the PR to include changes to all the files that have the A couple notes:
|
Thanks! Looks good to me. There are a lot of changes so I haven't excuasively checked every log, but the ones I did look at were correct. |
This PR uses the generated logs feature to simplify and reduce the log declarations.
closes #2529
Also my first PR so feedback would be lovely.