From 93060204269b41a4c0f606284e9eaa0adc637c84 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Fri, 24 Nov 2023 11:38:36 +0100 Subject: [PATCH] Loki: Update `getStats` logic and remove reliance on `timeSrv` (#78603) * Loki: For stats queries, use timeRange provided by query editor * Add comment * Update public/app/plugins/datasource/loki/components/LokiQueryEditor.tsx Co-authored-by: Matias Chomicki * Rebane variable --------- Co-authored-by: Matias Chomicki --- .../loki/components/LokiQueryEditor.tsx | 6 +-- .../datasource/loki/datasource.test.ts | 43 ++++++++++++------- .../app/plugins/datasource/loki/datasource.ts | 24 ++++++----- 3 files changed, 44 insertions(+), 29 deletions(-) diff --git a/public/app/plugins/datasource/loki/components/LokiQueryEditor.tsx b/public/app/plugins/datasource/loki/components/LokiQueryEditor.tsx index ac5f2d9222f..bd248c76a60 100644 --- a/public/app/plugins/datasource/loki/components/LokiQueryEditor.tsx +++ b/public/app/plugins/datasource/loki/components/LokiQueryEditor.tsx @@ -96,7 +96,7 @@ export const LokiQueryEditor = React.memo((props) => { }; useEffect(() => { - const update = shouldUpdateStats( + const shouldUpdate = shouldUpdateStats( query.expr, previousQueryExpr, timeRange, @@ -104,9 +104,9 @@ export const LokiQueryEditor = React.memo((props) => { query.queryType, previousQueryType ); - if (update) { + if (shouldUpdate && timeRange) { const makeAsyncRequest = async () => { - const stats = await datasource.getStats(query); + const stats = await datasource.getStats(query, timeRange); setQueryStats(stats); }; makeAsyncRequest(); diff --git a/public/app/plugins/datasource/loki/datasource.test.ts b/public/app/plugins/datasource/loki/datasource.test.ts index 8ec61abbfa7..93426033ba2 100644 --- a/public/app/plugins/datasource/loki/datasource.test.ts +++ b/public/app/plugins/datasource/loki/datasource.test.ts @@ -14,6 +14,7 @@ import { dateTime, FieldType, SupplementaryQueryType, + TimeRange, ToggleFilterAction, } from '@grafana/data'; import { @@ -112,6 +113,12 @@ const testLogsResponse: FetchResponse = { config: {} as unknown as BackendSrvRequest, }; +const mockTimeRange = { + from: dateTime(0), + to: dateTime(1), + raw: { from: dateTime(0), to: dateTime(1) }, +}; + interface AdHocFilter { condition: string; key: string; @@ -1349,22 +1356,22 @@ describe('LokiDatasource', () => { beforeEach(() => { ds = createLokiDatasource(templateSrvStub); ds.statsMetadataRequest = jest.fn().mockResolvedValue({ streams: 1, chunks: 1, bytes: 1, entries: 1 }); - ds.interpolateString = jest.fn().mockImplementation((value: string) => value.replace('$__interval', '1m')); + ds.interpolateString = jest.fn().mockImplementation((value: string) => value.replace('$__auto', '1m')); query = { refId: 'A', expr: '', queryType: LokiQueryType.Range }; }); it('uses statsMetadataRequest', async () => { query.expr = '{foo="bar"}'; - const result = await ds.getQueryStats(query); + const result = await ds.getQueryStats(query, mockTimeRange); expect(ds.statsMetadataRequest).toHaveBeenCalled(); expect(result).toEqual({ streams: 1, chunks: 1, bytes: 1, entries: 1 }); }); it('supports queries with template variables', async () => { - query.expr = 'rate({instance="server\\1"}[$__interval])'; - const result = await ds.getQueryStats(query); + query.expr = 'rate({instance="server\\1"}[$__auto])'; + const result = await ds.getQueryStats(query, mockTimeRange); expect(result).toEqual({ streams: 1, @@ -1376,7 +1383,7 @@ describe('LokiDatasource', () => { it('does not call stats if the query is invalid', async () => { query.expr = 'rate({label="value"}'; - const result = await ds.getQueryStats(query); + const result = await ds.getQueryStats(query, mockTimeRange); expect(ds.statsMetadataRequest).not.toHaveBeenCalled(); expect(result).toBe(undefined); @@ -1384,7 +1391,7 @@ describe('LokiDatasource', () => { it('combines the stats of each label matcher', async () => { query.expr = 'count_over_time({foo="bar"}[1m]) + count_over_time({test="test"}[1m])'; - const result = await ds.getQueryStats(query); + const result = await ds.getQueryStats(query, mockTimeRange); expect(ds.statsMetadataRequest).toHaveBeenCalled(); expect(result).toEqual({ streams: 2, chunks: 2, bytes: 2, entries: 2 }); @@ -1492,6 +1499,10 @@ describe('applyTemplateVariables', () => { describe('getStatsTimeRange', () => { let query: LokiQuery; let datasource: LokiDatasource; + const timeRange = { + from: 167255280000000, // 01 Jan 2023 06:00:00 GMT + to: 167263920000000, // 02 Jan 2023 06:00:00 GMT + } as unknown as TimeRange; beforeEach(() => { query = { refId: 'A', expr: '', queryType: LokiQueryType.Range }; @@ -1508,7 +1519,7 @@ describe('applyTemplateVariables', () => { // in this case (1 day) query.expr = '{job="grafana"}'; - expect(datasource.getStatsTimeRange(query, 0)).toEqual({ + expect(datasource.getStatsTimeRange(query, 0, timeRange)).toEqual({ start: 1672552800000000000, // 01 Jan 2023 06:00:00 GMT end: 1672639200000000000, // 02 Jan 2023 06:00:00 GMT }); @@ -1519,18 +1530,18 @@ describe('applyTemplateVariables', () => { query.queryType = LokiQueryType.Instant; query.expr = '{job="grafana"}'; - expect(datasource.getStatsTimeRange(query, 0)).toEqual({ + expect(datasource.getStatsTimeRange(query, 0, timeRange)).toEqual({ start: undefined, end: undefined, }); }); - it('should return the ds picker timerange', () => { + it('should return the ds picker time range', () => { // metric queries with range type should request ds picker timerange // in this case (1 day) query.expr = 'rate({job="grafana"}[5m])'; - expect(datasource.getStatsTimeRange(query, 0)).toEqual({ + expect(datasource.getStatsTimeRange(query, 0, timeRange)).toEqual({ start: 1672552800000000000, // 01 Jan 2023 06:00:00 GMT end: 1672639200000000000, // 02 Jan 2023 06:00:00 GMT }); @@ -1542,7 +1553,7 @@ describe('applyTemplateVariables', () => { query.queryType = LokiQueryType.Instant; query.expr = 'rate({job="grafana"}[5m])'; - expect(datasource.getStatsTimeRange(query, 0)).toEqual({ + expect(datasource.getStatsTimeRange(query, 0, timeRange)).toEqual({ start: 1672638900000000000, // 02 Jan 2023 05:55:00 GMT end: 1672639200000000000, // 02 Jan 2023 06:00:00 GMT }); @@ -1560,18 +1571,18 @@ describe('makeStatsRequest', () => { it('should return null if there is no query', () => { query.expr = ''; - expect(datasource.getStats(query)).resolves.toBe(null); + expect(datasource.getStats(query, mockTimeRange)).resolves.toBe(null); }); it('should return null if the query is invalid', () => { query.expr = '{job="grafana",'; - expect(datasource.getStats(query)).resolves.toBe(null); + expect(datasource.getStats(query, mockTimeRange)).resolves.toBe(null); }); it('should return null if the response has no data', () => { query.expr = '{job="grafana"}'; datasource.getQueryStats = jest.fn().mockResolvedValue({ streams: 0, chunks: 0, bytes: 0, entries: 0 }); - expect(datasource.getStats(query)).resolves.toBe(null); + expect(datasource.getStats(query, mockTimeRange)).resolves.toBe(null); }); it('should return the stats if the response has data', () => { @@ -1580,7 +1591,7 @@ describe('makeStatsRequest', () => { datasource.getQueryStats = jest .fn() .mockResolvedValue({ streams: 1, chunks: 12611, bytes: 12913664, entries: 78344 }); - expect(datasource.getStats(query)).resolves.toEqual({ + expect(datasource.getStats(query, mockTimeRange)).resolves.toEqual({ streams: 1, chunks: 12611, bytes: 12913664, @@ -1597,7 +1608,7 @@ describe('makeStatsRequest', () => { datasource.getQueryStats = jest .fn() .mockResolvedValue({ streams: 1, chunks: 12611, bytes: 12913664, entries: 78344 }); - expect(datasource.getStats(query)).resolves.toEqual({ + expect(datasource.getStats(query, mockTimeRange)).resolves.toEqual({ streams: 1, chunks: 12611, bytes: 12913664, diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index cda7a5cdf9f..456694d46ba 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -533,7 +533,7 @@ export class LokiDatasource * Used in `getStats`. Retrieves statistics for a Loki query and processes them into a QueryStats object. * @returns A Promise that resolves to a QueryStats object containing the query statistics or undefined if the query is invalid. */ - async getQueryStats(query: LokiQuery): Promise { + async getQueryStats(query: LokiQuery, timeRange: TimeRange): Promise { // if query is invalid, clear stats, and don't request if (isQueryWithError(this.interpolateString(query.expr, placeHolderScopedVars))) { return undefined; @@ -543,7 +543,7 @@ export class LokiDatasource let statsForAll: QueryStats = { streams: 0, chunks: 0, bytes: 0, entries: 0 }; for (const idx in labelMatchers) { - const { start, end } = this.getStatsTimeRange(query, Number(idx)); + const { start, end } = this.getStatsTimeRange(query, Number(idx), timeRange); if (start === undefined || end === undefined) { return { streams: 0, chunks: 0, bytes: 0, entries: 0, message: 'Query size estimate not available.' }; @@ -580,7 +580,11 @@ export class LokiDatasource * @returns An object containing the start and end time in nanoseconds (NS_IN_MS) or undefined if the time range cannot be estimated. */ - getStatsTimeRange(query: LokiQuery, idx: number): { start: number | undefined; end: number | undefined } { + getStatsTimeRange( + query: LokiQuery, + idx: number, + timeRange: TimeRange + ): { start: number | undefined; end: number | undefined } { let start: number, end: number; const NS_IN_MS = 1000000; const durationNodes = getNodesFromQuery(query.expr, [Duration]); @@ -592,7 +596,7 @@ export class LokiDatasource return { start: undefined, end: undefined }; } // logs query with range type - return this.getTimeRangeParams(); + return this.getTimeRangeParams(timeRange); } if (query.queryType === LokiQueryType.Instant) { @@ -600,7 +604,7 @@ export class LokiDatasource if (!!durations[idx]) { // if query has a duration e.g. [1m] - end = this.getTimeRangeParams().end; + end = this.getTimeRangeParams(timeRange).end; start = end - rangeUtil.intervalToMs(durations[idx]) * NS_IN_MS; return { start, end }; } else { @@ -608,7 +612,7 @@ export class LokiDatasource if (/(\$__auto|\$__range)/.test(query.expr)) { // if $__auto or $__range is used, we can estimate the time range using the selected range - return this.getTimeRangeParams(); + return this.getTimeRangeParams(timeRange); } // otherwise we cant estimate the time range @@ -617,19 +621,19 @@ export class LokiDatasource } // metric query with range type - return this.getTimeRangeParams(); + return this.getTimeRangeParams(timeRange); } /** * Retrieves statistics for a Loki query and returns the QueryStats object. * @returns A Promise that resolves to a QueryStats object or null if the query is invalid or has no statistics. */ - async getStats(query: LokiQuery): Promise { - if (!query) { + async getStats(query: LokiQuery, timeRange: TimeRange): Promise { + if (!query.expr) { return null; } - const response = await this.getQueryStats(query); + const response = await this.getQueryStats(query, timeRange); if (!response) { return null;