From d41dcd87513fd6b31b9ab54dbcef842bc627be6d Mon Sep 17 00:00:00 2001 From: Jacob Baungard Hansen Date: Mon, 20 Mar 2023 10:11:43 +0100 Subject: [PATCH] Proxy: Only generate debug messages in debug mode In debug mode we log information about which endpoints we skip and query, including the labels of each endpoint. These debug messages, can use significant memory, especially in larger setups with larger number of labelsets. Previously we would generate the debug messages, even when not in debug mode. With commit, we only generate the debug messages when debug logging is enabled. This fixes: https://github.com/thanos-io/thanos/issues/6198 Signed-off-by: Jacob Baungard Hansen --- cmd/thanos/query.go | 6 ++++-- cmd/thanos/receive.go | 5 ++++- pkg/api/query/grpc_test.go | 2 +- pkg/api/query/v1_test.go | 1 + pkg/query/querier_test.go | 1 + pkg/query/query_test.go | 2 +- pkg/store/proxy.go | 12 +++++++++--- pkg/store/proxy_test.go | 7 +++++++ 8 files changed, 28 insertions(+), 8 deletions(-) diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go index 5275fef55d3..a1e12e8ec1f 100644 --- a/cmd/thanos/query.go +++ b/cmd/thanos/query.go @@ -229,7 +229,7 @@ func registerQuery(app *extkingpin.App) { var storeRateLimits store.SeriesSelectLimits storeRateLimits.RegisterFlags(cmd) - cmd.Setup(func(g *run.Group, logger log.Logger, reg *prometheus.Registry, tracer opentracing.Tracer, _ <-chan struct{}, _ bool) error { + cmd.Setup(func(g *run.Group, logger log.Logger, reg *prometheus.Registry, tracer opentracing.Tracer, _ <-chan struct{}, debug bool) error { selectorLset, err := parseFlagLabels(*selectorLabels) if err != nil { return errors.Wrap(err, "parse federation labels") @@ -278,6 +278,7 @@ func registerQuery(app *extkingpin.App) { return runQuery( g, logger, + debug, reg, tracer, httpLogOpts, @@ -353,6 +354,7 @@ func registerQuery(app *extkingpin.App) { func runQuery( g *run.Group, logger log.Logger, + debug bool, reg *prometheus.Registry, tracer opentracing.Tracer, httpLogOpts []logging.Option, @@ -541,7 +543,7 @@ func runQuery( endpointInfoTimeout, queryConnMetricLabels..., ) - proxy = store.NewProxyStore(logger, reg, endpoints.GetStoreClients, component.Query, selectorLset, storeResponseTimeout, store.RetrievalStrategy(grpcProxyStrategy)) + proxy = store.NewProxyStore(logger, debug, reg, endpoints.GetStoreClients, component.Query, selectorLset, storeResponseTimeout, store.RetrievalStrategy(grpcProxyStrategy)) rulesProxy = rules.NewProxy(logger, endpoints.GetRulesClients) targetsProxy = targets.NewProxy(logger, endpoints.GetTargetsClients) metadataProxy = metadata.NewProxy(logger, endpoints.GetMetricMetadataClients) diff --git a/cmd/thanos/receive.go b/cmd/thanos/receive.go index 3c3b4da214d..93f46664292 100644 --- a/cmd/thanos/receive.go +++ b/cmd/thanos/receive.go @@ -58,7 +58,7 @@ func registerReceive(app *extkingpin.App) { conf := &receiveConfig{} conf.registerFlag(cmd) - cmd.Setup(func(g *run.Group, logger log.Logger, reg *prometheus.Registry, tracer opentracing.Tracer, _ <-chan struct{}, _ bool) error { + cmd.Setup(func(g *run.Group, logger log.Logger, reg *prometheus.Registry, tracer opentracing.Tracer, _ <-chan struct{}, debug bool) error { lset, err := parseFlagLabels(conf.labelStrs) if err != nil { return errors.Wrap(err, "parse labels") @@ -97,6 +97,7 @@ func registerReceive(app *extkingpin.App) { return runReceive( g, logger, + debug, reg, tracer, grpcLogOpts, tagOpts, @@ -113,6 +114,7 @@ func registerReceive(app *extkingpin.App) { func runReceive( g *run.Group, logger log.Logger, + debug bool, reg *prometheus.Registry, tracer opentracing.Tracer, grpcLogOpts []grpc_logging.Option, @@ -307,6 +309,7 @@ func runReceive( proxy := store.NewProxyStore( logger, + debug, reg, dbs.TSDBLocalClients, comp, diff --git a/pkg/api/query/grpc_test.go b/pkg/api/query/grpc_test.go index 9802aa1a41f..98d0c19bd44 100644 --- a/pkg/api/query/grpc_test.go +++ b/pkg/api/query/grpc_test.go @@ -25,7 +25,7 @@ import ( func TestGRPCQueryAPIErrorHandling(t *testing.T) { logger := log.NewNopLogger() reg := prometheus.NewRegistry() - proxy := store.NewProxyStore(logger, reg, func() []store.Client { return nil }, component.Store, nil, 1*time.Minute, store.LazyRetrieval) + proxy := store.NewProxyStore(logger, false, reg, func() []store.Client { return nil }, component.Store, nil, 1*time.Minute, store.LazyRetrieval) queryableCreator := query.NewQueryableCreator(logger, reg, proxy, 1, 1*time.Minute) lookbackDeltaFunc := func(i int64) time.Duration { return 5 * time.Minute } tests := []struct { diff --git a/pkg/api/query/v1_test.go b/pkg/api/query/v1_test.go index edacc7c6b9c..961931c3897 100644 --- a/pkg/api/query/v1_test.go +++ b/pkg/api/query/v1_test.go @@ -652,6 +652,7 @@ func newProxyStoreWithTSDBStore(db store.TSDBReader) *store.ProxyStore { return store.NewProxyStore( nil, + false, nil, func() []store.Client { return []store.Client{c} }, component.Query, diff --git a/pkg/query/querier_test.go b/pkg/query/querier_test.go index 65cf841381b..c34ef37e8a7 100644 --- a/pkg/query/querier_test.go +++ b/pkg/query/querier_test.go @@ -847,6 +847,7 @@ func newProxyStore(storeAPIs ...storepb.StoreServer) *store.ProxyStore { return store.NewProxyStore( nil, + false, nil, func() []store.Client { return cls }, component.Query, diff --git a/pkg/query/query_test.go b/pkg/query/query_test.go index 35949377e81..eee5e08de84 100644 --- a/pkg/query/query_test.go +++ b/pkg/query/query_test.go @@ -36,7 +36,7 @@ func TestQuerier_Proxy(t *testing.T) { q := NewQueryableCreator( logger, nil, - store.NewProxyStore(logger, nil, func() []store.Client { return clients }, + store.NewProxyStore(logger, false, nil, func() []store.Client { return clients }, component.Debug, nil, 5*time.Minute, store.EagerRetrieval), 1000000, 5*time.Minute, diff --git a/pkg/store/proxy.go b/pkg/store/proxy.go index 08c4e06a2e6..67620f5a8a6 100644 --- a/pkg/store/proxy.go +++ b/pkg/store/proxy.go @@ -68,6 +68,7 @@ type Client interface { // ProxyStore implements the store API that proxies request to all given underlying stores. type ProxyStore struct { logger log.Logger + debug bool stores func() []Client component component.StoreAPI selectorLabels labels.Labels @@ -103,6 +104,7 @@ func RegisterStoreServer(storeSrv storepb.StoreServer, logger log.Logger) func(* // Note that there is no deduplication support. Deduplication should be done on the highest level (just before PromQL). func NewProxyStore( logger log.Logger, + debug bool, reg prometheus.Registerer, stores func() []Client, component component.StoreAPI, @@ -117,6 +119,7 @@ func NewProxyStore( metrics := newProxyStoreMetrics(reg) s := &ProxyStore{ logger: logger, + debug: debug, stores: stores, component: component, selectorLabels: selectorLabels, @@ -273,7 +276,9 @@ func (s *ProxyStore) Series(originalRequest *storepb.SeriesRequest, srv storepb. for _, st := range s.stores() { // We might be able to skip the store if its meta information indicates it cannot have series matching our query. if ok, reason := storeMatches(srv.Context(), st, originalRequest.MinTime, originalRequest.MaxTime, matchers...); !ok { - storeDebugMsgs = append(storeDebugMsgs, fmt.Sprintf("store %s filtered out: %v", st, reason)) + if s.debug { + storeDebugMsgs = append(storeDebugMsgs, fmt.Sprintf("store %s filtered out: %v", st, reason)) + } continue } @@ -289,8 +294,9 @@ func (s *ProxyStore) Series(originalRequest *storepb.SeriesRequest, srv storepb. for _, st := range stores { st := st - - storeDebugMsgs = append(storeDebugMsgs, fmt.Sprintf("store %s queried", st)) + if s.debug { + storeDebugMsgs = append(storeDebugMsgs, fmt.Sprintf("store %s queried", st)) + } respSet, err := newAsyncRespSet(srv.Context(), st, r, s.responseTimeout, s.retrievalStrategy, &s.buffers, r.ShardInfo, reqLogger, s.metrics.emptyStreamResponses) if err != nil { diff --git a/pkg/store/proxy_test.go b/pkg/store/proxy_test.go index 61d9bc1965d..4e05fa9f5d3 100644 --- a/pkg/store/proxy_test.go +++ b/pkg/store/proxy_test.go @@ -62,6 +62,7 @@ func TestProxyStore_Info(t *testing.T) { defer cancel() q := NewProxyStore(nil, + false, nil, func() []Client { return nil }, component.Query, @@ -603,6 +604,7 @@ func TestProxyStore_Series(t *testing.T) { for _, strategy := range []RetrievalStrategy{EagerRetrieval, LazyRetrieval} { t.Run(string(strategy), func(t *testing.T) { q := NewProxyStore(nil, + false, nil, func() []Client { return tc.storeAPIs }, component.Query, @@ -1136,6 +1138,7 @@ func TestProxyStore_SeriesSlowStores(t *testing.T) { for _, strategy := range []RetrievalStrategy{EagerRetrieval, LazyRetrieval} { if ok := t.Run(string(strategy), func(t *testing.T) { q := NewProxyStore(nil, + false, nil, func() []Client { return tc.storeAPIs }, component.Query, @@ -1194,6 +1197,7 @@ func TestProxyStore_Series_RequestParamsProxied(t *testing.T) { }, } q := NewProxyStore(nil, + false, nil, func() []Client { return cls }, component.Query, @@ -1255,6 +1259,7 @@ func TestProxyStore_Series_RegressionFillResponseChannel(t *testing.T) { } q := NewProxyStore(nil, + false, nil, func() []Client { return cls }, component.Query, @@ -1302,6 +1307,7 @@ func TestProxyStore_LabelValues(t *testing.T) { }, } q := NewProxyStore(nil, + false, nil, func() []Client { return cls }, component.Query, @@ -1502,6 +1508,7 @@ func TestProxyStore_LabelNames(t *testing.T) { if ok := t.Run(tc.title, func(t *testing.T) { q := NewProxyStore( nil, + false, nil, func() []Client { return tc.storeAPIs }, component.Query,