From d173bcb9a920eb4c96f98eb029d5d85a3cc0be99 Mon Sep 17 00:00:00 2001 From: Matthias Loibl Date: Mon, 22 Mar 2021 15:05:50 +0100 Subject: [PATCH] Make NewInstrumentationMiddleware buckets configurable (#3948) * Make NewInstrumentationMiddleware buckets configurable Signed-off-by: Matthias Loibl * Add Changelog entry for #3948 Signed-off-by: Matthias Loibl --- CHANGELOG.md | 2 +- cmd/thanos/compact.go | 2 +- cmd/thanos/query.go | 2 +- cmd/thanos/query_frontend.go | 2 +- cmd/thanos/rule.go | 2 +- cmd/thanos/store.go | 2 +- cmd/thanos/tools_bucket.go | 2 +- pkg/extprom/http/instrument_server.go | 9 +++++++-- pkg/receive/handler.go | 4 +++- 9 files changed, 17 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a45f8f557..b087d0dc5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ We use _breaking :warning:_ to mark changes that are not backward compatible (re - [#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 diff --git a/cmd/thanos/compact.go b/cmd/thanos/compact.go index 19b6a1c80b..e8b532afa1 100644 --- 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 index fcac3abbed..5361ce5cca 100644 --- a/cmd/thanos/query.go +++ b/cmd/thanos/query.go @@ -496,7 +496,7 @@ func runQuery( // Configure Request Logging for HTTP calls. logMiddleware := logging.NewHTTPServerMiddleware(logger, httpLogOpts...) - 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 index b693e6fdda..68374b4500 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -213,7 +213,7 @@ func runQueryFrontend( // Configure Request Logging for HTTP calls. logMiddleware := logging.NewHTTPServerMiddleware(logger, httpLogOpts...) - 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 index 178452f988..e92ec70047 100644 --- a/cmd/thanos/rule.go +++ b/cmd/thanos/rule.go @@ -621,7 +621,7 @@ func runRule( } }) - ins := extpromhttp.NewInstrumentationMiddleware(reg) + ins := extpromhttp.NewInstrumentationMiddleware(reg, nil) // Configure Request Logging for HTTP calls. logMiddleware := logging.NewHTTPServerMiddleware(logger, httpLogOpts...) diff --git a/cmd/thanos/store.go b/cmd/thanos/store.go index e91f52e829..3538c94374 100644 --- a/cmd/thanos/store.go +++ b/cmd/thanos/store.go @@ -407,7 +407,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 index 41b1abc4ff..fa92ca739d 100644 --- a/cmd/thanos/tools_bucket.go +++ b/cmd/thanos/tools_bucket.go @@ -342,7 +342,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 index aa0445ed7d..df7217adfc 100644 --- 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, 240, 360, 720}, + Buckets: buckets, }, []string{"code", "handler", "method"}, ), diff --git a/pkg/receive/handler.go b/pkg/receive/handler.go index 0337fe495f..80a168f30c 100644 --- a/pkg/receive/handler.go +++ b/pkg/receive/handler.go @@ -150,7 +150,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