From 2c92e370dd3369eeb2d1f63da57e4b7177006932 Mon Sep 17 00:00:00 2001 From: Kristina Date: Thu, 26 Oct 2023 08:16:22 -0500 Subject: [PATCH] Explore: Fix broken interpolation for mixed datasources coming from dashboards (#76904) * Fix broken interpolation for mixed datasources in explore * Add interpolation tests * Use Promise.all and be better about dealing with invalid empty ds in query scenario * condense logic * Remove type declarations * Fix tests --- public/app/core/utils/explore.test.ts | 67 ++++++++++++++++++++++++++- public/app/core/utils/explore.ts | 57 +++++++++++------------ 2 files changed, 93 insertions(+), 31 deletions(-) diff --git a/public/app/core/utils/explore.test.ts b/public/app/core/utils/explore.test.ts index 38eaac8eb5b..f08d940f559 100644 --- a/public/app/core/utils/explore.test.ts +++ b/public/app/core/utils/explore.test.ts @@ -1,5 +1,6 @@ -import { dateTime, ExploreUrlState, LogsSortOrder } from '@grafana/data'; +import { DataSourceApi, dateTime, ExploreUrlState, LogsSortOrder } from '@grafana/data'; import { serializeStateToUrlParam } from '@grafana/data/src/utils/url'; +import { DataQuery } from '@grafana/schema'; import { RefreshPicker } from '@grafana/ui'; import store from 'app/core/store'; import { DEFAULT_RANGE } from 'app/features/explore/state/utils'; @@ -24,12 +25,36 @@ const DEFAULT_EXPLORE_STATE: ExploreUrlState = { }; const defaultDs = new MockDataSourceApi('default datasource', { data: ['default data'] }); - +const interpolateMockLoki = jest + .fn() + .mockReturnValue([{ refId: 'a', expr: 'replaced testDs loki' }]) as unknown as DataQuery[]; +const interpolateMockProm = jest + .fn() + .mockReturnValue([{ refId: 'a', expr: 'replaced testDs2 prom' }]) as unknown as DataQuery[]; const datasourceSrv = new DatasourceSrvMock(defaultDs, { 'generate empty query': new MockDataSourceApi('generateEmptyQuery'), ds1: { name: 'testDs', type: 'loki', + meta: { mixed: false }, + interpolateVariablesInQueries: interpolateMockLoki, + getRef: () => { + return 'ds1'; + }, + } as unknown as DataSourceApi, + ds2: { + name: 'testDs2', + type: 'prom', + meta: { mixed: false }, + interpolateVariablesInQueries: interpolateMockProm, + getRef: () => { + return 'ds2'; + }, + } as unknown as DataSourceApi, + dsMixed: { + name: 'testDSMixed', + type: 'mixed', + meta: { mixed: true }, } as MockDataSourceApi, }); @@ -70,6 +95,10 @@ describe('state functions', () => { }); describe('getExploreUrl', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + const args = { queries: [ { refId: 'A', expr: 'query1', legendFormat: 'legendFormat1' }, @@ -86,6 +115,40 @@ describe('getExploreUrl', () => { it('should omit expression target in explore url', async () => { expect(await getExploreUrl(args)).not.toMatch(/__expr__/g); }); + it('should interpolate queries with variables in a non-mixed datasource scenario', async () => { + // this is not actually valid (see root and query DS being different) but it will test the root DS mock was called + const nonMixedArgs = { + queries: [{ refId: 'A', expr: 'query1', datasource: { type: 'prom', uid: 'ds2' } }], + dsRef: { + uid: 'ds1', + meta: { mixed: false }, + }, + timeRange: { from: dateTime(), to: dateTime(), raw: { from: 'now-1h', to: 'now' } }, + scopedVars: {}, + }; + expect(await getExploreUrl(nonMixedArgs)).toMatch(/replaced%20testDs2%20prom/g); + expect(interpolateMockLoki).not.toBeCalled(); + expect(interpolateMockProm).toBeCalled(); + }); + it('should interpolate queries with variables in a mixed datasource scenario', async () => { + const nonMixedArgs = { + queries: [ + { refId: 'A', expr: 'query1', datasource: { type: 'loki', uid: 'ds1' } }, + { refId: 'B', expr: 'query2', datasource: { type: 'prom', uid: 'ds2' } }, + ], + dsRef: { + uid: 'dsMixed', + meta: { mixed: true }, + }, + timeRange: { from: dateTime(), to: dateTime(), raw: { from: 'now-1h', to: 'now' } }, + scopedVars: {}, + }; + const url = await getExploreUrl(nonMixedArgs); + expect(url).toMatch(/replaced%20testDs%20loki/g); + expect(url).toMatch(/replaced%20testDs2%20prom/g); + expect(interpolateMockLoki).toBeCalled(); + expect(interpolateMockProm).toBeCalled(); + }); }); describe('updateHistory()', () => { diff --git a/public/app/core/utils/explore.ts b/public/app/core/utils/explore.ts index 6d4851de77f..281f6fd997e 100644 --- a/public/app/core/utils/explore.ts +++ b/public/app/core/utils/explore.ts @@ -9,7 +9,6 @@ import { DataSourceApi, DataSourceRef, DefaultTimeZone, - ExploreUrlState, HistoryItem, IntervalValues, LogsDedupStrategy, @@ -61,36 +60,36 @@ export function generateExploreId() { */ export async function getExploreUrl(args: GetExploreUrlArguments): Promise { const { queries, dsRef, timeRange, scopedVars } = args; - let exploreDatasource = await getDataSourceSrv().get(dsRef); + const interpolatedQueries = ( + await Promise.allSettled( + queries + // Explore does not support expressions so filter those out + .filter((q) => q.datasource?.uid !== ExpressionDatasourceUID) + .map(async (q) => { + // if the query defines a datasource, use that one, otherwise use the one from the panel, which should always be defined. + // this will rejects if the datasource is not found, or return the default one if dsRef is not provided. + const queryDs = await getDataSourceSrv().get(q.datasource || dsRef); - /* - * Explore does not support expressions so filter those out - */ - let exploreTargets: DataQuery[] = queries.filter((t) => t.datasource?.uid !== ExpressionDatasourceUID); + return { + // interpolate the query using its datasource `interpolateVariablesInQueries` method if defined, othewise return the query as-is. + ...(queryDs.interpolateVariablesInQueries?.([q], scopedVars ?? {})[0] || q), + // But always set the datasource as it's required in Explore. + // NOTE: if for some reason the query has the "mixed" datasource, we omit the property; + // Upon initialization, Explore use its own logic to determine the datasource. + ...(!queryDs.meta.mixed && { datasource: queryDs.getRef() }), + }; + }) + ) + ) + .filter( + (promise: PromiseSettledResult): promise is PromiseFulfilledResult => promise.status === 'fulfilled' + ) + .map((q) => q.value); - let url: string | undefined; - - if (exploreDatasource) { - let state: Partial = { range: toURLRange(timeRange.raw) }; - if (exploreDatasource.interpolateVariablesInQueries) { - state = { - ...state, - datasource: exploreDatasource.uid, - queries: exploreDatasource.interpolateVariablesInQueries(exploreTargets, scopedVars ?? {}), - }; - } else { - state = { - ...state, - datasource: exploreDatasource.uid, - queries: exploreTargets, - }; - } - - const exploreState = JSON.stringify({ [generateExploreId()]: state }); - url = urlUtil.renderUrl('/explore', { panes: exploreState, schemaVersion: 1 }); - } - - return url; + const exploreState = JSON.stringify({ + [generateExploreId()]: { range: toURLRange(timeRange.raw), queries: interpolatedQueries, datasource: dsRef?.uid }, + }); + return urlUtil.renderUrl('/explore', { panes: exploreState, schemaVersion: 1 }); } export function buildQueryTransaction(