Minor perf and logging support changes #256
Conversation
| throw new NotImplementedException(); | ||
| } | ||
| } | ||
| public override long Length => Position; |
There was a problem hiding this comment.
Length is only expected to be available for seekable streams. https://msdn.microsoft.com/en-us/library/system.io.stream.length(v=vs.110).aspx NotSupportedException: A class derived from Stream does not support seeking. Position has the same expectation. https://msdn.microsoft.com/en-us/library/system.io.stream.position(v=vs.110).aspx
There was a problem hiding this comment.
Maybe we just add a new property to FrameResponseStream. BytesWritten perhaps? This way we can throw from Length and Position as expected.
@lodejard where were you planning on logging the this?
There was a problem hiding this comment.
From hosting, corresponding to "bytes written" you see in server logs.
There was a problem hiding this comment.
Is this logging meant to be server agnostic? Would using Stream.Length work for WebListener?
There was a problem hiding this comment.
No it won't, and it won't work for Nowin either. aspnet/Hosting#403 (comment)
There was a problem hiding this comment.
Both of those could easily be updated. But the idea you suggested of having a request statistics feature should also be considered.
Useful for bytes-written log value
lodejard
force-pushed
the
lodejard/perf-logging
branch
from
348604f to
a39529d
Compare
| { | ||
| SocketOutput.Write(_continueBytes); | ||
| foreach(var expect in _requestHeaders.HeaderExpect) |
|
@lodejard do you still want to merge this? |
|
I think this is too stale to remain open. |
Uses header properties directly in a few places
Provides a value for Response.Stream.Length in support of that logging the response content size