IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Explore: Memory leak fix due to dedup selector (#20107)

Browse Source 
Change custom hashing and lodash.memoize based selector for standard reselect.
This commit is contained in:
Andrej Ocenas
2019-11-01 16:38:34 +01:00
committed by GitHub
parent fe584efc70
commit dca872f75f
12 changed files with 266 additions and 242 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
public/app/features/explore/LiveLogs.test.tsx Normal file
View File

@@ -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(
<LiveLogsWithTheme
logRows={rows}
timeZone={'utc'}
stopLive={() => {}}
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(
<LiveLogsWithTheme
logRows={rows}
timeZone={'utc'}
stopLive={() => {}}
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>): 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,
};
};

14
public/app/features/explore/LiveLogs.tsx
View File

@@ -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<Props, State> {
@@ -70,7 +70,7 @@ class LiveLogs extends PureComponent<Props, State> {
constructor(props: Props) {
super(props);
this.state = {
logsResultToRender: props.logsResult,
logRowsToRender: props.logRows,
};
}
@@ -99,7 +99,7 @@ class LiveLogs extends PureComponent<Props, State> {
// 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<Props, State> {
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<Props, State> {
</button>
{isPaused || (
<span>
Last line received: <ElapsedTime resetKey={this.props.logsResult} humanize={true} /> ago
Last line received: <ElapsedTime resetKey={this.props.logRows} humanize={true} /> ago
</span>
)}
</div>

34
public/app/features/explore/Logs.tsx
View File

@@ -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<Props, State> {
render() {
const {
data,
logRows,
logsMeta,
logsSeries,
highlighterExpressions,
loading = false,
onClickFilterLabel,
@@ -104,22 +110,22 @@ export class Logs extends PureComponent<Props, State> {
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<Props, State> {
}
const scanText = scanRange ? `Scanning ${rangeUtil.describeTimeRange(scanRange)}` : 'Scanning...';
const series = data && data.series ? data.series : [];
const series = logsSeries ? logsSeries : [];
return (
<div className="logs-panel">
@@ -183,14 +189,14 @@ export class Logs extends PureComponent<Props, State> {
)}
<LogRows
data={data}
deduplicatedData={dedupedData}
logRows={logRows}
deduplicatedRows={dedupedRows}
dedupStrategy={dedupStrategy}
getRowContext={this.props.getRowContext}
highlighterExpressions={highlighterExpressions}
rowLimit={logRows ? logRows.length : undefined}
onClickFilterLabel={onClickFilterLabel}
onClickFilterOutLabel={onClickFilterOutLabel}
rowLimit={data ? data.rows.length : undefined}
showTime={showTime}
timeZone={timeZone}
/>

36
public/app/features/explore/LogsContainer.tsx
View File

@@ -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<LogsContainerProps> {
const {
loading,
logsHighlighterExpressions,
logsResult,
dedupedResult,
logRows,
logsMeta,
logsSeries,
dedupedRows,
onClickFilterLabel,
onClickFilterOutLabel,
onStartScanning,
@@ -108,7 +114,7 @@ export class LogsContainer extends PureComponent<LogsContainerProps> {
<LiveTailControls exploreId={exploreId}>
{controls => (
<LiveLogsWithTheme
logsResult={logsResult}
logRows={logRows}
timeZone={timeZone}
stopLive={controls.stop}
isPaused={this.props.isPaused}
@@ -123,8 +129,10 @@ export class LogsContainer extends PureComponent<LogsContainerProps> {
<Collapse label="Logs" loading={loading} isOpen>
<Logs
dedupStrategy={this.props.dedupStrategy || LogsDedupStrategy.none}
data={logsResult}
dedupedData={dedupedResult}
logRows={logRows}
logsMeta={logsMeta}
logsSeries={logsSeries}
dedupedRows={dedupedRows}
highlighterExpressions={logsHighlighterExpressions}
loading={loading}
onChangeTime={this.onChangeTime}
@@ -162,19 +170,21 @@ function mapStateToProps(state: StoreState, { exploreId }: { exploreId: string }
isPaused,
range,
absoluteRange,
dedupStrategy,
} = item;
const { dedupStrategy } = exploreItemUIStateSelector(item);
const dedupedResult = deduplicatedLogsSelector(item);
const dedupedRows = deduplicatedRowsSelector(item);
const timeZone = getTimeZone(state.user);
return {
loading,
logsHighlighterExpressions,
logsResult,
logRows: logsResult && logsResult.rows,
logsMeta: logsResult && logsResult.meta,
logsSeries: logsResult && logsResult.series,
scanning,
timeZone,
dedupStrategy,
dedupedResult,
dedupedRows,
datasourceInstance,
isLive,
isPaused,

88
public/app/features/explore/state/selectors.test.ts
View File

@@ -1,5 +1,5 @@
import { deduplicatedLogsSelector } from './selectors';
import { LogsDedupStrategy } from '@grafana/data';
import { deduplicatedRowsSelector } from './selectors';
import { LogLevel, LogsDedupStrategy } from '@grafana/data';
import { ExploreItemState } from 'app/types';
const state: any = {
@@ -7,33 +7,48 @@ const state: any = {
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,
},
],
},
@@ -42,67 +57,36 @@ const state: any = {
};
describe('Deduplication selector', () => {
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.
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);
});
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);
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({
it('should filter out log levels', () => {
let dedups = deduplicatedRowsSelector({
...state,
dedupStrategy: LogsDedupStrategy.signature,
hiddenLogLevels: [LogLevel.debug],
} as ExploreItemState);
expect(dedups.length).toBe(2);
expect(dedups).not.toBe(state.logsResult.rows);
deduplicatedLogsSelector({
...state,
dedupStrategy: LogsDedupStrategy.none,
} as ExploreItemState);
deduplicatedLogsSelector({
...state,
dedupStrategy: LogsDedupStrategy.exact,
} as ExploreItemState);
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);
});
});

28
public/app/features/explore/state/selectors.ts
View File

@@ -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);
}
);