From cf958e0b4f5289b2b199d938c273a65098dfc565 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Thu, 29 Apr 2021 18:26:30 +0200 Subject: [PATCH] Explore: Refactor deduplication, hiding of logs and Logs component (#33531) * Move fitlering and deduplication to comnponent to enable future caching * Clean up LogsMetaInfo * Update * Memoize component * Fix type errors * Clean uo * Add tests for filtering in combination with deduplication --- public/app/core/logs_model.test.ts | 58 +++++++ public/app/features/explore/Logs.tsx | 144 +++++------------- public/app/features/explore/LogsContainer.tsx | 27 +--- public/app/features/explore/LogsMetaRow.tsx | 116 ++++++++++++++ .../app/features/explore/state/explorePane.ts | 52 +------ .../features/explore/state/selectors.test.ts | 92 ----------- .../app/features/explore/state/selectors.ts | 20 +-- public/app/features/explore/state/utils.ts | 2 - public/app/types/explore.ts | 12 -- 9 files changed, 215 insertions(+), 308 deletions(-) create mode 100644 public/app/features/explore/LogsMetaRow.tsx delete mode 100644 public/app/features/explore/state/selectors.test.ts diff --git a/public/app/core/logs_model.test.ts b/public/app/core/logs_model.test.ts index 22df3385430..cac8a65d8b8 100644 --- a/public/app/core/logs_model.test.ts +++ b/public/app/core/logs_model.test.ts @@ -13,6 +13,7 @@ import { dedupLogRows, getSeriesProperties, logSeriesToLogsModel, + filterLogLevels, LIMIT_LABEL, } from './logs_model'; @@ -141,6 +142,63 @@ describe('dedupLogRows()', () => { }); }); +describe('filterLogLevels()', () => { + test('should correctly filter out log levels', () => { + const rows: LogRowModel[] = [ + { + entry: 'DEBUG 1', + logLevel: LogLevel.debug, + }, + { + entry: 'ERROR 1', + logLevel: LogLevel.error, + }, + { + entry: 'TRACE 1', + logLevel: LogLevel.trace, + }, + ] as any; + const filteredLogs = filterLogLevels(rows, new Set([LogLevel.debug])); + expect(filteredLogs.length).toBe(2); + expect(filteredLogs).toEqual([ + { entry: 'ERROR 1', logLevel: 'error' }, + { entry: 'TRACE 1', logLevel: 'trace' }, + ]); + }); + test('should correctly filter out log levels and then deduplicate', () => { + const rows: LogRowModel[] = [ + { + entry: 'DEBUG 1', + logLevel: LogLevel.debug, + }, + { + entry: 'DEBUG 2', + logLevel: LogLevel.debug, + }, + { + entry: 'DEBUG 2', + logLevel: LogLevel.debug, + }, + { + entry: 'ERROR 1', + logLevel: LogLevel.error, + }, + { + entry: 'TRACE 1', + logLevel: LogLevel.trace, + }, + ] as any; + const filteredLogs = filterLogLevels(rows, new Set([LogLevel.error])); + const deduplicatedLogs = dedupLogRows(filteredLogs, LogsDedupStrategy.exact); + expect(deduplicatedLogs.length).toBe(3); + expect(deduplicatedLogs).toEqual([ + { duplicates: 0, entry: 'DEBUG 1', logLevel: 'debug' }, + { duplicates: 1, entry: 'DEBUG 2', logLevel: 'debug' }, + { duplicates: 0, entry: 'TRACE 1', logLevel: 'trace' }, + ]); + }); +}); + const emptyLogsModel: any = { hasUniqueLabels: false, rows: [], diff --git a/public/app/features/explore/Logs.tsx b/public/app/features/explore/Logs.tsx index f9878439928..4c48f2c9e18 100644 --- a/public/app/features/explore/Logs.tsx +++ b/public/app/features/explore/Logs.tsx @@ -9,7 +9,6 @@ import { LogLevel, TimeZone, AbsoluteTimeRange, - LogsMetaKind, LogsDedupStrategy, LogRowModel, LogsDedupDescription, @@ -21,7 +20,6 @@ import { GrafanaTheme, } from '@grafana/data'; import { - LogLabels, RadioButtonGroup, LogRows, Button, @@ -30,14 +28,12 @@ import { InlineSwitch, withTheme, stylesFactory, - Icon, - Tooltip, } from '@grafana/ui'; import store from 'app/core/store'; +import { dedupLogRows, filterLogLevels } from 'app/core/logs_model'; import { ExploreGraphPanel } from './ExploreGraphPanel'; -import { MetaInfoText } from './MetaInfoText'; +import { LogsMetaRow } from './LogsMetaRow'; import { RowContextOptions } from '@grafana/ui/src/components/Logs/LogRowContextProvider'; -import { MAX_CHARACTERS } from '@grafana/ui/src/components/Logs/LogRowMessage'; const SETTINGS_KEYS = { showLabels: 'grafana.explore.logs.showLabels', @@ -45,24 +41,10 @@ const SETTINGS_KEYS = { wrapLogMessage: 'grafana.explore.logs.wrapLogMessage', }; -function renderMetaItem(value: any, kind: LogsMetaKind) { - if (kind === LogsMetaKind.LabelsMap) { - return ( - - - - ); - } else if (kind === LogsMetaKind.Error) { - return {value}; - } - return value; -} - interface Props { logRows: LogRowModel[]; logsMeta?: LogsMetaItem[]; logsSeries?: GraphSeriesXY[]; - dedupedRows?: LogRowModel[]; visibleRange?: AbsoluteTimeRange; width: number; theme: GrafanaTheme; @@ -72,15 +54,12 @@ interface Props { timeZone: TimeZone; scanning?: boolean; scanRange?: RawTimeRange; - dedupStrategy: LogsDedupStrategy; showContextToggle?: (row?: LogRowModel) => boolean; onChangeTime: (range: AbsoluteTimeRange) => void; onClickFilterLabel?: (key: string, value: string) => void; onClickFilterOutLabel?: (key: string, value: string) => void; onStartScanning?: () => void; onStopScanning?: () => void; - onDedupStrategyChange: (dedupStrategy: LogsDedupStrategy) => void; - onToggleLogLevel: (hiddenLogLevels: LogLevel[]) => void; getRowContext?: (row: LogRowModel, options?: RowContextOptions) => Promise; getFieldLinks: (field: Field, rowIndex: number) => Array>; } @@ -89,6 +68,8 @@ interface State { showLabels: boolean; showTime: boolean; wrapLogMessage: boolean; + dedupStrategy: LogsDedupStrategy; + hiddenLogLevels: LogLevel[]; logsSortOrder: LogsSortOrder | null; isFlipping: boolean; showDetectedFields: string[]; @@ -103,6 +84,8 @@ export class UnthemedLogs extends PureComponent { showLabels: store.getBool(SETTINGS_KEYS.showLabels, false), showTime: store.getBool(SETTINGS_KEYS.showTime, true), wrapLogMessage: store.getBool(SETTINGS_KEYS.wrapLogMessage, true), + dedupStrategy: LogsDedupStrategy.none, + hiddenLogLevels: [], logsSortOrder: null, isFlipping: false, showDetectedFields: [], @@ -134,12 +117,8 @@ export class UnthemedLogs extends PureComponent { })); }; - onChangeDedup = (dedup: LogsDedupStrategy) => { - const { onDedupStrategyChange } = this.props; - if (this.props.dedupStrategy === dedup) { - return onDedupStrategyChange(LogsDedupStrategy.none); - } - return onDedupStrategyChange(dedup); + onChangeDedup = (dedupStrategy: LogsDedupStrategy) => { + this.setState({ dedupStrategy }); }; onChangeLabels = (event?: React.SyntheticEvent) => { @@ -176,8 +155,8 @@ export class UnthemedLogs extends PureComponent { }; onToggleLogLevel = (hiddenRawLevels: string[]) => { - const hiddenLogLevels: LogLevel[] = hiddenRawLevels.map((level) => LogLevel[level as LogLevel]); - this.props.onToggleLogLevel(hiddenLogLevels); + const hiddenLogLevels = hiddenRawLevels.map((level) => LogLevel[level as LogLevel]); + this.setState({ hiddenLogLevels }); }; onClickScan = (event: React.SyntheticEvent) => { @@ -229,6 +208,16 @@ export class UnthemedLogs extends PureComponent { return !!logRows.some((r) => r.hasUnescapedContent); }); + dedupRows = memoizeOne((logRows: LogRowModel[], dedupStrategy: LogsDedupStrategy) => { + const dedupedRows = dedupLogRows(logRows, dedupStrategy); + const dedupCount = dedupedRows.reduce((sum, row) => (row.duplicates ? sum + row.duplicates : sum), 0); + return { dedupedRows, dedupCount }; + }); + + filterRows = memoizeOne((logRows: LogRowModel[], hiddenLogLevels: LogLevel[]) => { + return filterLogLevels(logRows, new Set(hiddenLogLevels)); + }); + render() { const { logRows, @@ -244,11 +233,9 @@ export class UnthemedLogs extends PureComponent { scanRange, showContextToggle, width, - dedupedRows, absoluteRange, onChangeTime, getFieldLinks, - dedupStrategy, theme, } = this.props; @@ -256,43 +243,27 @@ export class UnthemedLogs extends PureComponent { showLabels, showTime, wrapLogMessage, + dedupStrategy, + hiddenLogLevels, logsSortOrder, isFlipping, showDetectedFields, forceEscape, } = this.state; + const styles = getStyles(theme); const hasData = logRows && logRows.length > 0; - const dedupCount = dedupedRows - ? dedupedRows.reduce((sum, row) => (row.duplicates ? sum + row.duplicates : sum), 0) - : 0; - const meta = logsMeta ? [...logsMeta] : []; + const hasUnescapedContent = this.checkUnescapedContent(logRows); - if (dedupStrategy !== LogsDedupStrategy.none) { - meta.push({ - label: 'Dedup count', - value: dedupCount, - kind: LogsMetaKind.Number, - }); - } - - if (logRows.some((r) => r.entry.length > MAX_CHARACTERS)) { - meta.push({ - label: 'Info', - value: 'Logs with more than 100,000 characters could not be parsed and highlighted', - kind: LogsMetaKind.String, - }); - } + const filteredLogs = this.filterRows(logRows, hiddenLogLevels); + const { dedupedRows, dedupCount } = this.dedupRows(filteredLogs, dedupStrategy); const scanText = scanRange ? `Scanning ${rangeUtil.describeTimeRange(scanRange)}` : 'Scanning...'; - const series = logsSeries ? logsSeries : []; - const styles = getStyles(theme); - const hasUnescapedContent = this.checkUnescapedContent(logRows); return ( <> { - {meta && ( - { - return { - label: item.label, - value: renderMetaItem(item.value, item.kind), - }; - })} - /> - )} - - {showDetectedFields?.length > 0 && ( - - Show all detected fields - - ), - }, - ]} - /> - )} - - {hasUnescapedContent && ( - - - - ), - }, - ]} - /> - )} + { - this.props.changeDedupStrategy(this.props.exploreId, dedupStrategy); - }; - - handleToggleLogLevel = (hiddenLogLevels: LogLevel[]) => { - const { exploreId } = this.props; - this.props.toggleLogLevelAction({ - exploreId, - hiddenLogLevels, - }); - }; - getLogRowContext = async (row: LogRowModel, options?: any): Promise => { const { datasourceInstance } = this.props; @@ -80,7 +66,6 @@ export class LogsContainer extends PureComponent void; + clearDetectedFields: () => void; +}; + +export const LogsMetaRow: React.FC = React.memo( + ({ + meta, + dedupStrategy, + dedupCount, + showDetectedFields, + clearDetectedFields, + hasUnescapedContent, + forceEscape, + onEscapeNewlines, + logRows, + }) => { + const logsMetaItem: Array = [...meta]; + + // Add deduplication info + if (dedupStrategy !== LogsDedupStrategy.none) { + logsMetaItem.push({ + label: 'Dedup count', + value: dedupCount, + kind: LogsMetaKind.Number, + }); + } + // Add info about limit for highlighting + if (logRows.some((r) => r.entry.length > MAX_CHARACTERS)) { + logsMetaItem.push({ + label: 'Info', + value: 'Logs with more than 100,000 characters could not be parsed and highlighted', + kind: LogsMetaKind.String, + }); + } + + // Add detected fields info + if (showDetectedFields?.length > 0) { + logsMetaItem.push( + { + label: 'Showing only detected fields', + value: renderMetaItem(showDetectedFields, LogsMetaKind.LabelsMap), + }, + { + label: '', + value: ( + + ), + } + ); + } + + // Add unescaped content info + if (hasUnescapedContent) { + logsMetaItem.push({ + label: 'Your logs might have incorrectly escaped content', + value: ( + + + + ), + }); + } + + return ( + <> + {logsMetaItem && ( + { + return { + label: item.label, + value: 'kind' in item ? renderMetaItem(item.value, item.kind) : item.value, + }; + })} + /> + )} + + ); + } +); + +LogsMetaRow.displayName = 'LogsMetaRow'; + +function renderMetaItem(value: any, kind: LogsMetaKind) { + if (kind === LogsMetaKind.LabelsMap) { + return ( + + + + ); + } else if (kind === LogsMetaKind.Error) { + return {value}; + } + return value; +} diff --git a/public/app/features/explore/state/explorePane.ts b/public/app/features/explore/state/explorePane.ts index b72d750abdb..f88eb1a3425 100644 --- a/public/app/features/explore/state/explorePane.ts +++ b/public/app/features/explore/state/explorePane.ts @@ -21,16 +21,7 @@ import { getUrlStateFromPaneState, } from './utils'; import { createAction, PayloadAction } from '@reduxjs/toolkit'; -import { - EventBusExtended, - DataQuery, - ExploreUrlState, - LogLevel, - LogsDedupStrategy, - TimeRange, - HistoryItem, - DataSourceApi, -} from '@grafana/data'; +import { EventBusExtended, DataQuery, ExploreUrlState, TimeRange, HistoryItem, DataSourceApi } from '@grafana/data'; // Types import { ThunkResult } from 'app/types'; import { getTimeZone } from 'app/features/profile/state/selectors'; @@ -53,15 +44,6 @@ export interface ChangeSizePayload { } export const changeSizeAction = createAction('explore/changeSize'); -/** - * Change deduplication strategy for logs. - */ -export interface ChangeDedupStrategyPayload { - exploreId: ExploreId; - dedupStrategy: LogsDedupStrategy; -} -export const changeDedupStrategyAction = createAction('explore/changeDedupStrategyAction'); - /** * Highlight expressions in the log results */ @@ -89,12 +71,6 @@ export interface InitializeExplorePayload { } export const initializeExploreAction = createAction('explore/initializeExplore'); -export interface ToggleLogLevelPayload { - exploreId: ExploreId; - hiddenLogLevels: LogLevel[]; -} -export const toggleLogLevelAction = createAction('explore/toggleLogLevel'); - export interface SetUrlReplacedPayload { exploreId: ExploreId; } @@ -111,16 +87,6 @@ export function changeSize( return changeSizeAction({ exploreId, height, width }); } -/** - * Change logs deduplication strategy. - */ -export const changeDedupStrategy = ( - exploreId: ExploreId, - dedupStrategy: LogsDedupStrategy -): PayloadAction => { - return changeDedupStrategyAction({ exploreId, dedupStrategy }); -}; - /** * Initialize Explore state with state from the URL and the React component. * Call this only on components for with the Explore state has not been initialized. @@ -255,14 +221,6 @@ export const paneReducer = (state: ExploreItemState = makeExplorePaneState(), ac }; } - if (changeDedupStrategyAction.match(action)) { - const { dedupStrategy } = action.payload; - return { - ...state, - dedupStrategy, - }; - } - if (initializeExploreAction.match(action)) { const { containerWidth, eventBridge, queries, range, originPanelId, datasourceInstance, history } = action.payload; @@ -283,14 +241,6 @@ export const paneReducer = (state: ExploreItemState = makeExplorePaneState(), ac }; } - if (toggleLogLevelAction.match(action)) { - const { hiddenLogLevels } = action.payload; - return { - ...state, - hiddenLogLevels: Array.from(hiddenLogLevels), - }; - } - return state; }; diff --git a/public/app/features/explore/state/selectors.test.ts b/public/app/features/explore/state/selectors.test.ts deleted file mode 100644 index 86527447cc8..00000000000 --- a/public/app/features/explore/state/selectors.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { deduplicatedRowsSelector } from './selectors'; -import { LogLevel, LogsDedupStrategy } from '@grafana/data'; -import { ExploreItemState } from 'app/types'; - -const state: any = { - logsResult: { - rows: [ - { - entry: '2019-03-05T11:00:56Z sntpc sntpc[1]: offset=-0.033938, delay=0.000649', - logLevel: LogLevel.debug, - }, - { - entry: '2019-03-05T11:00:26Z sntpc sntpc[1]: offset=-0.033730, delay=0.000581', - logLevel: LogLevel.debug, - }, - { - entry: '2019-03-05T10:59:56Z sntpc sntpc[1]: offset=-0.034184, delay=0.001089', - logLevel: LogLevel.debug, - }, - { - entry: '2019-03-05T10:59:26Z sntpc sntpc[1]: offset=-0.033972, delay=0.000582', - logLevel: LogLevel.debug, - }, - { - entry: '2019-03-05T10:58:56Z sntpc sntpc[1]: offset=-0.033955, delay=0.000606', - logLevel: LogLevel.debug, - }, - { - entry: '2019-03-05T10:58:26Z sntpc sntpc[1]: offset=-0.034067, delay=0.000616', - logLevel: LogLevel.debug, - }, - { - entry: '2019-03-05T10:57:56Z sntpc sntpc[1]: offset=-0.034155, delay=0.001021', - logLevel: LogLevel.debug, - }, - { - entry: '2019-03-05T10:57:26Z sntpc sntpc[1]: offset=-0.035797, delay=0.000883', - logLevel: LogLevel.debug, - }, - { - entry: '2019-03-05T10:56:56Z sntpc sntpc[1]: offset=-0.046818, delay=0.000605', - logLevel: LogLevel.debug, - }, - { - entry: '2019-03-05T10:56:26Z sntpc sntpc[1]: offset=-0.049200, delay=0.000584', - logLevel: LogLevel.error, - }, - { - entry: - '2019-11-01T14:53:02Z lifecycle-server time="2019-11-01T14:53:02.563571300Z" level=debug msg="Calling GET /v1.30/containers/c8defad4025e23f503d91b66610f93b5380622c8e871b31a71e29ff0e67653e7/stats?stream=0"', - logLevel: LogLevel.trace, - }, - ], - }, - hiddenLogLevels: undefined, - dedupStrategy: LogsDedupStrategy.none, -}; - -describe('Deduplication selector', () => { - it('returns the same rows if no deduplication', () => { - const dedups = deduplicatedRowsSelector(state as ExploreItemState); - expect(dedups?.length).toBe(11); - expect(dedups).toBe(state.logsResult.rows); - }); - - it('should correctly extracts rows and deduplicates them', () => { - const dedups = deduplicatedRowsSelector({ - ...state, - dedupStrategy: LogsDedupStrategy.numbers, - } as ExploreItemState); - expect(dedups?.length).toBe(2); - expect(dedups).not.toBe(state.logsResult.rows); - }); - - it('should filter out log levels', () => { - let dedups = deduplicatedRowsSelector({ - ...state, - hiddenLogLevels: [LogLevel.debug], - } as ExploreItemState); - expect(dedups?.length).toBe(2); - expect(dedups).not.toBe(state.logsResult.rows); - - dedups = deduplicatedRowsSelector({ - ...state, - dedupStrategy: LogsDedupStrategy.numbers, - hiddenLogLevels: [LogLevel.debug], - } as ExploreItemState); - - expect(dedups?.length).toBe(2); - expect(dedups).not.toBe(state.logsResult.rows); - }); -}); diff --git a/public/app/features/explore/state/selectors.ts b/public/app/features/explore/state/selectors.ts index c1e8c0010ee..ec790dd8121 100644 --- a/public/app/features/explore/state/selectors.ts +++ b/public/app/features/explore/state/selectors.ts @@ -1,21 +1,3 @@ -import { createSelector } from 'reselect'; -import { ExploreId, ExploreItemState, StoreState } from 'app/types'; -import { filterLogLevels, dedupLogRows } from 'app/core/logs_model'; - -const logsRowsSelector = (state: ExploreItemState) => state.logsResult && state.logsResult.rows; -const hiddenLogLevelsSelector = (state: ExploreItemState) => state.hiddenLogLevels; -const dedupStrategySelector = (state: ExploreItemState) => state.dedupStrategy; -export const deduplicatedRowsSelector = createSelector( - logsRowsSelector, - hiddenLogLevelsSelector, - dedupStrategySelector, - function dedupRows(rows, hiddenLogLevels, dedupStrategy) { - if (!(rows && rows.length)) { - return rows; - } - const filteredRows = filterLogLevels(rows, new Set(hiddenLogLevels)); - return dedupLogRows(filteredRows, dedupStrategy); - } -); +import { ExploreId, StoreState } from 'app/types'; export const isSplit = (state: StoreState) => Boolean(state.explore[ExploreId.left] && state.explore[ExploreId.right]); diff --git a/public/app/features/explore/state/utils.ts b/public/app/features/explore/state/utils.ts index c7373f63b19..8b72d814782 100644 --- a/public/app/features/explore/state/utils.ts +++ b/public/app/features/explore/state/utils.ts @@ -5,7 +5,6 @@ import { getDefaultTimeRange, HistoryItem, LoadingState, - LogsDedupStrategy, PanelData, } from '@grafana/data'; @@ -49,7 +48,6 @@ export const makeExplorePaneState = (): ExploreItemState => ({ tableResult: null, graphResult: null, logsResult: null, - dedupStrategy: LogsDedupStrategy.none, eventBridge: (null as unknown) as EventBusExtended, }); diff --git a/public/app/types/explore.ts b/public/app/types/explore.ts index 06a9f5eaca0..e4f913cca21 100644 --- a/public/app/types/explore.ts +++ b/public/app/types/explore.ts @@ -6,8 +6,6 @@ import { DataQueryRequest, DataSourceApi, HistoryItem, - LogLevel, - LogsDedupStrategy, LogsModel, PanelData, QueryHint, @@ -119,16 +117,6 @@ export interface ExploreItemState { */ queryKeys: string[]; - /** - * Current logs deduplication strategy - */ - dedupStrategy: LogsDedupStrategy; - - /** - * Currently hidden log series - */ - hiddenLogLevels?: LogLevel[]; - /** * How often query should be refreshed */