Skip to content

Commit c05250c

Browse files
afhassanafhassan
committed
truncate getLabels and getSeries to max range if no start-end time provided
Signed-off-by: Ahmed Hassan <[email protected]>
1 parent 940adf6 commit c05250c

File tree

4 files changed

+97
-40
lines changed

4 files changed

+97
-40
lines changed

‎pkg/querier/distributor_queryable_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,8 @@ func TestDistributorQuerier_SelectShouldHonorQueryIngestersWithin(t *testing.T)
9898
overrides, err := validation.NewOverrides(limits, nil)
9999
require.NoError(t, err)
100100

101-
start, end, err := validateQueryTimeRange(ctx, "test", testData.queryMinT, testData.queryMaxT, overrides, 0)
101+
start, end, warnings, err := validateQueryTimeRange(ctx, "test", testData.queryMinT, testData.queryMaxT, overrides, 0)
102+
require.Nil(t, warnings)
102103
require.NoError(t, err)
103104
// Select hints are passed by Prometheus when querying /series.
104105
var hints *storage.SelectHints

‎pkg/querier/querier.go

+45-25
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/prometheus/prometheus/promql/parser"
2020
"github.com/prometheus/prometheus/storage"
2121
"github.com/prometheus/prometheus/util/annotations"
22+
v1 "github.com/prometheus/prometheus/web/api/v1"
2223
"github.com/thanos-io/promql-engine/engine"
2324
"github.com/thanos-io/promql-engine/logicalplan"
2425
"github.com/thanos-io/thanos/pkg/strutil"
@@ -27,6 +28,7 @@ import (
2728
"github.com/cortexproject/cortex/pkg/querier/batch"
2829
"github.com/cortexproject/cortex/pkg/querier/lazyquery"
2930
"github.com/cortexproject/cortex/pkg/querier/partialdata"
31+
seriesset "github.com/cortexproject/cortex/pkg/querier/series"
3032
querier_stats "github.com/cortexproject/cortex/pkg/querier/stats"
3133
"github.com/cortexproject/cortex/pkg/tenant"
3234
"github.com/cortexproject/cortex/pkg/util"
@@ -304,11 +306,12 @@ type querier struct {
304306
ignoreMaxQueryLength bool
305307
}
306308

307-
func (q querier) setupFromCtx(ctx context.Context) (context.Context, *querier_stats.QueryStats, string, int64, int64, storage.Querier, []storage.Querier, error) {
309+
func (q querier) setupFromCtx(ctx context.Context) (context.Context, *querier_stats.QueryStats, string, int64, int64, storage.Querier, []storage.Querier, annotations.Annotations, error) {
308310
stats := querier_stats.FromContext(ctx)
309311
userID, err := tenant.TenantID(ctx)
312+
warnings := annotations.Annotations(nil)
310313
if err != nil {
311-
return ctx, stats, userID, 0, 0, nil, nil, err
314+
return ctx, stats, userID, 0, 0, nil, nil, warnings, err
312315
}
313316

314317
q.limiterHolder.limiterInitializer.Do(func() {
@@ -317,14 +320,14 @@ func (q querier) setupFromCtx(ctx context.Context) (context.Context, *querier_st
317320

318321
ctx = limiter.AddQueryLimiterToContext(ctx, q.limiterHolder.limiter)
319322

320-
mint, maxt, err := validateQueryTimeRange(ctx, userID, q.mint, q.maxt, q.limits, q.maxQueryIntoFuture)
323+
mint, maxt, warnings, err := validateQueryTimeRange(ctx, userID, q.mint, q.maxt, q.limits, q.maxQueryIntoFuture)
321324
if err != nil {
322-
return ctx, stats, userID, 0, 0, nil, nil, err
325+
return ctx, stats, userID, 0, 0, nil, nil, warnings, err
323326
}
324327

325328
dqr, err := q.distributor.Querier(mint, maxt)
326329
if err != nil {
327-
return ctx, stats, userID, 0, 0, nil, nil, err
330+
return ctx, stats, userID, 0, 0, nil, nil, warnings, err
328331
}
329332
metadataQuerier := dqr
330333

@@ -340,18 +343,18 @@ func (q querier) setupFromCtx(ctx context.Context) (context.Context, *querier_st
340343

341344
cqr, err := s.Querier(mint, maxt)
342345
if err != nil {
343-
return ctx, stats, userID, 0, 0, nil, nil, err
346+
return ctx, stats, userID, 0, 0, nil, nil, warnings, err
344347
}
345348

346349
queriers = append(queriers, cqr)
347350
}
348-
return ctx, stats, userID, mint, maxt, metadataQuerier, queriers, nil
351+
return ctx, stats, userID, mint, maxt, metadataQuerier, queriers, warnings, nil
349352
}
350353

351354
// Select implements storage.Querier interface.
352355
// The bool passed is ignored because the series is always sorted.
353356
func (q querier) Select(ctx context.Context, sortSeries bool, sp *storage.SelectHints, matchers ...*labels.Matcher) storage.SeriesSet {
354-
ctx, stats, userID, mint, maxt, metadataQuerier, queriers, err := q.setupFromCtx(ctx)
357+
ctx, stats, userID, mint, maxt, metadataQuerier, queriers, warnings, err := q.setupFromCtx(ctx)
355358
if err == errEmptyTimeRange {
356359
return storage.EmptySeriesSet()
357360
} else if err != nil {
@@ -370,7 +373,8 @@ func (q querier) Select(ctx context.Context, sortSeries bool, sp *storage.Select
370373
}
371374

372375
if sp == nil {
373-
mint, maxt, err = validateQueryTimeRange(ctx, userID, mint, maxt, q.limits, q.maxQueryIntoFuture)
376+
mint, maxt, newWarnings, err := validateQueryTimeRange(ctx, userID, mint, maxt, q.limits, q.maxQueryIntoFuture)
377+
warnings.Merge(newWarnings)
374378
if err == errEmptyTimeRange {
375379
return storage.EmptySeriesSet()
376380
} else if err != nil {
@@ -383,7 +387,8 @@ func (q querier) Select(ctx context.Context, sortSeries bool, sp *storage.Select
383387
// Validate query time range. Even if the time range has already been validated when we created
384388
// the querier, we need to check it again here because the time range specified in hints may be
385389
// different.
386-
startMs, endMs, err := validateQueryTimeRange(ctx, userID, sp.Start, sp.End, q.limits, q.maxQueryIntoFuture)
390+
startMs, endMs, newWarnings, err := validateQueryTimeRange(ctx, userID, sp.Start, sp.End, q.limits, q.maxQueryIntoFuture)
391+
warnings.Merge(newWarnings)
387392
if err == errEmptyTimeRange {
388393
return storage.NoopSeriesSet()
389394
} else if err != nil {
@@ -437,12 +442,12 @@ func (q querier) Select(ctx context.Context, sortSeries bool, sp *storage.Select
437442
}
438443
}
439444

440-
return storage.NewMergeSeriesSet(result, 0, storage.ChainedSeriesMerge)
445+
return seriesset.NewSeriesSetWithWarnings(storage.NewMergeSeriesSet(result, 0, storage.ChainedSeriesMerge), warnings)
441446
}
442447

443448
// LabelValues implements storage.Querier.
444449
func (q querier) LabelValues(ctx context.Context, name string, hints *storage.LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) {
445-
ctx, stats, userID, mint, maxt, _, queriers, err := q.setupFromCtx(ctx)
450+
ctx, stats, userID, mint, maxt, _, queriers, warnings, err := q.setupFromCtx(ctx)
446451
if err == errEmptyTimeRange {
447452
return nil, nil, nil
448453
} else if err != nil {
@@ -456,7 +461,8 @@ func (q querier) LabelValues(ctx context.Context, name string, hints *storage.La
456461
startTime := model.Time(mint)
457462
endTime := model.Time(maxt)
458463

459-
if maxQueryLength := q.limits.MaxQueryLength(userID); maxQueryLength > 0 && endTime.Sub(startTime) > maxQueryLength {
464+
maxQueryLength := q.limits.MaxQueryLength(userID)
465+
if maxQueryLength > 0 && endTime.Sub(startTime) > maxQueryLength {
460466
limitErr := validation.LimitError(fmt.Sprintf(validation.ErrQueryTooLong, endTime.Sub(startTime), maxQueryLength))
461467
return nil, nil, limitErr
462468
}
@@ -466,9 +472,8 @@ func (q querier) LabelValues(ctx context.Context, name string, hints *storage.La
466472
}
467473

468474
var (
469-
g, _ = errgroup.WithContext(ctx)
470-
sets = [][]string{}
471-
warnings = annotations.Annotations(nil)
475+
g, _ = errgroup.WithContext(ctx)
476+
sets = [][]string{}
472477

473478
resMtx sync.Mutex
474479
)
@@ -505,7 +510,7 @@ func (q querier) LabelValues(ctx context.Context, name string, hints *storage.La
505510
}
506511

507512
func (q querier) LabelNames(ctx context.Context, hints *storage.LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) {
508-
ctx, stats, userID, mint, maxt, _, queriers, err := q.setupFromCtx(ctx)
513+
ctx, stats, userID, mint, maxt, _, queriers, warnings, err := q.setupFromCtx(ctx)
509514
if err == errEmptyTimeRange {
510515
return nil, nil, nil
511516
} else if err != nil {
@@ -519,7 +524,8 @@ func (q querier) LabelNames(ctx context.Context, hints *storage.LabelHints, matc
519524
startTime := model.Time(mint)
520525
endTime := model.Time(maxt)
521526

522-
if maxQueryLength := q.limits.MaxQueryLength(userID); maxQueryLength > 0 && endTime.Sub(startTime) > maxQueryLength {
527+
maxQueryLength := q.limits.MaxQueryLength(userID)
528+
if maxQueryLength > 0 && endTime.Sub(startTime) > maxQueryLength {
523529
limitErr := validation.LimitError(fmt.Sprintf(validation.ErrQueryTooLong, endTime.Sub(startTime), maxQueryLength))
524530
return nil, nil, limitErr
525531
}
@@ -529,9 +535,8 @@ func (q querier) LabelNames(ctx context.Context, hints *storage.LabelHints, matc
529535
}
530536

531537
var (
532-
g, _ = errgroup.WithContext(ctx)
533-
sets = [][]string{}
534-
warnings = annotations.Annotations(nil)
538+
g, _ = errgroup.WithContext(ctx)
539+
sets = [][]string{}
535540

536541
resMtx sync.Mutex
537542
)
@@ -622,11 +627,26 @@ func UseBeforeTimestampQueryable(queryable storage.Queryable, ts time.Time) Quer
622627
}
623628
}
624629

625-
func validateQueryTimeRange(ctx context.Context, userID string, startMs, endMs int64, limits *validation.Overrides, maxQueryIntoFuture time.Duration) (int64, int64, error) {
630+
func validateQueryTimeRange(ctx context.Context, userID string, startMs, endMs int64, limits *validation.Overrides, maxQueryIntoFuture time.Duration) (int64, int64, annotations.Annotations, error) {
631+
warnings := annotations.Annotations(nil)
626632
now := model.Now()
627633
startTime := model.Time(startMs)
628634
endTime := model.Time(endMs)
629635

636+
// Truncate time range to MaxQueryLength if time parameters are unspecified
637+
maxQueryLength := limits.MaxQueryLength(userID)
638+
if maxQueryLength > 0 && (util.TimeToMillis(v1.MinTime) == startMs || util.TimeToMillis(v1.MaxTime) == endMs) {
639+
startTime = now.Add(-maxQueryLength)
640+
endTime = now
641+
warnings.Add(validation.LimitError(fmt.Sprintf(validation.WarningTruncatedQueryLength, maxQueryLength)))
642+
643+
// Make sure to log it in traces to ease debugging.
644+
level.Debug(spanlogger.FromContext(ctx)).Log(
645+
"msg", "no time range specified, response truncated to maximum query length",
646+
"updatedStart", util.FormatTimeModel(startTime),
647+
"updatedEnd", util.FormatTimeModel(endTime))
648+
}
649+
630650
// Clamp time range based on max query into future.
631651
if maxQueryIntoFuture > 0 && endTime.After(now.Add(maxQueryIntoFuture)) {
632652
origEndTime := endTime
@@ -639,7 +659,7 @@ func validateQueryTimeRange(ctx context.Context, userID string, startMs, endMs i
639659
"updated", util.FormatTimeModel(endTime))
640660

641661
if endTime.Before(startTime) {
642-
return 0, 0, errEmptyTimeRange
662+
return 0, 0, warnings, errEmptyTimeRange
643663
}
644664
}
645665

@@ -655,7 +675,7 @@ func validateQueryTimeRange(ctx context.Context, userID string, startMs, endMs i
655675
"updated", util.FormatTimeModel(startTime))
656676

657677
if endTime.Before(startTime) {
658-
return 0, 0, errEmptyTimeRange
678+
return 0, 0, warnings, errEmptyTimeRange
659679
}
660680
}
661681

@@ -664,7 +684,7 @@ func validateQueryTimeRange(ctx context.Context, userID string, startMs, endMs i
664684
startTime = 0
665685
}
666686

667-
return int64(startTime), int64(endTime), nil
687+
return int64(startTime), int64(endTime), warnings, nil
668688
}
669689

670690
func EnableExperimentalPromQLFunctions(enablePromQLExperimentalFunctions, enableHoltWinters bool) {

‎pkg/querier/querier_test.go

+48-13
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/prometheus/prometheus/tsdb/chunkenc"
2424
"github.com/prometheus/prometheus/tsdb/tsdbutil"
2525
"github.com/prometheus/prometheus/util/annotations"
26+
v1 "github.com/prometheus/prometheus/web/api/v1"
2627
"github.com/stretchr/testify/assert"
2728
"github.com/stretchr/testify/mock"
2829
"github.com/stretchr/testify/require"
@@ -932,27 +933,34 @@ func TestQuerier_ValidateQueryTimeRange_MaxQueryLength_Labels(t *testing.T) {
932933
tests := map[string]struct {
933934
startTime time.Time
934935
endTime time.Time
935-
expected error
936+
expectedErr error
937+
expectedWarning error
936938
ignoreMaxQueryLength bool
937939
}{
938940
"time range shorter than maxQueryLength": {
939941
startTime: time.Now().Add(-maxQueryLength).Add(time.Hour),
940942
endTime: time.Now(),
941-
expected: nil,
943+
expectedErr: nil,
942944
ignoreMaxQueryLength: false,
943945
},
944946
"time range longer than maxQueryLength": {
945947
startTime: time.Now().Add(-maxQueryLength).Add(-time.Hour),
946948
endTime: time.Now(),
947-
expected: validation.LimitError("expanding series: the query time range exceeds the limit (query length: 721h0m0s, limit: 720h0m0s)"),
949+
expectedErr: validation.LimitError("expanding series: the query time range exceeds the limit (query length: 721h0m0s, limit: 720h0m0s)"),
948950
ignoreMaxQueryLength: false,
949951
},
950952
"time range longer than maxQueryLength and ignoreMaxQueryLength is true": {
951953
startTime: time.Now().Add(-maxQueryLength).Add(-time.Hour),
952954
endTime: time.Now(),
953-
expected: validation.LimitError("expanding series: the query time range exceeds the limit (query length: 721h0m0s, limit: 720h0m0s)"),
955+
expectedErr: validation.LimitError("expanding series: the query time range exceeds the limit (query length: 721h0m0s, limit: 720h0m0s)"),
954956
ignoreMaxQueryLength: true,
955957
},
958+
"time range not specified should be truncated with a warning": {
959+
startTime: v1.MinTime,
960+
endTime: v1.MaxTime,
961+
expectedWarning: validation.LimitError(fmt.Sprintf(validation.WarningTruncatedQueryLength, maxQueryLength)),
962+
ignoreMaxQueryLength: false,
963+
},
956964
}
957965

958966
for testName, testData := range tests {
@@ -978,23 +986,50 @@ func TestQuerier_ValidateQueryTimeRange_MaxQueryLength_Labels(t *testing.T) {
978986
q, err := queryable.Querier(util.TimeToMillis(testData.startTime), util.TimeToMillis(testData.endTime))
979987
require.NoError(t, err)
980988

981-
_, _, err = q.LabelNames(ctx, &storage.LabelHints{Limit: 0})
989+
_, warnings, err := q.LabelNames(ctx, &storage.LabelHints{Limit: 0})
982990

983-
if testData.expected != nil {
984-
require.NotNil(t, err)
985-
assert.True(t, strings.Contains(testData.expected.Error(), err.Error()))
991+
// Verify expected error is returned
992+
if testData.expectedErr != nil {
993+
assert.True(t, strings.Contains(testData.expectedErr.Error(), err.Error()))
986994
} else {
987995
assert.Nil(t, err)
988996
}
989997

990-
_, _, err = q.LabelValues(ctx, labels.MetricName, &storage.LabelHints{Limit: 0})
998+
// Verify expected warning is returned
999+
if testData.expectedWarning != nil {
1000+
foundWarning := false
1001+
for _, warning := range warnings {
1002+
if strings.Contains(testData.expectedWarning.Error(), warning.Error()) {
1003+
foundWarning = true
1004+
}
1005+
}
1006+
assert.True(t, foundWarning)
1007+
} else {
1008+
assert.Nil(t, warnings)
1009+
}
1010+
1011+
_, warnings, err = q.LabelValues(ctx, labels.MetricName, &storage.LabelHints{Limit: 0})
9911012

992-
if testData.expected != nil {
1013+
// Verify expected error is returned
1014+
if testData.expectedErr != nil {
9931015
require.NotNil(t, err)
994-
assert.True(t, strings.Contains(testData.expected.Error(), err.Error()))
1016+
assert.True(t, strings.Contains(testData.expectedErr.Error(), err.Error()))
9951017
} else {
9961018
assert.Nil(t, err)
9971019
}
1020+
1021+
// Verify expected warning is returned
1022+
if testData.expectedWarning != nil {
1023+
foundWarning := false
1024+
for _, warning := range warnings {
1025+
if strings.Contains(testData.expectedWarning.Error(), warning.Error()) {
1026+
foundWarning = true
1027+
}
1028+
}
1029+
assert.True(t, foundWarning)
1030+
} else {
1031+
assert.Nil(t, warnings)
1032+
}
9981033
})
9991034
}
10001035
}
@@ -1157,7 +1192,7 @@ func TestQuerier_ValidateQueryTimeRange_MaxQueryLookback(t *testing.T) {
11571192

11581193
// We apply the validation here again since when initializing querier we change the start/end time,
11591194
// but when querying series we don't validate again. So we should pass correct hints here.
1160-
start, end, err := validateQueryTimeRange(ctx, "test", util.TimeToMillis(testData.queryStartTime), util.TimeToMillis(testData.queryEndTime), overrides, 0)
1195+
start, end, _, err := validateQueryTimeRange(ctx, "test", util.TimeToMillis(testData.queryStartTime), util.TimeToMillis(testData.queryEndTime), overrides, 0)
11611196
// Skipped query will hit errEmptyTimeRange during validation.
11621197
if !testData.expectedSkipped {
11631198
require.NoError(t, err)
@@ -1318,7 +1353,7 @@ func TestValidateMaxQueryLength(t *testing.T) {
13181353
limits := DefaultLimitsConfig()
13191354
overrides, err := validation.NewOverrides(limits, nil)
13201355
require.NoError(t, err)
1321-
startMs, endMs, err := validateQueryTimeRange(ctx, "test", util.TimeToMillis(tc.start), util.TimeToMillis(tc.end), overrides, 0)
1356+
startMs, endMs, _, err := validateQueryTimeRange(ctx, "test", util.TimeToMillis(tc.start), util.TimeToMillis(tc.end), overrides, 0)
13221357
require.NoError(t, err)
13231358
startTime := model.Time(startMs)
13241359
endTime := model.Time(endMs)

‎pkg/util/validation/validate.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ const (
3636
unitTooLong = "unit_too_long"
3737

3838
// ErrQueryTooLong is used in chunk store, querier and query frontend.
39-
ErrQueryTooLong = "the query time range exceeds the limit (query length: %s, limit: %s)"
39+
ErrQueryTooLong = "the query time range exceeds the limit (query length: %s, limit: %s)"
40+
WarningTruncatedQueryLength = "no time range specified, response truncated to maximum query length (%s)"
4041

4142
missingMetricName = "missing_metric_name"
4243
invalidMetricName = "metric_name_invalid"

0 commit comments

Comments
 (0)