query/receive: Unify HTTP server instrumentation (#1458)

* Remove unncessary Query API metrics Signed-off-by: Kemal Akkoyun <[email protected]> * Refactor receive to use common instrumentation meiddleware Signed-off-by: Kemal Akkoyun <[email protected]> * Add changes to changelog Signed-off-by: Kemal Akkoyun <[email protected]> * Update changelog Signed-off-by: Kemal Akkoyun <[email protected]> * Update query metrics used in dashboards Signed-off-by: Kemal Akkoyun <[email protected]>
thanos-io · Aug 27, 2019 · c54c3f2 · c54c3f2
1 parent af53b38
commit c54c3f2
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 85 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ We use *breaking* word for marking changes that are not backward compatible (rel
 
 ### Added
 
+- [#1378](https://github.com/thanos-io/thanos/pull/1378) Thanos Receive now exposes `thanos_receive_config_hash`, `thanos_receive_config_last_reload_successful` and `thanos_receive_config_last_reload_success_timestamp_seconds` metrics to track latest configuration change
 - [#1358](https://github.com/thanos-io/thanos/pull/1358) Added `part_size` configuration option for HTTP multipart requests minimum part size for S3 storage type
 - [#1363](https://github.com/thanos-io/thanos/pull/1363) Thanos Receive now exposes `thanos_receive_hashring_nodes` and `thanos_receive_hashring_tenants` metrics to monitor status of hash-rings
 
@@ -32,6 +33,10 @@ We use *breaking* word for marking changes that are not backward compatible (rel
 - [#1327](https://github.com/thanos-io/thanos/pull/1327) `/series` API end-point now properly returns an empty array just like Prometheus if there are no results
 - [#1302](https://github.com/thanos-io/thanos/pull/1302) Thanos now efficiently reuses HTTP keep-alive connections
 
+### Deprecated
+
+- [#1458](https://github.com/thanos-io/thanos/pull/1458) Thanos Query and Receive now use common instrumentation middleware. As as result, for sake of `http_requests_total` and `http_request_duration_seconds_bucket`; Thanos Query no longer exposes `thanos_query_api_instant_query_duration_seconds`, `thanos_query_api_range_query_duration_second` metrics and Thanos Receive no longer exposes `thanos_http_request_duration_seconds`, `thanos_http_requests_total`, `thanos_http_response_size_bytes`.
+
 ## [v0.6.0](https://github.com/thanos-io/thanos/releases/tag/v0.6.0) - 2019.07.18
 
 ### Added

diff --git a/examples/grafana/thanos-query.json b/examples/grafana/thanos-query.json
@@ -306,14 +306,14 @@
       "steppedLine": false,
       "targets": [
         {
-          "expr": "histogram_quantile(0.9999, sum(rate(thanos_query_api_instant_query_duration_seconds_bucket{$labelselector=\"$labelvalue\",kubernetes_pod_name=~\"$pod\"}[$interval])) by (kubernetes_pod_name, le))",
+          "expr": "histogram_quantile(0.9999, sum(rate(http_request_duration_seconds_bucket{$labelselector=\"$labelvalue\",kubernetes_pod_name=~\"$pod\",handler=\"query\"}[$interval])) by (kubernetes_pod_name, le))",
           "format": "time_series",
           "intervalFactor": 1,
           "legendFormat": "instant_query {{kubernetes_pod_name}}",
           "refId": "A"
         },
         {
-          "expr": "histogram_quantile(0.9999, sum(rate(thanos_query_api_range_query_duration_seconds_bucket{$labelselector=\"$labelvalue\",kubernetes_pod_name=~\"$pod\"}[$interval])) by (kubernetes_pod_name, le))",
+          "expr": "histogram_quantile(0.9999, sum(rate(http_request_duration_seconds_bucket{$labelselector=\"$labelvalue\",kubernetes_pod_name=~\"$pod\",handler=\"query_range\"}[$interval])) by (kubernetes_pod_name, le))",
           "format": "time_series",
           "intervalFactor": 1,
           "legendFormat": "range_query {{kubernetes_pod_name}}",
@@ -1035,4 +1035,4 @@
   "title": "Thanos Query",
   "uid": "opwl5gSiz",
   "version": 2
-}
+}
diff --git a/pkg/extprom/http/instrument_server.go b/pkg/extprom/http/instrument_server.go
@@ -35,7 +35,7 @@ type defaultInstrumentationMiddleware struct {
 }
 
 // NewInstrumentationMiddleware provides default InstrumentationMiddleware.
-func NewInstrumentationMiddleware(reg *prometheus.Registry) InstrumentationMiddleware {
+func NewInstrumentationMiddleware(reg prometheus.Registerer) InstrumentationMiddleware {
 	ins := defaultInstrumentationMiddleware{
 		requestDuration: prometheus.NewHistogramVec(
 			prometheus.HistogramOpts{

diff --git a/pkg/query/api/v1.go b/pkg/query/api/v1.go
@@ -100,8 +100,6 @@ type API struct {
 	queryableCreate query.QueryableCreator
 	queryEngine     *promql.Engine
 
-	instantQueryDuration   prometheus.Histogram
-	rangeQueryDuration     prometheus.Histogram
 	enableAutodownsampling bool
 	enablePartialResponse  bool
 	reg                    prometheus.Registerer
@@ -118,31 +116,10 @@ func NewAPI(
 	enableAutodownsampling bool,
 	enablePartialResponse bool,
 ) *API {
-	instantQueryDuration := prometheus.NewHistogram(prometheus.HistogramOpts{
-		Name: "thanos_query_api_instant_query_duration_seconds",
-		Help: "Time it takes to perform instant query on promEngine backed up with thanos querier.",
-		Buckets: []float64{
-			0.05, 0.1, 0.25, 0.6, 1, 2, 3.5, 5, 7.5, 10, 15, 20,
-		},
-	})
-	rangeQueryDuration := prometheus.NewHistogram(prometheus.HistogramOpts{
-		Name: "thanos_query_api_range_query_duration_seconds",
-		Help: "Time it takes to perform range query on promEngine backed up with thanos querier.",
-		Buckets: []float64{
-			0.05, 0.1, 0.25, 0.6, 1, 2, 3.5, 5, 7.5, 10, 15, 20,
-		},
-	})
-
-	reg.MustRegister(
-		instantQueryDuration,
-		rangeQueryDuration,
-	)
 	return &API{
 		logger:                 logger,
 		queryEngine:            qe,
 		queryableCreate:        c,
-		instantQueryDuration:   instantQueryDuration,
-		rangeQueryDuration:     rangeQueryDuration,
 		enableAutodownsampling: enableAutodownsampling,
 		enablePartialResponse:  enablePartialResponse,
 		reg:                    reg,
@@ -284,7 +261,6 @@ func (api *API) query(r *http.Request) (interface{}, []error, *ApiError) {
 	span, ctx := tracing.StartSpan(ctx, "promql_instant_query")
 	defer span.Finish()
 
-	begin := api.now()
 	qry, err := api.queryEngine.NewInstantQuery(api.queryableCreate(enableDedup, 0, enablePartialResponse), r.FormValue("query"), ts)
 	if err != nil {
 		return nil, nil, &ApiError{errorBadData, err}
@@ -302,7 +278,6 @@ func (api *API) query(r *http.Request) (interface{}, []error, *ApiError) {
 		}
 		return nil, nil, &ApiError{errorExec, res.Err}
 	}
-	api.instantQueryDuration.Observe(time.Since(begin).Seconds())
 
 	return &queryData{
 		ResultType: res.Value.Type(),
@@ -372,7 +347,6 @@ func (api *API) queryRange(r *http.Request) (interface{}, []error, *ApiError) {
 	span, ctx := tracing.StartSpan(ctx, "promql_range_query")
 	defer span.Finish()
 
-	begin := api.now()
 	qry, err := api.queryEngine.NewRangeQuery(
 		api.queryableCreate(enableDedup, maxSourceResolution, enablePartialResponse),
 		r.FormValue("query"),
@@ -394,7 +368,6 @@ func (api *API) queryRange(r *http.Request) (interface{}, []error, *ApiError) {
 		}
 		return nil, nil, &ApiError{errorExec, res.Err}
 	}
-	api.rangeQueryDuration.Observe(time.Since(begin).Seconds())
 
 	return &queryData{
 		ResultType: res.Value.Type(),

diff --git a/pkg/query/api/v1_test.go b/pkg/query/api/v1_test.go
@@ -31,7 +31,6 @@ import (
 
 	"github.com/go-kit/kit/log"
 	opentracing "github.com/opentracing/opentracing-go"
-	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/common/route"
 	"github.com/prometheus/prometheus/pkg/labels"
 	"github.com/prometheus/prometheus/pkg/timestamp"
@@ -71,9 +70,6 @@ func TestEndpoints(t *testing.T) {
 		queryableCreate: testQueryableCreator(suite.Storage()),
 		queryEngine:     suite.QueryEngine(),
 
-		instantQueryDuration: prometheus.NewHistogram(prometheus.HistogramOpts{}),
-		rangeQueryDuration:   prometheus.NewHistogram(prometheus.HistogramOpts{}),
-
 		now: func() time.Time { return now },
 	}
 

diff --git a/pkg/receive/handler.go b/pkg/receive/handler.go
@@ -21,11 +21,11 @@ import (
 	opentracing "github.com/opentracing/opentracing-go"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/prometheus/common/route"
 	"github.com/prometheus/prometheus/prompb"
 	promtsdb "github.com/prometheus/prometheus/storage/tsdb"
 	terrors "github.com/prometheus/prometheus/tsdb/errors"
+	extpromhttp "github.com/thanos-io/thanos/pkg/extprom/http"
 	"github.com/thanos-io/thanos/pkg/runutil"
 )
 
@@ -54,9 +54,6 @@ type Handler struct {
 	hashring Hashring
 
 	// Metrics
-	requestDuration      *prometheus.HistogramVec
-	requestsTotal        *prometheus.CounterVec
-	responseSize         *prometheus.HistogramVec
 	forwardRequestsTotal *prometheus.CounterVec
 
 	// These fields are uint32 rather than boolean to be able to use atomic functions.
@@ -72,29 +69,8 @@ func NewHandler(logger log.Logger, o *Options) *Handler {
 		logger:       logger,
 		readyStorage: o.ReadyStorage,
 		receiver:     o.Receiver,
+		router:       route.New(),
 		options:      o,
-		requestDuration: prometheus.NewHistogramVec(
-			prometheus.HistogramOpts{
-				Name:    "thanos_http_request_duration_seconds",
-				Help:    "Histogram of latencies for HTTP requests.",
-				Buckets: []float64{.1, .2, .4, 1, 3, 8, 20, 60, 120},
-			},
-			[]string{"handler"},
-		),
-		requestsTotal: prometheus.NewCounterVec(
-			prometheus.CounterOpts{
-				Name: "thanos_http_requests_total",
-				Help: "Tracks the number of HTTP requests.",
-			}, []string{"code", "handler", "method"},
-		),
-		responseSize: prometheus.NewHistogramVec(
-			prometheus.HistogramOpts{
-				Name:    "thanos_http_response_size_bytes",
-				Help:    "Histogram of response size for HTTP requests.",
-				Buckets: prometheus.ExponentialBuckets(100, 10, 8),
-			},
-			[]string{"handler"},
-		),
 		forwardRequestsTotal: prometheus.NewCounterVec(
 			prometheus.CounterOpts{
 				Name: "thanos_receive_forward_requests_total",
@@ -103,35 +79,20 @@ func NewHandler(logger log.Logger, o *Options) *Handler {
 		),
 	}
 
-	router := route.New().WithInstrumentation(h.instrumentHandler)
-	h.router = router
+	ins := extpromhttp.NewNopInstrumentationMiddleware()
+	if o.Registry != nil {
+		ins = extpromhttp.NewInstrumentationMiddleware(o.Registry)
+		o.Registry.MustRegister(h.forwardRequestsTotal)
+	}
 
 	readyf := h.testReady
-	router.Post("/api/v1/receive", readyf(h.receive))
-
-	if o.Registry != nil {
-		o.Registry.MustRegister(
-			h.requestDuration,
-			h.requestsTotal,
-			h.responseSize,
-			h.forwardRequestsTotal,
-		)
+	instrf := func(name string, next func(w http.ResponseWriter, r *http.Request)) http.HandlerFunc {
+		return ins.NewHandler(name, http.HandlerFunc(next))
 	}
 
-	return h
-}
+	h.router.Post("/api/v1/receive", instrf("receive", readyf(h.receive)))
 
-func (h *Handler) instrumentHandler(handlerName string, handler http.HandlerFunc) http.HandlerFunc {
-	return promhttp.InstrumentHandlerDuration(
-		h.requestDuration.MustCurryWith(prometheus.Labels{"handler": handlerName}),
-		promhttp.InstrumentHandlerResponseSize(
-			h.responseSize.MustCurryWith(prometheus.Labels{"handler": handlerName}),
-			promhttp.InstrumentHandlerCounter(
-				h.requestsTotal.MustCurryWith(prometheus.Labels{"handler": handlerName}),
-				handler,
-			),
-		),
-	)
+	return h
 }
 
 // StorageReady marks the storage as ready.