http2: add Server.CountError hook for error metrics
Change-Id: Ieaa2e97a2c467a3cf0753fcbf1a143f7bf76ef29
Reviewed-on: https://go-review.googlesource.com/c/net/+/350649
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Trust: Tobias Klauser <tobias.klauser@gmail.com>
diff --git a/http2/server.go b/http2/server.go
index 19e449c..d986655 100644
--- a/http2/server.go
+++ b/http2/server.go
@@ -130,6 +130,12 @@
// If nil, a default scheduler is chosen.
NewWriteScheduler func() WriteScheduler
+ // CountError, if non-nil, is called on HTTP/2 server errors.
+ // It's intended to increment a metric for monitoring, such
+ // as an expvar or Prometheus metric.
+ // The errType consists of only ASCII word characters.
+ CountError func(errType string)
+
// Internal state. This is a pointer (rather than embedded directly)
// so that we don't embed a Mutex in this struct, which will make the
// struct non-copyable, which might break some callers.
@@ -1399,7 +1405,7 @@
// First frame received must be SETTINGS.
if !sc.sawFirstSettings {
if _, ok := f.(*SettingsFrame); !ok {
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("first_settings", ConnectionError(ErrCodeProtocol))
}
sc.sawFirstSettings = true
}
@@ -1424,7 +1430,7 @@
case *PushPromiseFrame:
// A client cannot push. Thus, servers MUST treat the receipt of a PUSH_PROMISE
// frame as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("push_promise", ConnectionError(ErrCodeProtocol))
default:
sc.vlogf("http2: server ignoring frame: %v", f.Header())
return nil
@@ -1444,7 +1450,7 @@
// identifier field value other than 0x0, the recipient MUST
// respond with a connection error (Section 5.4.1) of type
// PROTOCOL_ERROR."
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("ping_on_stream", ConnectionError(ErrCodeProtocol))
}
if sc.inGoAway && sc.goAwayCode != ErrCodeNo {
return nil
@@ -1463,7 +1469,7 @@
// or PRIORITY on a stream in this state MUST be
// treated as a connection error (Section 5.4.1) of
// type PROTOCOL_ERROR."
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("stream_idle", ConnectionError(ErrCodeProtocol))
}
if st == nil {
// "WINDOW_UPDATE can be sent by a peer that has sent a
@@ -1474,7 +1480,7 @@
return nil
}
if !st.flow.add(int32(f.Increment)) {
- return streamError(f.StreamID, ErrCodeFlowControl)
+ return sc.countError("bad_flow", streamError(f.StreamID, ErrCodeFlowControl))
}
default: // connection-level flow control
if !sc.flow.add(int32(f.Increment)) {
@@ -1495,7 +1501,7 @@
// identifying an idle stream is received, the
// recipient MUST treat this as a connection error
// (Section 5.4.1) of type PROTOCOL_ERROR.
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("reset_idle_stream", ConnectionError(ErrCodeProtocol))
}
if st != nil {
st.cancelCtx()
@@ -1547,7 +1553,7 @@
// Why is the peer ACKing settings we never sent?
// The spec doesn't mention this case, but
// hang up on them anyway.
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("ack_mystery", ConnectionError(ErrCodeProtocol))
}
return nil
}
@@ -1555,7 +1561,7 @@
// This isn't actually in the spec, but hang up on
// suspiciously large settings frames or those with
// duplicate entries.
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("settings_big_or_dups", ConnectionError(ErrCodeProtocol))
}
if err := f.ForeachSetting(sc.processSetting); err != nil {
return err
@@ -1622,7 +1628,7 @@
// control window to exceed the maximum size as a
// connection error (Section 5.4.1) of type
// FLOW_CONTROL_ERROR."
- return ConnectionError(ErrCodeFlowControl)
+ return sc.countError("setting_win_size", ConnectionError(ErrCodeFlowControl))
}
}
return nil
@@ -1655,7 +1661,7 @@
// or PRIORITY on a stream in this state MUST be
// treated as a connection error (Section 5.4.1) of
// type PROTOCOL_ERROR."
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("data_on_idle", ConnectionError(ErrCodeProtocol))
}
// "If a DATA frame is received whose stream is not in "open"
@@ -1672,7 +1678,7 @@
// and return any flow control bytes since we're not going
// to consume them.
if sc.inflow.available() < int32(f.Length) {
- return streamError(id, ErrCodeFlowControl)
+ return sc.countError("data_flow", streamError(id, ErrCodeFlowControl))
}
// Deduct the flow control from inflow, since we're
// going to immediately add it back in
@@ -1685,7 +1691,7 @@
// Already have a stream error in flight. Don't send another.
return nil
}
- return streamError(id, ErrCodeStreamClosed)
+ return sc.countError("closed", streamError(id, ErrCodeStreamClosed))
}
if st.body == nil {
panic("internal error: should have a body in this state")
@@ -1697,12 +1703,12 @@
// RFC 7540, sec 8.1.2.6: A request or response is also malformed if the
// value of a content-length header field does not equal the sum of the
// DATA frame payload lengths that form the body.
- return streamError(id, ErrCodeProtocol)
+ return sc.countError("send_too_much", streamError(id, ErrCodeProtocol))
}
if f.Length > 0 {
// Check whether the client has flow control quota.
if st.inflow.available() < int32(f.Length) {
- return streamError(id, ErrCodeFlowControl)
+ return sc.countError("flow_on_data_length", streamError(id, ErrCodeFlowControl))
}
st.inflow.take(int32(f.Length))
@@ -1710,7 +1716,7 @@
wrote, err := st.body.Write(data)
if err != nil {
sc.sendWindowUpdate(nil, int(f.Length)-wrote)
- return streamError(id, ErrCodeStreamClosed)
+ return sc.countError("body_write_err", streamError(id, ErrCodeStreamClosed))
}
if wrote != len(data) {
panic("internal error: bad Writer")
@@ -1796,7 +1802,7 @@
// stream identifier MUST respond with a connection error
// (Section 5.4.1) of type PROTOCOL_ERROR.
if id%2 != 1 {
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("headers_even", ConnectionError(ErrCodeProtocol))
}
// A HEADERS frame can be used to create a new stream or
// send a trailer for an open one. If we already have a stream
@@ -1813,7 +1819,7 @@
// this state, it MUST respond with a stream error (Section 5.4.2) of
// type STREAM_CLOSED.
if st.state == stateHalfClosedRemote {
- return streamError(id, ErrCodeStreamClosed)
+ return sc.countError("headers_half_closed", streamError(id, ErrCodeStreamClosed))
}
return st.processTrailerHeaders(f)
}
@@ -1824,7 +1830,7 @@
// receives an unexpected stream identifier MUST respond with
// a connection error (Section 5.4.1) of type PROTOCOL_ERROR.
if id <= sc.maxClientStreamID {
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("stream_went_down", ConnectionError(ErrCodeProtocol))
}
sc.maxClientStreamID = id
@@ -1841,14 +1847,14 @@
if sc.curClientStreams+1 > sc.advMaxStreams {
if sc.unackedSettings == 0 {
// They should know better.
- return streamError(id, ErrCodeProtocol)
+ return sc.countError("over_max_streams", streamError(id, ErrCodeProtocol))
}
// Assume it's a network race, where they just haven't
// received our last SETTINGS update. But actually
// this can't happen yet, because we don't yet provide
// a way for users to adjust server parameters at
// runtime.
- return streamError(id, ErrCodeRefusedStream)
+ return sc.countError("over_max_streams_race", streamError(id, ErrCodeRefusedStream))
}
initialState := stateOpen
@@ -1858,7 +1864,7 @@
st := sc.newStream(id, 0, initialState)
if f.HasPriority() {
- if err := checkPriority(f.StreamID, f.Priority); err != nil {
+ if err := sc.checkPriority(f.StreamID, f.Priority); err != nil {
return err
}
sc.writeSched.AdjustStream(st.id, f.Priority)
@@ -1902,15 +1908,15 @@
sc := st.sc
sc.serveG.check()
if st.gotTrailerHeader {
- return ConnectionError(ErrCodeProtocol)
+ return sc.countError("dup_trailers", ConnectionError(ErrCodeProtocol))
}
st.gotTrailerHeader = true
if !f.StreamEnded() {
- return streamError(st.id, ErrCodeProtocol)
+ return sc.countError("trailers_not_ended", streamError(st.id, ErrCodeProtocol))
}
if len(f.PseudoFields()) > 0 {
- return streamError(st.id, ErrCodeProtocol)
+ return sc.countError("trailers_pseudo", streamError(st.id, ErrCodeProtocol))
}
if st.trailer != nil {
for _, hf := range f.RegularFields() {
@@ -1919,7 +1925,7 @@
// TODO: send more details to the peer somehow. But http2 has
// no way to send debug data at a stream level. Discuss with
// HTTP folk.
- return streamError(st.id, ErrCodeProtocol)
+ return sc.countError("trailers_bogus", streamError(st.id, ErrCodeProtocol))
}
st.trailer[key] = append(st.trailer[key], hf.Value)
}
@@ -1928,13 +1934,13 @@
return nil
}
-func checkPriority(streamID uint32, p PriorityParam) error {
+func (sc *serverConn) checkPriority(streamID uint32, p PriorityParam) error {
if streamID == p.StreamDep {
// Section 5.3.1: "A stream cannot depend on itself. An endpoint MUST treat
// this as a stream error (Section 5.4.2) of type PROTOCOL_ERROR."
// Section 5.3.3 says that a stream can depend on one of its dependencies,
// so it's only self-dependencies that are forbidden.
- return streamError(streamID, ErrCodeProtocol)
+ return sc.countError("priority", streamError(streamID, ErrCodeProtocol))
}
return nil
}
@@ -1943,7 +1949,7 @@
if sc.inGoAway {
return nil
}
- if err := checkPriority(f.StreamID, f.PriorityParam); err != nil {
+ if err := sc.checkPriority(f.StreamID, f.PriorityParam); err != nil {
return err
}
sc.writeSched.AdjustStream(f.StreamID, f.PriorityParam)
@@ -2013,13 +2019,13 @@
// "All HTTP/2 requests MUST include exactly one valid
// value for the :method, :scheme, and :path
// pseudo-header fields"
- return nil, nil, streamError(f.StreamID, ErrCodeProtocol)
+ return nil, nil, sc.countError("bad_path_method", streamError(f.StreamID, ErrCodeProtocol))
}
bodyOpen := !f.StreamEnded()
if rp.method == "HEAD" && bodyOpen {
// HEAD requests can't have bodies
- return nil, nil, streamError(f.StreamID, ErrCodeProtocol)
+ return nil, nil, sc.countError("head_body", streamError(f.StreamID, ErrCodeProtocol))
}
rp.header = make(http.Header)
@@ -2102,7 +2108,7 @@
var err error
url_, err = url.ParseRequestURI(rp.path)
if err != nil {
- return nil, nil, streamError(st.id, ErrCodeProtocol)
+ return nil, nil, sc.countError("bad_path", streamError(st.id, ErrCodeProtocol))
}
requestURI = rp.path
}
@@ -2985,3 +2991,31 @@
}
return false
}
+
+func (sc *serverConn) countError(name string, err error) error {
+ if sc == nil || sc.srv == nil {
+ return err
+ }
+ f := sc.srv.CountError
+ if f == nil {
+ return err
+ }
+ var typ string
+ var code ErrCode
+ switch e := err.(type) {
+ case ConnectionError:
+ typ = "conn"
+ code = ErrCode(e)
+ case StreamError:
+ typ = "stream"
+ code = ErrCode(e.Code)
+ default:
+ return err
+ }
+ codeStr := errCodeName[code]
+ if codeStr == "" {
+ codeStr = strconv.Itoa(int(code))
+ }
+ f(fmt.Sprintf("%s_%s_%s", typ, codeStr, name))
+ return err
+}