server.logf is called for TLS handshake error which I guess bad clients could trigger? Otherwise it looks like internal server errors only.

Also see original discussion on #12745 - the original suggestion was to make sure a response was sent to clients and/or write to the log. I'd be happy without the logging as if we send a response body it's obvious to the client what is wrong with their request which is why I closed it with the first CL, but someone else did request logging too after it was closed.

I was just reviewing the 1.8 milestone and saw this issue.

I'm a little unclear on if this logging can be optionally disabled. If not, I would like to request that this is not included in 1.8, as some of our very high request-per-second endpoints would be negatively impacted by this. Specifically, we receive a fair number of malformed requests, and have more-or-less disabled logging for performance and reliability (ie, filling disk space) reasons.

Non-optionally logging malformed input would likely cause a substantial performance degradation and risk our systems over-logging, requiring excessive rotation or other techniques to deal with.

Thanks.

Thanks for cc'ing me. I didn't realize http.Server can generate 4xx error responses internally, without calling the request handler. This is a clear logging hole that makes me sympathize with https://golang.org/cl/27950. I run a service that logs metadata about every request/response pair. I didn't realize that our logs are missing a whole class of responses. (In practice, this is not a big deal, since we run behind a frontend that will reject most malformed requests, but still, there should be some hook that can be used to log such responses.)

At the same time, I agree with Russ: spamming stderr is not a good default behavior for this case. I vote to revert the logging portion of https://golang.org/cl/27950 for 1.8. Long term, I think something httptrace-like is the way to go, although I've given approximately zero thought to what such an API would look like.

@bradfitz perhaps we should just remove lines 1766 and 1780 so that the server returns a message to the client along with the status code but does no logging for now on 400 or 431 errors?

CL https://golang.org/cl/33617 mentions this issue.

Okay, it's removed for Go 1.8. We can reconsider what to do in Go 1.9.

For future ref, here is a summary of the current calls to logf in server.go:

ERRORS ACCEPTING CONNECTION:

// Accept error - things like hitting file limits 
srv.logf("http: Accept error: %v; retrying in %v", e, tempDelay)

ERRORS WRITING HEADERS:
// Caller error - writing on hijacked connection
w.conn.server.logf("http: response.WriteHeader on hijacked connection")

// Caller error - writing headers multiple times
w.conn.server.logf("http: multiple response.WriteHeader calls")

// Caller error - incorrect content length
w.conn.server.logf("http: invalid Content-Length of %q", cl)

// Caller error - incorrect mix of content length and transfer encoding
w.conn.server.logf("http: WriteHeader called with both Transfer-Encoding of %q and a Content-Length of %d",te, w.contentLength)

ERRORS WRITING RESPONSE:

// Caller error - trying to write on hijacked connection
w.conn.server.logf("http: response.Write on hijacked connection")

HANDLER ERRORS:

// Generic 500 error (server error - panic in handler)
c.server.logf("http: panic serving %v: %v\n%s", c.remoteAddr, err, buf)

CLIENT ERRORS:

// TLS handshake error (client error?)
c.server.logf("http: TLS handshake error from %s: %v", c.rwc.RemoteAddr(), err)

CLIENT ERRORS NOT LOGGED:
431 Request Header Fields Too Large
400 Bad Request (various reasons, mostly headers or url)

Info passed on with these current errors: RemoteAddr, contentLength, error string

There are also some related issues I think: #3344 #15949 #14205 #9383

So perhaps the best solution here is to add a ServerTrace to pkg httptrace with a handler for errors, which passes some info on to the caller about the reason for failure. See also the work started at #18997 for other tracing options for servers, this would extend that to provide a hook for errors.

I'm not sure on the signature for this error hook, to avoid leaking things like the connection, perhaps a struct containing some ConnInfo like the error, RemoteAddr, connection state, and perhaps an http status code?

This is now a dup of #18997. See the following comment:
#18997 (comment)