Make NewInstrumentationMiddleware buckets configurable (thanos-io#3948)

* Make NewInstrumentationMiddleware buckets configurable Signed-off-by: Matthias Loibl <mail@matthiasloibl.com> * Add Changelog entry for thanos-io#3948 Signed-off-by: Matthias Loibl <mail@matthiasloibl.com>
stolostron · Apr 27, 2021 · c2f2b83 · c2f2b83
1 parent e00c127
commit c2f2b83
Show file tree

Hide file tree

Showing 9 changed files with 30 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,15 +14,20 @@ We use _breaking :warning:_ to mark changes that are not backward compatible (re
 
 ### Added
 
-- [#3700](https://github.com/thanos-io/thanos/pull/3700) Compact/Web: Make old bucket viewer UI work with vanilla Prometheus blocks.
-- [#3657](https://github.com/thanos-io/thanos/pull/3657) *: It's now possible to configure HTTP transport options for S3 client.
-- [#3752](https://github.com/thanos-io/thanos/pull/3752) Compact/Store: Added `--block-meta-fetch-concurrency` allowing to configure number of go routines for block metadata synchronization.
-- [#3723](https://github.com/thanos-io/thanos/pull/3723) Query Frontend: Added `--query-range.request-downsampled` flag enabling additional queries for downsampled data in case of empty or incomplete response to range request.
-- [#3579](https://github.com/thanos-io/thanos/pull/3579) Cache: Added inmemory cache for caching bucket.
-- [#3792](https://github.com/thanos-io/thanos/pull/3792) Receiver: Added `--tsdb.allow-overlapping-blocks` flag to allow overlapping tsdb blocks and enable vertical compaction.
-- [#3740](https://github.com/thanos-io/thanos/pull/3740) Query: Added `--query.default-step` flag to set default step. Useful when your tenant scrape interval is stable and far from default UI's 1s.
-- [#3686](https://github.com/thanos-io/thanos/pull/3686) Query/Sidecar: Added metric metadata API support. You can now configure you Querier to fetch Prometheus metrics metadata from leaf Prometheus-es!
-- [#3031](https://github.com/thanos-io/thanos/pull/3031) Compact/Sidecar/Receive/Rule: Added `--hash-func`. If some function has been specified, writers calculate hashes using that function of each file in a block before uploading them. If those hashes exist in the `meta.json` file then Compact does not download the files if they already exist on disk and with the same hash. This also means that the data directory passed to Thanos Compact is only *cleared once at boot* or *if everything succeeds*. So, if you, for example, use persistent volumes on k8s and your Thanos Compact crashes or fails to make an iteration properly then the last downloaded files are not wiped from the disk. The directories that were created the last time are only wiped again after a successful iteration or if the previously picked up blocks have disappeared.
+### Fixed
+
+- [#3204](https://github.com/thanos-io/thanos/pull/3204) Mixin: Use sidecar's metric timestamp for healthcheck.
+- [#3922](https://github.com/thanos-io/thanos/pull/3922) Fix panic in http logging middleware.
+
+### Changed
+- [#3948](https://github.com/thanos-io/thanos/pull/3948) Receiver: Adjust `http_request_duration_seconds` buckets for low latency requests.
+- [#3856](https://github.com/thanos-io/thanos/pull/3856) Mixin: _breaking :warning:_ Introduce flexible multi-cluster/namespace mode for alerts and dashboards. Removes jobPrefix config option. Removes `namespace` by default.
+
+### Removed
+
+## [v0.19.0 - <in progress>](https://github.com/thanos-io/thanos/tree/release-0.19)
+
+### Added
 
 ### Fixed
 

diff --git a/cmd/thanos/compact.go b/cmd/thanos/compact.go
@@ -457,7 +457,7 @@ func runCompact(
 	if conf.wait {
 		r := route.New()
 
-		ins := extpromhttp.NewInstrumentationMiddleware(reg)
+		ins := extpromhttp.NewInstrumentationMiddleware(reg, nil)
 		compactorView.Register(r, true, ins)
 
 		global := ui.NewBucketUI(logger, conf.label, conf.webConf.externalPrefix, conf.webConf.prefixHeaderName, "/global", component)

diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go
@@ -476,7 +476,7 @@ func runQuery(
 		})}
 		logMiddleware := logging.NewHTTPServerMiddleware(logger, opts...)
 
-		ins := extpromhttp.NewInstrumentationMiddleware(reg)
+		ins := extpromhttp.NewInstrumentationMiddleware(reg, nil)
 		// TODO(bplotka in PR #513 review): pass all flags, not only the flags needed by prefix rewriting.
 		ui.NewQueryUI(logger, stores, webExternalPrefix, webPrefixHeaderName).Register(router, ins)
 

diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go
@@ -205,7 +205,7 @@ func runQueryFrontend(
 		return logging.LogDecision[cfg.RequestLoggingDecision]
 	})}
 	logMiddleware := logging.NewHTTPServerMiddleware(logger, opts...)
-	ins := extpromhttp.NewInstrumentationMiddleware(reg)
+	ins := extpromhttp.NewInstrumentationMiddleware(reg, nil)
 
 	// Start metrics HTTP server.
 	{

diff --git a/cmd/thanos/rule.go b/cmd/thanos/rule.go
@@ -600,7 +600,7 @@ func runRule(
 			}
 		})
 
-		ins := extpromhttp.NewInstrumentationMiddleware(reg)
+		ins := extpromhttp.NewInstrumentationMiddleware(reg, nil)
 
 		// Configure Request Logging for HTTP calls.
 		opts := []logging.Option{logging.WithDecider(func() logging.Decision {

diff --git a/cmd/thanos/store.go b/cmd/thanos/store.go
@@ -385,7 +385,7 @@ func runStore(
 	// Add bucket UI for loaded blocks.
 	{
 		r := route.New()
-		ins := extpromhttp.NewInstrumentationMiddleware(reg)
+		ins := extpromhttp.NewInstrumentationMiddleware(reg, nil)
 
 		compactorView := ui.NewBucketUI(logger, "", externalPrefix, prefixHeader, "/loaded", component)
 		compactorView.Register(r, true, ins)

diff --git a/cmd/thanos/tools_bucket.go b/cmd/thanos/tools_bucket.go
@@ -341,7 +341,7 @@ func registerBucketWeb(app extkingpin.AppClause, objStoreConfig *extflag.PathOrC
 		)
 
 		router := route.New()
-		ins := extpromhttp.NewInstrumentationMiddleware(reg)
+		ins := extpromhttp.NewInstrumentationMiddleware(reg, nil)
 
 		bucketUI := ui.NewBucketUI(logger, *label, *webExternalPrefix, *webPrefixHeaderName, "", component.Bucket)
 		bucketUI.Register(router, true, ins)

diff --git a/pkg/extprom/http/instrument_server.go b/pkg/extprom/http/instrument_server.go
@@ -39,13 +39,18 @@ type defaultInstrumentationMiddleware struct {
 }
 
 // NewInstrumentationMiddleware provides default InstrumentationMiddleware.
-func NewInstrumentationMiddleware(reg prometheus.Registerer) InstrumentationMiddleware {
+// Passing nil as buckets uses the default buckets.
+func NewInstrumentationMiddleware(reg prometheus.Registerer, buckets []float64) InstrumentationMiddleware {
+	if buckets == nil {
+		buckets = []float64{0.001, 0.01, 0.1, 0.3, 0.6, 1, 3, 6, 9, 20, 30, 60, 90, 120, 240, 360, 720}
+	}
+
 	ins := defaultInstrumentationMiddleware{
 		requestDuration: promauto.With(reg).NewHistogramVec(
 			prometheus.HistogramOpts{
 				Name:    "http_request_duration_seconds",
 				Help:    "Tracks the latencies for HTTP requests.",
-				Buckets: []float64{0.001, 0.01, 0.1, 0.3, 0.6, 1, 3, 6, 9, 20, 30, 60, 90, 120},
+				Buckets: buckets,
 			},
 			[]string{"code", "handler", "method"},
 		),

diff --git a/pkg/receive/handler.go b/pkg/receive/handler.go
@@ -151,7 +151,9 @@ func NewHandler(logger log.Logger, o *Options) *Handler {
 
 	ins := extpromhttp.NewNopInstrumentationMiddleware()
 	if o.Registry != nil {
-		ins = extpromhttp.NewInstrumentationMiddleware(o.Registry)
+		ins = extpromhttp.NewInstrumentationMiddleware(o.Registry,
+			[]float64{0.001, 0.005, 0.01, 0.02, 0.03, 0.04, 0.05, 0.06, 0.07, 0.08, 0.09, 0.1, 0.25, 0.5, 0.75, 1, 2, 3, 4, 5},
+		)
 	}
 
 	readyf := h.testReady