From f612a72f96fbb7d413a548eedf7e32b24854af76 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Tue, 18 Apr 2023 15:59:22 +0200 Subject: [PATCH] Loki: Update log context UI (#66730) * fix logrowcontext scrolling behavior * Loki: Update loki context ui menu * Update * Add test, update * Use escapeLabelValueInSelector when displaying labels * Update test for new appliedContextFilters --------- Co-authored-by: Sven Grossmann --- packages/grafana-data/src/types/logs.ts | 2 +- public/app/features/explore/Logs.tsx | 3 +- public/app/features/explore/LogsContainer.tsx | 7 +- .../app/features/logs/components/LogRows.tsx | 3 +- .../loki/LogContextProvider.test.ts | 26 +-- .../datasource/loki/LogContextProvider.ts | 17 +- .../loki/components/LokiContextUi.test.tsx | 99 +++++--- .../loki/components/LokiContextUi.tsx | 217 ++++++++++-------- .../app/plugins/datasource/loki/datasource.ts | 4 +- 9 files changed, 226 insertions(+), 152 deletions(-) diff --git a/packages/grafana-data/src/types/logs.ts b/packages/grafana-data/src/types/logs.ts index 85138e44de9..9200942492c 100644 --- a/packages/grafana-data/src/types/logs.ts +++ b/packages/grafana-data/src/types/logs.ts @@ -141,7 +141,7 @@ export interface DataSourceWithLogsContextSupport void): React.ReactNode; + getLogRowContextUi?(row: LogRowModel, runContextQuery?: () => void, origQuery?: TQuery): React.ReactNode; } export const hasLogsContextSupport = (datasource: unknown): datasource is DataSourceWithLogsContextSupport => { diff --git a/public/app/features/explore/Logs.tsx b/public/app/features/explore/Logs.tsx index 5c9854adb73..c748e8f0361 100644 --- a/public/app/features/explore/Logs.tsx +++ b/public/app/features/explore/Logs.tsx @@ -25,7 +25,6 @@ import { DataHoverEvent, DataHoverClearEvent, EventBus, - DataSourceWithLogsContextSupport, LogRowContextOptions, } from '@grafana/data'; import { reportInteraction } from '@grafana/runtime'; @@ -80,7 +79,7 @@ interface Props extends Themeable2 { onStartScanning?: () => void; onStopScanning?: () => void; getRowContext?: (row: LogRowModel, options?: LogRowContextOptions) => Promise; - getLogRowContextUi?: DataSourceWithLogsContextSupport['getLogRowContextUi']; + getLogRowContextUi?: (row: LogRowModel, runContextQuery?: () => void) => React.ReactNode; getFieldLinks: (field: Field, rowIndex: number, dataFrame: DataFrame) => Array>; addResultsToCache: () => void; clearCache: () => void; diff --git a/public/app/features/explore/LogsContainer.tsx b/public/app/features/explore/LogsContainer.tsx index b222fb8053e..eb98c9584b5 100644 --- a/public/app/features/explore/LogsContainer.tsx +++ b/public/app/features/explore/LogsContainer.tsx @@ -65,10 +65,13 @@ class LogsContainer extends PureComponent { }; getLogRowContextUi = (row: LogRowModel, runContextQuery?: () => void): React.ReactNode => { - const { datasourceInstance } = this.props; + const { datasourceInstance, logsQueries } = this.props; if (hasLogsContextUiSupport(datasourceInstance) && datasourceInstance.getLogRowContextUi) { - return datasourceInstance.getLogRowContextUi(row, runContextQuery); + const query = (logsQueries ?? []).find( + (q) => q.refId === row.dataFrame.refId && q.datasource != null && q.datasource.type === datasourceInstance.type + ); + return datasourceInstance.getLogRowContextUi(row, runContextQuery, query); } return <>; diff --git a/public/app/features/logs/components/LogRows.tsx b/public/app/features/logs/components/LogRows.tsx index 14a65bffe4a..dc71062d073 100644 --- a/public/app/features/logs/components/LogRows.tsx +++ b/public/app/features/logs/components/LogRows.tsx @@ -10,7 +10,6 @@ import { LogsSortOrder, CoreApp, DataFrame, - DataSourceWithLogsContextSupport, DataQueryResponse, LogRowContextOptions, } from '@grafana/data'; @@ -43,7 +42,7 @@ export interface Props extends Themeable2 { onClickFilterLabel?: (key: string, value: string) => void; onClickFilterOutLabel?: (key: string, value: string) => void; getRowContext?: (row: LogRowModel, options?: LogRowContextOptions) => Promise; - getLogRowContextUi?: DataSourceWithLogsContextSupport['getLogRowContextUi']; + getLogRowContextUi?: (row: LogRowModel, runContextQuery?: () => void) => React.ReactNode; getFieldLinks?: (field: Field, rowIndex: number, dataFrame: DataFrame) => Array>; onClickShowField?: (key: string) => void; onClickHideField?: (key: string) => void; diff --git a/public/app/plugins/datasource/loki/LogContextProvider.test.ts b/public/app/plugins/datasource/loki/LogContextProvider.test.ts index 7a4feeb7fd5..d2db8aa9e5a 100644 --- a/public/app/plugins/datasource/loki/LogContextProvider.test.ts +++ b/public/app/plugins/datasource/loki/LogContextProvider.test.ts @@ -42,7 +42,7 @@ describe('LogContextProvider', () => { beforeEach(() => { logContextProvider = new LogContextProvider(defaultDatasourceMock); logContextProvider.getInitContextFiltersFromLabels = jest.fn(() => - Promise.resolve([{ value: 'bar', enabled: true, fromParser: false, label: 'bar' }]) + Promise.resolve([{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]) ); }); @@ -59,8 +59,8 @@ describe('LogContextProvider', () => { it('should not call getInitContextFilters if appliedContextFilters', async () => { logContextProvider.appliedContextFilters = [ - { value: 'bar', enabled: true, fromParser: false, label: 'bar' }, - { value: 'xyz', enabled: true, fromParser: false, label: 'xyz' }, + { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, + { value: 'abc', enabled: true, fromParser: false, label: 'xyz' }, ]; await logContextProvider.getLogRowContext(defaultLogRow, { limit: 10, @@ -89,9 +89,9 @@ describe('LogContextProvider', () => { it('should not apply parsed labels', async () => { logContextProvider.appliedContextFilters = [ - { value: 'bar', enabled: true, fromParser: false, label: 'bar' }, - { value: 'xyz', enabled: true, fromParser: false, label: 'xyz' }, - { value: 'foo', enabled: true, fromParser: true, label: 'foo' }, + { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, + { value: 'abc', enabled: true, fromParser: false, label: 'xyz' }, + { value: 'uniqueParsedLabel', enabled: true, fromParser: true, label: 'foo' }, ]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, @@ -107,8 +107,8 @@ describe('LogContextProvider', () => { describe('query with parser', () => { it('should apply parser', async () => { logContextProvider.appliedContextFilters = [ - { value: 'bar', enabled: true, fromParser: false, label: 'bar' }, - { value: 'xyz', enabled: true, fromParser: false, label: 'xyz' }, + { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, + { value: 'abc', enabled: true, fromParser: false, label: 'xyz' }, ]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, @@ -124,9 +124,9 @@ describe('LogContextProvider', () => { it('should apply parser and parsed labels', async () => { logContextProvider.appliedContextFilters = [ - { value: 'bar', enabled: true, fromParser: false, label: 'bar' }, - { value: 'xyz', enabled: true, fromParser: false, label: 'xyz' }, - { value: 'foo', enabled: true, fromParser: true, label: 'foo' }, + { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, + { value: 'abc', enabled: true, fromParser: false, label: 'xyz' }, + { value: 'uniqueParsedLabel', enabled: true, fromParser: true, label: 'foo' }, ]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, @@ -143,8 +143,8 @@ describe('LogContextProvider', () => { it('should not apply parser and parsed labels if more parsers in original query', async () => { logContextProvider.appliedContextFilters = [ - { value: 'bar', enabled: true, fromParser: false, label: 'bar' }, - { value: 'foo', enabled: true, fromParser: true, label: 'foo' }, + { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, + { value: 'uniqueParsedLabel', enabled: true, fromParser: true, label: 'foo' }, ]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, diff --git a/public/app/plugins/datasource/loki/LogContextProvider.ts b/public/app/plugins/datasource/loki/LogContextProvider.ts index 27fc206eddc..cbd4eb47981 100644 --- a/public/app/plugins/datasource/loki/LogContextProvider.ts +++ b/public/app/plugins/datasource/loki/LogContextProvider.ts @@ -137,7 +137,7 @@ export class LogContextProvider { }; } - getLogRowContextUi(row: LogRowModel, runContextQuery: () => void): React.ReactNode { + getLogRowContextUi(row: LogRowModel, runContextQuery?: () => void, originalQuery?: DataQuery): React.ReactNode { const updateFilter = (contextFilters: ContextFilter[]) => { this.appliedContextFilters = contextFilters; @@ -153,8 +153,15 @@ export class LogContextProvider { this.appliedContextFilters = []; }); + let origQuery: LokiQuery | undefined = undefined; + // Type guard for LokiQuery + if (originalQuery && isLokiQuery(originalQuery)) { + origQuery = originalQuery; + } + return LokiContextUi({ row, + origQuery, updateFilter, onClose: this.onContextClose, logContextProvider: this, @@ -164,10 +171,9 @@ export class LogContextProvider { processContextFiltersToExpr = (row: LogRowModel, contextFilters: ContextFilter[], query: LokiQuery | undefined) => { const labelFilters = contextFilters .map((filter) => { - const label = filter.value; if (!filter.fromParser && filter.enabled) { // escape backslashes in label as users can't escape them by themselves - return `${label}="${escapeLabelValueInExactSelector(row.labels[label])}"`; + return `${filter.label}="${escapeLabelValueInExactSelector(filter.value)}"`; } return ''; }) @@ -186,7 +192,7 @@ export class LogContextProvider { const parsedLabels = contextFilters.filter((filter) => filter.fromParser && filter.enabled); for (const parsedLabel of parsedLabels) { if (parsedLabel.enabled) { - expr = addLabelToQuery(expr, parsedLabel.label, '=', row.labels[parsedLabel.label]); + expr = addLabelToQuery(expr, parsedLabel.label, '=', parsedLabel.value); } } } @@ -202,10 +208,9 @@ export class LogContextProvider { Object.entries(labels).forEach(([label, value]) => { const filter: ContextFilter = { label, - value: label, // this looks weird in the first place, but we need to set the label as value here + value: value, enabled: allLabels.includes(label), fromParser: !allLabels.includes(label), - description: value, }; contextFilters.push(filter); }); diff --git a/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx b/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx index fce625d2f5d..0dc2ae6495b 100644 --- a/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx +++ b/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx @@ -1,10 +1,12 @@ import { act, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import React from 'react'; import { selectOptionInTest } from 'test/helpers/selectOptionInTest'; import { LogRowModel } from '@grafana/data'; import { LogContextProvider } from '../LogContextProvider'; +import { ContextFilter, LokiQuery } from '../types'; import { LokiContextUi, LokiContextUiProps } from './LokiContextUi'; @@ -14,6 +16,49 @@ jest.mock('@grafana/runtime', () => ({ reportInteraction: () => null, })); +jest.mock('app/core/store', () => { + return { + set() {}, + getBool() { + return true; + }, + }; +}); + +const setupProps = (): LokiContextUiProps => { + const defaults: LokiContextUiProps = { + logContextProvider: mockLogContextProvider as unknown as LogContextProvider, + updateFilter: jest.fn(), + row: { + entry: 'WARN test 1.23 on [xxx]', + labels: { + label1: 'value1', + label3: 'value3', + }, + } as unknown as LogRowModel, + onClose: jest.fn(), + }; + + return defaults; +}; + +const mockLogContextProvider = { + getInitContextFiltersFromLabels: jest.fn().mockImplementation(() => + Promise.resolve([ + { value: 'value1', enabled: true, fromParser: false, label: 'label1' }, + { value: 'value3', enabled: false, fromParser: true, label: 'label3' }, + ]) + ), + processContextFiltersToExpr: jest.fn().mockImplementation( + (row: LogRowModel, contextFilters: ContextFilter[], query: LokiQuery | undefined) => + `{${contextFilters + .filter((filter) => filter.enabled) + .map((filter) => `${filter.label}="${filter.value}"`) + .join('` ')}}` + ), + getLogRowContext: jest.fn(), +}; + describe('LokiContextUi', () => { const savedGlobal = global; beforeAll(() => { @@ -29,38 +74,11 @@ describe('LokiContextUi', () => { afterAll(() => { global = savedGlobal; }); - const setupProps = (): LokiContextUiProps => { - const mockLogContextProvider = { - getInitContextFiltersFromLabels: jest.fn().mockImplementation(() => - Promise.resolve([ - { value: 'label1', enabled: true, fromParser: false, label: 'label1' }, - { value: 'label3', enabled: false, fromParser: true, label: 'label3' }, - ]) - ), - }; - const defaults: LokiContextUiProps = { - logContextProvider: mockLogContextProvider as unknown as LogContextProvider, - updateFilter: jest.fn(), - row: { - entry: 'WARN test 1.23 on [xxx]', - labels: { - label1: 'value1', - label3: 'value3', - }, - } as unknown as LogRowModel, - onClose: jest.fn(), - }; - - return defaults; - }; - - it('renders and shows basic text', async () => { + it('renders and shows executed query text', async () => { const props = setupProps(); render(); - - // Initial set of labels is available and not selected - expect(await screen.findByText(/Select labels to be included in the context query/)).toBeInTheDocument(); + expect(await screen.findByText(/Executed log context query:/)).toBeInTheDocument(); }); it('initialize context filters', async () => { @@ -79,7 +97,7 @@ describe('LokiContextUi', () => { expect(props.logContextProvider.getInitContextFiltersFromLabels).toHaveBeenCalled(); }); const select = await screen.findAllByRole('combobox'); - await selectOptionInTest(select[0], 'label1'); + await selectOptionInTest(select[0], 'label1="value1"'); }); it('finds label3 as a parsed label', async () => { @@ -89,7 +107,7 @@ describe('LokiContextUi', () => { expect(props.logContextProvider.getInitContextFiltersFromLabels).toHaveBeenCalled(); }); const select = await screen.findAllByRole('combobox'); - await selectOptionInTest(select[1], 'label3'); + await selectOptionInTest(select[1], 'label3="value3"'); }); it('calls updateFilter when selecting a label', async () => { @@ -100,7 +118,7 @@ describe('LokiContextUi', () => { expect(props.logContextProvider.getInitContextFiltersFromLabels).toHaveBeenCalled(); expect(screen.getAllByRole('combobox')).toHaveLength(2); }); - await selectOptionInTest(screen.getAllByRole('combobox')[1], 'label3'); + await selectOptionInTest(screen.getAllByRole('combobox')[1], 'label3="value3"'); act(() => { jest.runAllTimers(); }); @@ -117,4 +135,21 @@ describe('LokiContextUi', () => { expect(props.onClose).toHaveBeenCalled(); }); }); + + it('displays executed query even if context ui closed', async () => { + const props = setupProps(); + render(); + // We start with the context ui open + expect(await screen.findByText(/Executed log context query:/)).toBeInTheDocument(); + // We click on it to close + await userEvent.click(screen.getByText(/Executed log context query:/)); + await waitFor(() => { + // We should see the query text (it is split into multiple spans) + expect(screen.getByText('{')).toBeInTheDocument(); + expect(screen.getByText('label1')).toBeInTheDocument(); + expect(screen.getByText('=')).toBeInTheDocument(); + expect(screen.getByText('"value1"')).toBeInTheDocument(); + expect(screen.getByText('}')).toBeInTheDocument(); + }); + }); }); diff --git a/public/app/plugins/datasource/loki/components/LokiContextUi.tsx b/public/app/plugins/datasource/loki/components/LokiContextUi.tsx index 4532dce3b5d..ba82209170d 100644 --- a/public/app/plugins/datasource/loki/components/LokiContextUi.tsx +++ b/public/app/plugins/datasource/loki/components/LokiContextUi.tsx @@ -1,40 +1,38 @@ import { css } from '@emotion/css'; -import memoizeOne from 'memoize-one'; -import React, { useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import { useAsync } from 'react-use'; import { GrafanaTheme2, LogRowModel, SelectableValue } from '@grafana/data'; import { reportInteraction } from '@grafana/runtime'; -import { LoadingPlaceholder, MultiSelect, Tag, Tooltip, useStyles2 } from '@grafana/ui'; +import { Collapse, Label, LoadingPlaceholder, MultiSelect, Tag, Tooltip, useStyles2 } from '@grafana/ui'; +import store from 'app/core/store'; +import { RawQuery } from '../../prometheus/querybuilder/shared/RawQuery'; import { LogContextProvider } from '../LogContextProvider'; -import { ContextFilter } from '../types'; +import { escapeLabelValueInSelector } from '../languageUtils'; +import { lokiGrammar } from '../syntax'; +import { ContextFilter, LokiQuery } from '../types'; export interface LokiContextUiProps { logContextProvider: LogContextProvider; row: LogRowModel; updateFilter: (value: ContextFilter[]) => void; onClose: () => void; + origQuery?: LokiQuery; } function getStyles(theme: GrafanaTheme2) { return { labels: css` display: flex; - gap: 2px; + gap: ${theme.spacing(0.5)}; `, - multiSelectWrapper: css` + wrapper: css` display: flex; flex-direction: column; flex: 1; - margin-top: ${theme.spacing(1)}; gap: ${theme.spacing(0.5)}; `, - multiSelect: css` - & .scrollbar-view { - overscroll-behavior: contain; - } - `, loadingPlaceholder: css` margin-bottom: 0px; float: right; @@ -48,23 +46,37 @@ function getStyles(theme: GrafanaTheme2) { hidden: css` visibility: hidden; `, + tag: css` + padding: ${theme.spacing(0.25)} ${theme.spacing(0.75)}; + `, + label: css` + max-width: 100%; + margin: ${theme.spacing(2)} 0; + `, + query: css` + text-align: start; + line-break: anywhere; + margin-top: ${theme.spacing(0.5)}; + `, + ui: css` + background-color: ${theme.colors.background.secondary}; + padding: ${theme.spacing(2)}; + `, }; } -const formatOptionLabel = memoizeOne(({ label, description }: SelectableValue) => ( - - {label} - -)); +const IS_LOKI_LOG_CONTEXT_UI_OPEN = 'isLogContextQueryUiOpen'; export function LokiContextUi(props: LokiContextUiProps) { - const { row, logContextProvider, updateFilter, onClose } = props; + const { row, logContextProvider, updateFilter, onClose, origQuery } = props; const styles = useStyles2(getStyles); const [contextFilters, setContextFilters] = useState([]); const [initialized, setInitialized] = useState(false); const [loading, setLoading] = useState(false); + const [isOpen, setIsOpen] = useState(store.getBool(IS_LOKI_LOG_CONTEXT_UI_OPEN, true)); + const timerHandle = React.useRef(); const previousInitialized = React.useRef(false); const previousContextFilters = React.useRef([]); @@ -136,108 +148,129 @@ export function LokiContextUi(props: LokiContextUiProps) { const parsedLabels = contextFilters.filter(({ fromParser }) => fromParser); const parsedLabelsEnabled = parsedLabels.filter(({ enabled }) => enabled); + const contextFilterToSelectFilter = useCallback((contextFilter: ContextFilter): SelectableValue => { + return { + label: `${contextFilter.label}="${escapeLabelValueInSelector(contextFilter.value)}"`, + value: contextFilter.label, + }; + }, []); + return ( -
-
- {' '} - - - {' '} - Select labels to be included in the context query: - -
-
- { - if (actionMeta.action === 'select-option') { - reportInteraction('grafana_explore_logs_loki_log_context_filtered', { - logRowUid: row.uid, - type: 'label', - action: 'select', - }); +
+ + { + store.set(IS_LOKI_LOG_CONTEXT_UI_OPEN, !isOpen); + setIsOpen((isOpen) => !isOpen); + }} + label={ +
+ + enabled), + origQuery + )} + /> +
+ } + > +
+ { - if (filter.fromParser) { - return filter; - } - filter.enabled = keys.some((key) => key.value === filter.value); - return filter; - }) - ); - }} - /> -
- {parsedLabels.length > 0 && ( -
+ placement="top" + > + + {' '} + { if (actionMeta.action === 'select-option') { reportInteraction('grafana_explore_logs_loki_log_context_filtered', { logRowUid: row.uid, - type: 'parsed_label', + type: 'label', action: 'select', }); } if (actionMeta.action === 'remove-value') { reportInteraction('grafana_explore_logs_loki_log_context_filtered', { logRowUid: row.uid, - type: 'parsed_label', + type: 'label', action: 'remove', }); } - setContextFilters( + return setContextFilters( contextFilters.map((filter) => { - if (!filter.fromParser) { + if (filter.fromParser) { return filter; } - filter.enabled = keys.some((key) => key.value === filter.value); + filter.enabled = keys.some((key) => key.value === filter.label); return filter; }) ); }} /> + {parsedLabels.length > 0 && ( + <> + + { + if (actionMeta.action === 'select-option') { + reportInteraction('grafana_explore_logs_loki_log_context_filtered', { + logRowUid: row.uid, + type: 'parsed_label', + action: 'select', + }); + } + if (actionMeta.action === 'remove-value') { + reportInteraction('grafana_explore_logs_loki_log_context_filtered', { + logRowUid: row.uid, + type: 'parsed_label', + action: 'remove', + }); + } + setContextFilters( + contextFilters.map((filter) => { + if (!filter.fromParser) { + return filter; + } + filter.enabled = keys.some((key) => key.value === filter.label); + return filter; + }) + ); + }} + /> + + )}
- )} +
); } diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index 384a298e81b..e65b09fc2af 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -651,8 +651,8 @@ export class LokiDatasource return await this.logContextProvider.getLogRowContext(row, options, origQuery); }; - getLogRowContextUi(row: LogRowModel, runContextQuery: () => void): React.ReactNode { - return this.logContextProvider.getLogRowContextUi(row, runContextQuery); + getLogRowContextUi(row: LogRowModel, runContextQuery: () => void, origQuery: DataQuery): React.ReactNode { + return this.logContextProvider.getLogRowContextUi(row, runContextQuery, origQuery); } testDatasource(): Promise<{ status: string; message: string }> {