fix(api): Send warnings only if the limit is really exceeded (prometh…

…eus#14116) for the the series, label names and label values APIs Add warnings count check to TestEndpoints The limit param was added in prometheus#13396 Signed-off-by: machine424 <[email protected]>
rajagopalanand · May 21, 2024 · fabcd7e · fabcd7e
1 parent 5c85a55
commit fabcd7e
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 6 deletions.
diff --git a/web/api/v1/api.go b/web/api/v1/api.go
@@ -704,7 +704,7 @@ func (api *API) labelNames(r *http.Request) apiFuncResult {
 		names = []string{}
 	}
 
-	if len(names) >= limit {
+	if len(names) > limit {
 		names = names[:limit]
 		warnings = warnings.Add(errors.New("results truncated due to limit"))
 	}
@@ -793,7 +793,7 @@ func (api *API) labelValues(r *http.Request) (result apiFuncResult) {
 
 	slices.Sort(vals)
 
-	if len(vals) >= limit {
+	if len(vals) > limit {
 		vals = vals[:limit]
 		warnings = warnings.Add(errors.New("results truncated due to limit"))
 	}
@@ -889,7 +889,8 @@ func (api *API) series(r *http.Request) (result apiFuncResult) {
 		}
 		metrics = append(metrics, set.At().Labels())
 
-		if len(metrics) >= limit {
+		if len(metrics) > limit {
+			metrics = metrics[:limit]
 			warnings.Add(errors.New("results truncated due to limit"))
 			return apiFuncResult{metrics, nil, warnings, closer}
 		}

diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go
@@ -1060,6 +1060,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 		responseLen           int // If nonzero, check only the length; `response` is ignored.
 		responseMetadataTotal int
 		responseAsJSON        string
+		warningsCount         int
 		errType               errorType
 		sorter                func(interface{})
 		metadata              []targetMetadata
@@ -1417,7 +1418,17 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 				"match[]": []string{"test_metric1"},
 				"limit":   []string{"1"},
 			},
-			responseLen: 1, // API does not specify which particular value will come back.
+			responseLen:   1, // API does not specify which particular value will come back.
+			warningsCount: 1,
+		},
+		{
+			endpoint: api.series,
+			query: url.Values{
+				"match[]": []string{"test_metric1"},
+				"limit":   []string{"2"},
+			},
+			responseLen:   2, // API does not specify which particular value will come back.
+			warningsCount: 0, // No warnings if limit isn't exceeded.
 		},
 		// Missing match[] query params in series requests.
 		{
@@ -2700,7 +2711,19 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 				query: url.Values{
 					"limit": []string{"2"},
 				},
-				responseLen: 2, // API does not specify which particular values will come back.
+				responseLen:   2, // API does not specify which particular values will come back.
+				warningsCount: 1,
+			},
+			{
+				endpoint: api.labelValues,
+				params: map[string]string{
+					"name": "__name__",
+				},
+				query: url.Values{
+					"limit": []string{"4"},
+				},
+				responseLen:   4, // API does not specify which particular values will come back.
+				warningsCount: 0, // No warnings if limit isn't exceeded.
 			},
 			// Label names.
 			{
@@ -2847,7 +2870,16 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 				query: url.Values{
 					"limit": []string{"2"},
 				},
-				responseLen: 2, // API does not specify which particular values will come back.
+				responseLen:   2, // API does not specify which particular values will come back.
+				warningsCount: 1,
+			},
+			{
+				endpoint: api.labelNames,
+				query: url.Values{
+					"limit": []string{"3"},
+				},
+				responseLen:   3, // API does not specify which particular values will come back.
+				warningsCount: 0, // No warnings if limit isn't exceeded.
 			},
 		}...)
 	}
@@ -2924,6 +2956,8 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 						require.NoError(t, err)
 						require.JSONEq(t, test.responseAsJSON, string(s))
 					}
+
+					require.Len(t, res.warnings, test.warningsCount)
 				})
 			}
 		})