From fabd099aa9752766e3d206b6b430d494a4e3cb66 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 4 May 2022 09:55:56 -0400 Subject: [PATCH 1/9] Added to overrides Signed-off-by: Joe Elliott --- modules/overrides/limits.go | 3 +++ modules/overrides/limits_test.go | 6 ++++- modules/overrides/overrides.go | 5 ++++ modules/overrides/overrides_test.go | 42 ++++++++++++++++++----------- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/modules/overrides/limits.go b/modules/overrides/limits.go index 83df014613a..0394c4a16de 100644 --- a/modules/overrides/limits.go +++ b/modules/overrides/limits.go @@ -70,6 +70,9 @@ type Limits struct { // Querier enforced limits. MaxBytesPerTagValuesQuery int `yaml:"max_bytes_per_tag_values_query" json:"max_bytes_per_tag_values_query"` + // QueryFrontend enforced limits + MaxSearchDuration model.Duration `yaml:"max_search_duration" json:"max_search_duration"` + // MaxBytesPerTrace is enforced in the Ingester, Compactor, Querier (Search) and Serverless (Search). It // it not enforce currently when doing a trace by id lookup. MaxBytesPerTrace int `yaml:"max_bytes_per_trace" json:"max_bytes_per_trace"` diff --git a/modules/overrides/limits_test.go b/modules/overrides/limits_test.go index 435b2006949..2e10fff2659 100644 --- a/modules/overrides/limits_test.go +++ b/modules/overrides/limits_test.go @@ -53,6 +53,8 @@ per_tenant_override_period: 1m metrics_generator_send_queue_size: 10 metrics_generator_send_workers: 1 + +max_search_duration: 5m ` inputJSON := ` { @@ -73,7 +75,9 @@ metrics_generator_send_workers: 1 "per_tenant_override_period": "1m", "metrics_generator_send_queue_size": 10, - "metrics_generator_send_workers": 1 + "metrics_generator_send_workers": 1, + + "max_search_duration": "5m" }` limitsYAML := Limits{} diff --git a/modules/overrides/overrides.go b/modules/overrides/overrides.go index 802e87ad647..80dafe7191b 100644 --- a/modules/overrides/overrides.go +++ b/modules/overrides/overrides.go @@ -307,6 +307,11 @@ func (o *Overrides) BlockRetention(userID string) time.Duration { return time.Duration(o.getOverridesForUser(userID).BlockRetention) } +// MaxSearchDuration is the duration of the max search duration for this tenant. +func (o *Overrides) MaxSearchDuration(userID string) time.Duration { + return time.Duration(o.getOverridesForUser(userID).MaxSearchDuration) +} + func (o *Overrides) getOverridesForUser(userID string) *Limits { if tenantOverrides := o.tenantOverrides(); tenantOverrides != nil { l := tenantOverrides.forUser(userID) diff --git a/modules/overrides/overrides_test.go b/modules/overrides/overrides_test.go index 6991c7ab325..a8a52f2941d 100644 --- a/modules/overrides/overrides_test.go +++ b/modules/overrides/overrides_test.go @@ -26,22 +26,24 @@ func TestOverrides(t *testing.T) { expectedMaxBytesPerTrace map[string]int expectedIngestionRateSpans map[string]int expectedIngestionBurstSpans map[string]int + expectedMaxSearchDuration map[string]int }{ - { - name: "limits only", - limits: Limits{ - MaxGlobalTracesPerUser: 1, - MaxLocalTracesPerUser: 2, - MaxBytesPerTrace: 3, - IngestionBurstSizeBytes: 4, - IngestionRateLimitBytes: 5, - }, - expectedMaxGlobalTraces: map[string]int{"user1": 1, "user2": 1}, - expectedMaxLocalTraces: map[string]int{"user1": 2, "user2": 2}, - expectedMaxBytesPerTrace: map[string]int{"user1": 3, "user2": 3}, - expectedIngestionBurstSpans: map[string]int{"user1": 4, "user2": 4}, - expectedIngestionRateSpans: map[string]int{"user1": 5, "user2": 5}, - }, + // { + // name: "limits only", + // limits: Limits{ + // MaxGlobalTracesPerUser: 1, + // MaxLocalTracesPerUser: 2, + // MaxBytesPerTrace: 3, + // IngestionBurstSizeBytes: 4, + // IngestionRateLimitBytes: 5, + // }, + // expectedMaxGlobalTraces: map[string]int{"user1": 1, "user2": 1}, + // expectedMaxLocalTraces: map[string]int{"user1": 2, "user2": 2}, + // expectedMaxBytesPerTrace: map[string]int{"user1": 3, "user2": 3}, + // expectedIngestionBurstSpans: map[string]int{"user1": 4, "user2": 4}, + // expectedIngestionRateSpans: map[string]int{"user1": 5, "user2": 5}, + // expectedMaxSearchDuration: map[string]int{"user1": 0, "user2": 0}, + // }, { name: "basic overrides", limits: Limits{ @@ -59,6 +61,7 @@ func TestOverrides(t *testing.T) { MaxBytesPerTrace: 8, IngestionBurstSizeBytes: 9, IngestionRateLimitBytes: 10, + MaxSearchDuration: model.Duration(11 * time.Second), }, }, }, @@ -67,6 +70,7 @@ func TestOverrides(t *testing.T) { expectedMaxBytesPerTrace: map[string]int{"user1": 8, "user2": 3}, expectedIngestionBurstSpans: map[string]int{"user1": 9, "user2": 4}, expectedIngestionRateSpans: map[string]int{"user1": 10, "user2": 5}, + expectedMaxSearchDuration: map[string]int{"user1": int(11 * time.Second), "user2": 0}, }, { name: "wildcard override", @@ -92,6 +96,7 @@ func TestOverrides(t *testing.T) { MaxBytesPerTrace: 13, IngestionBurstSizeBytes: 14, IngestionRateLimitBytes: 15, + MaxSearchDuration: model.Duration(16 * time.Second), }, }, }, @@ -100,11 +105,12 @@ func TestOverrides(t *testing.T) { expectedMaxBytesPerTrace: map[string]int{"user1": 8, "user2": 13}, expectedIngestionBurstSpans: map[string]int{"user1": 9, "user2": 14}, expectedIngestionRateSpans: map[string]int{"user1": 10, "user2": 15}, + expectedMaxSearchDuration: map[string]int{"user1": 0, "user2": int(16 * time.Second)}, }, } for _, tt := range tests { - t.Run(t.Name(), func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { if tt.overrides != nil { overridesFile := filepath.Join(t.TempDir(), "overrides.yaml") @@ -140,6 +146,10 @@ func TestOverrides(t *testing.T) { assert.Equal(t, float64(expectedVal), overrides.IngestionRateLimitBytes(user)) } + for user, expectedVal := range tt.expectedMaxSearchDuration { + assert.Equal(t, time.Duration(expectedVal), overrides.MaxSearchDuration(user)) + } + //if srv != nil { err = services.StopAndAwaitTerminated(context.TODO(), overrides) require.NoError(t, err) From 9b863c2a188f44aac09b7c85dc36541176454a80 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 4 May 2022 11:30:37 -0400 Subject: [PATCH 2/9] Added support for overrides Signed-off-by: Joe Elliott --- cmd/tempo/app/modules.go | 4 +-- modules/frontend/config.go | 2 +- modules/frontend/frontend.go | 9 ++--- modules/frontend/frontend_test.go | 12 +++---- modules/frontend/searchsharding.go | 44 +++++++++++++++++-------- modules/frontend/searchsharding_test.go | 42 ++++++++++++++++++++--- tempodb/wal/wal_test.go | 2 +- 7 files changed, 83 insertions(+), 32 deletions(-) diff --git a/cmd/tempo/app/modules.go b/cmd/tempo/app/modules.go index 59553bb40ea..3cf492de487 100644 --- a/cmd/tempo/app/modules.go +++ b/cmd/tempo/app/modules.go @@ -215,7 +215,7 @@ func (t *App) initQueryFrontend() (services.Service, error) { t.frontend = v1 // create query frontend - queryFrontend, err := frontend.New(t.cfg.Frontend, cortexTripper, t.store, log.Logger, prometheus.DefaultRegisterer) + queryFrontend, err := frontend.New(t.cfg.Frontend, cortexTripper, t.overrides, t.store, log.Logger, prometheus.DefaultRegisterer) if err != nil { return nil, err } @@ -327,7 +327,7 @@ func (t *App) setupModuleManager() error { // Store: nil, Overrides: {Server}, MemberlistKV: {Server}, - QueryFrontend: {Store, Server}, + QueryFrontend: {Store, Server, Overrides}, Ring: {Server, MemberlistKV}, MetricsGeneratorRing: {Server, MemberlistKV}, Distributor: {Ring, Server, Overrides}, diff --git a/modules/frontend/config.go b/modules/frontend/config.go index 2702557ac50..76442a015e0 100644 --- a/modules/frontend/config.go +++ b/modules/frontend/config.go @@ -25,7 +25,7 @@ type Config struct { } type SearchConfig struct { - Sharder SearchSharderConfig `yaml:",inline"` // jpe - what did this look like before + Sharder SearchSharderConfig `yaml:",inline"` } func (cfg *Config) RegisterFlagsAndApplyDefaults(prefix string, f *flag.FlagSet) { diff --git a/modules/frontend/frontend.go b/modules/frontend/frontend.go index 51dae338ce1..3baf67f6cea 100644 --- a/modules/frontend/frontend.go +++ b/modules/frontend/frontend.go @@ -19,6 +19,7 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" "github.com/weaveworks/common/user" + "github.com/grafana/tempo/modules/overrides" "github.com/grafana/tempo/modules/storage" "github.com/grafana/tempo/pkg/api" "github.com/grafana/tempo/pkg/tempopb" @@ -38,7 +39,7 @@ type QueryFrontend struct { } // New returns a new QueryFrontend -func New(cfg Config, next http.RoundTripper, store storage.Store, logger log.Logger, registerer prometheus.Registerer) (*QueryFrontend, error) { +func New(cfg Config, next http.RoundTripper, o *overrides.Overrides, store storage.Store, logger log.Logger, registerer prometheus.Registerer) (*QueryFrontend, error) { level.Info(logger).Log("msg", "creating middleware in query frontend") if cfg.QueryShards < minQueryShards || cfg.QueryShards > maxQueryShards { @@ -67,7 +68,7 @@ func New(cfg Config, next http.RoundTripper, store storage.Store, logger log.Log // tracebyid middleware traceByIDMiddleware := MergeMiddlewares(newTraceByIDMiddleware(cfg, logger), retryWare) - searchMiddleware := MergeMiddlewares(newSearchMiddleware(cfg, store, logger), retryWare) + searchMiddleware := MergeMiddlewares(newSearchMiddleware(cfg, o, store, logger), retryWare) traceByIDCounter := queriesPerTenant.MustCurryWith(prometheus.Labels{ "op": traceByIDOp, @@ -174,10 +175,10 @@ func newTraceByIDMiddleware(cfg Config, logger log.Logger) Middleware { } // newSearchMiddleware creates a new frontend middleware to handle search and search tags requests. -func newSearchMiddleware(cfg Config, reader tempodb.Reader, logger log.Logger) Middleware { +func newSearchMiddleware(cfg Config, o *overrides.Overrides, reader tempodb.Reader, logger log.Logger) Middleware { return MiddlewareFunc(func(next http.RoundTripper) http.RoundTripper { ingesterSearchRT := next - backendSearchRT := NewRoundTripper(next, newSearchSharder(reader, cfg.Search.Sharder, logger)) + backendSearchRT := NewRoundTripper(next, newSearchSharder(reader, o, cfg.Search.Sharder, logger)) return RoundTripperFunc(func(r *http.Request) (*http.Response, error) { // backend search queries require sharding so we pass through a special roundtripper diff --git a/modules/frontend/frontend_test.go b/modules/frontend/frontend_test.go index 3cba41a9378..e0247d4b750 100644 --- a/modules/frontend/frontend_test.go +++ b/modules/frontend/frontend_test.go @@ -31,7 +31,7 @@ func TestFrontendRoundTripsSearch(t *testing.T) { TargetBytesPerRequest: defaultTargetBytesPerRequest, }, }, - }, next, nil, log.NewNopLogger(), nil) + }, next, nil, nil, log.NewNopLogger(), nil) require.NoError(t, err) req := httptest.NewRequest("GET", "/", nil) @@ -50,7 +50,7 @@ func TestFrontendBadConfigFails(t *testing.T) { TargetBytesPerRequest: defaultTargetBytesPerRequest, }, }, - }, nil, nil, log.NewNopLogger(), nil) + }, nil, nil, nil, log.NewNopLogger(), nil) assert.EqualError(t, err, "frontend query shards should be between 2 and 256 (both inclusive)") assert.Nil(t, f) @@ -61,7 +61,7 @@ func TestFrontendBadConfigFails(t *testing.T) { TargetBytesPerRequest: defaultTargetBytesPerRequest, }, }, - }, nil, nil, log.NewNopLogger(), nil) + }, nil, nil, nil, log.NewNopLogger(), nil) assert.EqualError(t, err, "frontend query shards should be between 2 and 256 (both inclusive)") assert.Nil(t, f) @@ -72,7 +72,7 @@ func TestFrontendBadConfigFails(t *testing.T) { TargetBytesPerRequest: defaultTargetBytesPerRequest, }, }, - }, nil, nil, log.NewNopLogger(), nil) + }, nil, nil, nil, log.NewNopLogger(), nil) assert.EqualError(t, err, "frontend search concurrent requests should be greater than 0") assert.Nil(t, f) @@ -83,7 +83,7 @@ func TestFrontendBadConfigFails(t *testing.T) { TargetBytesPerRequest: 0, }, }, - }, nil, nil, log.NewNopLogger(), nil) + }, nil, nil, nil, log.NewNopLogger(), nil) assert.EqualError(t, err, "frontend search target bytes per request should be greater than 0") assert.Nil(t, f) @@ -96,7 +96,7 @@ func TestFrontendBadConfigFails(t *testing.T) { QueryBackendAfter: time.Hour, }, }, - }, nil, nil, log.NewNopLogger(), nil) + }, nil, nil, nil, log.NewNopLogger(), nil) assert.EqualError(t, err, "query backend after should be less than or equal to query ingester until") assert.Nil(t, f) } diff --git a/modules/frontend/searchsharding.go b/modules/frontend/searchsharding.go index 03a2e6d1a46..1c3ed204d57 100644 --- a/modules/frontend/searchsharding.go +++ b/modules/frontend/searchsharding.go @@ -13,6 +13,7 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" "github.com/gogo/protobuf/jsonpb" + "github.com/grafana/tempo/modules/overrides" "github.com/grafana/tempo/pkg/api" "github.com/grafana/tempo/pkg/boundedwaitgroup" "github.com/grafana/tempo/pkg/tempopb" @@ -124,8 +125,9 @@ func (r *searchResponse) result() *tempopb.SearchResponse { } type searchSharder struct { - next http.RoundTripper - reader tempodb.Reader + next http.RoundTripper + reader tempodb.Reader + overrides *overrides.Overrides cfg SearchSharderConfig logger log.Logger @@ -142,13 +144,14 @@ type SearchSharderConfig struct { } // newSearchSharder creates a sharding middleware for search -func newSearchSharder(reader tempodb.Reader, cfg SearchSharderConfig, logger log.Logger) Middleware { +func newSearchSharder(reader tempodb.Reader, o *overrides.Overrides, cfg SearchSharderConfig, logger log.Logger) Middleware { return MiddlewareFunc(func(next http.RoundTripper) http.RoundTripper { return searchSharder{ - next: next, - reader: reader, - logger: logger, - cfg: cfg, + next: next, + reader: reader, + overrides: o, + logger: logger, + cfg: cfg, } }) } @@ -169,15 +172,9 @@ func (s searchSharder) RoundTrip(r *http.Request) (*http.Response, error) { }, nil } + // adjust limit based on config searchReq.Limit = adjustLimit(searchReq.Limit, s.cfg.DefaultLimit, s.cfg.MaxLimit) - if s.cfg.MaxDuration != 0 && time.Duration(searchReq.End-searchReq.Start)*time.Second > s.cfg.MaxDuration { - return &http.Response{ - StatusCode: http.StatusBadRequest, - Body: io.NopCloser(strings.NewReader(fmt.Sprintf("range specified by start and end exceeds %s. received start=%d end=%d", s.cfg.MaxDuration, searchReq.Start, searchReq.End))), - }, nil - } - ctx := r.Context() tenantID, err := user.ExtractOrgID(ctx) if err != nil { @@ -189,6 +186,15 @@ func (s searchSharder) RoundTrip(r *http.Request) (*http.Response, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "frontend.ShardSearch") defer span.Finish() + // calculate and enforce max search duration + maxDuration := s.maxDuration(tenantID) + if maxDuration != 0 && time.Duration(searchReq.End-searchReq.Start)*time.Second > maxDuration { + return &http.Response{ + StatusCode: http.StatusBadRequest, + Body: io.NopCloser(strings.NewReader(fmt.Sprintf("range specified by start and end exceeds %s. received start=%d end=%d", s.cfg.MaxDuration, searchReq.Start, searchReq.End))), + }, nil + } + ingesterReq, err := s.ingesterRequest(ctx, tenantID, r, *searchReq) if err != nil { return nil, err @@ -439,3 +445,13 @@ func adjustLimit(limit, defaultLimit, maxLimit uint32) uint32 { return limit } + +func (s *searchSharder) maxDuration(tenantID string) time.Duration { + // check overrides first, if no overrides then grab from our config + maxDuration := s.overrides.MaxSearchDuration(tenantID) + if maxDuration != 0 { + return maxDuration + } + + return s.cfg.MaxDuration +} diff --git a/modules/frontend/searchsharding_test.go b/modules/frontend/searchsharding_test.go index d2d7a809302..787c45ed91a 100644 --- a/modules/frontend/searchsharding_test.go +++ b/modules/frontend/searchsharding_test.go @@ -16,11 +16,13 @@ import ( "github.com/go-kit/log" "github.com/golang/protobuf/jsonpb" "github.com/google/uuid" + "github.com/grafana/tempo/modules/overrides" "github.com/grafana/tempo/pkg/api" "github.com/grafana/tempo/pkg/tempopb" "github.com/grafana/tempo/tempodb/backend" "github.com/grafana/tempo/tempodb/blocklist" "github.com/grafana/tempo/tempodb/encoding/common" + "github.com/prometheus/common/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/weaveworks/common/user" @@ -592,6 +594,9 @@ func TestSearchSharderRoundTrip(t *testing.T) { }, nil }) + o, err := overrides.NewOverrides(overrides.Limits{}) + require.NoError(t, err) + sharder := newSearchSharder(&mockReader{ metas: []*backend.BlockMeta{ // one block with 2 records that are each the target bytes per request will force 2 sub queries { @@ -602,7 +607,7 @@ func TestSearchSharderRoundTrip(t *testing.T) { BlockID: uuid.MustParse("00000000-0000-0000-0000-000000000000"), }, }, - }, SearchSharderConfig{ + }, o, SearchSharderConfig{ ConcurrentRequests: 1, // 1 concurrent request to force order TargetBytesPerRequest: defaultTargetBytesPerRequest, }, log.NewNopLogger()) @@ -641,7 +646,10 @@ func TestSearchSharderRoundTripBadRequest(t *testing.T) { return nil, nil }) - sharder := newSearchSharder(&mockReader{}, SearchSharderConfig{ + o, err := overrides.NewOverrides(overrides.Limits{}) + require.NoError(t, err) + + sharder := newSearchSharder(&mockReader{}, o, SearchSharderConfig{ ConcurrentRequests: defaultConcurrentRequests, TargetBytesPerRequest: defaultTargetBytesPerRequest, MaxDuration: 5 * time.Minute, @@ -655,13 +663,12 @@ func TestSearchSharderRoundTripBadRequest(t *testing.T) { // start/end outside of max duration req = httptest.NewRequest("GET", "/?start=1000&end=1500", nil) - req.Header.Set(user.OrgIDHeaderName, "blerg") + req = req.WithContext(user.InjectOrgID(req.Context(), "blerg")) resp, err = testRT.RoundTrip(req) testBadRequest(t, resp, err, "range specified by start and end exceeds 5m0s. received start=1000 end=1500") // bad request req = httptest.NewRequest("GET", "/?start=asdf&end=1500", nil) - req.Header.Set(user.OrgIDHeaderName, "blerg") resp, err = testRT.RoundTrip(req) testBadRequest(t, resp, err, "invalid start: strconv.ParseInt: parsing \"asdf\": invalid syntax") } @@ -680,3 +687,30 @@ func TestAdjustLimit(t *testing.T) { assert.Equal(t, uint32(3), adjustLimit(3, 10, 20)) assert.Equal(t, uint32(20), adjustLimit(25, 10, 20)) } + +func TestMaxDuration(t *testing.T) { + // + o, err := overrides.NewOverrides(overrides.Limits{}) + require.NoError(t, err) + sharder := searchSharder{ + cfg: SearchSharderConfig{ + MaxDuration: 5 * time.Minute, + }, + overrides: o, + } + actual := sharder.maxDuration("test") + assert.Equal(t, 5*time.Minute, actual) + + o, err = overrides.NewOverrides(overrides.Limits{ + MaxSearchDuration: model.Duration(10 * time.Minute), + }) + require.NoError(t, err) + sharder = searchSharder{ + cfg: SearchSharderConfig{ + MaxDuration: 5 * time.Minute, + }, + overrides: o, + } + actual = sharder.maxDuration("test") + assert.Equal(t, 10*time.Minute, actual) +} diff --git a/tempodb/wal/wal_test.go b/tempodb/wal/wal_test.go index d9d89f8eeb4..abfeb4c9bba 100644 --- a/tempodb/wal/wal_test.go +++ b/tempodb/wal/wal_test.go @@ -165,7 +165,7 @@ func TestErrorConditions(t *testing.T) { require.NoFileExists(t, filepath.Join(tempDir, "fe0b83eb-a86b-4b6c-9a74-dc272cd5700e:blerg:v2:gzip")) } -func TestAppendBlockStartEnd(t *testing.T) { // jpe extend +func TestAppendBlockStartEnd(t *testing.T) { wal, err := New(&Config{ Filepath: t.TempDir(), Encoding: backend.EncNone, From 516a0bfbef15120cf6ef5036e341439be014b577 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 4 May 2022 11:47:13 -0400 Subject: [PATCH 3/9] Added overrides and patched up jsonnet Signed-off-by: Joe Elliott --- example/tk/lib/dashboards/grafana.libsonnet | 4 ++-- operations/jsonnet/microservices/frontend.libsonnet | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/example/tk/lib/dashboards/grafana.libsonnet b/example/tk/lib/dashboards/grafana.libsonnet index 65917f6196c..c96a6c246e0 100644 --- a/example/tk/lib/dashboards/grafana.libsonnet +++ b/example/tk/lib/dashboards/grafana.libsonnet @@ -7,7 +7,7 @@ local mixins = import 'mixins.libsonnet'; deploy(frontend_url='http://query-frontend'): grafana + grafana.withReplicas(1) - + grafana.withImage('grafana/grafana:8.3.0-beta2') + + grafana.withImage('grafana/grafana:8.5.2') + grafana.withRootUrl('http://grafana') + grafana.withTheme('dark') + grafana.withAnonymous() @@ -15,7 +15,7 @@ local mixins = import 'mixins.libsonnet'; + grafana.withGrafanaIniConfig({ sections+: { feature_toggles: { - enable: 'tempoSearch', + enable: 'tempoSearch tempoBackendSearch', }, }, }) diff --git a/operations/jsonnet/microservices/frontend.libsonnet b/operations/jsonnet/microservices/frontend.libsonnet index 3ec220463e0..d9f42902866 100644 --- a/operations/jsonnet/microservices/frontend.libsonnet +++ b/operations/jsonnet/microservices/frontend.libsonnet @@ -12,6 +12,7 @@ local tempo_config_volume = 'tempo-conf', local tempo_query_config_volume = 'tempo-query-conf', local tempo_data_volume = 'tempo-data', + local tempo_overrides_config_volume = 'overrides', tempo_query_frontend_container:: container.new(target_name, $._images.tempo) + @@ -26,6 +27,7 @@ (if $._config.variables_expansion then container.withEnvMixin($._config.variables_expansion_env_mixin) else {}) + container.withVolumeMounts([ volumeMount.new(tempo_config_volume, '/conf'), + volumeMount.new(tempo_overrides_config_volume, '/overrides'), ]) + $.util.withResources($._config.query_frontend.resources) + $.util.readinessProbe + @@ -63,6 +65,7 @@ deployment.mixin.spec.template.spec.withVolumes([ volume.fromConfigMap(tempo_query_config_volume, $.tempo_query_configmap.metadata.name), volume.fromConfigMap(tempo_config_volume, $.tempo_query_frontend_configmap.metadata.name), + volume.fromConfigMap(tempo_overrides_config_volume, $._config.overrides_configmap_name), ]), tempo_query_frontend_service: From 52e0e5a34697a89001f50ee6e881d2b43df36833 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 4 May 2022 11:52:40 -0400 Subject: [PATCH 4/9] Fixed error message with overrides and added test Signed-off-by: Joe Elliott --- modules/frontend/searchsharding.go | 2 +- modules/frontend/searchsharding_test.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/modules/frontend/searchsharding.go b/modules/frontend/searchsharding.go index 1c3ed204d57..92cc9e1ed54 100644 --- a/modules/frontend/searchsharding.go +++ b/modules/frontend/searchsharding.go @@ -191,7 +191,7 @@ func (s searchSharder) RoundTrip(r *http.Request) (*http.Response, error) { if maxDuration != 0 && time.Duration(searchReq.End-searchReq.Start)*time.Second > maxDuration { return &http.Response{ StatusCode: http.StatusBadRequest, - Body: io.NopCloser(strings.NewReader(fmt.Sprintf("range specified by start and end exceeds %s. received start=%d end=%d", s.cfg.MaxDuration, searchReq.Start, searchReq.End))), + Body: io.NopCloser(strings.NewReader(fmt.Sprintf("range specified by start and end exceeds %s. received start=%d end=%d", maxDuration, searchReq.Start, searchReq.End))), }, nil } diff --git a/modules/frontend/searchsharding_test.go b/modules/frontend/searchsharding_test.go index 787c45ed91a..333c32ea965 100644 --- a/modules/frontend/searchsharding_test.go +++ b/modules/frontend/searchsharding_test.go @@ -671,6 +671,24 @@ func TestSearchSharderRoundTripBadRequest(t *testing.T) { req = httptest.NewRequest("GET", "/?start=asdf&end=1500", nil) resp, err = testRT.RoundTrip(req) testBadRequest(t, resp, err, "invalid start: strconv.ParseInt: parsing \"asdf\": invalid syntax") + + // test max duration error with overrides + o, err = overrides.NewOverrides(overrides.Limits{ + MaxSearchDuration: model.Duration(time.Minute), + }) + require.NoError(t, err) + + sharder = newSearchSharder(&mockReader{}, o, SearchSharderConfig{ + ConcurrentRequests: defaultConcurrentRequests, + TargetBytesPerRequest: defaultTargetBytesPerRequest, + MaxDuration: 5 * time.Minute, + }, log.NewNopLogger()) + testRT = NewRoundTripper(next, sharder) + + req = httptest.NewRequest("GET", "/?start=1000&end=1500", nil) + req = req.WithContext(user.InjectOrgID(req.Context(), "blerg")) + resp, err = testRT.RoundTrip(req) + testBadRequest(t, resp, err, "range specified by start and end exceeds 1m0s. received start=1000 end=1500") } func testBadRequest(t *testing.T, resp *http.Response, err error, expectedBody string) { From a15d8b87d4781e5d39406c51f2fcd05599adfe3f Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 4 May 2022 11:58:13 -0400 Subject: [PATCH 5/9] manifest Signed-off-by: Joe Elliott --- docs/tempo/website/configuration/_index.md | 10 +++++++++- docs/tempo/website/configuration/manifest.md | 21 ++++++++++++++------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/docs/tempo/website/configuration/_index.md b/docs/tempo/website/configuration/_index.md index 1d79af797de..d9ebaec5ada 100644 --- a/docs/tempo/website/configuration/_index.md +++ b/docs/tempo/website/configuration/_index.md @@ -987,7 +987,15 @@ overrides: # This setting is useful if you wish to test how many active series a tenant will generate, without # actually writing these metrics. [metrics_generator_disable_collection: | default = false] - + + # Per-user block retention. If this value is set to 0 (default) then block_retention + # in the compactor config is used. + [block_retention: | default = 0s] + + # Per-user max search duration. If this value is set to 0 (default) then max_duration + # in the frontend config is used. + [max_search_duration: | default = 0s] + # Tenant-specific overrides settings configuration file. The empty string (default # value) disables using an overrides file. [per_tenant_override_config: | default = ""] diff --git a/docs/tempo/website/configuration/manifest.md b/docs/tempo/website/configuration/manifest.md index fbfa106195e..d3cf762bff1 100644 --- a/docs/tempo/website/configuration/manifest.md +++ b/docs/tempo/website/configuration/manifest.md @@ -112,7 +112,7 @@ ingester_client: remote_timeout: 5s grpc_client_config: max_recv_msg_size: 104857600 - max_send_msg_size: 16777216 + max_send_msg_size: 104857600 grpc_compression: snappy rate_limit: 0 rate_limit_burst: 0 @@ -135,7 +135,7 @@ metrics_generator_client: remote_timeout: 5s grpc_client_config: max_recv_msg_size: 104857600 - max_send_msg_size: 16777216 + max_send_msg_size: 104857600 grpc_compression: snappy rate_limit: 0 rate_limit_burst: 0 @@ -316,9 +316,13 @@ ingester: join_after: 0s min_ready_duration: 15s interface_names: - - eth0 - - en0 - final_sleep: 30s + - wlp2s0 + - docker0 + - br-f163873defd4 + - br-f56e9de73d01 + - br-16536cce4aa3 + - br-3bc02eb7efdd + final_sleep: 0s tokens_file_path: "" availability_zone: "" unregister_on_shutdown: true @@ -384,6 +388,7 @@ metrics_generator: - 3.2 - 6.4 - 12.8 + dimensions: [] span_metrics: histogram_buckets: - 0.002 @@ -448,9 +453,9 @@ storage: bucket_name: "" chunk_buffer_size: 10485760 endpoint: "" - insecure: false hedge_requests_at: 0s hedge_requests_up_to: 2 + insecure: false object_cache_control: "" object_metadata: {} s3: @@ -494,8 +499,12 @@ overrides: metrics_generator_processors: null metrics_generator_max_active_series: 0 metrics_generator_collection_interval: 0s + metrics_generator_disable_collection: false + metrics_generator_forwarder_queue_size: 0 + metrics_generator_forwarder_workers: 0 block_retention: 0s max_bytes_per_tag_values_query: 5000000 + max_search_duration: 0s max_bytes_per_trace: 5000000 per_tenant_override_config: "" per_tenant_override_period: 10s From e85607e041a6c308f943919dbc4e0198bb1752cd Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 4 May 2022 12:07:58 -0400 Subject: [PATCH 6/9] restored test Signed-off-by: Joe Elliott --- modules/overrides/overrides_test.go | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/modules/overrides/overrides_test.go b/modules/overrides/overrides_test.go index a8a52f2941d..0cc3d7615fb 100644 --- a/modules/overrides/overrides_test.go +++ b/modules/overrides/overrides_test.go @@ -28,22 +28,22 @@ func TestOverrides(t *testing.T) { expectedIngestionBurstSpans map[string]int expectedMaxSearchDuration map[string]int }{ - // { - // name: "limits only", - // limits: Limits{ - // MaxGlobalTracesPerUser: 1, - // MaxLocalTracesPerUser: 2, - // MaxBytesPerTrace: 3, - // IngestionBurstSizeBytes: 4, - // IngestionRateLimitBytes: 5, - // }, - // expectedMaxGlobalTraces: map[string]int{"user1": 1, "user2": 1}, - // expectedMaxLocalTraces: map[string]int{"user1": 2, "user2": 2}, - // expectedMaxBytesPerTrace: map[string]int{"user1": 3, "user2": 3}, - // expectedIngestionBurstSpans: map[string]int{"user1": 4, "user2": 4}, - // expectedIngestionRateSpans: map[string]int{"user1": 5, "user2": 5}, - // expectedMaxSearchDuration: map[string]int{"user1": 0, "user2": 0}, - // }, + { + name: "limits only", + limits: Limits{ + MaxGlobalTracesPerUser: 1, + MaxLocalTracesPerUser: 2, + MaxBytesPerTrace: 3, + IngestionBurstSizeBytes: 4, + IngestionRateLimitBytes: 5, + }, + expectedMaxGlobalTraces: map[string]int{"user1": 1, "user2": 1}, + expectedMaxLocalTraces: map[string]int{"user1": 2, "user2": 2}, + expectedMaxBytesPerTrace: map[string]int{"user1": 3, "user2": 3}, + expectedIngestionBurstSpans: map[string]int{"user1": 4, "user2": 4}, + expectedIngestionRateSpans: map[string]int{"user1": 5, "user2": 5}, + expectedMaxSearchDuration: map[string]int{"user1": 0, "user2": 0}, + }, { name: "basic overrides", limits: Limits{ From 1d699627db10112748cc61d8d608bdeb8f615c66 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 4 May 2022 12:09:46 -0400 Subject: [PATCH 7/9] changelog Signed-off-by: Joe Elliott --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62f9dab15cb..cfc4878bcc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## main / unreleased * [BUGFIX] metrics-generator: don't inject X-Scope-OrgID header for single-tenant setups [1417](https://github.com/grafana/tempo/pull/1417) (@kvrhdn) +* [ENHANCEMENT] Added the ability to have a per tenant max search duration. [#1421](https://github.com/grafana/tempo/pull/1421) (@joe-elliott) ## v1.4.0 / 2022-04-28 From d396a705a9359ad5fb48c91c7c66d0bcd12c0a9a Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 4 May 2022 13:12:47 -0400 Subject: [PATCH 8/9] Update docs/tempo/website/configuration/_index.md Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com> --- docs/tempo/website/configuration/_index.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/tempo/website/configuration/_index.md b/docs/tempo/website/configuration/_index.md index d9ebaec5ada..9d7e776f9ab 100644 --- a/docs/tempo/website/configuration/_index.md +++ b/docs/tempo/website/configuration/_index.md @@ -988,12 +988,12 @@ overrides: # actually writing these metrics. [metrics_generator_disable_collection: | default = false] - # Per-user block retention. If this value is set to 0 (default) then block_retention - # in the compactor config is used. + # Per-user block retention. If this value is set to 0 (default), then block_retention + # in the compactor configuration is used. [block_retention: | default = 0s] - # Per-user max search duration. If this value is set to 0 (default) then max_duration - # in the frontend config is used. + # Per-user max search duration. If this value is set to 0 (default), then max_duration + # in the front-end configuration is used. [max_search_duration: | default = 0s] # Tenant-specific overrides settings configuration file. The empty string (default From bd23e84f785714ffdaef967162a4c40414d65f8f Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 4 May 2022 13:57:07 -0400 Subject: [PATCH 9/9] make kube-manifests Signed-off-by: Joe Elliott --- operations/kube-manifests/Deployment-query-frontend.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/operations/kube-manifests/Deployment-query-frontend.yaml b/operations/kube-manifests/Deployment-query-frontend.yaml index b993b5055a6..a4d91e1f8dc 100644 --- a/operations/kube-manifests/Deployment-query-frontend.yaml +++ b/operations/kube-manifests/Deployment-query-frontend.yaml @@ -50,6 +50,8 @@ spec: volumeMounts: - mountPath: /conf name: tempo-conf + - mountPath: /overrides + name: overrides - args: - --query.base-path=/tempo - --grpc-storage-plugin.configuration-file=/conf/tempo-query.yaml @@ -72,3 +74,6 @@ spec: - configMap: name: tempo-query-frontend name: tempo-conf + - configMap: + name: tempo-overrides + name: overrides