From d199c33d7e0bc815ac456c3350e1bd45261ea0fd Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Tue, 11 Feb 2025 20:25:52 +0100 Subject: [PATCH] Log Context: Fix bug where variables are not replaced in dashboards (#100433) * Log Context: Fix bug where variables are not replaced in dashboards * add objects to act as `row` and `options` * run prettier * fix lint --- packages/grafana-data/src/types/logs.ts | 9 ++- .../loki/LogContextProvider.test.ts | 57 +++++++++++++++++-- .../datasource/loki/LogContextProvider.ts | 17 +++++- .../app/plugins/datasource/loki/datasource.ts | 14 ++++- .../app/plugins/panel/logs/LogsPanel.test.tsx | 2 +- public/app/plugins/panel/logs/LogsPanel.tsx | 8 ++- 6 files changed, 95 insertions(+), 12 deletions(-) diff --git a/packages/grafana-data/src/types/logs.ts b/packages/grafana-data/src/types/logs.ts index 67a194202a7..bfd54b0ec6f 100644 --- a/packages/grafana-data/src/types/logs.ts +++ b/packages/grafana-data/src/types/logs.ts @@ -4,6 +4,7 @@ import { DataQuery, LogsSortOrder } from '@grafana/schema'; import { BusEventWithPayload } from '../events/types'; +import { ScopedVars } from './ScopedVars'; import { KeyValue, Labels } from './data'; import { DataFrame } from './dataFrame'; import { DataQueryRequest, DataQueryResponse, DataSourceApi, QueryFixAction, QueryFixType } from './datasource'; @@ -134,6 +135,7 @@ export enum LogsDedupDescription { export interface LogRowContextOptions { direction?: LogRowContextQueryDirection; limit?: number; + scopedVars?: ScopedVars; } export enum LogRowContextQueryDirection { @@ -172,7 +174,12 @@ export interface DataSourceWithLogsContextSupport void, origQuery?: TQuery): React.ReactNode; + getLogRowContextUi?( + row: LogRowModel, + runContextQuery?: () => void, + origQuery?: TQuery, + scopedVars?: ScopedVars + ): React.ReactNode; } export const hasLogsContextSupport = (datasource: unknown): datasource is DataSourceWithLogsContextSupport => { diff --git a/public/app/plugins/datasource/loki/LogContextProvider.test.ts b/public/app/plugins/datasource/loki/LogContextProvider.test.ts index 08c4643d475..dcd86744b47 100644 --- a/public/app/plugins/datasource/loki/LogContextProvider.test.ts +++ b/public/app/plugins/datasource/loki/LogContextProvider.test.ts @@ -1,4 +1,5 @@ import { of } from 'rxjs'; +import { initTemplateSrv } from 'test/helpers/initTemplateSrv'; import { DataQueryResponse, @@ -8,6 +9,7 @@ import { createDataFrame, dateTime, } from '@grafana/data'; +import { setTemplateSrv } from '@grafana/runtime'; import LokiLanguageProvider from './LanguageProvider'; import { @@ -24,10 +26,6 @@ const defaultLanguageProviderMock = { getLabelKeys: jest.fn(() => ['bar', 'xyz']), } as unknown as LokiLanguageProvider; -const defaultDatasourceMock = createLokiDatasource(); -defaultDatasourceMock.query = jest.fn(() => of({ data: [] } as DataQueryResponse)); -defaultDatasourceMock.languageProvider = defaultLanguageProviderMock; - const defaultLogRow = { rowIndex: 0, dataFrame: createDataFrame({ @@ -68,6 +66,11 @@ const frameWithoutTypes = { describe('LogContextProvider', () => { let logContextProvider: LogContextProvider; beforeEach(() => { + const templateSrv = initTemplateSrv('key', [{ type: 'query', name: 'foo', current: { value: 'baz' } }]); + setTemplateSrv(templateSrv); + const defaultDatasourceMock = createLokiDatasource(templateSrv); + defaultDatasourceMock.query = jest.fn(() => of({ data: [] } as DataQueryResponse)); + defaultDatasourceMock.languageProvider = defaultLanguageProviderMock; logContextProvider = new LogContextProvider(defaultDatasourceMock); }); @@ -107,6 +110,32 @@ describe('LogContextProvider', () => { expect(logContextProvider.cachedContextFilters).toHaveLength(1); }); + it('should replace variables before getInitContextFilters', async () => { + logContextProvider.getInitContextFilters = jest.fn().mockResolvedValue({ + contextFilters: [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }], + preservedFiltersApplied: false, + }); + + expect(logContextProvider.cachedContextFilters).toHaveLength(0); + await logContextProvider.getLogRowContext( + defaultLogRow, + { + limit: 10, + direction: LogRowContextQueryDirection.Backward, + scopedVars: { test: { value: 'baz', text: 'baz' } }, + }, + { + expr: '{bar="$test"}', + refId: 'A', + } + ); + expect(logContextProvider.getInitContextFilters).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ expr: '{bar="baz"}', refId: 'A' }), + expect.anything() + ); + }); + it('should not call getInitContextFilters if cachedContextFilters', async () => { logContextProvider.getInitContextFilters = jest .fn() @@ -140,6 +169,26 @@ describe('LogContextProvider', () => { expect(logContextProvider.getInitContextFilters).toHaveBeenCalled(); }); + it('should replace variables before getInitContextFilters', async () => { + logContextProvider.getInitContextFilters = jest.fn().mockResolvedValue({ + contextFilters: [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }], + preservedFiltersApplied: false, + }); + + const query = await logContextProvider.getLogRowContextQuery( + defaultLogRow, + { + limit: 10, + direction: LogRowContextQueryDirection.Backward, + }, + { + expr: '{bar="$test"}', + refId: 'A', + } + ); + expect(query.expr).toBe('{bar="baz"}'); + }); + it('should also call getInitContextFilters if cacheFilters is not set', async () => { logContextProvider.getInitContextFilters = jest.fn().mockResolvedValue({ contextFilters: [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }], diff --git a/public/app/plugins/datasource/loki/LogContextProvider.ts b/public/app/plugins/datasource/loki/LogContextProvider.ts index 4fa2da27792..00873a8d920 100644 --- a/public/app/plugins/datasource/loki/LogContextProvider.ts +++ b/public/app/plugins/datasource/loki/LogContextProvider.ts @@ -14,6 +14,7 @@ import { LogRowContextQueryDirection, LogRowContextOptions, dateTime, + ScopedVars, } from '@grafana/data'; import { LabelParser, LabelFilter, LineFilters, PipelineStage, Logfmt, Json } from '@grafana/lezer-logql'; @@ -79,6 +80,9 @@ export class LogContextProvider { origQuery?: LokiQuery, cacheFilters = true ): Promise => { + if (origQuery && options?.scopedVars) { + origQuery = this.datasource.applyTemplateVariables(origQuery, options?.scopedVars); + } const { query } = await this.getQueryAndRange(row, options, origQuery, cacheFilters); if (!cacheFilters) { @@ -94,6 +98,9 @@ export class LogContextProvider { options?: LogRowContextOptions, origQuery?: LokiQuery ): Promise<{ data: DataFrame[] }> => { + if (origQuery && options?.scopedVars) { + origQuery = this.datasource.applyTemplateVariables(origQuery, options?.scopedVars); + } const direction = (options && options.direction) || LogRowContextQueryDirection.Backward; const { query, range } = await this.getQueryAndRange(row, options, origQuery); @@ -185,7 +192,15 @@ export class LogContextProvider { }; } - getLogRowContextUi(row: LogRowModel, runContextQuery?: () => void, origQuery?: LokiQuery): React.ReactNode { + getLogRowContextUi( + row: LogRowModel, + runContextQuery?: () => void, + origQuery?: LokiQuery, + scopedVars?: ScopedVars + ): React.ReactNode { + if (origQuery && scopedVars) { + origQuery = this.datasource.applyTemplateVariables(origQuery, scopedVars); + } const updateFilter = (contextFilters: ContextFilter[]) => { this.cachedContextFilters = contextFilters; diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index 3ceade42594..66c9236ae27 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -1014,8 +1014,18 @@ export class LokiDatasource * Part of `DataSourceWithLogsContextSupport`, used to retrieve the log context UI for the provided log row and original query. * @returns A React component or element representing the log context UI for the log row. */ - getLogRowContextUi(row: LogRowModel, runContextQuery: () => void, origQuery: DataQuery): React.ReactNode { - return this.logContextProvider.getLogRowContextUi(row, runContextQuery, getLokiQueryFromDataQuery(origQuery)); + getLogRowContextUi( + row: LogRowModel, + runContextQuery: () => void, + origQuery: DataQuery, + scopedVars?: ScopedVars + ): React.ReactNode { + return this.logContextProvider.getLogRowContextUi( + row, + runContextQuery, + getLokiQueryFromDataQuery(origQuery), + scopedVars + ); } /** diff --git a/public/app/plugins/panel/logs/LogsPanel.test.tsx b/public/app/plugins/panel/logs/LogsPanel.test.tsx index 76c04ca75a7..7def6ca6cb5 100644 --- a/public/app/plugins/panel/logs/LogsPanel.test.tsx +++ b/public/app/plugins/panel/logs/LogsPanel.test.tsx @@ -414,7 +414,7 @@ describe('LogsPanel', () => { await userEvent.click(screen.getByLabelText(/show context/i)); const getRowContextCb = logRowContextModalMock.mock.calls[0][0].getRowContext; - getRowContextCb(); + getRowContextCb({}, {}); expect(showContextDs.getLogRowContext).toBeCalled(); }); }); diff --git a/public/app/plugins/panel/logs/LogsPanel.tsx b/public/app/plugins/panel/logs/LogsPanel.tsx index 33043b913eb..51ae908d9b5 100644 --- a/public/app/plugins/panel/logs/LogsPanel.tsx +++ b/public/app/plugins/panel/logs/LogsPanel.tsx @@ -232,9 +232,11 @@ export const LogsPanel = ({ return Promise.resolve({ data: [] }); } + options.scopedVars = panelData.request?.scopedVars; + return dataSource.getLogRowContext(row, options, query); }, - [panelData.request?.targets, dataSourcesMap] + [panelData.request?.targets, panelData.request?.scopedVars, dataSourcesMap] ); const getLogRowContextUi = useCallback( @@ -257,9 +259,9 @@ export const LogsPanel = ({ return <>; } - return dataSource.getLogRowContextUi(origRow, runContextQuery, query); + return dataSource.getLogRowContextUi(origRow, runContextQuery, query, panelData.request?.scopedVars); }, - [panelData.request?.targets, dataSourcesMap] + [panelData.request?.targets, panelData.request?.scopedVars, dataSourcesMap] ); // Important to memoize stuff here, as panel rerenders a lot for example when resizing.