http: record the number of bytes read when response writer is hijacked #6173
Conversation
|
@Mygod. What do you think of this? |
|
I no longer uses caddy now but my old use case would really prefer a raw connection for kernel space |
|
It sounds like this is no longer needed... is this a bug fix or mostly a feature? If it's a feature I'm not sure it's worth the complexity especially if it's not needed. That said, I thought we do kernel-space transfers when possible... 🤔 |
|
I think it's a bug fix. For websockets, number of bytes written is logged but not the number of bytes read. And I can't think of a non-invasive way to record that without adding a new field to the log. |
| @@ -240,6 +242,12 @@ func (rr *responseRecorder) FlushError() error { | |||
| return nil | |||
| } | |||
|
|
|||
| // Private interface so it can only be used in this package | |||
| // #TODO: maybe export it later | |||
| func (rr *responseRecorder) setReadSize(size *int) { | |||
There was a problem hiding this comment.
Passing in a *int is a bit weird. Can we just pass in a int? Any reason we need a pointer at all, actually?
There was a problem hiding this comment.
This pointer points to the member of the wrapped request body. So when it's updated, the corresponding log entry is updated as well. int requires more changes to the code.
There was a problem hiding this comment.
Ah, I see. Is that thread-safe? (Would it ever be used across goroutines?)
There was a problem hiding this comment.
The net.Conn interface is not thread-safe by default unless specified explicitly. But the only race I know of is when caddy sends websocket goaway frame, which is not thread-safe in the first place. But clients are expected to reconnect anyway, the stats are a bit off (goaway frames are small), nothing breaking.
Closes 5728.