From 3bc622838e3a6c2aa9bacbd8b219745ac16dfbf5 Mon Sep 17 00:00:00 2001 From: Ben Ye Date: Fri, 24 Jul 2026 06:13:11 +0000 Subject: [PATCH 1/2] Track data selection time range in query stats independent of query priority and rejection Previously, rejectQueryOrSetPriority returned early when both query priority and query rejection were disabled, so DataSelectMinTime and DataSelectMaxTime were never set in the request stats. Track the data selection time range for query and query_range requests regardless of whether query priority or query rejection is enabled. Fixes #6484 Signed-off-by: Ben Ye --- CHANGELOG.md | 1 + .../tripperware/query_attribute_matcher.go | 19 +++++-- .../query_attribute_matcher_test.go | 55 +++++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 541cc6856e3..93e19bc1e5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ * [BUGFIX] Querier: Fix unbounded resource leak in the bucket-scan blocks finder (used when the bucket index is disabled). Per-tenant metadata fetchers, their Prometheus registries, and on-disk meta caches are now evicted once a tenant is no longer active, instead of being retained for the lifetime of the process. #7573 * [BUGFIX] Alertmanager: Fix data race between ApplyConfig's dispatcher/inhibitor startup and Stop during config reload and shutdown, and reject lazy tenant creation after shutdown begins. #7618 * [BUGFIX] Distributor: Release the push worker pool goroutines on shutdown by stopping the async executor during the stopping phase when `-distributor.num-push-workers` is set. #7602 +* [BUGFIX] Query Frontend: Track data selection min and max time of query requests in query stats regardless of whether query priority and query rejection are enabled. #7722 * [BUGFIX] Querier: Fix flake in integration tests TestQuerierWithStoreGatewayDataBytesLimits and TestQuerierWithBlocksStorageLimits by waiting for the querier to see the store-gateway ACTIVE in the ring before querying. #7614 * [BUGFIX] Ruler: Register xfunctions (xincrease, xrate, xdelta) in the global parser before loading rule files. #7621 * [BUGFIX] Security: Reject empty entries in `-distributor.sign-write-requests-keys` caused by stray or trailing commas (e.g. `newkey,`). Previously these were silently accepted and produced an empty signing key, which downgraded HMAC stream-push authentication to a forgeable signature. Misconfigured flags now fail at process startup; audit your configs before upgrading. #7587 diff --git a/pkg/querier/tripperware/query_attribute_matcher.go b/pkg/querier/tripperware/query_attribute_matcher.go index 7e0990162bb..54dcaee85cf 100644 --- a/pkg/querier/tripperware/query_attribute_matcher.go +++ b/pkg/querier/tripperware/query_attribute_matcher.go @@ -18,9 +18,6 @@ import ( const QueryRejectErrorMessage = "This query does not perform well and has been rejected by the service operator." func rejectQueryOrSetPriority(r *http.Request, now time.Time, lookbackDelta time.Duration, limits Limits, userStr string, rejectedQueriesPerTenant *prometheus.CounterVec) error { - if limits == nil || (!limits.QueryPriority(userStr).Enabled && !limits.QueryRejection(userStr).Enabled) { - return nil - } op := getOperation(r) reqStats := stats.FromContext(r.Context()) @@ -32,6 +29,15 @@ func rejectQueryOrSetPriority(r *http.Request, now time.Time, lookbackDelta time } minTime, maxTime := util.FindMinMaxTime(r, expr, lookbackDelta, now) + // Track data selection time range for query stats purpose. It should + // be tracked regardless of query priority and query rejection being enabled. + reqStats.SetDataSelectMaxTime(maxTime) + reqStats.SetDataSelectMinTime(minTime) + + if limits == nil || (!limits.QueryPriority(userStr).Enabled && !limits.QueryRejection(userStr).Enabled) { + return nil + } + if queryReject := limits.QueryRejection(userStr); queryReject.Enabled && query != "" { for _, attribute := range queryReject.QueryAttributes { if matchAttributeForExpressionQuery(attribute, op, r, query, now, minTime, maxTime) { @@ -41,9 +47,6 @@ func rejectQueryOrSetPriority(r *http.Request, now time.Time, lookbackDelta time } } - reqStats.SetDataSelectMaxTime(maxTime) - reqStats.SetDataSelectMinTime(minTime) - if queryPriority := limits.QueryPriority(userStr); queryPriority.Enabled && len(queryPriority.Priorities) != 0 && query != "" { for _, priority := range queryPriority.Priorities { for _, attribute := range priority.QueryAttributes { @@ -56,6 +59,10 @@ func rejectQueryOrSetPriority(r *http.Request, now time.Time, lookbackDelta time reqStats.SetPriority(queryPriority.DefaultPriority) } } else { + if limits == nil || (!limits.QueryPriority(userStr).Enabled && !limits.QueryRejection(userStr).Enabled) { + return nil + } + if queryReject := limits.QueryRejection(userStr); queryReject.Enabled && (op == "series" || op == "labels" || op == "label_values") { for _, attribute := range queryReject.QueryAttributes { if matchAttributeForMetadataQuery(attribute, op, r, now) { diff --git a/pkg/querier/tripperware/query_attribute_matcher_test.go b/pkg/querier/tripperware/query_attribute_matcher_test.go index 31d6b455999..60d8d82f5a2 100644 --- a/pkg/querier/tripperware/query_attribute_matcher_test.go +++ b/pkg/querier/tripperware/query_attribute_matcher_test.go @@ -88,6 +88,61 @@ func Test_rejectQueryOrSetPriorityShouldReturnDefaultPriorityIfNotEnabledOrInval } } +func Test_rejectQueryOrSetPriorityShouldTrackDataSelectTimeRangeIndependentOfQueryPriorityAndRejection(t *testing.T) { + disabledLimits := mockLimits{ + queryPriority: validation.QueryPriority{Enabled: false}, + queryRejection: validation.QueryRejection{Enabled: false}, + } + lookbackDelta := 5 * time.Minute + + type testCase struct { + limits Limits + path string + expectedMinTime int64 + expectedMaxTime int64 + } + + tests := map[string]testCase{ + "should track data select time range for instant query when query priority and rejection are disabled": { + limits: disabledLimits, + path: "/api/v1/query?time=1536716898&query=sum%28container_memory_rss%29+by+%28namespace%29", + // min time is (time - lookbackDelta) exclusive. + expectedMinTime: 1536716898000 - lookbackDelta.Milliseconds() + 1, + expectedMaxTime: 1536716898000, + }, + "should track data select time range for range query when query priority and rejection are disabled": { + limits: disabledLimits, + path: "/api/v1/query_range?start=1536716898&end=1536720498&step=15&query=sum%28container_memory_rss%29+by+%28namespace%29", + expectedMinTime: 1536716898000 - lookbackDelta.Milliseconds() + 1, + expectedMaxTime: 1536720498000, + }, + "should track data select time range when limits is nil": { + limits: nil, + path: "/api/v1/query?time=1536716898&query=up", + expectedMinTime: 1536716898000 - lookbackDelta.Milliseconds() + 1, + expectedMaxTime: 1536716898000, + }, + "should not track data select time range for metadata query": { + limits: disabledLimits, + path: "/api/v1/labels?match[]=up", + expectedMinTime: 0, + expectedMaxTime: 0, + }, + } + + for testName, testData := range tests { + t.Run(testName, func(t *testing.T) { + req, err := http.NewRequest("GET", testData.path, http.NoBody) + require.NoError(t, err) + reqStats, ctx := stats.ContextWithEmptyStats(context.Background()) + req = req.WithContext(ctx) + require.NoError(t, rejectQueryOrSetPriority(req, time.Now(), lookbackDelta, testData.limits, "", rejectedQueriesPerTenant)) + assert.Equal(t, testData.expectedMinTime, reqStats.LoadDataSelectMinTime()) + assert.Equal(t, testData.expectedMaxTime, reqStats.LoadDataSelectMaxTime()) + }) + } +} + func Test_rejectQueryOrSetPriorityShouldRejectIfMatches(t *testing.T) { now := time.Now() limits := mockLimits{ From 7635bd5cf91769241d5ba369a80e85faf314203e Mon Sep 17 00:00:00 2001 From: Ben Ye Date: Fri, 24 Jul 2026 06:14:03 +0000 Subject: [PATCH 2/2] Fix PR number in CHANGELOG entry Signed-off-by: Ben Ye --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93e19bc1e5c..330b9e0020f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,7 +68,7 @@ * [BUGFIX] Querier: Fix unbounded resource leak in the bucket-scan blocks finder (used when the bucket index is disabled). Per-tenant metadata fetchers, their Prometheus registries, and on-disk meta caches are now evicted once a tenant is no longer active, instead of being retained for the lifetime of the process. #7573 * [BUGFIX] Alertmanager: Fix data race between ApplyConfig's dispatcher/inhibitor startup and Stop during config reload and shutdown, and reject lazy tenant creation after shutdown begins. #7618 * [BUGFIX] Distributor: Release the push worker pool goroutines on shutdown by stopping the async executor during the stopping phase when `-distributor.num-push-workers` is set. #7602 -* [BUGFIX] Query Frontend: Track data selection min and max time of query requests in query stats regardless of whether query priority and query rejection are enabled. #7722 +* [BUGFIX] Query Frontend: Track data selection min and max time of query requests in query stats regardless of whether query priority and query rejection are enabled. #7724 * [BUGFIX] Querier: Fix flake in integration tests TestQuerierWithStoreGatewayDataBytesLimits and TestQuerierWithBlocksStorageLimits by waiting for the querier to see the store-gateway ACTIVE in the ring before querying. #7614 * [BUGFIX] Ruler: Register xfunctions (xincrease, xrate, xdelta) in the global parser before loading rule files. #7621 * [BUGFIX] Security: Reject empty entries in `-distributor.sign-write-requests-keys` caused by stray or trailing commas (e.g. `newkey,`). Previously these were silently accepted and produced an empty signing key, which downgraded HMAC stream-push authentication to a forgeable signature. Misconfigured flags now fail at process startup; audit your configs before upgrading. #7587