Skip to content

Commit 98823ff

Browse files
committed
fix(api): cap label values returned by ClickHouse's Prometheus handler
1 parent a09fcfa commit 98823ff

2 files changed

Lines changed: 43 additions & 1 deletion

File tree

‎packages/api/src/routers/api/__tests__/prometheus.int.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
clickhousePrometheusUpstream,
1818
joinPrometheusUpstreamUrl,
1919
} from '@/controllers/promql';
20+
import { PROMETHEUS_MAX_RESULT_ROWS } from '@/controllers/timeseriesEngine';
2021
import {
2122
DEFAULT_DATABASE,
2223
dropTimeSeriesTable,
@@ -2283,6 +2284,35 @@ describe('prometheus router', () => {
22832284
},
22842285
);
22852286

2287+
// The handler returns all values in one row, which max_result_rows can't cap.
2288+
it.each([
2289+
['no limit', ''],
2290+
['limit=0', '&limit=0'],
2291+
['a limit over the cap', `&limit=${PROMETHEUS_MAX_RESULT_ROWS + 1}`],
2292+
])(
2293+
'caps the values the handler returns when the request has %s',
2294+
async (_case, limitQuery) => {
2295+
stubServerVersion([26, 10, 0, 0]);
2296+
const { agent, team } = await getLoggedInAgent(server);
2297+
const source = await seedClickHouseSource(team._id);
2298+
mockFetch
2299+
.mockResolvedValueOnce(handlerProbeResponse())
2300+
.mockResolvedValueOnce(
2301+
fakeUpstreamResponse({ status: 'success', data: [] }),
2302+
);
2303+
2304+
await agent
2305+
.get('/v1/prometheus/label/job/values')
2306+
.query(`sourceId=${source._id.toString()}${limitQuery}`)
2307+
.expect(200);
2308+
2309+
const url = new URL(String(mockFetch.mock.calls[1][0]));
2310+
expect(url.searchParams.getAll('limit')).toEqual([
2311+
String(PROMETHEUS_MAX_RESULT_ROWS),
2312+
]);
2313+
},
2314+
);
2315+
22862316
// 26.8's handler answers label lookups with a 400 ("not implemented"), so
22872317
// the route must never send them there.
22882318
it.each(ROUTES)(

‎packages/api/src/routers/api/prometheus.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
resolvePromqlTarget,
2323
} from '@/controllers/promql';
2424
import {
25+
PROMETHEUS_MAX_RESULT_ROWS,
2526
queryInstantViaTableFunction,
2627
queryLabelNames,
2728
queryLabelValues,
@@ -860,10 +861,21 @@ async function handleLabelLookup(
860861
backend = 'clickhouse';
861862
if ((await resolveClickhouseLabelBackend(target)) === 'clickhouse-http') {
862863
const { upstream, headers } = clickhousePrometheusRequest(connection);
864+
// The handler returns every value in a single row, so the upstream's
865+
// max_result_rows never applies. Cap the values themselves instead.
866+
const cappedLimit = Math.min(
867+
limit || PROMETHEUS_MAX_RESULT_ROWS,
868+
PROMETHEUS_MAX_RESULT_ROWS,
869+
);
863870
const status = await proxyToPrometheus(
864871
upstream,
865872
`${CLICKHOUSE_PROMETHEUS_API_PREFIX}${apiPath}`,
866-
{ ...proxyParams, database: databaseName, table: tableName },
873+
{
874+
...proxyParams,
875+
limit: String(cappedLimit),
876+
database: databaseName,
877+
table: tableName,
878+
},
867879
res,
868880
headers,
869881
CLICKHOUSE_HTTP_API_UNAVAILABLE_MESSAGE,

0 commit comments

Comments
 (0)