[ARVADOS] created: 1.1.4-613-gbc66b072e

Git user git at public.curoverse.com
Tue Jul 17 11:40:31 EDT 2018


        at  bc66b072e80a69cd9cb4f0c4bf746995305eaf8d (commit)


commit bc66b072e80a69cd9cb4f0c4bf746995305eaf8d
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Jul 17 11:38:45 2018 -0400

    13198: Track & serve keep-web metrics.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/httpserver/metrics.go b/sdk/go/httpserver/metrics.go
index 1f7d44752..77525a80f 100644
--- a/sdk/go/httpserver/metrics.go
+++ b/sdk/go/httpserver/metrics.go
@@ -19,6 +19,11 @@ import (
 
 type Handler interface {
 	http.Handler
+
+	// Returns an http.Handler that serves the Handler's metrics
+	// data at /metrics and /metrics.json, and passes other
+	// requests through to next.
+	ServeAPI(next http.Handler) http.Handler
 }
 
 type metrics struct {
@@ -34,6 +39,8 @@ func (*metrics) Levels() []logrus.Level {
 	return logrus.AllLevels
 }
 
+// Fire implements logrus.Hook in order to collect data points from
+// request logs.
 func (m *metrics) Fire(ent *logrus.Entry) error {
 	if tts, ok := ent.Data["timeToStatus"].(stats.Duration); !ok {
 	} else if method, ok := ent.Data["reqMethod"].(string); !ok {
@@ -57,19 +64,41 @@ func (m *metrics) exportJSON(w http.ResponseWriter, req *http.Request) {
 	w.Write([]byte{']'})
 }
 
+// ServeHTTP implements http.Handler.
 func (m *metrics) ServeHTTP(w http.ResponseWriter, req *http.Request) {
-	switch {
-	case req.Method != "GET" && req.Method != "HEAD":
-		m.next.ServeHTTP(w, req)
-	case req.URL.Path == "/metrics.json":
-		m.exportJSON(w, req)
-	case req.URL.Path == "/metrics":
-		m.exportProm.ServeHTTP(w, req)
-	default:
-		m.next.ServeHTTP(w, req)
-	}
+	m.next.ServeHTTP(w, req)
+}
+
+// ServeAPI returns a new http.Handler that serves current data at
+// metrics API endpoints (currently "GET /metrics(.json)?") and passes
+// other requests through to next.
+//
+// Typical example:
+//
+//	m := Instrument(...)
+//	srv := http.Server{Handler: m.ServeAPI(m)}
+func (m *metrics) ServeAPI(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		switch {
+		case req.Method != "GET" && req.Method != "HEAD":
+			next.ServeHTTP(w, req)
+		case req.URL.Path == "/metrics.json":
+			m.exportJSON(w, req)
+		case req.URL.Path == "/metrics":
+			m.exportProm.ServeHTTP(w, req)
+		default:
+			next.ServeHTTP(w, req)
+		}
+	})
 }
 
+// Instrument returns a new Handler that passes requests through to
+// the next handler in the stack, and tracks metrics of those
+// requests.
+//
+// For the metrics to be accurate, the caller must ensure every
+// request passed to the Handler also passes through
+// LogRequests(logger, ...), and vice versa.
 func Instrument(logger *logrus.Logger, next http.Handler) Handler {
 	if logger == nil {
 		logger = logrus.StandardLogger()
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 7d17be6e7..d0ba431aa 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -31,6 +31,7 @@ import (
 
 type handler struct {
 	Config        *Config
+	MetricsAPI    http.Handler
 	clientPool    *arvadosclient.ClientPool
 	setupOnce     sync.Once
 	healthHandler http.Handler
@@ -259,6 +260,9 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	} else if r.URL.Path == "/status.json" {
 		h.serveStatus(w, r)
 		return
+	} else if strings.HasPrefix(r.URL.Path, "/metrics") {
+		h.MetricsAPI.ServeHTTP(w, r)
+		return
 	} else if siteFSDir[pathParts[0]] {
 		useSiteFS = true
 	} else if len(pathParts) >= 1 && strings.HasPrefix(pathParts[0], "c=") {
diff --git a/services/keep-web/server.go b/services/keep-web/server.go
index 269f5d368..58ec348c8 100644
--- a/services/keep-web/server.go
+++ b/services/keep-web/server.go
@@ -5,6 +5,8 @@
 package main
 
 import (
+	"net/http"
+
 	"git.curoverse.com/arvados.git/sdk/go/httpserver"
 )
 
@@ -14,7 +16,10 @@ type server struct {
 }
 
 func (srv *server) Start() error {
-	srv.Handler = httpserver.Instrument(nil, httpserver.AddRequestIDs(httpserver.LogRequests(nil, &handler{Config: srv.Config})))
+	h := &handler{Config: srv.Config}
+	mh := httpserver.Instrument(nil, httpserver.AddRequestIDs(httpserver.LogRequests(nil, h)))
+	h.MetricsAPI = mh.ServeAPI(http.NotFoundHandler())
+	srv.Handler = mh
 	srv.Addr = srv.Config.Listen
 	return srv.Server.Start()
 }
diff --git a/services/keep-web/server_test.go b/services/keep-web/server_test.go
index 63a84289c..6688cc2ee 100644
--- a/services/keep-web/server_test.go
+++ b/services/keep-web/server_test.go
@@ -360,6 +360,7 @@ func (s *IntegrationSuite) TestMetrics(c *check.C) {
 	// must not intercept that route.
 	req, _ = http.NewRequest("GET", origin+"/metrics.json", nil)
 	req.Host = strings.Replace(arvadostest.FooCollectionPDH, "+", "-", -1) + ".example.com"
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
 	resp, err = http.DefaultClient.Do(req)
 	c.Assert(err, check.IsNil)
 	c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index 847486fbd..d19be61e9 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -88,8 +88,9 @@ func MakeRESTRouter() http.Handler {
 
 	rtr.limiter = httpserver.NewRequestLimiter(theConfig.MaxRequests, rtr)
 
-	return httpserver.Instrument(nil,
+	stack := httpserver.Instrument(nil,
 		httpserver.AddRequestIDs(httpserver.LogRequests(nil, rtr.limiter)))
+	return stack.ServeAPI(stack)
 }
 
 // BadRequestHandler is a HandleFunc to address bad requests.

commit 84f21d5634e17be62748f29f4303a86e0be6715b
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Jul 16 15:01:42 2018 -0400

    13198: Test case for keep-web metrics.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 206bf6f43..68ed06216 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -29,7 +29,7 @@ type UnitSuite struct{}
 
 func (s *UnitSuite) TestCORSPreflight(c *check.C) {
 	h := handler{Config: DefaultConfig()}
-	u, _ := url.Parse("http://keep-web.example/c=" + arvadostest.FooCollection + "/foo")
+	u := mustParseURL("http://keep-web.example/c=" + arvadostest.FooCollection + "/foo")
 	req := &http.Request{
 		Method:     "OPTIONS",
 		Host:       u.Host,
@@ -70,8 +70,7 @@ func (s *UnitSuite) TestInvalidUUID(c *check.C) {
 		"http://" + bogusID + ".keep-web/t=" + token + "/" + bogusID + "/foo",
 	} {
 		c.Log(trial)
-		u, err := url.Parse(trial)
-		c.Assert(err, check.IsNil)
+		u := mustParseURL(trial)
 		req := &http.Request{
 			Method:     "GET",
 			Host:       u.Host,
diff --git a/services/keep-web/server.go b/services/keep-web/server.go
index e51376c3b..269f5d368 100644
--- a/services/keep-web/server.go
+++ b/services/keep-web/server.go
@@ -14,7 +14,7 @@ type server struct {
 }
 
 func (srv *server) Start() error {
-	srv.Handler = httpserver.AddRequestIDs(httpserver.LogRequests(nil, &handler{Config: srv.Config}))
+	srv.Handler = httpserver.Instrument(nil, httpserver.AddRequestIDs(httpserver.LogRequests(nil, &handler{Config: srv.Config})))
 	srv.Addr = srv.Config.Listen
 	return srv.Server.Start()
 }
diff --git a/services/keep-web/server_test.go b/services/keep-web/server_test.go
index ee585ad5b..63a84289c 100644
--- a/services/keep-web/server_test.go
+++ b/services/keep-web/server_test.go
@@ -6,10 +6,12 @@ package main
 
 import (
 	"crypto/md5"
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"net"
+	"net/http"
 	"os"
 	"os/exec"
 	"strings"
@@ -294,6 +296,75 @@ func (s *IntegrationSuite) runCurl(c *check.C, token, host, uri string, args ...
 	return
 }
 
+func (s *IntegrationSuite) TestMetrics(c *check.C) {
+	origin := "http://" + s.testServer.Addr
+	req, _ := http.NewRequest("GET", origin+"/notfound", nil)
+	_, err := http.DefaultClient.Do(req)
+	c.Assert(err, check.IsNil)
+	req, _ = http.NewRequest("GET", origin+"/by_id/", nil)
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	resp, err := http.DefaultClient.Do(req)
+	c.Assert(err, check.IsNil)
+	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
+	req, _ = http.NewRequest("GET", origin+"/foo", nil)
+	req.Host = arvadostest.FooCollection + ".example.com"
+	req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+	resp, err = http.DefaultClient.Do(req)
+	c.Assert(err, check.IsNil)
+	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
+	buf, _ := ioutil.ReadAll(resp.Body)
+	c.Check(buf, check.DeepEquals, []byte("foo"))
+
+	req, _ = http.NewRequest("GET", origin+"/metrics.json", nil)
+	resp, err = http.DefaultClient.Do(req)
+	c.Assert(err, check.IsNil)
+	c.Check(resp.StatusCode, check.Equals, http.StatusOK)
+	type summary struct {
+		SampleCount string  `json:"sample_count"`
+		SampleSum   float64 `json:"sample_sum"`
+		Quantile    []struct {
+			Quantile float64
+			Value    float64
+		}
+	}
+	var ents []struct {
+		Name   string
+		Help   string
+		Type   string
+		Metric []struct {
+			Label []struct {
+				Name  string
+				Value string
+			}
+			Summary summary
+		}
+	}
+	json.NewDecoder(resp.Body).Decode(&ents)
+	flat := map[string]summary{}
+	for _, e := range ents {
+		for _, m := range e.Metric {
+			labels := map[string]string{}
+			for _, lbl := range m.Label {
+				labels[lbl.Name] = lbl.Value
+			}
+			flat[e.Name+"/"+labels["method"]+"/"+labels["code"]] = m.Summary
+		}
+	}
+	c.Check(flat["request_duration_seconds/get/200"].SampleSum, check.Not(check.Equals), 0)
+	c.Check(flat["request_duration_seconds/get/200"].SampleCount, check.Equals, "2")
+	c.Check(flat["request_duration_seconds/get/404"].SampleCount, check.Equals, "1")
+	c.Check(flat["time_to_status_seconds/get/404"].SampleCount, check.Equals, "1")
+
+	// If the Host header indicates a collection, /metrics.json
+	// refers to a file in the collection -- the metrics handler
+	// must not intercept that route.
+	req, _ = http.NewRequest("GET", origin+"/metrics.json", nil)
+	req.Host = strings.Replace(arvadostest.FooCollectionPDH, "+", "-", -1) + ".example.com"
+	resp, err = http.DefaultClient.Do(req)
+	c.Assert(err, check.IsNil)
+	c.Check(resp.StatusCode, check.Equals, http.StatusNotFound)
+}
+
 func (s *IntegrationSuite) SetUpSuite(c *check.C) {
 	arvadostest.StartAPI()
 	arvadostest.StartKeep(2, true)

commit d27d94532c173335cae9dddc30f1cc9a2e372bd7
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Jul 16 10:42:23 2018 -0400

    13198: Move metrics to httpserver package.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/httpserver/metrics.go b/sdk/go/httpserver/metrics.go
new file mode 100644
index 000000000..1f7d44752
--- /dev/null
+++ b/sdk/go/httpserver/metrics.go
@@ -0,0 +1,100 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package httpserver
+
+import (
+	"net/http"
+	"strconv"
+	"strings"
+	"time"
+
+	"git.curoverse.com/arvados.git/sdk/go/stats"
+	"github.com/Sirupsen/logrus"
+	"github.com/gogo/protobuf/jsonpb"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promhttp"
+)
+
+type Handler interface {
+	http.Handler
+}
+
+type metrics struct {
+	next         http.Handler
+	logger       *logrus.Logger
+	registry     *prometheus.Registry
+	reqDuration  *prometheus.SummaryVec
+	timeToStatus *prometheus.SummaryVec
+	exportProm   http.Handler
+}
+
+func (*metrics) Levels() []logrus.Level {
+	return logrus.AllLevels
+}
+
+func (m *metrics) Fire(ent *logrus.Entry) error {
+	if tts, ok := ent.Data["timeToStatus"].(stats.Duration); !ok {
+	} else if method, ok := ent.Data["reqMethod"].(string); !ok {
+	} else if code, ok := ent.Data["respStatusCode"].(int); !ok {
+	} else {
+		m.timeToStatus.WithLabelValues(strconv.Itoa(code), strings.ToLower(method)).Observe(time.Duration(tts).Seconds())
+	}
+	return nil
+}
+
+func (m *metrics) exportJSON(w http.ResponseWriter, req *http.Request) {
+	jm := jsonpb.Marshaler{Indent: "  "}
+	mfs, _ := m.registry.Gather()
+	w.Write([]byte{'['})
+	for i, mf := range mfs {
+		if i > 0 {
+			w.Write([]byte{','})
+		}
+		jm.Marshal(w, mf)
+	}
+	w.Write([]byte{']'})
+}
+
+func (m *metrics) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+	switch {
+	case req.Method != "GET" && req.Method != "HEAD":
+		m.next.ServeHTTP(w, req)
+	case req.URL.Path == "/metrics.json":
+		m.exportJSON(w, req)
+	case req.URL.Path == "/metrics":
+		m.exportProm.ServeHTTP(w, req)
+	default:
+		m.next.ServeHTTP(w, req)
+	}
+}
+
+func Instrument(logger *logrus.Logger, next http.Handler) Handler {
+	if logger == nil {
+		logger = logrus.StandardLogger()
+	}
+	reqDuration := prometheus.NewSummaryVec(prometheus.SummaryOpts{
+		Name: "request_duration_seconds",
+		Help: "Summary of request duration.",
+	}, []string{"code", "method"})
+	timeToStatus := prometheus.NewSummaryVec(prometheus.SummaryOpts{
+		Name: "time_to_status_seconds",
+		Help: "Summary of request TTFB.",
+	}, []string{"code", "method"})
+	registry := prometheus.NewRegistry()
+	registry.MustRegister(timeToStatus)
+	registry.MustRegister(reqDuration)
+	m := &metrics{
+		next:         promhttp.InstrumentHandlerDuration(reqDuration, next),
+		logger:       logger,
+		registry:     registry,
+		reqDuration:  reqDuration,
+		timeToStatus: timeToStatus,
+		exportProm: promhttp.HandlerFor(registry, promhttp.HandlerOpts{
+			ErrorLog: logger,
+		}),
+	}
+	m.logger.AddHook(m)
+	return m
+}
diff --git a/services/keepstore/config.go b/services/keepstore/config.go
index 3db20e29c..1f8c7e31a 100644
--- a/services/keepstore/config.go
+++ b/services/keepstore/config.go
@@ -9,17 +9,11 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
-	"net/http"
-	"strconv"
 	"strings"
 	"time"
 
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
-	"git.curoverse.com/arvados.git/sdk/go/stats"
 	"github.com/Sirupsen/logrus"
-	"github.com/golang/protobuf/jsonpb"
-	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/client_golang/prometheus/promhttp"
 )
 
 type Config struct {
@@ -54,8 +48,6 @@ type Config struct {
 
 	ManagementToken string `doc: The secret key that must be provided by monitoring services
 wishing to access the health check endpoint (/_health).`
-
-	metrics
 }
 
 var (
@@ -161,62 +153,6 @@ func (cfg *Config) Start() error {
 	return nil
 }
 
-type metrics struct {
-	registry     *prometheus.Registry
-	reqDuration  *prometheus.SummaryVec
-	timeToStatus *prometheus.SummaryVec
-	exportProm   http.Handler
-}
-
-func (*metrics) Levels() []logrus.Level {
-	return logrus.AllLevels
-}
-
-func (m *metrics) Fire(ent *logrus.Entry) error {
-	if tts, ok := ent.Data["timeToStatus"].(stats.Duration); !ok {
-	} else if method, ok := ent.Data["reqMethod"].(string); !ok {
-	} else if code, ok := ent.Data["respStatusCode"].(int); !ok {
-	} else {
-		m.timeToStatus.WithLabelValues(strconv.Itoa(code), strings.ToLower(method)).Observe(time.Duration(tts).Seconds())
-	}
-	return nil
-}
-
-func (m *metrics) setup() {
-	m.registry = prometheus.NewRegistry()
-	m.timeToStatus = prometheus.NewSummaryVec(prometheus.SummaryOpts{
-		Name: "time_to_status_seconds",
-		Help: "Summary of request TTFB.",
-	}, []string{"code", "method"})
-	m.reqDuration = prometheus.NewSummaryVec(prometheus.SummaryOpts{
-		Name: "request_duration_seconds",
-		Help: "Summary of request duration.",
-	}, []string{"code", "method"})
-	m.registry.MustRegister(m.timeToStatus)
-	m.registry.MustRegister(m.reqDuration)
-	m.exportProm = promhttp.HandlerFor(m.registry, promhttp.HandlerOpts{
-		ErrorLog: log,
-	})
-	log.AddHook(m)
-}
-
-func (m *metrics) exportJSON(w http.ResponseWriter, req *http.Request) {
-	jm := jsonpb.Marshaler{Indent: "  "}
-	mfs, _ := m.registry.Gather()
-	w.Write([]byte{'['})
-	for i, mf := range mfs {
-		if i > 0 {
-			w.Write([]byte{','})
-		}
-		jm.Marshal(w, mf)
-	}
-	w.Write([]byte{']'})
-}
-
-func (m *metrics) Instrument(next http.Handler) http.Handler {
-	return promhttp.InstrumentHandlerDuration(m.reqDuration, next)
-}
-
 // VolumeTypes is built up by init() funcs in the source files that
 // define the volume types.
 var VolumeTypes = []func() VolumeWithExamples{}
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index fb327a386..847486fbd 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -86,17 +86,10 @@ func MakeRESTRouter() http.Handler {
 	// 400 Bad Request.
 	rtr.NotFoundHandler = http.HandlerFunc(BadRequestHandler)
 
-	theConfig.metrics.setup()
-
 	rtr.limiter = httpserver.NewRequestLimiter(theConfig.MaxRequests, rtr)
 
-	mux := http.NewServeMux()
-	mux.Handle("/", theConfig.metrics.Instrument(
-		httpserver.AddRequestIDs(httpserver.LogRequests(nil, rtr.limiter))))
-	mux.HandleFunc("/metrics.json", theConfig.metrics.exportJSON)
-	mux.Handle("/metrics", theConfig.metrics.exportProm)
-
-	return mux
+	return httpserver.Instrument(nil,
+		httpserver.AddRequestIDs(httpserver.LogRequests(nil, rtr.limiter)))
 }
 
 // BadRequestHandler is a HandleFunc to address bad requests.
diff --git a/vendor/vendor.json b/vendor/vendor.json
index f18d4e464..aa6b2d773 100644
--- a/vendor/vendor.json
+++ b/vendor/vendor.json
@@ -252,26 +252,32 @@
 			"revisionTime": "2018-06-25T08:58:08Z"
 		},
 		{
+			"checksumSHA1": "8UEp6v0Dczw/SlasE0DivB0mAHA=",
+			"path": "github.com/gogo/protobuf/jsonpb",
+			"revision": "30cf7ac33676b5786e78c746683f0d4cd64fa75b",
+			"revisionTime": "2018-05-09T16:24:41Z"
+		},
+		{
 			"checksumSHA1": "wn2shNJMwRZpvuvkf1s7h0wvqHI=",
 			"path": "github.com/gogo/protobuf/proto",
 			"revision": "160de10b2537169b5ae3e7e221d28269ef40d311",
 			"revisionTime": "2018-01-04T10:21:28Z"
 		},
 		{
-			"checksumSHA1": "iVfdaLxIDjfk2KLP8dCMIbsxZZM=",
-			"path": "github.com/golang/protobuf/jsonpb",
-			"revision": "1e59b77b52bf8e4b449a57e6f79f21226d571845",
-			"revisionTime": "2017-11-13T18:07:20Z"
+			"checksumSHA1": "HPVQZu059/Rfw2bAWM538bVTcUc=",
+			"path": "github.com/gogo/protobuf/sortkeys",
+			"revision": "30cf7ac33676b5786e78c746683f0d4cd64fa75b",
+			"revisionTime": "2018-05-09T16:24:41Z"
 		},
 		{
-			"checksumSHA1": "yqF125xVSkmfLpIVGrLlfE05IUk=",
-			"path": "github.com/golang/protobuf/proto",
-			"revision": "1e59b77b52bf8e4b449a57e6f79f21226d571845",
-			"revisionTime": "2017-11-13T18:07:20Z"
+			"checksumSHA1": "SkxU1+wPGUJyLyQENrZtr2/OUBs=",
+			"path": "github.com/gogo/protobuf/types",
+			"revision": "30cf7ac33676b5786e78c746683f0d4cd64fa75b",
+			"revisionTime": "2018-05-09T16:24:41Z"
 		},
 		{
-			"checksumSHA1": "Ylq6kq3KWBy6mu68oyEwenhNMdg=",
-			"path": "github.com/golang/protobuf/ptypes/struct",
+			"checksumSHA1": "yqF125xVSkmfLpIVGrLlfE05IUk=",
+			"path": "github.com/golang/protobuf/proto",
 			"revision": "1e59b77b52bf8e4b449a57e6f79f21226d571845",
 			"revisionTime": "2017-11-13T18:07:20Z"
 		},

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list