tsweb: ensure in-flight requests are always marked as finished

The inflight request tracker only starts recording a new bucket after
the first non-error request. Unfortunately, it's written in such a way
that ONLY successful requests are ever marked as being finished. Once a
bucket has had at least one successful request and begun to be tracked,
all subsequent error cases are never marked finished and always appear
as in-flight.

This change ensures that if a request is recorded has having been
started, we also mark it as finished at the end.

Updates tailscale/corp#19767

Signed-off-by: Will Norris <will@tailscale.com>
pull/12002/head
Will Norris 6 months ago committed by Will Norris
parent 817badf9ca
commit 46980c9664

@ -313,6 +313,7 @@ func (h retHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
var bucket string var bucket string
bumpStartIfNeeded := func() {} bumpStartIfNeeded := func() {}
var startRecorded bool
if bs := h.opts.BucketedStats; bs != nil { if bs := h.opts.BucketedStats; bs != nil {
bucket = bs.bucketForRequest(r) bucket = bs.bucketForRequest(r)
if bs.Started != nil { if bs.Started != nil {
@ -320,6 +321,7 @@ func (h retHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
case *expvar.Int: case *expvar.Int:
// If we've already seen this bucket for, count it immediately. // If we've already seen this bucket for, count it immediately.
v.Add(1) v.Add(1)
startRecorded = true
case nil: case nil:
// Otherwise, for newly seen paths, only count retroactively // Otherwise, for newly seen paths, only count retroactively
// (so started-finished doesn't go negative) so we don't fill // (so started-finished doesn't go negative) so we don't fill
@ -437,12 +439,13 @@ func (h retHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
} }
if bs := h.opts.BucketedStats; bs != nil && bs.Finished != nil { if bs := h.opts.BucketedStats; bs != nil && bs.Finished != nil {
// Only increment metrics for buckets that result in good HTTP statuses. // Only increment metrics for buckets that result in good HTTP statuses
// or when we know the start was already counted.
// Otherwise they get full of internet scanning noise. Only filtering 404 // Otherwise they get full of internet scanning noise. Only filtering 404
// gets most of the way there but there are also plenty of URLs that are // gets most of the way there but there are also plenty of URLs that are
// almost right but result in 400s too. Seem easier to just only ignore // almost right but result in 400s too. Seem easier to just only ignore
// all 4xx and 5xx. // all 4xx and 5xx.
if msg.Code < 400 { if startRecorded || msg.Code < 400 {
bumpStartIfNeeded() bumpStartIfNeeded()
bs.Finished.Add(bucket, 1) bs.Finished.Add(bucket, 1)
} }

Loading…
Cancel
Save