From 177496a686fa80787c7dcf8dd57cd0ab35b7883a Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Mon, 27 Nov 2023 14:29:00 +0100 Subject: [PATCH] Loki: Filter by labels based on the type of label (structured, indexed, parsed) (#78595) * add label addition based on labeltype * add `logRowToDataFrame` * change to single row dataframe * add documentation * add tests for `LogDetailsRow` * add tests for datasource * remove row * update tests * fix tests * PR comments * removed comment * add comment * remove params * remove unused jsdoc * move `getLabelTypeFromFrame` to `languageUtils` * add tests * remove `refId` and use `frame` * fix tests * Update public/app/plugins/datasource/loki/modifyQuery.ts --- packages/grafana-data/src/types/datasource.ts | 12 +- packages/grafana-data/src/types/logs.ts | 1 + public/app/features/explore/Explore.tsx | 24 +- public/app/features/explore/Logs/Logs.tsx | 4 +- .../features/explore/Logs/LogsContainer.tsx | 4 +- .../app/features/explore/Logs/LogsTable.tsx | 8 +- .../features/explore/Logs/LogsTableWrap.tsx | 4 +- .../logs/components/LogDetails.test.tsx | 26 ++- .../features/logs/components/LogDetails.tsx | 4 +- .../logs/components/LogDetailsRow.test.tsx | 38 +++- .../logs/components/LogDetailsRow.tsx | 21 +- .../app/features/logs/components/LogRow.tsx | 4 +- .../app/features/logs/components/LogRows.tsx | 4 +- .../logs/components/__mocks__/logRow.ts | 15 +- public/app/features/logs/logsModel.test.ts | 52 +++++ public/app/features/logs/logsModel.ts | 19 ++ .../datasource/loki/datasource.test.ts | 210 +++++++++++++++++- .../app/plugins/datasource/loki/datasource.ts | 26 +-- .../datasource/loki/languageUtils.test.ts | 50 ++++- .../plugins/datasource/loki/languageUtils.ts | 25 ++- .../datasource/loki/modifyQuery.test.ts | 21 +- .../plugins/datasource/loki/modifyQuery.ts | 107 +++++---- 22 files changed, 561 insertions(+), 118 deletions(-) diff --git a/packages/grafana-data/src/types/datasource.ts b/packages/grafana-data/src/types/datasource.ts index 6fec58a2af1..96a1e792762 100644 --- a/packages/grafana-data/src/types/datasource.ts +++ b/packages/grafana-data/src/types/datasource.ts @@ -582,10 +582,20 @@ export interface QueryFix { export type QueryFixType = 'ADD_FILTER' | 'ADD_FILTER_OUT' | 'ADD_STRING_FILTER' | 'ADD_STRING_FILTER_OUT'; export interface QueryFixAction { - type: QueryFixType | string; query?: string; preventSubmit?: boolean; + /** + * The type of action to perform. Will be passed to the data source to handle. + */ + type: QueryFixType | string; + /** + * A key value map of options that will be passed. Usually used to pass e.g. the label and value. + */ options?: KeyValue; + /** + * An optional single row data frame containing the row that triggered the the QueryFixAction. + */ + frame?: DataFrame; } export interface QueryHint { diff --git a/packages/grafana-data/src/types/logs.ts b/packages/grafana-data/src/types/logs.ts index 33560345c67..3518fa78443 100644 --- a/packages/grafana-data/src/types/logs.ts +++ b/packages/grafana-data/src/types/logs.ts @@ -264,6 +264,7 @@ export interface QueryFilterOptions extends KeyValue {} export interface ToggleFilterAction { type: 'FILTER_FOR' | 'FILTER_OUT'; options: QueryFilterOptions; + frame?: DataFrame; } /** * Data sources that support toggleable filters through `toggleQueryFilter`, and displaying the active diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx index 6a21db4beb4..b3ebd9e0f3e 100644 --- a/public/app/features/explore/Explore.tsx +++ b/public/app/features/explore/Explore.tsx @@ -7,6 +7,7 @@ import AutoSizer from 'react-virtualized-auto-sizer'; import { AbsoluteTimeRange, + DataFrame, EventBus, GrafanaTheme2, hasToggleableQueryFiltersSupport, @@ -219,15 +220,29 @@ export class Explore extends React.PureComponent { /** * Used by Logs details. */ - onClickFilterLabel = (key: string, value: string, refId?: string) => { - this.onModifyQueries({ type: 'ADD_FILTER', options: { key, value } }, refId); + onClickFilterLabel = (key: string, value: string, frame?: DataFrame) => { + this.onModifyQueries( + { + type: 'ADD_FILTER', + options: { key, value }, + frame, + }, + frame?.refId + ); }; /** * Used by Logs details. */ - onClickFilterOutLabel = (key: string, value: string, refId?: string) => { - this.onModifyQueries({ type: 'ADD_FILTER_OUT', options: { key, value } }, refId); + onClickFilterOutLabel = (key: string, value: string, frame?: DataFrame) => { + this.onModifyQueries( + { + type: 'ADD_FILTER_OUT', + options: { key, value }, + frame, + }, + frame?.refId + ); }; /** @@ -269,6 +284,7 @@ export class Explore extends React.PureComponent { return ds.toggleQueryFilter(query, { type: modification.type === 'ADD_FILTER' ? 'FILTER_FOR' : 'FILTER_OUT', options: modification.options ?? {}, + frame: modification.frame, }); } if (ds.modifyQuery) { diff --git a/public/app/features/explore/Logs/Logs.tsx b/public/app/features/explore/Logs/Logs.tsx index 5ff5719cda8..d86baa5bf0d 100644 --- a/public/app/features/explore/Logs/Logs.tsx +++ b/public/app/features/explore/Logs/Logs.tsx @@ -86,8 +86,8 @@ interface Props extends Themeable2 { loadLogsVolumeData: () => void; showContextToggle?: (row: LogRowModel) => boolean; onChangeTime: (range: AbsoluteTimeRange) => void; - onClickFilterLabel?: (key: string, value: string, refId?: string) => void; - onClickFilterOutLabel?: (key: string, value: string, refId?: string) => void; + onClickFilterLabel?: (key: string, value: string, frame?: DataFrame) => void; + onClickFilterOutLabel?: (key: string, value: string, frame?: DataFrame) => void; onStartScanning?: () => void; onStopScanning?: () => void; getRowContext?: (row: LogRowModel, origRow: LogRowModel, options: LogRowContextOptions) => Promise; diff --git a/public/app/features/explore/Logs/LogsContainer.tsx b/public/app/features/explore/Logs/LogsContainer.tsx index a1d66e5d7aa..c711f8a883c 100644 --- a/public/app/features/explore/Logs/LogsContainer.tsx +++ b/public/app/features/explore/Logs/LogsContainer.tsx @@ -50,8 +50,8 @@ interface LogsContainerProps extends PropsFromRedux { scanRange?: RawTimeRange; syncedTimes: boolean; loadingState: LoadingState; - onClickFilterLabel: (key: string, value: string, refId?: string) => void; - onClickFilterOutLabel: (key: string, value: string, refId?: string) => void; + onClickFilterLabel: (key: string, value: string, frame?: DataFrame) => void; + onClickFilterOutLabel: (key: string, value: string, frame?: DataFrame) => void; onStartScanning: () => void; onStopScanning: () => void; eventBus: EventBus; diff --git a/public/app/features/explore/Logs/LogsTable.tsx b/public/app/features/explore/Logs/LogsTable.tsx index 808b747df26..1489853096d 100644 --- a/public/app/features/explore/Logs/LogsTable.tsx +++ b/public/app/features/explore/Logs/LogsTable.tsx @@ -34,8 +34,8 @@ interface Props { logsSortOrder: LogsSortOrder; columnsWithMeta: Record; height: number; - onClickFilterLabel?: (key: string, value: string, refId?: string) => void; - onClickFilterOutLabel?: (key: string, value: string, refId?: string) => void; + onClickFilterLabel?: (key: string, value: string, frame?: DataFrame) => void; + onClickFilterOutLabel?: (key: string, value: string, frame?: DataFrame) => void; } export function LogsTable(props: Props) { @@ -147,11 +147,11 @@ export function LogsTable(props: Props) { return; } if (operator === FILTER_FOR_OPERATOR) { - onClickFilterLabel(key, value, dataFrame.refId); + onClickFilterLabel(key, value, dataFrame); } if (operator === FILTER_OUT_OPERATOR) { - onClickFilterOutLabel(key, value, dataFrame.refId); + onClickFilterOutLabel(key, value, dataFrame); } }; diff --git a/public/app/features/explore/Logs/LogsTableWrap.tsx b/public/app/features/explore/Logs/LogsTableWrap.tsx index 2e898c4379e..d46b2352ccd 100644 --- a/public/app/features/explore/Logs/LogsTableWrap.tsx +++ b/public/app/features/explore/Logs/LogsTableWrap.tsx @@ -31,8 +31,8 @@ interface Props extends Themeable2 { logsSortOrder: LogsSortOrder; panelState: ExploreLogsPanelState | undefined; updatePanelState: (panelState: Partial) => void; - onClickFilterLabel?: (key: string, value: string, refId?: string) => void; - onClickFilterOutLabel?: (key: string, value: string, refId?: string) => void; + onClickFilterLabel?: (key: string, value: string, frame?: DataFrame) => void; + onClickFilterOutLabel?: (key: string, value: string, frame?: DataFrame) => void; } export type fieldNameMeta = { diff --git a/public/app/features/logs/components/LogDetails.test.tsx b/public/app/features/logs/components/LogDetails.test.tsx index c10c35277e0..0512aec6f26 100644 --- a/public/app/features/logs/components/LogDetails.test.tsx +++ b/public/app/features/logs/components/LogDetails.test.tsx @@ -90,11 +90,33 @@ describe('LogDetails', () => { await userEvent.click(screen.getByLabelText('Filter for value in query A')); expect(onClickFilterLabelMock).toHaveBeenCalledTimes(1); - expect(onClickFilterLabelMock).toHaveBeenCalledWith('key1', 'label1', mockRow.dataFrame.refId); + expect(onClickFilterLabelMock).toHaveBeenCalledWith( + 'key1', + 'label1', + expect.objectContaining({ + fields: [ + expect.objectContaining({ values: [0] }), + expect.objectContaining({ values: ['line1'] }), + expect.objectContaining({ values: [{ app: 'app01' }] }), + ], + length: 1, + }) + ); await userEvent.click(screen.getByLabelText('Filter out value in query A')); expect(onClickFilterOutLabelMock).toHaveBeenCalledTimes(1); - expect(onClickFilterOutLabelMock).toHaveBeenCalledWith('key1', 'label1', mockRow.dataFrame.refId); + expect(onClickFilterOutLabelMock).toHaveBeenCalledWith( + 'key1', + 'label1', + expect.objectContaining({ + fields: [ + expect.objectContaining({ values: [0] }), + expect.objectContaining({ values: ['line1'] }), + expect.objectContaining({ values: [{ app: 'app01' }] }), + ], + length: 1, + }) + ); }); }); it('should not render filter controls when the callbacks are not provided', () => { diff --git a/public/app/features/logs/components/LogDetails.tsx b/public/app/features/logs/components/LogDetails.tsx index 3233d03d0e7..6d489d91b51 100644 --- a/public/app/features/logs/components/LogDetails.tsx +++ b/public/app/features/logs/components/LogDetails.tsx @@ -20,8 +20,8 @@ export interface Props extends Themeable2 { app?: CoreApp; styles: LogRowStyles; - onClickFilterLabel?: (key: string, value: string, refId?: string) => void; - onClickFilterOutLabel?: (key: string, value: string, refId?: string) => void; + onClickFilterLabel?: (key: string, value: string, frame?: DataFrame) => void; + onClickFilterOutLabel?: (key: string, value: string, frame?: DataFrame) => void; getFieldLinks?: (field: Field, rowIndex: number, dataFrame: DataFrame) => Array>; displayedFields?: string[]; onClickShowField?: (key: string) => void; diff --git a/public/app/features/logs/components/LogDetailsRow.test.tsx b/public/app/features/logs/components/LogDetailsRow.test.tsx index 380c916655f..bcaab5cd40e 100644 --- a/public/app/features/logs/components/LogDetailsRow.test.tsx +++ b/public/app/features/logs/components/LogDetailsRow.test.tsx @@ -8,8 +8,8 @@ type Props = ComponentProps; const setup = (propOverrides?: Partial) => { const props: Props = { - parsedValues: [''], - parsedKeys: [''], + parsedValues: ['value'], + parsedKeys: ['key'], isLabel: true, wrapLogMessage: false, getStats: () => null, @@ -66,6 +66,40 @@ describe('LogDetailsRow', () => { }); expect(await screen.findByRole('button', { name: 'Remove filter in query A' })).toBeInTheDocument(); }); + it('should trigger a call to `onClickFilterOutLabel` when the filter out button is clicked', () => { + const onClickFilterOutLabel = jest.fn(); + setup({ onClickFilterOutLabel }); + fireEvent.click(screen.getByRole('button', { name: 'Filter out value in query A' })); + expect(onClickFilterOutLabel).toHaveBeenCalledWith( + 'key', + 'value', + expect.objectContaining({ + fields: [ + expect.objectContaining({ values: [0] }), + expect.objectContaining({ values: ['line1'] }), + expect.objectContaining({ values: [{ app: 'app01' }] }), + ], + length: 1, + }) + ); + }); + it('should trigger a call to `onClickFilterLabel` when the filter button is clicked', () => { + const onClickFilterLabel = jest.fn(); + setup({ onClickFilterLabel }); + fireEvent.click(screen.getByRole('button', { name: 'Filter for value in query A' })); + expect(onClickFilterLabel).toHaveBeenCalledWith( + 'key', + 'value', + expect.objectContaining({ + fields: [ + expect.objectContaining({ values: [0] }), + expect.objectContaining({ values: ['line1'] }), + expect.objectContaining({ values: [{ app: 'app01' }] }), + ], + length: 1, + }) + ); + }); }); describe('if props is not a label', () => { diff --git a/public/app/features/logs/components/LogDetailsRow.tsx b/public/app/features/logs/components/LogDetailsRow.tsx index b96dd64d38c..ac1ef650e5e 100644 --- a/public/app/features/logs/components/LogDetailsRow.tsx +++ b/public/app/features/logs/components/LogDetailsRow.tsx @@ -3,10 +3,21 @@ import { isEqual } from 'lodash'; import memoizeOne from 'memoize-one'; import React, { PureComponent, useState } from 'react'; -import { CoreApp, Field, GrafanaTheme2, IconName, LinkModel, LogLabelStatsModel, LogRowModel } from '@grafana/data'; +import { + CoreApp, + DataFrame, + Field, + GrafanaTheme2, + IconName, + LinkModel, + LogLabelStatsModel, + LogRowModel, +} from '@grafana/data'; import { reportInteraction } from '@grafana/runtime'; import { ClipboardButton, DataLinkButton, IconButton, Themeable2, withTheme2 } from '@grafana/ui'; +import { logRowToSingleRowDataFrame } from '../logsModel'; + import { LogLabelStats } from './LogLabelStats'; import { getLogRowStyles } from './getLogRowStyles'; @@ -16,8 +27,8 @@ export interface Props extends Themeable2 { disableActions: boolean; wrapLogMessage?: boolean; isLabel?: boolean; - onClickFilterLabel?: (key: string, value: string, refId?: string) => void; - onClickFilterOutLabel?: (key: string, value: string, refId?: string) => void; + onClickFilterLabel?: (key: string, value: string, frame?: DataFrame) => void; + onClickFilterOutLabel?: (key: string, value: string, frame?: DataFrame) => void; links?: Array>; getStats: () => LogLabelStatsModel[] | null; displayedFields?: string[]; @@ -143,7 +154,7 @@ class UnThemedLogDetailsRow extends PureComponent { filterLabel = () => { const { onClickFilterLabel, parsedKeys, parsedValues, row } = this.props; if (onClickFilterLabel) { - onClickFilterLabel(parsedKeys[0], parsedValues[0], row.dataFrame?.refId); + onClickFilterLabel(parsedKeys[0], parsedValues[0], logRowToSingleRowDataFrame(row) || undefined); } reportInteraction('grafana_explore_logs_log_details_filter_clicked', { @@ -156,7 +167,7 @@ class UnThemedLogDetailsRow extends PureComponent { filterOutLabel = () => { const { onClickFilterOutLabel, parsedKeys, parsedValues, row } = this.props; if (onClickFilterOutLabel) { - onClickFilterOutLabel(parsedKeys[0], parsedValues[0], row.dataFrame?.refId); + onClickFilterOutLabel(parsedKeys[0], parsedValues[0], logRowToSingleRowDataFrame(row) || undefined); } reportInteraction('grafana_explore_logs_log_details_filter_clicked', { diff --git a/public/app/features/logs/components/LogRow.tsx b/public/app/features/logs/components/LogRow.tsx index 923eb1feaa0..04d61a3cc5c 100644 --- a/public/app/features/logs/components/LogRow.tsx +++ b/public/app/features/logs/components/LogRow.tsx @@ -30,8 +30,8 @@ interface Props extends Themeable2 { app?: CoreApp; displayedFields?: string[]; getRows: () => LogRowModel[]; - onClickFilterLabel?: (key: string, value: string, refId?: string) => void; - onClickFilterOutLabel?: (key: string, value: string, refId?: string) => void; + onClickFilterLabel?: (key: string, value: string, frame?: DataFrame) => void; + onClickFilterOutLabel?: (key: string, value: string, frame?: DataFrame) => void; onContextClick?: () => void; getFieldLinks?: (field: Field, rowIndex: number, dataFrame: DataFrame) => Array>; showContextToggle?: (row: LogRowModel) => boolean; diff --git a/public/app/features/logs/components/LogRows.tsx b/public/app/features/logs/components/LogRows.tsx index 9b1e009c6a1..4485bad5a1d 100644 --- a/public/app/features/logs/components/LogRows.tsx +++ b/public/app/features/logs/components/LogRows.tsx @@ -41,8 +41,8 @@ export interface Props extends Themeable2 { displayedFields?: string[]; app?: CoreApp; showContextToggle?: (row: LogRowModel) => boolean; - onClickFilterLabel?: (key: string, value: string, refId?: string) => void; - onClickFilterOutLabel?: (key: string, value: string, refId?: string) => void; + onClickFilterLabel?: (key: string, value: string, frame?: DataFrame) => void; + onClickFilterOutLabel?: (key: string, value: string, frame?: DataFrame) => void; getFieldLinks?: (field: Field, rowIndex: number, dataFrame: DataFrame) => Array>; onClickShowField?: (key: string) => void; onClickHideField?: (key: string) => void; diff --git a/public/app/features/logs/components/__mocks__/logRow.ts b/public/app/features/logs/components/__mocks__/logRow.ts index fde85c69dc8..0968f242a84 100644 --- a/public/app/features/logs/components/__mocks__/logRow.ts +++ b/public/app/features/logs/components/__mocks__/logRow.ts @@ -1,4 +1,4 @@ -import { LogLevel, LogRowModel, MutableDataFrame } from '@grafana/data'; +import { FieldType, LogLevel, LogRowModel, toDataFrame } from '@grafana/data'; export const createLogRow = (overrides?: Partial): LogRowModel => { const uid = overrides?.uid || '1'; @@ -8,7 +8,18 @@ export const createLogRow = (overrides?: Partial): LogRowModel => { return { entryFieldIndex: 0, rowIndex: 0, - dataFrame: new MutableDataFrame({ refId: 'A', fields: [] }), + dataFrame: toDataFrame({ + refId: 'A', + fields: [ + { name: 'Time', type: FieldType.time, values: [0, 1] }, + { + name: 'Line', + type: FieldType.string, + values: ['line1', 'line2'], + }, + { name: 'labels', type: FieldType.other, values: [{ app: 'app01' }, { app: 'app02' }] }, + ], + }), uid, logLevel: LogLevel.info, entry, diff --git a/public/app/features/logs/logsModel.test.ts b/public/app/features/logs/logsModel.test.ts index 78fd89cd80d..eaca7a1e9ce 100644 --- a/public/app/features/logs/logsModel.test.ts +++ b/public/app/features/logs/logsModel.test.ts @@ -28,6 +28,7 @@ import { filterLogLevels, getSeriesProperties, LIMIT_LABEL, + logRowToSingleRowDataFrame, logSeriesToLogsModel, queryLogsSample, queryLogsVolume, @@ -1458,3 +1459,54 @@ describe('logs sample', () => { }); }); }); + +const mockLogRow = { + dataFrame: toDataFrame({ + fields: [ + { name: 'Time', type: FieldType.time, values: [0, 1] }, + { + name: 'Line', + type: FieldType.string, + values: ['line1', 'line2'], + }, + { name: 'labels', type: FieldType.other, values: [{ app: 'app01' }, { app: 'app02' }] }, + ], + }), + rowIndex: 0, +} as unknown as LogRowModel; + +describe('logRowToDataFrame', () => { + it('should return a DataFrame with the values from the specified row', () => { + const result = logRowToSingleRowDataFrame(mockLogRow); + + expect(result?.length).toBe(1); + + expect(result?.fields[0].values[0]).toEqual(0); + expect(result?.fields[1].values[0]).toEqual('line1'); + expect(result?.fields[2].values[0]).toEqual({ app: 'app01' }); + }); + + it('should return a DataFrame with the values from the specified different row', () => { + const result = logRowToSingleRowDataFrame({ ...mockLogRow, rowIndex: 1 }); + + expect(result?.length).toBe(1); + + expect(result?.fields[0].values[0]).toEqual(1); + expect(result?.fields[1].values[0]).toEqual('line2'); + expect(result?.fields[2].values[0]).toEqual({ app: 'app02' }); + }); + + it('should handle an empty DataFrame', () => { + const emptyLogRow = { dataFrame: { fields: [] }, rowIndex: 0 } as unknown as LogRowModel; + const result = logRowToSingleRowDataFrame(emptyLogRow); + + expect(result?.length).toBe(0); + }); + + it('should handle rowIndex exceeding array bounds', () => { + const invalidRowIndex = 10; + const result = logRowToSingleRowDataFrame({ ...mockLogRow, rowIndex: invalidRowIndex }); + + expect(result).toBe(null); + }); +}); diff --git a/public/app/features/logs/logsModel.ts b/public/app/features/logs/logsModel.ts index 2eb8d3025f3..468406f123d 100644 --- a/public/app/features/logs/logsModel.ts +++ b/public/app/features/logs/logsModel.ts @@ -3,6 +3,7 @@ import { from, isObservable, Observable } from 'rxjs'; import { AbsoluteTimeRange, + createDataFrame, DataFrame, DataQuery, DataQueryRequest, @@ -789,3 +790,21 @@ function getIntervalInfo(scopedVars: ScopedVars, timespanMs: number): { interval return { interval: '$__interval' }; } } + +/** + * Creates a new data frame containing only the single row from `logRow`. + */ +export function logRowToSingleRowDataFrame(logRow: LogRowModel): DataFrame | null { + const originFrame = logRow.dataFrame; + + if (originFrame.length === 0 || originFrame.length <= logRow.rowIndex) { + return null; + } + + // create a new data frame containing only the single row from `logRow` + const frame = createDataFrame({ + fields: originFrame.fields.map((field) => ({ ...field, values: [field.values[logRow.rowIndex]] })), + }); + + return frame; +} diff --git a/public/app/plugins/datasource/loki/datasource.test.ts b/public/app/plugins/datasource/loki/datasource.test.ts index 9b9bc74ced6..01027706bae 100644 --- a/public/app/plugins/datasource/loki/datasource.test.ts +++ b/public/app/plugins/datasource/loki/datasource.test.ts @@ -13,7 +13,9 @@ import { DataSourceInstanceSettings, dateTime, FieldType, + QueryFixAction, SupplementaryQueryType, + toDataFrame, TimeRange, ToggleFilterAction, } from '@grafana/data'; @@ -555,6 +557,28 @@ describe('LokiDatasource', () => { }); describe('modifyQuery', () => { + const frameWithTypes = toDataFrame({ + fields: [ + { name: 'Time', type: FieldType.time, values: [0] }, + { + name: 'Line', + type: FieldType.string, + values: ['line1'], + }, + { name: 'labelTypes', type: FieldType.other, values: [{ indexed: 'I', parsed: 'P', structured: 'S' }] }, + ], + }); + const frameWithoutTypes = toDataFrame({ + fields: [ + { name: 'Time', type: FieldType.time, values: [0] }, + { + name: 'Line', + type: FieldType.string, + values: ['line1'], + }, + { name: 'labels', type: FieldType.other, values: [{ job: 'test' }] }, + ], + }); describe('when called with ADD_FILTER', () => { let ds: LokiDatasource; beforeEach(() => { @@ -590,14 +614,186 @@ describe('LokiDatasource', () => { expect(result.expr).toEqual('rate({bar="baz", job="grafana"}[5m])'); }); - it('then the correct label should be added for non-indexed metadata as LabelFilter', () => { - const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; - const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' }; - ds.languageProvider.labelKeys = ['bar']; - const result = ds.modifyQuery(query, action); + describe('with a frame with label types', () => { + it('then the correct structured metadata label should be added as LabelFilter', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; - expect(result.refId).toEqual('A'); - expect(result.expr).toEqual('{bar="baz"} | job=`grafana`'); + const action: QueryFixAction = { + options: { key: 'structured', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"} | structured=`foo`'); + }); + + it('then the correct parsed label should be added as LabelFilter', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; + + const action: QueryFixAction = { + options: { key: 'parsed', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"} | parsed=`foo`'); + }); + + it('then the correct indexed label should be added as LabelFilter', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; + + const action: QueryFixAction = { + options: { key: 'indexed', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz", indexed="foo"}'); + }); + + it('then the correct structured metadata label should be added as LabelFilter with parser', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | json' }; + + const action: QueryFixAction = { + options: { key: 'structured', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"} | json | structured=`foo`'); + }); + + it('then the correct parsed label should be added as LabelFilter with parser', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | json' }; + + const action: QueryFixAction = { + options: { key: 'parsed', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"} | json | parsed=`foo`'); + }); + + it('then the correct indexed label should be added as LabelFilter with parser', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | json' }; + + const action: QueryFixAction = { + options: { key: 'indexed', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz", indexed="foo"} | json'); + }); + }); + describe('with a frame without label types', () => { + it('then the correct structured metadata label should be added as LabelFilter', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; + + const action: QueryFixAction = { + options: { key: 'structured', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithoutTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz", structured="foo"}'); + }); + + it('then the correct parsed label should be added to the stream selector', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; + + const action: QueryFixAction = { + options: { key: 'parsed', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithoutTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz", parsed="foo"}'); + }); + + it('then the correct indexed label should be added as LabelFilter', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; + + const action: QueryFixAction = { + options: { key: 'indexed', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithoutTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz", indexed="foo"}'); + }); + it('then the correct structured metadata label should be added as LabelFilter with parser', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | json' }; + + const action: QueryFixAction = { + options: { key: 'structured', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithoutTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"} | json | structured=`foo`'); + }); + + it('then the correct parsed label should be added as LabelFilter with parser', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | json' }; + + const action: QueryFixAction = { + options: { key: 'parsed', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithoutTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"} | json | parsed=`foo`'); + }); + + it('then the correct indexed label should be added as LabelFilter with parser', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | json' }; + + const action: QueryFixAction = { + options: { key: 'indexed', value: 'foo' }, + type: 'ADD_FILTER', + frame: frameWithoutTypes, + }; + ds.languageProvider.labelKeys = ['bar']; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"} | json | indexed=`foo`'); + }); }); }); describe('and query has parser', () => { diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index 88aaedb77b2..05cf21c475e 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -58,7 +58,7 @@ import { LokiVariableSupport } from './LokiVariableSupport'; import { transformBackendResult } from './backendResultTransformer'; import { LokiAnnotationsQueryEditor } from './components/AnnotationsQueryEditor'; import { placeHolderScopedVars } from './components/monaco-query-field/monaco-completion-provider/validation'; -import { escapeLabelValueInSelector, isRegexSelector } from './languageUtils'; +import { escapeLabelValueInSelector, isRegexSelector, getLabelTypeFromFrame } from './languageUtils'; import { labelNamesRegex, labelValuesRegex } from './migrations/variableQueryMigrations'; import { addLabelFormatToQuery, @@ -795,6 +795,7 @@ export class LokiDatasource */ toggleQueryFilter(query: LokiQuery, filter: ToggleFilterAction): LokiQuery { let expression = query.expr ?? ''; + const labelType = getLabelTypeFromFrame(filter.options.key, filter.frame, 0); switch (filter.type) { case 'FILTER_FOR': { if (filter.options?.key && filter.options?.value) { @@ -803,7 +804,7 @@ export class LokiDatasource // This gives the user the ability to toggle a filter on and off. expression = queryHasFilter(expression, filter.options.key, '=', value) ? removeLabelFromQuery(expression, filter.options.key, '=', value) - : addLabelToQuery(expression, filter.options.key, '=', value); + : addLabelToQuery(expression, filter.options.key, '=', value, labelType); } break; } @@ -820,7 +821,7 @@ export class LokiDatasource expression = removeLabelFromQuery(expression, filter.options.key, '=', value); } - expression = addLabelToQuery(expression, filter.options.key, '!=', value); + expression = addLabelToQuery(expression, filter.options.key, '!=', value, labelType); } break; } @@ -849,31 +850,20 @@ export class LokiDatasource // NB: Usually the labelKeys should be fetched and cached in the datasource, // but there might be some edge cases where this wouldn't be the case. // However the changed would make this method `async`. - const allLabels = this.languageProvider.getLabelKeys(); switch (action.type) { case 'ADD_FILTER': { if (action.options?.key && action.options?.value) { + const labelType = getLabelTypeFromFrame(action.options.key, action.frame, 0); const value = escapeLabelValueInSelector(action.options.value); - expression = addLabelToQuery( - expression, - action.options.key, - '=', - value, - allLabels.includes(action.options.key) === false - ); + expression = addLabelToQuery(expression, action.options.key, '=', value, labelType); } break; } case 'ADD_FILTER_OUT': { if (action.options?.key && action.options?.value) { + const labelType = getLabelTypeFromFrame(action.options.key, action.frame, 0); const value = escapeLabelValueInSelector(action.options.value); - expression = addLabelToQuery( - expression, - action.options.key, - '!=', - value, - allLabels.includes(action.options.key) === false - ); + expression = addLabelToQuery(expression, action.options.key, '!=', value, labelType); } break; } diff --git a/public/app/plugins/datasource/loki/languageUtils.test.ts b/public/app/plugins/datasource/loki/languageUtils.test.ts index 80d7b2aa7e6..f240d816a12 100644 --- a/public/app/plugins/datasource/loki/languageUtils.test.ts +++ b/public/app/plugins/datasource/loki/languageUtils.test.ts @@ -1,4 +1,12 @@ -import { escapeLabelValueInExactSelector, isBytesString, unescapeLabelValue } from './languageUtils'; +import { toDataFrame, FieldType } from '@grafana/data'; + +import { + escapeLabelValueInExactSelector, + getLabelTypeFromFrame, + isBytesString, + unescapeLabelValue, +} from './languageUtils'; +import { LabelType } from './types'; describe('isBytesString', () => { it('correctly matches bytes string with integers', () => { @@ -42,3 +50,43 @@ describe('unescapeLabelValueInExactSelector', () => { expect(unescapeLabelValue(value)).toEqual(unescapedValue); }); }); + +describe('getLabelTypeFromFrame', () => { + const frameWithTypes = toDataFrame({ + fields: [ + { name: 'Time', type: FieldType.time, values: [0] }, + { + name: 'Line', + type: FieldType.string, + values: ['line1'], + }, + { name: 'labelTypes', type: FieldType.other, values: [{ indexed: 'I', parsed: 'P', structured: 'S' }] }, + ], + }); + const frameWithoutTypes = toDataFrame({ + fields: [ + { name: 'Time', type: FieldType.time, values: [0] }, + { + name: 'Line', + type: FieldType.string, + values: ['line1'], + }, + { name: 'labels', type: FieldType.other, values: [{ job: 'test' }] }, + ], + }); + it('returns structuredMetadata', () => { + expect(getLabelTypeFromFrame('structured', frameWithTypes, 0)).toBe(LabelType.StructuredMetadata); + }); + it('returns indexed', () => { + expect(getLabelTypeFromFrame('indexed', frameWithTypes, 0)).toBe(LabelType.Indexed); + }); + it('returns parsed', () => { + expect(getLabelTypeFromFrame('parsed', frameWithTypes, 0)).toBe(LabelType.Parsed); + }); + it('returns null for unknown field', () => { + expect(getLabelTypeFromFrame('unknown', frameWithTypes, 0)).toBe(null); + }); + it('returns null for frame without types', () => { + expect(getLabelTypeFromFrame('job', frameWithoutTypes, 0)).toBe(null); + }); +}); diff --git a/public/app/plugins/datasource/loki/languageUtils.ts b/public/app/plugins/datasource/loki/languageUtils.ts index 581d8a991a1..c79d720fbd4 100644 --- a/public/app/plugins/datasource/loki/languageUtils.ts +++ b/public/app/plugins/datasource/loki/languageUtils.ts @@ -1,4 +1,6 @@ -import { TimeRange } from '@grafana/data'; +import { DataFrame, TimeRange } from '@grafana/data'; + +import { LabelType } from './types'; function roundMsToMin(milliseconds: number): number { return roundSecToMin(milliseconds / 1000); @@ -88,3 +90,24 @@ export function isBytesString(string: string) { const match = string.match(regex); return !!match; } + +export function getLabelTypeFromFrame(labelKey: string, frame?: DataFrame, index?: number): null | LabelType { + if (!frame || index === undefined) { + return null; + } + + const typeField = frame.fields.find((field) => field.name === 'labelTypes')?.values[index]; + if (!typeField) { + return null; + } + switch (typeField[labelKey]) { + case 'I': + return LabelType.Indexed; + case 'S': + return LabelType.StructuredMetadata; + case 'P': + return LabelType.Parsed; + default: + return null; + } +} diff --git a/public/app/plugins/datasource/loki/modifyQuery.test.ts b/public/app/plugins/datasource/loki/modifyQuery.test.ts index 8965e27d2cc..f3c953816f3 100644 --- a/public/app/plugins/datasource/loki/modifyQuery.test.ts +++ b/public/app/plugins/datasource/loki/modifyQuery.test.ts @@ -11,6 +11,7 @@ import { removeCommentsFromQuery, removeLabelFromQuery, } from './modifyQuery'; +import { LabelType } from './types'; describe('addLabelToQuery()', () => { it.each` @@ -74,14 +75,26 @@ describe('addLabelToQuery()', () => { } ); - it('should always add label as labelFilter if force flag is given', () => { - expect(addLabelToQuery('{foo="bar"}', 'forcedLabel', '=', 'value', true)).toEqual( + it('should always add label as labelFilter if label type is parsed', () => { + expect(addLabelToQuery('{foo="bar"}', 'forcedLabel', '=', 'value', LabelType.Parsed)).toEqual( '{foo="bar"} | forcedLabel=`value`' ); }); - it('should always add label as labelFilter if force flag is given with a parser', () => { - expect(addLabelToQuery('{foo="bar"} | logfmt', 'forcedLabel', '=', 'value', true)).toEqual( + it('should always add label as labelFilter if label type is parsed with parser', () => { + expect(addLabelToQuery('{foo="bar"} | logfmt', 'forcedLabel', '=', 'value', LabelType.Parsed)).toEqual( + '{foo="bar"} | logfmt | forcedLabel=`value`' + ); + }); + + it('should always add label as labelFilter if label type is structured', () => { + expect(addLabelToQuery('{foo="bar"}', 'forcedLabel', '=', 'value', LabelType.StructuredMetadata)).toEqual( + '{foo="bar"} | forcedLabel=`value`' + ); + }); + + it('should always add label as labelFilter if label type is structured with parser', () => { + expect(addLabelToQuery('{foo="bar"} | logfmt', 'forcedLabel', '=', 'value', LabelType.StructuredMetadata)).toEqual( '{foo="bar"} | logfmt | forcedLabel=`value`' ); }); diff --git a/public/app/plugins/datasource/loki/modifyQuery.ts b/public/app/plugins/datasource/loki/modifyQuery.ts index 5ef8aa70cb6..db28c7a4ba2 100644 --- a/public/app/plugins/datasource/loki/modifyQuery.ts +++ b/public/app/plugins/datasource/loki/modifyQuery.ts @@ -28,6 +28,7 @@ import { unescapeLabelValue } from './languageUtils'; import { getNodePositionsFromQuery } from './queryUtils'; import { lokiQueryModeller as modeller } from './querybuilder/LokiQueryModeller'; import { buildVisualQueryFromString, handleQuotes } from './querybuilder/parsing'; +import { LabelType } from './types'; export class NodePosition { from: number; @@ -142,19 +143,13 @@ function getMatchersWithFilter(query: string, label: string, operator: string, v * * This operates on substrings of the query with labels and operates just on those. This makes this * more robust and can alter even invalid queries, and preserves in general the query structure and whitespace. - * - * @param {string} query - * @param {string} key - * @param {string} operator - * @param {string} value - * @param {boolean} [forceAsLabelFilter=false] - if true, it will add a LabelFilter expression even if there is no parser in the query */ export function addLabelToQuery( query: string, key: string, operator: string, value: string, - forceAsLabelFilter = false + labelType?: LabelType | null ): string { if (!key || !value) { throw new Error('Need label to add to query.'); @@ -165,6 +160,8 @@ export function addLabelToQuery( return query; } + const parserPositions = getParserPositions(query); + const labelFilterPositions = getLabelFilterPositions(query); const hasStreamSelectorMatchers = getMatcherInStreamPositions(query); const everyStreamSelectorHasMatcher = streamSelectorPositions.every((streamSelectorPosition) => hasStreamSelectorMatchers.some( @@ -172,38 +169,35 @@ export function addLabelToQuery( matcherPosition.from >= streamSelectorPosition.from && matcherPosition.to <= streamSelectorPosition.to ) ); - const parserPositions = getParserPositions(query); - const labelFilterPositions = getLabelFilterPositions(query); const filter = toLabelFilter(key, value, operator); - // If we have non-empty stream selector and parser/label filter, we want to add a new label filter after the last one. - // If some of the stream selectors don't have matchers, we want to add new matcher to the all stream selectors. - if (forceAsLabelFilter) { - // `forceAsLabelFilter` is mostly used for structured metadata labels. Those are not - // very well distinguishable from real labels, but need to be added as label - // filters after the last stream selector, parser or label filter. This is - // just a quickfix for now and still has edge-cases where it can fail. - // TODO: improve this once we have a better API in Loki to distinguish - // between the origins of labels. + if (labelType === LabelType.Parsed || labelType === LabelType.StructuredMetadata) { const positionToAdd = findLastPosition([...streamSelectorPositions, ...labelFilterPositions, ...parserPositions]); return addFilterAsLabelFilter(query, [positionToAdd], filter); - } else if (everyStreamSelectorHasMatcher && (labelFilterPositions.length || parserPositions.length)) { - // in case we are not adding the label to stream selectors we need to find the last position to add in each expression - const subExpressions = findLeaves(getNodePositionsFromQuery(query, [Expr])); - const parserFilterPositions = [...parserPositions, ...labelFilterPositions]; - - // find last position for each subexpression - const lastPositionsPerExpression = subExpressions.map((subExpression) => { - return findLastPosition( - parserFilterPositions.filter((p) => { - return subExpression.contains(p); - }) - ); - }); - - return addFilterAsLabelFilter(query, lastPositionsPerExpression, filter); - } else { + } else if (labelType === LabelType.Indexed) { return addFilterToStreamSelector(query, streamSelectorPositions, filter); + } else { + // labelType is not set, so we need to figure out where to add the label + // if we don't have a parser, or have empty stream selectors, we will just add it to the stream selector + if (parserPositions.length === 0 || everyStreamSelectorHasMatcher === false) { + return addFilterToStreamSelector(query, streamSelectorPositions, filter); + } else { + // If `labelType` is not set, it indicates a potential metric query (`labelType` is present only in log queries that came from a Loki instance supporting the `categorize-labels` API). In case we are not adding the label to stream selectors we need to find the last position to add in each expression. + // E.g. in `sum(rate({foo="bar"} | logfmt [$__auto])) / sum(rate({foo="baz"} | logfmt [$__auto]))` we need to add the label at two places. + const subExpressions = findLeaves(getNodePositionsFromQuery(query, [Expr])); + const parserFilterPositions = [...parserPositions, ...labelFilterPositions]; + + // find last position for each subexpression + const lastPositionsPerExpression = subExpressions.map((subExpression) => { + return findLastPosition( + parserFilterPositions.filter((p) => { + return subExpression.contains(p); + }) + ); + }); + + return addFilterAsLabelFilter(query, lastPositionsPerExpression, filter); + } } } @@ -297,19 +291,6 @@ export function getStreamSelectorPositions(query: string): NodePosition[] { return positions; } -function getMatcherInStreamPositions(query: string): NodePosition[] { - const tree = parser.parse(query); - const positions: NodePosition[] = []; - tree.iterate({ - enter: ({ node }): false | void => { - if (node.type.id === Selector) { - positions.push(...getAllPositionsInNodeByType(node, Matcher)); - } - }, - }); - return positions; -} - /** * Parse the string and get all LabelParser positions in the query. * @param query @@ -579,9 +560,22 @@ function labelExists(labels: QueryBuilderLabelFilter[], filter: QueryBuilderLabe * @param positions */ export function findLastPosition(positions: NodePosition[]): NodePosition { + if (!positions.length) { + return new NodePosition(0, 0); + } return positions.reduce((prev, current) => (prev.to > current.to ? prev : current)); } +/** + * Gets all leaves of the nodes given. Leaves are nodes that don't contain any other nodes. + * + * @param {NodePosition[]} nodes + * @return + */ +function findLeaves(nodes: NodePosition[]): NodePosition[] { + return nodes.filter((node) => nodes.every((n) => node.contains(n) === false || node === n)); +} + function getAllPositionsInNodeByType(node: SyntaxNode, type: number): NodePosition[] { if (node.type.id === type) { return [NodePosition.fromNode(node)]; @@ -598,12 +592,15 @@ function getAllPositionsInNodeByType(node: SyntaxNode, type: number): NodePositi return positions; } -/** - * Gets all leaves of the nodes given. Leaves are nodes that don't contain any other nodes. - * - * @param {NodePosition[]} nodes - * @return - */ -function findLeaves(nodes: NodePosition[]): NodePosition[] { - return nodes.filter((node) => nodes.every((n) => node.contains(n) === false || node === n)); +function getMatcherInStreamPositions(query: string): NodePosition[] { + const tree = parser.parse(query); + const positions: NodePosition[] = []; + tree.iterate({ + enter: ({ node }): false | void => { + if (node.type.id === Selector) { + positions.push(...getAllPositionsInNodeByType(node, Matcher)); + } + }, + }); + return positions; }