From de0ce87fd4632976cc291e61bf36024761fad929 Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Thu, 26 Jun 2025 19:04:04 +0200 Subject: [PATCH] Prometheus: Apply limit to metadata request (#107068) * use limit when calling metadata endpoint * request metadata with limit * betterer --- .betterer.results | 6 +- .../src/language_provider.test.ts | 149 +++++++++++++++++- .../src/language_provider.ts | 12 +- 3 files changed, 156 insertions(+), 11 deletions(-) diff --git a/.betterer.results b/.betterer.results index 7c38c251319..67d05506a29 100644 --- a/.betterer.results +++ b/.betterer.results @@ -438,7 +438,11 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"] + [0, 0, 0, "Unexpected any. Specify a different type.", "3"], + [0, 0, 0, "Unexpected any. Specify a different type.", "4"], + [0, 0, 0, "Unexpected any. Specify a different type.", "5"], + [0, 0, 0, "Unexpected any. Specify a different type.", "6"], + [0, 0, 0, "Unexpected any. Specify a different type.", "7"] ], "packages/grafana-prometheus/src/language_provider.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/packages/grafana-prometheus/src/language_provider.test.ts b/packages/grafana-prometheus/src/language_provider.test.ts index ec5cba785e5..5d7396cda95 100644 --- a/packages/grafana-prometheus/src/language_provider.test.ts +++ b/packages/grafana-prometheus/src/language_provider.test.ts @@ -726,6 +726,7 @@ describe('PrometheusLanguageProvider with feature toggle', () => { cacheLevel: PrometheusCacheLevel.None, getIntervalVars: () => ({}), getRangeScopedVars: () => ({}), + seriesLimit: DEFAULT_SERIES_LIMIT, } as unknown as PrometheusDatasource; describe('constructor', () => { @@ -770,21 +771,127 @@ describe('PrometheusLanguageProvider with feature toggle', () => { expect(provider.retrieveMetricsMetadata()).toEqual(mockMetadata); expect(provider.metricsMetadata).toEqual(mockMetadata); // Check backward compatibility }); + + it('should call queryMetricsMetadata with datasource seriesLimit during start', async () => { + const customSeriesLimit = 5000; + const datasourceWithCustomLimit = { + ...defaultDatasource, + seriesLimit: customSeriesLimit, + } as PrometheusDatasource; + + const provider = new PrometheusLanguageProvider(datasourceWithCustomLimit); + const mockMetadata = { metric1: { type: 'counter', help: 'help text' } }; + + // Mock the resource client's start method + const resourceClientStartSpy = jest.spyOn(provider['resourceClient'], 'start').mockResolvedValue(); + const queryMetricsMetadataSpy = jest.spyOn(provider, 'queryMetricsMetadata').mockResolvedValue(mockMetadata); + + await provider.start(); + + expect(resourceClientStartSpy).toHaveBeenCalled(); + expect(queryMetricsMetadataSpy).toHaveBeenCalledWith(customSeriesLimit); + }); }); describe('queryMetricsMetadata', () => { - it('should fetch and store metadata', async () => { + it('should fetch and store metadata without limit', async () => { const provider = new PrometheusLanguageProvider(defaultDatasource); const mockMetadata = { metric1: { type: 'counter', help: 'help text' } }; const queryMetadataSpy = jest.spyOn(provider as any, '_queryMetadata').mockResolvedValue(mockMetadata); const result = await provider.queryMetricsMetadata(); - expect(queryMetadataSpy).toHaveBeenCalled(); + expect(queryMetadataSpy).toHaveBeenCalledWith(undefined); expect(result).toEqual(mockMetadata); expect(provider.retrieveMetricsMetadata()).toEqual(mockMetadata); }); + it('should fetch and store metadata with custom limit', async () => { + const provider = new PrometheusLanguageProvider(defaultDatasource); + const mockMetadata = { metric1: { type: 'counter', help: 'help text' } }; + const customLimit = 1000; + const queryMetadataSpy = jest.spyOn(provider as any, '_queryMetadata').mockResolvedValue(mockMetadata); + + const result = await provider.queryMetricsMetadata(customLimit); + + expect(queryMetadataSpy).toHaveBeenCalledWith(customLimit); + expect(result).toEqual(mockMetadata); + expect(provider.retrieveMetricsMetadata()).toEqual(mockMetadata); + }); + + it('should pass limit parameter to the metadata API endpoint', async () => { + const provider = new PrometheusLanguageProvider(defaultDatasource); + const requestSpy = jest.spyOn(provider, 'request').mockResolvedValue({ + metric1: { type: 'counter', help: 'help text' }, + }); + const customLimit = 500; + + await provider.queryMetricsMetadata(customLimit); + + expect(requestSpy).toHaveBeenCalledWith( + '/api/v1/metadata', + { limit: customLimit }, + expect.objectContaining({ + showErrorAlert: false, + }) + ); + }); + + it('should use DEFAULT_SERIES_LIMIT when no limit is provided', async () => { + const provider = new PrometheusLanguageProvider(defaultDatasource); + const requestSpy = jest.spyOn(provider, 'request').mockResolvedValue({ + metric1: { type: 'counter', help: 'help text' }, + }); + + await provider.queryMetricsMetadata(); + + expect(requestSpy).toHaveBeenCalledWith( + '/api/v1/metadata', + { limit: DEFAULT_SERIES_LIMIT }, + expect.objectContaining({ + showErrorAlert: false, + }) + ); + }); + + it('should pass zero limit when explicitly set', async () => { + const provider = new PrometheusLanguageProvider(defaultDatasource); + const requestSpy = jest.spyOn(provider, 'request').mockResolvedValue({ + metric1: { type: 'counter', help: 'help text' }, + }); + + await provider.queryMetricsMetadata(0); + + expect(requestSpy).toHaveBeenCalledWith( + '/api/v1/metadata', + { limit: 0 }, + expect.objectContaining({ + showErrorAlert: false, + }) + ); + }); + + it('should include cache headers in the request', async () => { + const provider = new PrometheusLanguageProvider({ + ...defaultDatasource, + cacheLevel: PrometheusCacheLevel.Medium, + } as PrometheusDatasource); + const requestSpy = jest.spyOn(provider, 'request').mockResolvedValue({}); + + await provider.queryMetricsMetadata(1000); + + expect(requestSpy).toHaveBeenCalledWith( + '/api/v1/metadata', + { limit: 1000 }, + expect.objectContaining({ + showErrorAlert: false, + headers: expect.objectContaining({ + 'X-Grafana-Cache': expect.stringMatching(/private, max-age=\d+/), + }), + }) + ); + }); + it('should handle undefined metadata response', async () => { const provider = new PrometheusLanguageProvider(defaultDatasource); const queryMetadataSpy = jest.spyOn(provider as any, '_queryMetadata').mockResolvedValue(undefined); @@ -796,18 +903,52 @@ describe('PrometheusLanguageProvider with feature toggle', () => { expect(provider.retrieveMetricsMetadata()).toEqual({}); }); + it('should handle null metadata response', async () => { + const provider = new PrometheusLanguageProvider(defaultDatasource); + const queryMetadataSpy = jest.spyOn(provider as any, '_queryMetadata').mockResolvedValue(null); + + const result = await provider.queryMetricsMetadata(1000); + + expect(queryMetadataSpy).toHaveBeenCalledWith(1000); + expect(result).toEqual({}); + expect(provider.retrieveMetricsMetadata()).toEqual({}); + }); + it('should handle endpoint errors and set empty metadata', async () => { const provider = new PrometheusLanguageProvider(defaultDatasource); const queryMetadataSpy = jest .spyOn(provider as any, '_queryMetadata') .mockRejectedValue(new Error('Endpoint not found')); - const result = await provider.queryMetricsMetadata(); + const result = await provider.queryMetricsMetadata(1000); - expect(queryMetadataSpy).toHaveBeenCalled(); + expect(queryMetadataSpy).toHaveBeenCalledWith(1000); expect(result).toEqual({}); expect(provider.retrieveMetricsMetadata()).toEqual({}); }); + + it('should handle network timeout errors gracefully', async () => { + const provider = new PrometheusLanguageProvider(defaultDatasource); + const timeoutError = new Error('Request timeout'); + timeoutError.name = 'TimeoutError'; + const queryMetadataSpy = jest.spyOn(provider as any, '_queryMetadata').mockRejectedValue(timeoutError); + + const result = await provider.queryMetricsMetadata(500); + + expect(queryMetadataSpy).toHaveBeenCalledWith(500); + expect(result).toEqual({}); + expect(provider.retrieveMetricsMetadata()).toEqual({}); + }); + + it('should maintain backward compatibility by setting deprecated metricsMetadata property', async () => { + const provider = new PrometheusLanguageProvider(defaultDatasource); + const mockMetadata = { metric1: { type: 'counter', help: 'help text' } }; + jest.spyOn(provider as any, '_queryMetadata').mockResolvedValue(mockMetadata); + + await provider.queryMetricsMetadata(250); + + expect(provider.retrieveMetricsMetadata()).toEqual(mockMetadata); + }); }); describe('queryLabelKeys and queryLabelValues', () => { diff --git a/packages/grafana-prometheus/src/language_provider.ts b/packages/grafana-prometheus/src/language_provider.ts index cfedcc9bdd0..a61be690cb8 100644 --- a/packages/grafana-prometheus/src/language_provider.ts +++ b/packages/grafana-prometheus/src/language_provider.ts @@ -522,7 +522,7 @@ export interface PrometheusLanguageProviderInterface retrieveMetrics: () => string[]; retrieveLabelKeys: () => string[]; - queryMetricsMetadata: () => Promise; + queryMetricsMetadata: (limit?: number) => Promise; queryLabelKeys: (timeRange: TimeRange, match?: string, limit?: number) => Promise; queryLabelValues: (timeRange: TimeRange, labelKey: string, match?: string, limit?: number) => Promise; } @@ -577,7 +577,7 @@ export class PrometheusLanguageProvider extends PromQlLanguageProvider implement if (this.datasource.lookupsDisabled) { return []; } - await Promise.all([this.resourceClient.start(timeRange), this.queryMetricsMetadata()]); + await Promise.all([this.resourceClient.start(timeRange), this.queryMetricsMetadata(this.datasource.seriesLimit)]); return this._backwardCompatibleStart(); }; @@ -599,12 +599,12 @@ export class PrometheusLanguageProvider extends PromQlLanguageProvider implement * * @returns {Promise} Promise that resolves when metadata has been fetched */ - private _queryMetadata = async () => { + private _queryMetadata = async (limit?: number): Promise => { const secondsInDay = 86400; const headers = buildCacheHeaders(getDaysToCacheMetadata(this.datasource.cacheLevel) * secondsInDay); const metadata = await this.request( API_V1.METADATA, - {}, + { limit: limit ?? this.datasource.seriesLimit }, { showErrorAlert: false, ...headers, @@ -660,9 +660,9 @@ export class PrometheusLanguageProvider extends PromQlLanguageProvider implement * * @returns {Promise} Promise that resolves to the fetched metadata */ - public queryMetricsMetadata = async (): Promise => { + public queryMetricsMetadata = async (limit?: number): Promise => { try { - this._metricsMetadata = (await this._queryMetadata()) ?? {}; + this._metricsMetadata = (await this._queryMetadata(limit)) ?? {}; } catch (error) { this._metricsMetadata = {}; }