Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: report gateway http metrics only when response is successful #8827

Merged
merged 5 commits into from
Apr 8, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions core/corehttp/gateway_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ const (
immutableCacheControl = "public, max-age=29030400, immutable"
)

var onlyAscii = regexp.MustCompile("[[:^ascii:]]")
var noModtime = time.Unix(0, 0) // disables Last-Modified header if passed as modtime
var (
onlyAscii = regexp.MustCompile("[[:^ascii:]]")
noModtime = time.Unix(0, 0) // disables Last-Modified header if passed as modtime
)

// HTML-based redirect for errors which can be recovered from, but we want
// to provide hint to people that they should fix things on their end.
Expand Down Expand Up @@ -96,6 +98,48 @@ func (sw *statusResponseWriter) WriteHeader(code int) {
sw.ResponseWriter.WriteHeader(code)
}

// ServeContent replies to the request using the content in the provided ReadSeeker
// and returns the status code written and any error encountered during a write.
// It wraps http.ServeContent which takes care of If-None-Match+Etag,
// Content-Length and range requests.
func ServeContent(w http.ResponseWriter, req *http.Request, name string, modtime time.Time, content io.ReadSeeker) (int, error) {
ew := &errRecordingResponseWriter{ResponseWriter: w}
http.ServeContent(ew, req, name, modtime, content)
return ew.code, ew.err
}

// errRecordingResponseWriter wraps a ResponseWriter to record the status code and any write error.
type errRecordingResponseWriter struct {
http.ResponseWriter
code int
err error
}

func (w *errRecordingResponseWriter) WriteHeader(code int) {
if w.code == 0 {
w.code = code
}
w.ResponseWriter.WriteHeader(code)
}

func (w *errRecordingResponseWriter) Write(p []byte) (int, error) {
n, err := w.ResponseWriter.Write(p)
if err != nil && w.err == nil {
w.err = nil
}
return n, err
}

// ReadFrom exposes errRecordingResponseWriter's underlying ResponseWriter to io.Copy
// to allow optimized methods to be taken advantage of.
func (w *errRecordingResponseWriter) ReadFrom(r io.Reader) (n int64, err error) {
n, err = io.Copy(w.ResponseWriter, r)
if err != nil && w.err == nil {
w.err = nil
}
return n, err
}

func newGatewaySummaryMetric(name string, help string) *prometheus.SummaryVec {
summaryMetric := prometheus.NewSummaryVec(
prometheus.SummaryOpts{
Expand Down Expand Up @@ -644,7 +688,6 @@ func addCacheControlHeaders(w http.ResponseWriter, r *http.Request, contentPath

// TODO: set Cache-Control based on TTL of IPNS/DNSLink: https://github.com/ipfs/go-ipfs/issues/1818#issuecomment-1015849462
// TODO: set Last-Modified based on /ipns/ publishing timestamp?

} else {
// immutable! CACHE ALL THE THINGS, FOREVER! wolololol
w.Header().Set("Cache-Control", immutableCacheControl)
Expand Down
11 changes: 6 additions & 5 deletions core/corehttp/gateway_handler_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,11 @@ func (i *gatewayHandler) serveRawBlock(w http.ResponseWriter, r *http.Request, r
w.Header().Set("Content-Type", "application/vnd.ipld.raw")
w.Header().Set("X-Content-Type-Options", "nosniff") // no funny business in the browsers :^)

// Done: http.ServeContent will take care of
// If-None-Match+Etag, Content-Length and range requests
http.ServeContent(w, r, name, modtime, content)
code, err := ServeContent(w, r, name, modtime, content)

// Update metrics
i.rawBlockGetMetric.WithLabelValues(contentPath.Namespace()).Observe(time.Since(begin).Seconds())
// Was response successful?
if code/100 == 2 && err == nil {
Copy link
Member

@lidel lidel Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭

This also ignores 304 Not Modified responses when calculating metrics – i think that was intentional, because those also lower response times, right?

Would it be useful to add a global metric counting 304s, just to know the % of "skipped" responses? (in separate PR)

// Update metrics
i.rawBlockGetMetric.WithLabelValues(contentPath.Namespace()).Observe(time.Since(begin).Seconds())
}
}
11 changes: 6 additions & 5 deletions core/corehttp/gateway_handler_unixfs_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,11 @@ func (i *gatewayHandler) serveFile(w http.ResponseWriter, r *http.Request, resol
// special fixup around redirects
w = &statusResponseWriter{w}

// Done: http.ServeContent will take care of
// If-None-Match+Etag, Content-Length and range requests
http.ServeContent(w, r, name, modtime, content)
code, err := ServeContent(w, r, name, modtime, content)

// Update metrics
i.unixfsFileGetMetric.WithLabelValues(contentPath.Namespace()).Observe(time.Since(begin).Seconds())
// Was response successful?
if code/100 == 2 && err == nil {
// Update metrics
i.unixfsFileGetMetric.WithLabelValues(contentPath.Namespace()).Observe(time.Since(begin).Seconds())
}
}