diff --git a/packages/grafana-ui/src/components/Logs/LogRows.test.tsx b/packages/grafana-ui/src/components/Logs/LogRows.test.tsx index 05b082af02d..7956d1e02f8 100644 --- a/packages/grafana-ui/src/components/Logs/LogRows.test.tsx +++ b/packages/grafana-ui/src/components/Logs/LogRows.test.tsx @@ -10,10 +10,7 @@ describe('LogRows', () => { const rows: LogRowModel[] = [makeLog({ uid: '1' }), makeLog({ uid: '2' }), makeLog({ uid: '3' })]; const wrapper = mount( { jest.useFakeTimers(); const wrapper = mount( { const dedupedRows: LogRowModel[] = [makeLog({ uid: '4' }), makeLog({ uid: '5' })]; const wrapper = mount( { const rows: LogRowModel[] = range(PREVIEW_LIMIT * 2 + 1).map(num => makeLog({ uid: num.toString() })); const wrapper = mount( { componentDidMount() { // Staged rendering - const { data, previewLimit } = this.props; - const rowCount = data ? data.rows.length : 0; + const { logRows, previewLimit } = this.props; + const rowCount = logRows ? logRows.length : 0; // Render all right away if not too far over the limit const renderAll = rowCount <= previewLimit! * 2; if (renderAll) { @@ -70,8 +70,8 @@ class UnThemedLogRows extends PureComponent { const { dedupStrategy, showTime, - data, - deduplicatedData, + logRows, + deduplicatedRows, highlighterExpressions, timeZone, onClickFilterLabel, @@ -82,15 +82,15 @@ class UnThemedLogRows extends PureComponent { previewLimit, } = this.props; const { renderAll } = this.state; - const dedupedData = deduplicatedData ? deduplicatedData : data; - const hasData = data && data.rows && data.rows.length > 0; - const dedupCount = dedupedData - ? dedupedData.rows.reduce((sum, row) => (row.duplicates ? sum + row.duplicates : sum), 0) + const dedupedRows = deduplicatedRows ? deduplicatedRows : logRows; + const hasData = logRows && logRows.length > 0; + const dedupCount = dedupedRows + ? dedupedRows.reduce((sum, row) => (row.duplicates ? sum + row.duplicates : sum), 0) : 0; const showDuplicates = dedupStrategy !== LogsDedupStrategy.none && dedupCount > 0; // Staged rendering - const processedRows = dedupedData ? dedupedData.rows : []; + const processedRows = dedupedRows ? dedupedRows : []; const firstRows = processedRows.slice(0, previewLimit!); const rowCount = Math.min(processedRows.length, rowLimit!); const lastRows = processedRows.slice(previewLimit!, rowCount); diff --git a/public/app/core/logs_model.ts b/public/app/core/logs_model.ts index f7385bc163d..80e11d04f98 100644 --- a/public/app/core/logs_model.ts +++ b/public/app/core/logs_model.ts @@ -57,12 +57,12 @@ function isDuplicateRow(row: LogRowModel, other: LogRowModel, strategy: LogsDedu } } -export function dedupLogRows(logs: LogsModel, strategy: LogsDedupStrategy): LogsModel { +export function dedupLogRows(rows: LogRowModel[], strategy: LogsDedupStrategy): LogRowModel[] { if (strategy === LogsDedupStrategy.none) { - return logs; + return rows; } - const dedupedRows = logs.rows.reduce((result: LogRowModel[], row: LogRowModel, index, list) => { + return rows.reduce((result: LogRowModel[], row: LogRowModel, index) => { const rowCopy = { ...row }; const previous = result[result.length - 1]; if (index > 0 && isDuplicateRow(row, previous, strategy)) { @@ -73,29 +73,16 @@ export function dedupLogRows(logs: LogsModel, strategy: LogsDedupStrategy): Logs } return result; }, []); - - return { - ...logs, - rows: dedupedRows, - }; } -export function filterLogLevels(logs: LogsModel, hiddenLogLevels: Set): LogsModel { +export function filterLogLevels(logRows: LogRowModel[], hiddenLogLevels: Set): LogRowModel[] { if (hiddenLogLevels.size === 0) { - return logs; + return logRows; } - const filteredRows = logs.rows.reduce((result: LogRowModel[], row: LogRowModel, index, list) => { - if (!hiddenLogLevels.has(row.logLevel)) { - result.push(row); - } - return result; - }, []); - - return { - ...logs, - rows: filteredRows, - }; + return logRows.filter((row: LogRowModel) => { + return !hiddenLogLevels.has(row.logLevel); + }); } export function makeSeriesForLogs(rows: LogRowModel[], intervalMs: number): GraphSeriesXY[] { diff --git a/public/app/core/specs/logs_model.test.ts b/public/app/core/specs/logs_model.test.ts index 0b3a240cc85..d09134f9485 100644 --- a/public/app/core/specs/logs_model.test.ts +++ b/public/app/core/specs/logs_model.test.ts @@ -1,48 +1,44 @@ import { DataFrame, FieldType, - LogsModel, LogsMetaKind, LogsDedupStrategy, LogLevel, MutableDataFrame, toDataFrame, + LogRowModel, } from '@grafana/data'; import { dedupLogRows, dataFrameToLogsModel } from '../logs_model'; describe('dedupLogRows()', () => { test('should return rows as is when dedup is set to none', () => { - const logs = { - rows: [ - { - entry: 'WARN test 1.23 on [xxx]', - }, - { - entry: 'WARN test 1.23 on [xxx]', - }, - ], - }; - expect(dedupLogRows(logs as LogsModel, LogsDedupStrategy.none).rows).toMatchObject(logs.rows); + const rows: LogRowModel[] = [ + { + entry: 'WARN test 1.23 on [xxx]', + }, + { + entry: 'WARN test 1.23 on [xxx]', + }, + ] as any; + expect(dedupLogRows(rows, LogsDedupStrategy.none)).toMatchObject(rows); }); test('should dedup on exact matches', () => { - const logs = { - rows: [ - { - entry: 'WARN test 1.23 on [xxx]', - }, - { - entry: 'WARN test 1.23 on [xxx]', - }, - { - entry: 'INFO test 2.44 on [xxx]', - }, - { - entry: 'WARN test 1.23 on [xxx]', - }, - ], - }; - expect(dedupLogRows(logs as LogsModel, LogsDedupStrategy.exact).rows).toEqual([ + const rows: LogRowModel[] = [ + { + entry: 'WARN test 1.23 on [xxx]', + }, + { + entry: 'WARN test 1.23 on [xxx]', + }, + { + entry: 'INFO test 2.44 on [xxx]', + }, + { + entry: 'WARN test 1.23 on [xxx]', + }, + ] as any; + expect(dedupLogRows(rows, LogsDedupStrategy.exact)).toEqual([ { duplicates: 1, entry: 'WARN test 1.23 on [xxx]', @@ -59,23 +55,21 @@ describe('dedupLogRows()', () => { }); test('should dedup on number matches', () => { - const logs = { - rows: [ - { - entry: 'WARN test 1.2323423 on [xxx]', - }, - { - entry: 'WARN test 1.23 on [xxx]', - }, - { - entry: 'INFO test 2.44 on [xxx]', - }, - { - entry: 'WARN test 1.23 on [xxx]', - }, - ], - }; - expect(dedupLogRows(logs as LogsModel, LogsDedupStrategy.numbers).rows).toEqual([ + const rows: LogRowModel[] = [ + { + entry: 'WARN test 1.2323423 on [xxx]', + }, + { + entry: 'WARN test 1.23 on [xxx]', + }, + { + entry: 'INFO test 2.44 on [xxx]', + }, + { + entry: 'WARN test 1.23 on [xxx]', + }, + ] as any; + expect(dedupLogRows(rows, LogsDedupStrategy.numbers)).toEqual([ { duplicates: 1, entry: 'WARN test 1.2323423 on [xxx]', @@ -92,23 +86,21 @@ describe('dedupLogRows()', () => { }); test('should dedup on signature matches', () => { - const logs = { - rows: [ - { - entry: 'WARN test 1.2323423 on [xxx]', - }, - { - entry: 'WARN test 1.23 on [xxx]', - }, - { - entry: 'INFO test 2.44 on [xxx]', - }, - { - entry: 'WARN test 1.23 on [xxx]', - }, - ], - }; - expect(dedupLogRows(logs as LogsModel, LogsDedupStrategy.signature).rows).toEqual([ + const rows: LogRowModel[] = [ + { + entry: 'WARN test 1.2323423 on [xxx]', + }, + { + entry: 'WARN test 1.23 on [xxx]', + }, + { + entry: 'INFO test 2.44 on [xxx]', + }, + { + entry: 'WARN test 1.23 on [xxx]', + }, + ] as any; + expect(dedupLogRows(rows, LogsDedupStrategy.signature)).toEqual([ { duplicates: 3, entry: 'WARN test 1.2323423 on [xxx]', @@ -117,20 +109,18 @@ describe('dedupLogRows()', () => { }); test('should return to non-deduped state on same log result', () => { - const logs = { - rows: [ - { - entry: 'INFO 123', - }, - { - entry: 'WARN 123', - }, - { - entry: 'WARN 123', - }, - ], - }; - expect(dedupLogRows(logs as LogsModel, LogsDedupStrategy.exact).rows).toEqual([ + const rows: LogRowModel[] = [ + { + entry: 'INFO 123', + }, + { + entry: 'WARN 123', + }, + { + entry: 'WARN 123', + }, + ] as any; + expect(dedupLogRows(rows, LogsDedupStrategy.exact)).toEqual([ { duplicates: 0, entry: 'INFO 123', @@ -141,7 +131,7 @@ describe('dedupLogRows()', () => { }, ]); - expect(dedupLogRows(logs as LogsModel, LogsDedupStrategy.none).rows).toEqual(logs.rows); + expect(dedupLogRows(rows, LogsDedupStrategy.none)).toEqual(rows); }); }); diff --git a/public/app/core/utils/reselect.ts b/public/app/core/utils/reselect.ts deleted file mode 100644 index 037c7d8f80a..00000000000 --- a/public/app/core/utils/reselect.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { memoize } from 'lodash'; -import { createSelectorCreator } from 'reselect'; - -const hashFn = (...args: any[]) => args.reduce((acc, val) => acc + '-' + JSON.stringify(val), ''); -export const createLodashMemoizedSelector = createSelectorCreator(memoize as any, hashFn); diff --git a/public/app/features/explore/LiveLogs.test.tsx b/public/app/features/explore/LiveLogs.test.tsx new file mode 100644 index 00000000000..12bd1bc5d35 --- /dev/null +++ b/public/app/features/explore/LiveLogs.test.tsx @@ -0,0 +1,77 @@ +import React from 'react'; +import { LogLevel, LogRowModel } from '@grafana/data'; +import { mount } from 'enzyme'; +import { LiveLogsWithTheme } from './LiveLogs'; + +describe('LiveLogs', () => { + it('renders logs', () => { + const rows: LogRowModel[] = [makeLog({ uid: '1' }), makeLog({ uid: '2' }), makeLog({ uid: '3' })]; + const wrapper = mount( + {}} + onPause={() => {}} + onResume={() => {}} + isPaused={true} + /> + ); + + expect(wrapper.contains('log message 1')).toBeTruthy(); + expect(wrapper.contains('log message 2')).toBeTruthy(); + expect(wrapper.contains('log message 3')).toBeTruthy(); + }); + + it('renders new logs only when not paused', () => { + const rows: LogRowModel[] = [makeLog({ uid: '1' }), makeLog({ uid: '2' }), makeLog({ uid: '3' })]; + const wrapper = mount( + {}} + onPause={() => {}} + onResume={() => {}} + isPaused={true} + /> + ); + + wrapper.setProps({ + ...wrapper.props(), + logRows: [makeLog({ uid: '4' }), makeLog({ uid: '5' }), makeLog({ uid: '6' })], + }); + + expect(wrapper.contains('log message 1')).toBeTruthy(); + expect(wrapper.contains('log message 2')).toBeTruthy(); + expect(wrapper.contains('log message 3')).toBeTruthy(); + + (wrapper.find('LiveLogs').instance() as any).liveEndDiv.scrollIntoView = () => {}; + + wrapper.setProps({ + ...wrapper.props(), + isPaused: false, + }); + + expect(wrapper.contains('log message 4')).toBeTruthy(); + expect(wrapper.contains('log message 5')).toBeTruthy(); + expect(wrapper.contains('log message 6')).toBeTruthy(); + }); +}); + +const makeLog = (overides: Partial): LogRowModel => { + const uid = overides.uid || '1'; + const entry = `log message ${uid}`; + return { + uid, + logLevel: LogLevel.debug, + entry, + hasAnsi: false, + labels: {}, + raw: entry, + timestamp: '', + timeFromNow: '', + timeEpochMs: 1, + timeLocal: '', + timeUtc: '', + ...overides, + }; +}; diff --git a/public/app/features/explore/LiveLogs.tsx b/public/app/features/explore/LiveLogs.tsx index 7081b59759d..f7b861827b2 100644 --- a/public/app/features/explore/LiveLogs.tsx +++ b/public/app/features/explore/LiveLogs.tsx @@ -3,7 +3,7 @@ import { css, cx } from 'emotion'; import tinycolor from 'tinycolor2'; import { Themeable, withTheme, getLogRowStyles } from '@grafana/ui'; -import { GrafanaTheme, LogsModel, LogRowModel, TimeZone } from '@grafana/data'; +import { GrafanaTheme, LogRowModel, TimeZone } from '@grafana/data'; import ElapsedTime from './ElapsedTime'; @@ -50,7 +50,7 @@ const getStyles = (theme: GrafanaTheme) => ({ }); export interface Props extends Themeable { - logsResult?: LogsModel; + logRows?: LogRowModel[]; timeZone: TimeZone; stopLive: () => void; onPause: () => void; @@ -59,7 +59,7 @@ export interface Props extends Themeable { } interface State { - logsResultToRender?: LogsModel; + logRowsToRender?: LogRowModel[]; } class LiveLogs extends PureComponent { @@ -70,7 +70,7 @@ class LiveLogs extends PureComponent { constructor(props: Props) { super(props); this.state = { - logsResultToRender: props.logsResult, + logRowsToRender: props.logRows, }; } @@ -99,7 +99,7 @@ class LiveLogs extends PureComponent { // We update what we show only if not paused. We keep any background subscriptions running and keep updating // our state, but we do not show the updates, this allows us start again showing correct result after resuming // without creating a gap in the log results. - logsResultToRender: nextProps.logsResult, + logRowsToRender: nextProps.logRows, }; } else { return null; @@ -123,7 +123,7 @@ class LiveLogs extends PureComponent { rowsToRender = () => { const { isPaused } = this.props; - let rowsToRender: LogRowModel[] = this.state.logsResultToRender ? this.state.logsResultToRender.rows : []; + let rowsToRender: LogRowModel[] = this.state.logRowsToRender; if (!isPaused) { // A perf optimisation here. Show just 100 rows when streaming and full length when the streaming is paused. rowsToRender = rowsToRender.slice(-100); @@ -184,7 +184,7 @@ class LiveLogs extends PureComponent { {isPaused || ( - Last line received: ago + Last line received: ago )} diff --git a/public/app/features/explore/Logs.tsx b/public/app/features/explore/Logs.tsx index d2b68a5c7c1..46803d91787 100644 --- a/public/app/features/explore/Logs.tsx +++ b/public/app/features/explore/Logs.tsx @@ -7,10 +7,11 @@ import { TimeZone, AbsoluteTimeRange, LogsMetaKind, - LogsModel, LogsDedupStrategy, LogRowModel, LogsDedupDescription, + LogsMetaItem, + GraphSeriesXY, } from '@grafana/data'; import { Switch, LogLabels, ToggleButtonGroup, ToggleButton, LogRows } from '@grafana/ui'; @@ -28,8 +29,11 @@ function renderMetaItem(value: any, kind: LogsMetaKind) { } interface Props { - data?: LogsModel; - dedupedData?: LogsModel; + logRows?: LogRowModel[]; + logsMeta?: LogsMetaItem[]; + logsSeries?: GraphSeriesXY[]; + dedupedRows?: LogRowModel[]; + width: number; highlighterExpressions: string[]; loading: boolean; @@ -95,7 +99,9 @@ export class Logs extends PureComponent { render() { const { - data, + logRows, + logsMeta, + logsSeries, highlighterExpressions, loading = false, onClickFilterLabel, @@ -104,22 +110,22 @@ export class Logs extends PureComponent { scanning, scanRange, width, - dedupedData, + dedupedRows, absoluteRange, onChangeTime, } = this.props; - if (!data) { + if (!logRows) { return null; } const { showTime } = this.state; const { dedupStrategy } = this.props; - const hasData = data && data.rows && data.rows.length > 0; - const dedupCount = dedupedData - ? dedupedData.rows.reduce((sum, row) => (row.duplicates ? sum + row.duplicates : sum), 0) + const hasData = logRows && logRows.length > 0; + const dedupCount = dedupedRows + ? dedupedRows.reduce((sum, row) => (row.duplicates ? sum + row.duplicates : sum), 0) : 0; - const meta = data && data.meta ? [...data.meta] : []; + const meta = logsMeta ? [...logsMeta] : []; if (dedupStrategy !== LogsDedupStrategy.none) { meta.push({ @@ -130,7 +136,7 @@ export class Logs extends PureComponent { } const scanText = scanRange ? `Scanning ${rangeUtil.describeTimeRange(scanRange)}` : 'Scanning...'; - const series = data && data.series ? data.series : []; + const series = logsSeries ? logsSeries : []; return (
@@ -183,14 +189,14 @@ export class Logs extends PureComponent { )} diff --git a/public/app/features/explore/LogsContainer.tsx b/public/app/features/explore/LogsContainer.tsx index 6c41b14fbfd..a7df15cf537 100644 --- a/public/app/features/explore/LogsContainer.tsx +++ b/public/app/features/explore/LogsContainer.tsx @@ -9,10 +9,11 @@ import { LogLevel, TimeZone, AbsoluteTimeRange, - LogsModel, LogRowModel, LogsDedupStrategy, TimeRange, + LogsMetaItem, + GraphSeriesXY, } from '@grafana/data'; import { ExploreId, ExploreItemState } from 'app/types/explore'; @@ -20,7 +21,7 @@ import { StoreState } from 'app/types'; import { changeDedupStrategy, updateTimeRange } from './state/actions'; import { toggleLogLevelAction } from 'app/features/explore/state/actionTypes'; -import { deduplicatedLogsSelector, exploreItemUIStateSelector } from 'app/features/explore/state/selectors'; +import { deduplicatedRowsSelector } from 'app/features/explore/state/selectors'; import { getTimeZone } from '../profile/state/selectors'; import { LiveLogsWithTheme } from './LiveLogs'; import { Logs } from './Logs'; @@ -33,8 +34,11 @@ interface LogsContainerProps { loading: boolean; logsHighlighterExpressions?: string[]; - logsResult?: LogsModel; - dedupedResult?: LogsModel; + logRows?: LogRowModel[]; + logsMeta?: LogsMetaItem[]; + logsSeries?: GraphSeriesXY[]; + dedupedRows?: LogRowModel[]; + onClickFilterLabel?: (key: string, value: string) => void; onClickFilterOutLabel?: (key: string, value: string) => void; onStartScanning: () => void; @@ -86,8 +90,10 @@ export class LogsContainer extends PureComponent { const { loading, logsHighlighterExpressions, - logsResult, - dedupedResult, + logRows, + logsMeta, + logsSeries, + dedupedRows, onClickFilterLabel, onClickFilterOutLabel, onStartScanning, @@ -108,7 +114,7 @@ export class LogsContainer extends PureComponent { {controls => ( { { - it('should correctly deduplicate log rows when changing strategy multiple times', () => { - // Simulating sequence of UI actions that was causing a problem with deduplication counter being visible when unnecessary. - // The sequence was changing dedup strategy: (none -> exact -> numbers -> signature -> none) *2 -> exact. After that the first - // row contained information that was deduped, while it shouldn't be. - // Problem was caused by mutating the log results entries in redux state. The memoisation hash for deduplicatedLogsSelector - // was changing depending on duplicates information from log row state, while should be dependand on log row only. - - let dedups = deduplicatedLogsSelector(state as ExploreItemState); - expect(dedups.rows.length).toBe(10); - - deduplicatedLogsSelector({ - ...state, - dedupStrategy: LogsDedupStrategy.none, - } as ExploreItemState); - - deduplicatedLogsSelector({ - ...state, - dedupStrategy: LogsDedupStrategy.exact, - } as ExploreItemState); + 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); + }); - deduplicatedLogsSelector({ + 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); + }); - deduplicatedLogsSelector({ - ...state, - dedupStrategy: LogsDedupStrategy.signature, - } as ExploreItemState); - - deduplicatedLogsSelector({ - ...state, - dedupStrategy: LogsDedupStrategy.none, - } as ExploreItemState); - - deduplicatedLogsSelector({ + it('should filter out log levels', () => { + let dedups = deduplicatedRowsSelector({ ...state, - dedupStrategy: LogsDedupStrategy.exact, + hiddenLogLevels: [LogLevel.debug], } as ExploreItemState); + expect(dedups.length).toBe(2); + expect(dedups).not.toBe(state.logsResult.rows); - deduplicatedLogsSelector({ + dedups = deduplicatedRowsSelector({ ...state, dedupStrategy: LogsDedupStrategy.numbers, + hiddenLogLevels: [LogLevel.debug], } as ExploreItemState); - deduplicatedLogsSelector({ - ...state, - dedupStrategy: LogsDedupStrategy.signature, - } as ExploreItemState); - - deduplicatedLogsSelector({ - ...state, - dedupStrategy: LogsDedupStrategy.none, - } as ExploreItemState); - - dedups = deduplicatedLogsSelector({ - ...state, - dedupStrategy: LogsDedupStrategy.exact, - } as ExploreItemState); - - // Expecting that no row has duplicates now - expect(dedups.rows.reduce((acc, row) => acc + row.duplicates, 0)).toBe(0); + 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 6925e706d4f..32e097b4f62 100644 --- a/public/app/features/explore/state/selectors.ts +++ b/public/app/features/explore/state/selectors.ts @@ -1,29 +1,19 @@ -import { createLodashMemoizedSelector } from 'app/core/utils/reselect'; +import { createSelector } from 'reselect'; import { ExploreItemState } from 'app/types'; import { filterLogLevels, dedupLogRows } from 'app/core/logs_model'; -export const exploreItemUIStateSelector = (itemState: ExploreItemState) => { - const { showingGraph, showingTable, showingStartPage, dedupStrategy } = itemState; - return { - showingGraph, - showingTable, - showingStartPage, - dedupStrategy, - }; -}; - -const logsSelector = (state: ExploreItemState) => state.logsResult; +const logsRowsSelector = (state: ExploreItemState) => state.logsResult && state.logsResult.rows; const hiddenLogLevelsSelector = (state: ExploreItemState) => state.hiddenLogLevels; const dedupStrategySelector = (state: ExploreItemState) => state.dedupStrategy; -export const deduplicatedLogsSelector = createLodashMemoizedSelector( - logsSelector, +export const deduplicatedRowsSelector = createSelector( + logsRowsSelector, hiddenLogLevelsSelector, dedupStrategySelector, - (logs, hiddenLogLevels, dedupStrategy) => { - if (!logs) { - return null; + function dedupRows(rows, hiddenLogLevels, dedupStrategy) { + if (!(rows && rows.length)) { + return rows; } - const filteredData = filterLogLevels(logs, new Set(hiddenLogLevels)); - return dedupLogRows(filteredData, dedupStrategy); + const filteredRows = filterLogLevels(rows, new Set(hiddenLogLevels)); + return dedupLogRows(filteredRows, dedupStrategy); } ); diff --git a/public/app/plugins/panel/logs/LogsPanel.tsx b/public/app/plugins/panel/logs/LogsPanel.tsx index a4f0527077f..32fa547c203 100644 --- a/public/app/plugins/panel/logs/LogsPanel.tsx +++ b/public/app/plugins/panel/logs/LogsPanel.tsx @@ -27,7 +27,7 @@ export const LogsPanel: React.FunctionComponent = ({ return (