From a07ff8e541b784a33bca53864c63f3336adb2f76 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 5 Jan 2018 15:03:15 +0000 Subject: [PATCH 1/2] Extend default histogram buckets up to 100 seconds Previously we would clamp at 10 seconds, giving a misleading impression for things that can take a lot longer to respond. --- Gopkg.lock | 2 +- common/monitoring.go | 5 +++-- users/api/services.go | 2 +- vendor/github.com/weaveworks/common/instrument/instrument.go | 4 ++++ vendor/github.com/weaveworks/common/server/server.go | 4 +++- 5 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 9298dc3b2..863c545a8 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -540,7 +540,7 @@ branch = "master" name = "github.com/weaveworks/common" packages = ["errors","http/client","httpgrpc","httpgrpc/server","instrument","logging","mflag","mflagext","middleware","mtime","server","signals","user"] - revision = "92029dbfb76a2b715629d9f7e68e62574028df22" + revision = "d85bab03a1b1327e94db89d64a84dfd1e79f193f" [[projects]] branch = "master" diff --git a/common/monitoring.go b/common/monitoring.go index fd391a3d1..040096054 100644 --- a/common/monitoring.go +++ b/common/monitoring.go @@ -2,6 +2,7 @@ package common import ( "github.com/prometheus/client_golang/prometheus" + "github.com/weaveworks/common/instrument" ) const ( @@ -15,7 +16,7 @@ var ( Namespace: PrometheusNamespace, Name: "request_duration_seconds", Help: "Time (in seconds) spent serving HTTP requests.", - Buckets: prometheus.DefBuckets, + Buckets: instrument.DefBuckets, }, []string{"method", "route", "status_code", "ws"}) // DatabaseRequestDuration is our standard database histogram vector. @@ -23,6 +24,6 @@ var ( Namespace: PrometheusNamespace, Name: "database_request_duration_seconds", Help: "Time spent (in seconds) doing database requests.", - Buckets: prometheus.DefBuckets, + Buckets: instrument.DefBuckets, }, []string{"method", "status_code"}) ) diff --git a/users/api/services.go b/users/api/services.go index fe026b97b..4822f47bf 100644 --- a/users/api/services.go +++ b/users/api/services.go @@ -239,7 +239,7 @@ var ServiceStatusRequestDuration = prometheus.NewHistogramVec(prometheus.Histogr Namespace: "users", Name: "get_service_status_request_duration_seconds", Help: "Time spent (in seconds) doing service status requests.", - Buckets: prometheus.DefBuckets, + Buckets: instrument.DefBuckets, }, []string{"service_name", "status_code"}) func errorCode(err error) string { diff --git a/vendor/github.com/weaveworks/common/instrument/instrument.go b/vendor/github.com/weaveworks/common/instrument/instrument.go index 63f6b4964..c993c4eb1 100644 --- a/vendor/github.com/weaveworks/common/instrument/instrument.go +++ b/vendor/github.com/weaveworks/common/instrument/instrument.go @@ -11,6 +11,10 @@ import ( oldcontext "golang.org/x/net/context" ) +// DefBuckets are histogram buckets for the response time (in seconds) +// of a network service, including one that is responding very slowly. +var DefBuckets = []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10, 25, 50, 100} + // Collector describes something that collects data before and/or after a task. type Collector interface { Register() diff --git a/vendor/github.com/weaveworks/common/server/server.go b/vendor/github.com/weaveworks/common/server/server.go index 09df87a0d..cf4dd3f19 100644 --- a/vendor/github.com/weaveworks/common/server/server.go +++ b/vendor/github.com/weaveworks/common/server/server.go @@ -21,6 +21,7 @@ import ( "github.com/weaveworks-experiments/loki/pkg/client" "github.com/weaveworks/common/httpgrpc" httpgrpc_server "github.com/weaveworks/common/httpgrpc/server" + "github.com/weaveworks/common/instrument" "github.com/weaveworks/common/middleware" "github.com/weaveworks/common/signals" ) @@ -95,7 +96,7 @@ func New(cfg Config) (*Server, error) { Namespace: cfg.MetricsNamespace, Name: "request_duration_seconds", Help: "Time (in seconds) spent serving HTTP requests.", - Buckets: prometheus.DefBuckets, + Buckets: instrument.DefBuckets, }, []string{"method", "route", "status_code", "ws"}) prometheus.MustRegister(requestDuration) @@ -110,6 +111,7 @@ func New(cfg Config) (*Server, error) { grpc.UnaryInterceptor(grpc_middleware.ChainUnaryServer( grpcMiddleware..., )), + grpc.RPCDecompressor(grpc.NewGZIPDecompressor()), ) // Setup HTTP server From 608b348da790b532090f9add54cdc7cd39503fdf Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sun, 28 Jan 2018 11:07:45 +0000 Subject: [PATCH 2/2] Update weaveworks/common for buckets and error-logging changes This brings in the merged commits for: - #81 "Extend histogram buckets for a server up to 100 seconds" - #83 "Let servers set any gRPC options they like " - #84 "Don't log request body and headers for 502 errors" (#82 was relaced by #83) --- Gopkg.lock | 2 +- vendor/github.com/weaveworks/common/middleware/logging.go | 2 +- vendor/github.com/weaveworks/common/server/server.go | 8 +++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 863c545a8..17ec752a1 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -540,7 +540,7 @@ branch = "master" name = "github.com/weaveworks/common" packages = ["errors","http/client","httpgrpc","httpgrpc/server","instrument","logging","mflag","mflagext","middleware","mtime","server","signals","user"] - revision = "d85bab03a1b1327e94db89d64a84dfd1e79f193f" + revision = "61d550eccac1e7a022723f7e36386206282913ef" [[projects]] branch = "master" diff --git a/vendor/github.com/weaveworks/common/middleware/logging.go b/vendor/github.com/weaveworks/common/middleware/logging.go index c7000c4d1..4e3f739c8 100644 --- a/vendor/github.com/weaveworks/common/middleware/logging.go +++ b/vendor/github.com/weaveworks/common/middleware/logging.go @@ -36,7 +36,7 @@ func (l Log) Wrap(next http.Handler) http.Handler { wrapped := newBadResponseLoggingWriter(w, &buf) next.ServeHTTP(wrapped, r) statusCode := wrapped.statusCode - if 100 <= statusCode && statusCode < 500 { + if 100 <= statusCode && statusCode < 500 || statusCode == 502 { logWithRequest(r).Debugf("%s %s (%d) %s", r.Method, uri, statusCode, time.Since(begin)) if l.LogRequestHeaders && headers != nil { logWithRequest(r).Debugf("Is websocket request: %v\n%s", IsWSHandshakeRequest(r), string(headers)) diff --git a/vendor/github.com/weaveworks/common/server/server.go b/vendor/github.com/weaveworks/common/server/server.go index cf4dd3f19..5feb7c452 100644 --- a/vendor/github.com/weaveworks/common/server/server.go +++ b/vendor/github.com/weaveworks/common/server/server.go @@ -48,6 +48,7 @@ type Config struct { HTTPServerWriteTimeout time.Duration HTTPServerIdleTimeout time.Duration + GRPCOptions []grpc.ServerOption GRPCMiddleware []grpc.UnaryServerInterceptor HTTPMiddleware []middleware.Interface } @@ -107,12 +108,13 @@ func New(cfg Config) (*Server, error) { otgrpc.OpenTracingServerInterceptor(opentracing.GlobalTracer()), } grpcMiddleware = append(grpcMiddleware, cfg.GRPCMiddleware...) - grpcServer := grpc.NewServer( + grpcOptions := []grpc.ServerOption{ grpc.UnaryInterceptor(grpc_middleware.ChainUnaryServer( grpcMiddleware..., )), - grpc.RPCDecompressor(grpc.NewGZIPDecompressor()), - ) + } + grpcOptions = append(grpcOptions, cfg.GRPCOptions...) + grpcServer := grpc.NewServer(grpcOptions...) // Setup HTTP server router := mux.NewRouter()