Skip to content

Commit

Permalink
Proxy: Only generate debug messages in debug mode
Browse files Browse the repository at this point in the history 
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: #6198

Signed-off-by: Jacob Baungard Hansen <jachanse@redhat.com>
  • Loading branch information
@jacobbaungard
jacobbaungard committed Mar 20, 2023
1 parent 36de497 commit d41dcd8
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 8 deletions.
6 changes: 4 additions & 2 deletions cmd/thanos/query.go
View file
Expand Up @@ -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")
Expand Down Expand Up @@ -278,6 +278,7 @@ func registerQuery(app *extkingpin.App) {
return runQuery(
g,
logger,
debug,
reg,
tracer,
httpLogOpts,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion cmd/thanos/receive.go
View file
Expand Up @@ -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")
Expand Down Expand Up @@ -97,6 +97,7 @@ func registerReceive(app *extkingpin.App) {
return runReceive(
g,
logger,
debug,
reg,
tracer,
grpcLogOpts, tagOpts,
Expand All @@ -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,
Expand Down Expand Up @@ -307,6 +309,7 @@ func runReceive(

proxy := store.NewProxyStore(
logger,
debug,
reg,
dbs.TSDBLocalClients,
comp,
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/query/grpc_test.go
View file
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions pkg/api/query/v1_test.go
View file
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions pkg/query/querier_test.go
View file
Expand Up @@ -847,6 +847,7 @@ func newProxyStore(storeAPIs ...storepb.StoreServer) *store.ProxyStore {

return store.NewProxyStore(
nil,
false,
nil,
func() []store.Client { return cls },
component.Query,
Expand Down
2 changes: 1 addition & 1 deletion pkg/query/query_test.go
View file
Expand Up @@ -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,
Expand Down
12 changes: 9 additions & 3 deletions pkg/store/proxy.go
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -117,6 +119,7 @@ func NewProxyStore(
metrics := newProxyStoreMetrics(reg)
s := &ProxyStore{
logger: logger,
debug: debug,
stores: stores,
component: component,
selectorLabels: selectorLabels,
Expand Down Expand Up @@ -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
}

Expand All @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions pkg/store/proxy_test.go
View file
Expand Up @@ -62,6 +62,7 @@ func TestProxyStore_Info(t *testing.T) {
defer cancel()

q := NewProxyStore(nil,
false,
nil,
func() []Client { return nil },
component.Query,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1194,6 +1197,7 @@ func TestProxyStore_Series_RequestParamsProxied(t *testing.T) {
},
}
q := NewProxyStore(nil,
false,
nil,
func() []Client { return cls },
component.Query,
Expand Down Expand Up @@ -1255,6 +1259,7 @@ func TestProxyStore_Series_RegressionFillResponseChannel(t *testing.T) {
}

q := NewProxyStore(nil,
false,
nil,
func() []Client { return cls },
component.Query,
Expand Down Expand Up @@ -1302,6 +1307,7 @@ func TestProxyStore_LabelValues(t *testing.T) {
},
}
q := NewProxyStore(nil,
false,
nil,
func() []Client { return cls },
component.Query,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit d41dcd8

Please sign in to comment.