Track data selection time range in query stats independent of query priority and rejection

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. #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

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) {

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{