diff --git a/CHANGELOG.md b/CHANGELOG.md index b54c6abb1b4..2694bb64067 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * [ENHANCEMENT] Index Cache: Multi level cache adds config `max_backfill_items` to cap max items to backfill per async operation. #5686 * [ENHANCEMENT] Query Frontend: Log number of split queries in `query stats` log. #5703 * [ENHANCEMENT] Logging: Added new options for logging HTTP request headers: `-server.log-request-headers` enables logging HTTP request headers, `-server.log-request-headers-exclude-list` allows users to specify headers which should not be logged. #5744 +* [ENHANCEMENT] Query Frontend/Scheduler: Time check in query priority now considers overall data select time window (including range selectors, modifiers and lookback delta). #5758 * [ENHANCEMENT] Querier: Added `querier.store-gateway-query-stats-enabled` to enable or disable store gateway query stats log. #5749 * [BUGFIX] Distributor: Do not use label with empty values for sharding #5717 * [BUGFIX] Query Frontend: queries with negative offset should check whether it is cacheable or not. #5719 diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index efea8a6e1b5..6db5383c4d1 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -5095,14 +5095,18 @@ otel: # Regex that the query string should match. If not set, it won't be checked. [regex: | default = ""] -# Time window that the query should be within. If not set, it won't be checked. +# Overall data select time window (including range selectors, modifiers and +# lookback delta) that the query should be within. If not set, it won't be +# checked. time_window: - # Start of the time window that the query should be within. If set to 0, it - # won't be checked. + # Start of the data select time window (including range selectors, modifiers + # and lookback delta) that the query should be within. If set to 0, it won't + # be checked. [start: | default = 0] - # End of the time window that the query should be within. If set to 0, it - # won't be checked. + # End of the data select time window (including range selectors, modifiers and + # lookback delta) that the query should be within. If set to 0, it won't be + # checked. [end: | default = 0] ``` diff --git a/pkg/frontend/transport/handler.go b/pkg/frontend/transport/handler.go index f6554acb252..2845198975d 100644 --- a/pkg/frontend/transport/handler.go +++ b/pkg/frontend/transport/handler.go @@ -295,6 +295,8 @@ func (f *Handler) reportQueryStats(r *http.Request, userID string, queryString u numChunkBytes := stats.LoadFetchedChunkBytes() numDataBytes := stats.LoadFetchedDataBytes() splitQueries := stats.LoadSplitQueries() + dataSelectMaxTime := stats.LoadDataSelectMaxTime() + dataSelectMinTime := stats.LoadDataSelectMinTime() // Track stats. f.querySeconds.WithLabelValues(userID).Add(wallTime.Seconds()) @@ -339,14 +341,20 @@ func (f *Handler) reportQueryStats(r *http.Request, userID string, queryString u logMessage = append(logMessage, "content_encoding", encoding) } + if dataSelectMaxTime > 0 { + logMessage = append(logMessage, "data_select_max_time", util.FormatMillisToSeconds(dataSelectMaxTime)) + } + if dataSelectMinTime > 0 { + logMessage = append(logMessage, "data_select_min_time", util.FormatMillisToSeconds(dataSelectMinTime)) + } if query := queryString.Get("query"); len(query) > 0 { logMessage = append(logMessage, "query_length", len(query)) } if ua := r.Header.Get("User-Agent"); len(ua) > 0 { logMessage = append(logMessage, "user_agent", ua) } - if queryPriority := r.Header.Get(util.QueryPriorityHeaderKey); len(queryPriority) > 0 { - logMessage = append(logMessage, "priority", queryPriority) + if priority, ok := stats.LoadPriority(); ok { + logMessage = append(logMessage, "priority", priority) } if error != nil { diff --git a/pkg/frontend/transport/handler_test.go b/pkg/frontend/transport/handler_test.go index dae62012fb8..425369ac3ee 100644 --- a/pkg/frontend/transport/handler_test.go +++ b/pkg/frontend/transport/handler_test.go @@ -21,7 +21,6 @@ import ( "github.com/weaveworks/common/user" querier_stats "github.com/cortexproject/cortex/pkg/querier/stats" - "github.com/cortexproject/cortex/pkg/util" ) type roundTripperFunc func(*http.Request) (*http.Response, error) @@ -348,9 +347,20 @@ func TestReportQueryStatsFormat(t *testing.T) { }, "should include query priority": { queryString: url.Values(map[string][]string{"query": {"up"}}), - header: http.Header{util.QueryPriorityHeaderKey: []string{"99"}}, + queryStats: &querier_stats.QueryStats{ + Priority: 99, + PriorityAssigned: true, + }, expectedLog: `level=info msg="query stats" component=query-frontend method=GET path=/prometheus/api/v1/query response_time=1s query_wall_time_seconds=0 fetched_series_count=0 fetched_chunks_count=0 fetched_samples_count=0 fetched_chunks_bytes=0 fetched_data_bytes=0 split_queries=0 status_code=200 response_size=1000 query_length=2 priority=99 param_query=up`, }, + "should include data fetch min and max time": { + queryString: url.Values(map[string][]string{"query": {"up"}}), + queryStats: &querier_stats.QueryStats{ + DataSelectMaxTime: 1704153600000, + DataSelectMinTime: 1704067200000, + }, + expectedLog: `level=info msg="query stats" component=query-frontend method=GET path=/prometheus/api/v1/query response_time=1s query_wall_time_seconds=0 fetched_series_count=0 fetched_chunks_count=0 fetched_samples_count=0 fetched_chunks_bytes=0 fetched_data_bytes=0 split_queries=0 status_code=200 response_size=1000 data_select_max_time=1704153600 data_select_min_time=1704067200 query_length=2 param_query=up`, + }, } for testName, testData := range tests { diff --git a/pkg/frontend/v1/frontend.go b/pkg/frontend/v1/frontend.go index 024c1f961a5..060020be183 100644 --- a/pkg/frontend/v1/frontend.go +++ b/pkg/frontend/v1/frontend.go @@ -5,7 +5,6 @@ import ( "flag" "fmt" "net/http" - "strconv" "time" "github.com/go-kit/log" @@ -100,8 +99,8 @@ type request struct { } func (r request) Priority() int64 { - priority, err := strconv.ParseInt(httpgrpcutil.GetHeader(*r.request, util.QueryPriorityHeaderKey), 10, 64) - if err != nil { + priority, ok := stats.FromContext(r.originalCtx).LoadPriority() + if !ok { return 0 } diff --git a/pkg/querier/stats/stats.go b/pkg/querier/stats/stats.go index 81b9724abb7..67ec874ef36 100644 --- a/pkg/querier/stats/stats.go +++ b/pkg/querier/stats/stats.go @@ -16,7 +16,11 @@ var ctxKey = contextKey(0) type QueryStats struct { Stats - m sync.Mutex + PriorityAssigned bool + Priority int64 + DataSelectMaxTime int64 + DataSelectMinTime int64 + m sync.Mutex } // ContextWithEmptyStats returns a context with empty stats. @@ -196,6 +200,58 @@ func (s *QueryStats) LoadSplitQueries() uint64 { return atomic.LoadUint64(&s.SplitQueries) } +func (s *QueryStats) SetPriority(priority int64) { + if s == nil { + return + } + + if !s.PriorityAssigned { + s.PriorityAssigned = true + } + + atomic.StoreInt64(&s.Priority, priority) +} + +func (s *QueryStats) LoadPriority() (int64, bool) { + if s == nil { + return 0, false + } + + return atomic.LoadInt64(&s.Priority), s.PriorityAssigned +} + +func (s *QueryStats) SetDataSelectMaxTime(dataSelectMaxTime int64) { + if s == nil { + return + } + + atomic.StoreInt64(&s.DataSelectMaxTime, dataSelectMaxTime) +} + +func (s *QueryStats) LoadDataSelectMaxTime() int64 { + if s == nil { + return 0 + } + + return atomic.LoadInt64(&s.DataSelectMaxTime) +} + +func (s *QueryStats) SetDataSelectMinTime(dataSelectMinTime int64) { + if s == nil { + return + } + + atomic.StoreInt64(&s.DataSelectMinTime, dataSelectMinTime) +} + +func (s *QueryStats) LoadDataSelectMinTime() int64 { + if s == nil { + return 0 + } + + return atomic.LoadInt64(&s.DataSelectMinTime) +} + // Merge the provided Stats into this one. func (s *QueryStats) Merge(other *QueryStats) { if s == nil || other == nil { diff --git a/pkg/querier/tripperware/priority.go b/pkg/querier/tripperware/priority.go index a87a31888b4..07326d22d50 100644 --- a/pkg/querier/tripperware/priority.go +++ b/pkg/querier/tripperware/priority.go @@ -1,66 +1,16 @@ package tripperware import ( - "errors" - "net/http" - "strings" "time" - "github.com/prometheus/prometheus/promql/parser" - - "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/validation" ) -var ( - errParseExpr = errors.New("failed to parse expr") -) - -func GetPriority(r *http.Request, userID string, limits Limits, now time.Time, lookbackDelta time.Duration) (int64, error) { - isQuery := strings.HasSuffix(r.URL.Path, "/query") - isQueryRange := strings.HasSuffix(r.URL.Path, "/query_range") - queryPriority := limits.QueryPriority(userID) - query := r.FormValue("query") - - if (!isQuery && !isQueryRange) || !queryPriority.Enabled || query == "" { - return 0, nil - } - - expr, err := parser.ParseExpr(query) - if err != nil { - // If query fails to be parsed, we throw a simple parse error - // and fail query later on querier. - return 0, errParseExpr - } - - if len(queryPriority.Priorities) == 0 { - return queryPriority.DefaultPriority, nil - } - - var startTime, endTime int64 - if isQuery { - if t, err := util.ParseTimeParam(r, "time", now.Unix()); err == nil { - startTime = t - endTime = t - } - } else if isQueryRange { - if st, err := util.ParseTime(r.FormValue("start")); err == nil { - if et, err := util.ParseTime(r.FormValue("end")); err == nil { - startTime = st - endTime = et - } - } +func GetPriority(query string, minTime, maxTime int64, now time.Time, queryPriority validation.QueryPriority) int64 { + if !queryPriority.Enabled || query == "" || len(queryPriority.Priorities) == 0 { + return queryPriority.DefaultPriority } - es := &parser.EvalStmt{ - Expr: expr, - Start: util.TimeFromMillis(startTime), - End: util.TimeFromMillis(endTime), - LookbackDelta: lookbackDelta, - } - - minTime, maxTime := FindMinMaxTime(es) - for _, priority := range queryPriority.Priorities { for _, attribute := range priority.QueryAttributes { if attribute.Regex != "" && attribute.Regex != ".*" && attribute.Regex != ".+" { @@ -70,12 +20,12 @@ func GetPriority(r *http.Request, userID string, limits Limits, now time.Time, l } if isWithinTimeAttributes(attribute.TimeWindow, now, minTime, maxTime) { - return priority.Priority, nil + return priority.Priority } } } - return queryPriority.DefaultPriority, nil + return queryPriority.DefaultPriority } func isWithinTimeAttributes(timeWindow validation.TimeWindow, now time.Time, startTime, endTime int64) bool { @@ -84,14 +34,14 @@ func isWithinTimeAttributes(timeWindow validation.TimeWindow, now time.Time, sta } if timeWindow.Start != 0 { - startTimeThreshold := now.Add(-1 * time.Duration(timeWindow.Start).Abs()).Truncate(time.Second).Unix() + startTimeThreshold := now.Add(-1 * time.Duration(timeWindow.Start).Abs()).Add(-1 * time.Minute).Truncate(time.Minute).UnixMilli() if startTime < startTimeThreshold { return false } } if timeWindow.End != 0 { - endTimeThreshold := now.Add(-1 * time.Duration(timeWindow.End).Abs()).Add(1 * time.Second).Truncate(time.Second).Unix() + endTimeThreshold := now.Add(-1 * time.Duration(timeWindow.End).Abs()).Add(1 * time.Minute).Truncate(time.Minute).UnixMilli() if endTime > endTimeThreshold { return false } @@ -99,9 +49,3 @@ func isWithinTimeAttributes(timeWindow validation.TimeWindow, now time.Time, sta return true } - -func FindMinMaxTime(s *parser.EvalStmt) (int64, int64) { - // Placeholder until Prometheus is updated to include - // https://github.com/prometheus/prometheus/commit/9e3df532d8294d4fe3284bde7bc96db336a33552 - return s.Start.Unix(), s.End.Unix() -} diff --git a/pkg/querier/tripperware/priority_test.go b/pkg/querier/tripperware/priority_test.go index d6913e06b56..12c1243ed61 100644 --- a/pkg/querier/tripperware/priority_test.go +++ b/pkg/querier/tripperware/priority_test.go @@ -1,10 +1,7 @@ package tripperware import ( - "bytes" - "net/http" "regexp" - "strconv" "testing" "time" @@ -31,35 +28,16 @@ func Test_GetPriorityShouldReturnDefaultPriorityIfNotEnabledOrInvalidQueryString }} type testCase struct { - url string + query string queryPriorityEnabled bool - err error } tests := map[string]testCase{ "should miss if query priority not enabled": { - url: "/query?query=up", + query: "up", }, "should miss if query string empty": { - url: "/query?query=", - queryPriorityEnabled: true, - }, - "shouldn't return error if query is invalid": { - url: "/query?query=up[4h", - queryPriorityEnabled: true, - err: errParseExpr, - }, - "should miss if query string empty - range query": { - url: "/query_range?query=", - queryPriorityEnabled: true, - }, - "shouldn't return error if query is invalid, range query": { - url: "/query_range?query=up[4h", - queryPriorityEnabled: true, - err: errParseExpr, - }, - "should miss if neither instant nor range query": { - url: "/series", + query: "", queryPriorityEnabled: true, }, } @@ -67,13 +45,7 @@ func Test_GetPriorityShouldReturnDefaultPriorityIfNotEnabledOrInvalidQueryString for testName, testData := range tests { t.Run(testName, func(t *testing.T) { limits.queryPriority.Enabled = testData.queryPriorityEnabled - req, _ := http.NewRequest(http.MethodPost, testData.url, bytes.NewReader([]byte{})) - priority, err := GetPriority(req, "", limits, now, 0) - if err != nil { - assert.Equal(t, testData.err, err) - } else { - assert.NoError(t, err) - } + priority := GetPriority(testData.query, 0, 0, now, limits.queryPriority) assert.Equal(t, int64(0), priority) }) } @@ -131,9 +103,7 @@ func Test_GetPriorityShouldConsiderRegex(t *testing.T) { t.Run(testName, func(t *testing.T) { limits.queryPriority.Priorities[0].QueryAttributes[0].Regex = testData.regex limits.queryPriority.Priorities[0].QueryAttributes[0].CompiledRegex = regexp.MustCompile(testData.regex) - req, _ := http.NewRequest(http.MethodPost, "/query?query="+testData.query, bytes.NewReader([]byte{})) - priority, err := GetPriority(req, "", limits, now, 0) - assert.NoError(t, err) + priority := GetPriority(testData.query, 0, 0, now, limits.queryPriority) assert.Equal(t, int64(testData.expectedPriority), priority) }) } @@ -161,58 +131,38 @@ func Test_GetPriorityShouldConsiderStartAndEndTime(t *testing.T) { }} type testCase struct { - time time.Time start time.Time end time.Time expectedPriority int } tests := map[string]testCase{ - "should hit instant query between start and end time": { - time: now.Add(-30 * time.Minute), - expectedPriority: 1, - }, - "should hit instant query equal to start time": { - time: now.Add(-45 * time.Minute), - expectedPriority: 1, - }, - "should hit instant query equal to end time": { - time: now.Add(-15 * time.Minute), - expectedPriority: 1, - }, - "should miss instant query outside of end time": { - expectedPriority: 0, - }, - "should miss instant query outside of start time": { - time: now.Add(-60 * time.Minute), - expectedPriority: 0, - }, - "should hit range query between start and end time": { + "should hit between start and end time": { start: now.Add(-40 * time.Minute), end: now.Add(-20 * time.Minute), expectedPriority: 1, }, - "should hit range query equal to start and end time": { + "should hit equal to start and end time": { start: now.Add(-45 * time.Minute), end: now.Add(-15 * time.Minute), expectedPriority: 1, }, - "should miss range query outside of start time": { + "should miss outside of start time": { start: now.Add(-50 * time.Minute), end: now.Add(-15 * time.Minute), expectedPriority: 0, }, - "should miss range query completely outside of start time": { + "should miss completely outside of start time": { start: now.Add(-50 * time.Minute), end: now.Add(-45 * time.Minute), expectedPriority: 0, }, - "should miss range query outside of end time": { + "should miss outside of end time": { start: now.Add(-45 * time.Minute), end: now.Add(-10 * time.Minute), expectedPriority: 0, }, - "should miss range query completely outside of end time": { + "should miss completely outside of end time": { start: now.Add(-15 * time.Minute), end: now.Add(-10 * time.Minute), expectedPriority: 0, @@ -221,18 +171,7 @@ func Test_GetPriorityShouldConsiderStartAndEndTime(t *testing.T) { for testName, testData := range tests { t.Run(testName, func(t *testing.T) { - var url string - if !testData.time.IsZero() { - url = "/query?query=sum(up)&time=" + strconv.FormatInt(testData.time.Unix(), 10) - } else if !testData.start.IsZero() { - url = "/query_range?query=sum(up)&start=" + strconv.FormatInt(testData.start.Unix(), 10) - url += "&end=" + strconv.FormatInt(testData.end.Unix(), 10) - } else { - url = "/query?query=sum(up)" - } - req, _ := http.NewRequest(http.MethodPost, url, bytes.NewReader([]byte{})) - priority, err := GetPriority(req, "", limits, now, 0) - assert.NoError(t, err) + priority := GetPriority("sum(up)", testData.start.UnixMilli(), testData.end.UnixMilli(), now, limits.queryPriority) assert.Equal(t, int64(testData.expectedPriority), priority) }) } @@ -255,20 +194,20 @@ func Test_GetPriorityShouldNotConsiderStartAndEndTimeIfEmpty(t *testing.T) { }} type testCase struct { - time time.Time start time.Time end time.Time } tests := map[string]testCase{ - "should hit instant query with no time": {}, - "should hit instant query with future time": { - time: now.Add(1000000 * time.Hour), + "should hit with future time": { + start: now, + end: now.Add(1000000 * time.Hour), }, - "should hit instant query with very old time": { - time: now.Add(-1000000 * time.Hour), + "should hit with very old time": { + start: now.Add(-1000000 * time.Hour), + end: now, }, - "should hit range query with very wide time window": { + "should hit with very wide time window": { start: now.Add(-1000000 * time.Hour), end: now.Add(1000000 * time.Hour), }, @@ -276,18 +215,7 @@ func Test_GetPriorityShouldNotConsiderStartAndEndTimeIfEmpty(t *testing.T) { for testName, testData := range tests { t.Run(testName, func(t *testing.T) { - var url string - if !testData.time.IsZero() { - url = "/query?query=sum(up)&time=" + strconv.FormatInt(testData.time.Unix(), 10) - } else if !testData.start.IsZero() { - url = "/query_range?query=sum(up)&start=" + strconv.FormatInt(testData.start.Unix(), 10) - url += "&end=" + strconv.FormatInt(testData.end.Unix(), 10) - } else { - url = "/query?query=sum(up)" - } - req, _ := http.NewRequest(http.MethodPost, url, bytes.NewReader([]byte{})) - priority, err := GetPriority(req, "", limits, now, 0) - assert.NoError(t, err) + priority := GetPriority("sum(up)", testData.start.Unix(), testData.end.Unix(), now, limits.queryPriority) assert.Equal(t, int64(1), priority) }) } diff --git a/pkg/querier/tripperware/roundtrip.go b/pkg/querier/tripperware/roundtrip.go index f3526e39254..b137429815f 100644 --- a/pkg/querier/tripperware/roundtrip.go +++ b/pkg/querier/tripperware/roundtrip.go @@ -19,7 +19,6 @@ import ( "context" "io" "net/http" - "strconv" "strings" "time" @@ -28,10 +27,12 @@ import ( "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prometheus/prometheus/promql/parser" "github.com/thanos-io/thanos/pkg/querysharding" "github.com/weaveworks/common/httpgrpc" "github.com/weaveworks/common/user" + "github.com/cortexproject/cortex/pkg/querier/stats" "github.com/cortexproject/cortex/pkg/tenant" "github.com/cortexproject/cortex/pkg/util" util_log "github.com/cortexproject/cortex/pkg/util/log" @@ -159,14 +160,20 @@ func NewQueryTripperware( } } + expr, err := parser.ParseExpr(query) + if err != nil { + // If query is invalid, no need to go through tripperwares for further splitting. + return next.RoundTrip(r) + } + + reqStats := stats.FromContext(r.Context()) + minTime, maxTime := util.FindMinMaxTime(r, expr, lookbackDelta, now) + reqStats.SetDataSelectMaxTime(maxTime) + reqStats.SetDataSelectMinTime(minTime) + if limits != nil && limits.QueryPriority(userStr).Enabled { - priority, err := GetPriority(r, userStr, limits, now, lookbackDelta) - if err != nil && err == errParseExpr { - // If query is invalid, no need to go through tripperwares - // for further splitting. - return next.RoundTrip(r) - } - r.Header.Set(util.QueryPriorityHeaderKey, strconv.FormatInt(priority, 10)) + priority := GetPriority(query, minTime, maxTime, now, limits.QueryPriority(userStr)) + reqStats.SetPriority(priority) } } diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index fa28485298f..128a9c0631e 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -5,7 +5,6 @@ import ( "flag" "io" "net/http" - "strconv" "sync" "time" @@ -22,6 +21,8 @@ import ( "google.golang.org/grpc" "github.com/cortexproject/cortex/pkg/frontend/v2/frontendv2pb" + //lint:ignore faillint scheduler needs to retrieve priority from the context + "github.com/cortexproject/cortex/pkg/querier/stats" "github.com/cortexproject/cortex/pkg/scheduler/queue" "github.com/cortexproject/cortex/pkg/scheduler/schedulerpb" "github.com/cortexproject/cortex/pkg/tenant" @@ -167,8 +168,8 @@ type schedulerRequest struct { } func (s schedulerRequest) Priority() int64 { - priority, err := strconv.ParseInt(httpgrpcutil.GetHeader(*s.request, util.QueryPriorityHeaderKey), 10, 64) - if err != nil { + priority, ok := stats.FromContext(s.ctx).LoadPriority() + if !ok { return 0 } diff --git a/pkg/util/http.go b/pkg/util/http.go index 41daae0fc65..09fb3df38c1 100644 --- a/pkg/util/http.go +++ b/pkg/util/http.go @@ -21,7 +21,6 @@ import ( yamlv3 "gopkg.in/yaml.v3" ) -const QueryPriorityHeaderKey = "X-Cortex-Query-Priority" const messageSizeLargerErrFmt = "received message larger than max (%d vs %d)" // IsRequestBodyTooLarge returns true if the error is "http: request body too large". diff --git a/pkg/util/time.go b/pkg/util/time.go index a28c84b0462..96142d977bd 100644 --- a/pkg/util/time.go +++ b/pkg/util/time.go @@ -5,10 +5,13 @@ import ( "math/rand" "net/http" "strconv" + "strings" "time" "github.com/pkg/errors" "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/promql" + "github.com/prometheus/prometheus/promql/parser" "github.com/weaveworks/common/httpgrpc" ) @@ -35,6 +38,10 @@ func FormatTimeModel(t model.Time) string { return TimeFromMillis(int64(t)).String() } +func FormatMillisToSeconds(ms int64) string { + return strconv.FormatFloat(float64(ms)/float64(1000), 'f', -1, 64) +} + // ParseTime parses the string into an int64, milliseconds since epoch. func ParseTime(s string) (int64, error) { if t, err := strconv.ParseFloat(s, 64); err == nil { @@ -98,3 +105,34 @@ func NewDisableableTicker(interval time.Duration) (func(), <-chan time.Time) { tick := time.NewTicker(interval) return func() { tick.Stop() }, tick.C } + +// FindMinMaxTime returns the time in milliseconds of the earliest and latest point in time the statement will try to process. +// This takes into account offsets, @ modifiers, and range selectors. +// If the expression does not select series, then FindMinMaxTime returns (0, 0). +func FindMinMaxTime(r *http.Request, expr parser.Expr, lookbackDelta time.Duration, now time.Time) (int64, int64) { + isQuery := strings.HasSuffix(r.URL.Path, "/query") + + var startTime, endTime int64 + if isQuery { + if t, err := ParseTimeParam(r, "time", now.UnixMilli()); err == nil { + startTime = t + endTime = t + } + } else { + if st, err := ParseTime(r.FormValue("start")); err == nil { + if et, err := ParseTime(r.FormValue("end")); err == nil { + startTime = st + endTime = et + } + } + } + + es := &parser.EvalStmt{ + Expr: expr, + Start: TimeFromMillis(startTime), + End: TimeFromMillis(endTime), + LookbackDelta: lookbackDelta, + } + + return promql.FindMinMaxTime(es) +} diff --git a/pkg/util/time_test.go b/pkg/util/time_test.go index ab1da4a85bf..97ee82e8e42 100644 --- a/pkg/util/time_test.go +++ b/pkg/util/time_test.go @@ -1,10 +1,14 @@ package util import ( + "bytes" "fmt" + "net/http" + "strconv" "testing" "time" + "github.com/prometheus/prometheus/promql/parser" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -131,3 +135,56 @@ func TestNewDisableableTicker_Disabled(t *testing.T) { break } } + +func TestFindMinMaxTime(t *testing.T) { + now := time.Now() + + type testCase struct { + query string + lookbackDelta time.Duration + queryStartTime time.Time + queryEndTime time.Time + expectedMinTime time.Time + expectedMaxTime time.Time + } + + tests := map[string]testCase{ + "should consider min and max of the query param": { + query: "up", + queryStartTime: now.Add(-1 * time.Hour), + queryEndTime: now, + expectedMinTime: now.Add(-1 * time.Hour), + expectedMaxTime: now, + }, + "should consider min and max of inner queries": { + query: "go_gc_duration_seconds_count[2h] offset 30m + go_gc_duration_seconds_count[3h] offset 1h", + queryStartTime: now.Add(-1 * time.Hour), + queryEndTime: now, + expectedMinTime: now.Add(-5 * time.Hour), + expectedMaxTime: now.Add(-30 * time.Minute), + }, + "should consider lookback delta": { + query: "up", + lookbackDelta: 1 * time.Hour, + queryStartTime: now.Add(-1 * time.Hour), + queryEndTime: now, + expectedMinTime: now.Add(-2 * time.Hour), + expectedMaxTime: now, + }, + } + + for testName, testData := range tests { + t.Run(testName, func(t *testing.T) { + expr, _ := parser.ParseExpr(testData.query) + + url := "/query_range?query=" + testData.query + + "&start=" + strconv.FormatInt(testData.queryStartTime.Truncate(time.Minute).Unix(), 10) + + "&end=" + strconv.FormatInt(testData.queryEndTime.Truncate(time.Minute).Unix(), 10) + req, _ := http.NewRequest(http.MethodPost, url, bytes.NewReader([]byte{})) + + minTime, maxTime := FindMinMaxTime(req, expr, testData.lookbackDelta, now) + assert.Equal(t, testData.expectedMinTime.Truncate(time.Minute).UnixMilli(), minTime) + assert.Equal(t, testData.expectedMaxTime.Truncate(time.Minute).UnixMilli(), maxTime) + }) + } +} diff --git a/pkg/util/validation/limits.go b/pkg/util/validation/limits.go index f5cbe5f2b54..54174cc6be4 100644 --- a/pkg/util/validation/limits.go +++ b/pkg/util/validation/limits.go @@ -64,13 +64,13 @@ type PriorityDef struct { type QueryAttribute struct { Regex string `yaml:"regex" json:"regex" doc:"nocli|description=Regex that the query string should match. If not set, it won't be checked."` - TimeWindow TimeWindow `yaml:"time_window" json:"time_window" doc:"nocli|description=Time window that the query should be within. If not set, it won't be checked."` + TimeWindow TimeWindow `yaml:"time_window" json:"time_window" doc:"nocli|description=Overall data select time window (including range selectors, modifiers and lookback delta) that the query should be within. If not set, it won't be checked."` CompiledRegex *regexp.Regexp } type TimeWindow struct { - Start model.Duration `yaml:"start" json:"start" doc:"nocli|description=Start of the time window that the query should be within. If set to 0, it won't be checked.|default=0"` - End model.Duration `yaml:"end" json:"end" doc:"nocli|description=End of the time window that the query should be within. If set to 0, it won't be checked.|default=0"` + Start model.Duration `yaml:"start" json:"start" doc:"nocli|description=Start of the data select time window (including range selectors, modifiers and lookback delta) that the query should be within. If set to 0, it won't be checked.|default=0"` + End model.Duration `yaml:"end" json:"end" doc:"nocli|description=End of the data select time window (including range selectors, modifiers and lookback delta) that the query should be within. If set to 0, it won't be checked.|default=0"` } // Limits describe all the limits for users; can be used to describe global default