fix(gw): update metrics only when payload data sent (#8827)
* fix: report gateway http metrics only when response is successful * fix(gw): 304 Not Modified as no-op This fix ensures we don't do any additional work when Etag match what user already has in their own cache. Co-authored-by: Marcin Rataj <lidel@lidel.org>
Ian Davis committed
Apr 8, 2022 at 21:07 UTC
fbf76663f4db6f3c4ed89d8c017d9319d2727121
4 files changed
+74
-14
core/corehttp/gateway_handler.go
+55
-5
@@ -36,8 +36,10 @@ const (
36
immutableCacheControl = "public, max-age=29030400, immutable"
37
)
38
39
-var onlyAscii = regexp.MustCompile("[[:^ascii:]]")
40
-var noModtime = time.Unix(0, 0) // disables Last-Modified header if passed as modtime
39
+var (
40
+ onlyAscii = regexp.MustCompile("[[:^ascii:]]")
41
+ noModtime = time.Unix(0, 0) // disables Last-Modified header if passed as modtime
42
+)
43
44
// HTML-based redirect for errors which can be recovered from, but we want
45
// to provide hint to people that they should fix things on their end.
@@ -96,6 +98,54 @@ func (sw *statusResponseWriter) WriteHeader(code int) {
98
sw.ResponseWriter.WriteHeader(code)
99
}
100
101
+// ServeContent replies to the request using the content in the provided ReadSeeker
102
+// and returns the status code written and any error encountered during a write.
103
+// It wraps http.ServeContent which takes care of If-None-Match+Etag,
104
+// Content-Length and range requests.
105
+func ServeContent(w http.ResponseWriter, req *http.Request, name string, modtime time.Time, content io.ReadSeeker) (int, bool, error) {
106
+ ew := &errRecordingResponseWriter{ResponseWriter: w}
107
+ http.ServeContent(ew, req, name, modtime, content)
108
+
109
+ // When we calculate some metrics we want a flag that lets us to ignore
110
+ // errors and 304 Not Modified, and only care when requested data
111
+ // was sent in full.
112
+ dataSent := ew.code/100 == 2 && ew.err == nil
113
+
114
+ return ew.code, dataSent, ew.err
115
+}
116
+
117
+// errRecordingResponseWriter wraps a ResponseWriter to record the status code and any write error.
118
+type errRecordingResponseWriter struct {
119
+ http.ResponseWriter
120
+ code int
121
+ err error
122
+}
123
+
124
+func (w *errRecordingResponseWriter) WriteHeader(code int) {
125
+ if w.code == 0 {
126
+ w.code = code
127
+ }
128
+ w.ResponseWriter.WriteHeader(code)
129
+}
130
+
131
+func (w *errRecordingResponseWriter) Write(p []byte) (int, error) {
132
+ n, err := w.ResponseWriter.Write(p)
133
+ if err != nil && w.err == nil {
134
+ w.err = err
135
+ }
136
+ return n, err
137
+}
138
+
139
+// ReadFrom exposes errRecordingResponseWriter's underlying ResponseWriter to io.Copy
140
+// to allow optimized methods to be taken advantage of.
141
+func (w *errRecordingResponseWriter) ReadFrom(r io.Reader) (n int64, err error) {
142
+ n, err = io.Copy(w.ResponseWriter, r)
143
+ if err != nil && w.err == nil {
144
+ w.err = err
145
+ }
146
+ return n, err
147
+}
148
+
149
func newGatewaySummaryMetric(name string, help string) *prometheus.SummaryVec {
150
summaryMetric := prometheus.NewSummaryVec(
151
prometheus.SummaryOpts{
@@ -360,7 +410,8 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
410
trace.SpanFromContext(r.Context()).SetAttributes(attribute.String("ResolvedPath", resolvedPath.String()))
411
412
// Finish early if client already has matching Etag
363
- if r.Header.Get("If-None-Match") == getEtag(r, resolvedPath.Cid()) {
413
+ ifNoneMatch := r.Header.Get("If-None-Match")
414
+ if ifNoneMatch == getEtag(r, resolvedPath.Cid()) || ifNoneMatch == getDirListingEtag(resolvedPath.Cid()) {
415
w.WriteHeader(http.StatusNotModified)
416
return
417
}
@@ -401,7 +452,7 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
452
case "application/vnd.ipld.car":
453
logger.Debugw("serving car stream", "path", contentPath)
454
carVersion := formatParams["version"]
404
- i.serveCar(w, r, resolvedPath, contentPath, carVersion, begin)
455
+ i.serveCar(w, r, resolvedPath, contentPath, carVersion, begin)
456
return
457
default: // catch-all for unsuported application/vnd.*
458
err := fmt.Errorf("unsupported format %q", responseFormat)
@@ -644,7 +695,6 @@ func addCacheControlHeaders(w http.ResponseWriter, r *http.Request, contentPath
695
696
// TODO: set Cache-Control based on TTL of IPNS/DNSLink: https://github.com/ipfs/go-ipfs/issues/1818#issuecomment-1015849462
697
// TODO: set Last-Modified based on /ipns/ publishing timestamp?
647
-
698
} else {
699
// immutable! CACHE ALL THE THINGS, FOREVER! wolololol
700
w.Header().Set("Cache-Control", immutableCacheControl)
core/corehttp/gateway_handler_block.go
+6
-4
@@ -38,10 +38,12 @@ func (i *gatewayHandler) serveRawBlock(w http.ResponseWriter, r *http.Request, r
38
w.Header().Set("Content-Type", "application/vnd.ipld.raw")
39
w.Header().Set("X-Content-Type-Options", "nosniff") // no funny business in the browsers :^)
40
41
- // Done: http.ServeContent will take care of
41
+ // ServeContent will take care of
42
// If-None-Match+Etag, Content-Length and range requests
43
- http.ServeContent(w, r, name, modtime, content)
43
+ _, dataSent, _ := ServeContent(w, r, name, modtime, content)
44
45
- // Update metrics
46
- i.rawBlockGetMetric.WithLabelValues(contentPath.Namespace()).Observe(time.Since(begin).Seconds())
45
+ if dataSent {
46
+ // Update metrics
47
+ i.rawBlockGetMetric.WithLabelValues(contentPath.Namespace()).Observe(time.Since(begin).Seconds())
48
+ }
49
}
core/corehttp/gateway_handler_unixfs_dir.go
+6
-1
@@ -8,6 +8,7 @@ import (
8
"time"
9
10
"github.com/dustin/go-humanize"
11
+ cid "github.com/ipfs/go-cid"
12
files "github.com/ipfs/go-ipfs-files"
13
"github.com/ipfs/go-ipfs/assets"
14
"github.com/ipfs/go-ipfs/tracing"
@@ -93,7 +94,7 @@ func (i *gatewayHandler) serveDirectory(w http.ResponseWriter, r *http.Request,
94
95
// Generated dir index requires custom Etag (it may change between go-ipfs versions)
96
if assets.BindataVersionHash != "" {
96
- dirEtag := `"DirIndex-` + assets.BindataVersionHash + `_CID-` + resolvedPath.Cid().String() + `"`
97
+ dirEtag := getDirListingEtag(resolvedPath.Cid())
98
w.Header().Set("Etag", dirEtag)
99
if r.Header.Get("If-None-Match") == dirEtag {
100
w.WriteHeader(http.StatusNotModified)
@@ -204,3 +205,7 @@ func (i *gatewayHandler) serveDirectory(w http.ResponseWriter, r *http.Request,
205
// Update metrics
206
i.unixfsGenDirGetMetric.WithLabelValues(contentPath.Namespace()).Observe(time.Since(begin).Seconds())
207
}
208
+
209
+func getDirListingEtag(dirCid cid.Cid) string {
210
+ return `"DirIndex-` + assets.BindataVersionHash + `_CID-` + dirCid.String() + `"`
211
+}
core/corehttp/gateway_handler_unixfs_file.go
+7
-4
@@ -82,10 +82,13 @@ func (i *gatewayHandler) serveFile(w http.ResponseWriter, r *http.Request, resol
82
// special fixup around redirects
83
w = &statusResponseWriter{w}
84
85
- // Done: http.ServeContent will take care of
85
+ // ServeContent will take care of
86
// If-None-Match+Etag, Content-Length and range requests
87
- http.ServeContent(w, r, name, modtime, content)
87
+ _, dataSent, _ := ServeContent(w, r, name, modtime, content)
88
89
- // Update metrics
90
- i.unixfsFileGetMetric.WithLabelValues(contentPath.Namespace()).Observe(time.Since(begin).Seconds())
89
+ // Was response successful?
90
+ if dataSent {
91
+ // Update metrics
92
+ i.unixfsFileGetMetric.WithLabelValues(contentPath.Namespace()).Observe(time.Since(begin).Seconds())
93
+ }
94
}