-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Don't capture HttpContext into the log objects #11723
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
Conversation
45f61c7
to
116b599
Compare
|
||
Protocol = request.Protocol; | ||
Method = request.Method; | ||
ContentType = !string.IsNullOrEmpty(request.ContentType) ? Uri.EscapeUriString(request.ContentType) : string.Empty; |
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.
Allocations!
I think this change is fine but there's no issue with console and other loggers that store messages in a queue for further processing (at least the ones that we ship). They materialize the message before en-queuing. |
If you are happy with the current state, I'll close this as it does make |
The logger I'm developing has encountered this issue: ZLogger adopts an architecture that consistently delays formatting and writes to an Looking at the PR, it seems that incorporating this would solve the issue. Also, I think it's bad to capture the HttpContext. |
Don't capture HttpContext into the log objects from #10319
Resolves #10556
Resolves #10557
/cc @davidfowl