Variables: Improves inspection performance and unknown filtering (#31811)

* Refactor: moves inspect calculation to Redux

* Refactor: adds valid filters and tests
This commit is contained in:
Hugo Häggmark 2021-03-09 12:49:05 +01:00 committed by GitHub
parent d60727311e
commit 63746d027b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 270 additions and 111 deletions

View File

@ -17,6 +17,11 @@ const mapStateToProps = (state: StoreState) => ({
variables: getEditorVariables(state),
idInEditor: state.templating.editor.id,
dashboard: state.dashboard.getModel(),
unknownsNetwork: state.templating.inspect.unknownsNetwork,
unknownExists: state.templating.inspect.unknownExits,
usagesNetwork: state.templating.inspect.usagesNetwork,
unknown: state.templating.inspect.unknown,
usages: state.templating.inspect.usages,
});
const mapDispatchToProps = {
@ -67,6 +72,7 @@ class VariableEditorContainerUnconnected extends PureComponent<Props> {
render() {
const variableToEdit = this.props.variables.find((s) => s.id === this.props.idInEditor) ?? null;
return (
<div>
<div className="page-action-bar">
@ -114,8 +120,10 @@ class VariableEditorContainerUnconnected extends PureComponent<Props> {
onChangeVariableOrder={this.onChangeVariableOrder}
onDuplicateVariable={this.onDuplicateVariable}
onRemoveVariable={this.onRemoveVariable}
usages={this.props.usages}
usagesNetwork={this.props.usagesNetwork}
/>
<VariablesUnknownTable dashboard={this.props.dashboard} variables={this.props.variables} />
{this.props.unknownExists ? <VariablesUnknownTable usages={this.props.unknownsNetwork} /> : null}
</>
)}
{variableToEdit && <VariableEditorEditor identifier={toVariableIdentifier(variableToEdit)} />}

View File

@ -8,13 +8,15 @@ import EmptyListCTA from '../../../core/components/EmptyListCTA/EmptyListCTA';
import { QueryVariableModel, VariableModel } from '../types';
import { toVariableIdentifier, VariableIdentifier } from '../state/types';
import { DashboardModel } from '../../dashboard/state';
import { getVariableUsages } from '../inspect/utils';
import { getVariableUsages, UsagesToNetwork, VariableUsageTree } from '../inspect/utils';
import { isAdHoc } from '../guard';
import { VariableUsagesButton } from '../inspect/VariableUsagesButton';
export interface Props {
variables: VariableModel[];
dashboard: DashboardModel | null;
usages: VariableUsageTree[];
usagesNetwork: UsagesToNetwork[];
onAddClick: (event: MouseEvent) => void;
onEditClick: (identifier: VariableIdentifier) => void;
onChangeVariableOrder: (identifier: VariableIdentifier, fromIndex: number, toIndex: number) => void;
@ -97,7 +99,7 @@ export class VariableEditorList extends PureComponent<Props> {
: typeof variable.query === 'string'
? variable.query
: '';
const usages = getVariableUsages(variable.id, this.props.variables, this.props.dashboard);
const usages = getVariableUsages(variable.id, this.props.usages);
const passed = usages > 0 || isAdHoc(variable);
return (
<tr key={`${variable.name}-${index}`}>
@ -129,9 +131,9 @@ export class VariableEditorList extends PureComponent<Props> {
<td style={{ width: '1%' }}>
<VariableUsagesButton
variable={variable}
variables={this.props.variables}
dashboard={this.props.dashboard}
id={variable.id}
isAdhoc={isAdHoc(variable)}
usages={this.props.usagesNetwork}
/>
</td>

View File

@ -1,5 +1,5 @@
import { ThunkResult } from '../../../types';
import { getNewVariabelIndex, getVariable, getVariables } from '../state/selectors';
import { getEditorVariables, getNewVariabelIndex, getVariable, getVariables } from '../state/selectors';
import {
changeVariableNameFailed,
changeVariableNameSucceeded,
@ -15,6 +15,8 @@ import { VariableType } from '@grafana/data';
import { addVariable, removeVariable } from '../state/sharedReducer';
import { updateOptions } from '../state/actions';
import { VariableModel } from '../types';
import { initInspect } from '../inspect/reducer';
import { createUsagesNetwork, transformUsagesToNetwork } from '../inspect/utils';
export const variableEditorMount = (identifier: VariableIdentifier): ThunkResult<void> => {
return async (dispatch) => {
@ -102,8 +104,17 @@ export const switchToEditMode = (identifier: VariableIdentifier): ThunkResult<vo
dispatch(setIdInEditor({ id: identifier.id }));
};
export const switchToListMode = (): ThunkResult<void> => (dispatch) => {
export const switchToListMode = (): ThunkResult<void> => (dispatch, getState) => {
dispatch(clearIdInEditor());
const state = getState();
const variables = getEditorVariables(state);
const dashboard = state.dashboard.getModel();
const { unknown, usages } = createUsagesNetwork(variables, dashboard);
const unknownsNetwork = transformUsagesToNetwork(unknown);
const unknownExits = Object.keys(unknown).length > 0;
const usagesNetwork = transformUsagesToNetwork(usages);
dispatch(initInspect({ unknown, usages, usagesNetwork, unknownsNetwork, unknownExits }));
};
export function getNextAvailableId(type: VariableType, variables: VariableModel[]): string {

View File

@ -1,33 +1,18 @@
import React, { FC, useMemo } from 'react';
import { Provider } from 'react-redux';
import { IconButton } from '@grafana/ui';
import { createUsagesNetwork, transformUsagesToNetwork } from './utils';
import { store } from '../../../store/store';
import { isAdHoc } from '../guard';
import { UsagesToNetwork } from './utils';
import { NetworkGraphModal } from './NetworkGraphModal';
import { VariableModel } from '../types';
import { DashboardModel } from '../../dashboard/state';
interface OwnProps {
variables: VariableModel[];
variable: VariableModel;
dashboard: DashboardModel | null;
interface Props {
id: string;
usages: UsagesToNetwork[];
isAdhoc: boolean;
}
interface ConnectedProps {}
interface DispatchProps {}
type Props = OwnProps & ConnectedProps & DispatchProps;
export const UnProvidedVariableUsagesGraphButton: FC<Props> = ({ variables, variable, dashboard }) => {
const { id } = variable;
const { usages } = useMemo(() => createUsagesNetwork(variables, dashboard), [variables, dashboard]);
const network = useMemo(() => transformUsagesToNetwork(usages).find((n) => n.variable.id === id), [usages, id]);
const adhoc = useMemo(() => isAdHoc(variable), [variable]);
if (usages.length === 0 || adhoc || !network) {
export const VariableUsagesButton: FC<Props> = ({ id, usages, isAdhoc }) => {
const network = useMemo(() => usages.find((n) => n.variable.id === id), [usages, id]);
if (usages.length === 0 || isAdhoc || !network) {
return null;
}
@ -46,9 +31,3 @@ export const UnProvidedVariableUsagesGraphButton: FC<Props> = ({ variables, vari
</NetworkGraphModal>
);
};
export const VariableUsagesButton: FC<Props> = (props) => (
<Provider store={store}>
<UnProvidedVariableUsagesGraphButton {...props} />
</Provider>
);

View File

@ -1,31 +1,17 @@
import React, { FC, useMemo } from 'react';
import { Provider } from 'react-redux';
import { IconButton } from '@grafana/ui';
import { createUsagesNetwork, transformUsagesToNetwork } from './utils';
import { store } from '../../../store/store';
import { VariableModel } from '../types';
import { DashboardModel } from '../../dashboard/state';
import { UsagesToNetwork } from './utils';
import { NetworkGraphModal } from './NetworkGraphModal';
interface OwnProps {
variable: VariableModel;
variables: VariableModel[];
dashboard: DashboardModel | null;
interface Props {
id: string;
usages: UsagesToNetwork[];
}
interface ConnectedProps {}
export const VariablesUnknownButton: FC<Props> = ({ id, usages }) => {
const network = useMemo(() => usages.find((n) => n.variable.id === id), [id, usages]);
interface DispatchProps {}
type Props = OwnProps & ConnectedProps & DispatchProps;
export const UnProvidedVariablesUnknownGraphButton: FC<Props> = ({ variable, variables, dashboard }) => {
const { id } = variable;
const { unknown } = useMemo(() => createUsagesNetwork(variables, dashboard), [variables, dashboard]);
const network = useMemo(() => transformUsagesToNetwork(unknown).find((n) => n.variable.id === id), [id, unknown]);
const unknownExist = useMemo(() => Object.keys(unknown).length > 0, [unknown]);
if (!unknownExist || !network) {
if (!network) {
return null;
}
@ -44,9 +30,3 @@ export const UnProvidedVariablesUnknownGraphButton: FC<Props> = ({ variable, var
</NetworkGraphModal>
);
};
export const VariablesUnknownButton: FC<Props> = (props) => (
<Provider store={store}>
<UnProvidedVariablesUnknownGraphButton {...props} />
</Provider>
);

View File

@ -1,35 +1,16 @@
import React, { FC, useMemo } from 'react';
import { Provider } from 'react-redux';
import React, { FC } from 'react';
import { css } from 'emotion';
import { Icon, Tooltip, useStyles } from '@grafana/ui';
import { GrafanaTheme } from '@grafana/data';
import { createUsagesNetwork, transformUsagesToNetwork } from './utils';
import { store } from '../../../store/store';
import { UsagesToNetwork } from './utils';
import { VariablesUnknownButton } from './VariablesUnknownButton';
import { VariableModel } from '../types';
import { DashboardModel } from '../../dashboard/state';
interface OwnProps {
variables: VariableModel[];
dashboard: DashboardModel | null;
interface Props {
usages: UsagesToNetwork[];
}
interface ConnectedProps {}
interface DispatchProps {}
type Props = OwnProps & ConnectedProps & DispatchProps;
export const UnProvidedVariablesUnknownTable: FC<Props> = ({ variables, dashboard }) => {
export const VariablesUnknownTable: FC<Props> = ({ usages }) => {
const style = useStyles(getStyles);
const { unknown } = useMemo(() => createUsagesNetwork(variables, dashboard), [variables, dashboard]);
const networks = useMemo(() => transformUsagesToNetwork(unknown), [unknown]);
const unknownExist = useMemo(() => Object.keys(unknown).length > 0, [unknown]);
if (!unknownExist) {
return null;
}
return (
<div className={style.container}>
<h5>
@ -48,8 +29,8 @@ export const UnProvidedVariablesUnknownTable: FC<Props> = ({ variables, dashboar
</tr>
</thead>
<tbody>
{networks.map((network) => {
const { variable } = network;
{usages.map((usage) => {
const { variable } = usage;
const { id, name } = variable;
return (
<tr key={id}>
@ -60,7 +41,7 @@ export const UnProvidedVariablesUnknownTable: FC<Props> = ({ variables, dashboar
<td className={style.defaultColumn} />
<td className={style.defaultColumn} />
<td className={style.lastColumn}>
<VariablesUnknownButton variable={variable} variables={variables} dashboard={dashboard} />
<VariablesUnknownButton id={variable.id} usages={usages} />
</td>
</tr>
);
@ -97,9 +78,3 @@ const getStyles = (theme: GrafanaTheme) => ({
text-align: right;
`,
});
export const VariablesUnknownTable: FC<Props> = (props) => (
<Provider store={store}>
<UnProvidedVariablesUnknownTable {...props} />
</Provider>
);

View File

@ -0,0 +1,29 @@
import { reducerTester } from '../../../../test/core/redux/reducerTester';
import { initialVariableInspectState, initInspect, variableInspectReducer } from './reducer';
import { textboxBuilder } from '../shared/testing/builders';
describe('variableInspectReducer', () => {
describe('when initInspect is dispatched', () => {
it('then state should be correct', () => {
const variable = textboxBuilder().withId('text').withName('text').build();
reducerTester()
.givenReducer(variableInspectReducer, { ...initialVariableInspectState })
.whenActionIsDispatched(
initInspect({
unknownExits: true,
unknownsNetwork: [{ edges: [], nodes: [], showGraph: true, variable }],
usagesNetwork: [{ edges: [], nodes: [], showGraph: true, variable }],
usages: [{ variable, tree: {} }],
unknown: [{ variable, tree: {} }],
})
)
.thenStateShouldEqual({
unknownExits: true,
unknownsNetwork: [{ edges: [], nodes: [], showGraph: true, variable }],
usagesNetwork: [{ edges: [], nodes: [], showGraph: true, variable }],
usages: [{ variable, tree: {} }],
unknown: [{ variable, tree: {} }],
});
});
});
});

View File

@ -0,0 +1,46 @@
import { UsagesToNetwork, VariableUsageTree } from './utils';
import { createSlice, PayloadAction } from '@reduxjs/toolkit';
export interface VariableInspectState {
unknown: VariableUsageTree[];
usages: VariableUsageTree[];
unknownsNetwork: UsagesToNetwork[];
usagesNetwork: UsagesToNetwork[];
unknownExits: boolean;
}
export const initialVariableInspectState: VariableInspectState = {
unknown: [],
usages: [],
unknownsNetwork: [],
usagesNetwork: [],
unknownExits: false,
};
const variableInspectReducerSlice = createSlice({
name: 'templating/inspect',
initialState: initialVariableInspectState,
reducers: {
initInspect: (
state,
action: PayloadAction<{
unknown: VariableUsageTree[];
usages: VariableUsageTree[];
unknownsNetwork: UsagesToNetwork[];
usagesNetwork: UsagesToNetwork[];
unknownExits: boolean;
}>
) => {
const { unknown, usages, unknownExits, unknownsNetwork, usagesNetwork } = action.payload;
state.usages = usages;
state.unknown = unknown;
state.unknownsNetwork = unknownsNetwork;
state.unknownExits = unknownExits;
state.usagesNetwork = usagesNetwork;
},
},
});
export const variableInspectReducer = variableInspectReducerSlice.reducer;
export const { initInspect } = variableInspectReducerSlice.actions;

View File

@ -44,4 +44,104 @@ describe('getPropsWithVariable', () => {
},
});
});
it('when called with a valid an id that is not part of valid names it should return the correct graph', () => {
const value = {
targets: [
{
id: 'A',
description: '$tag_host-[[tag_host]]',
query:
'SELECT mean(total) AS "total" FROM "disk" WHERE "host" =~ /$host$/ AND $timeFilter GROUP BY time($interval), "host", "path"',
alias: '$tag_host [[tag_host]] $col $host',
},
],
};
const result = getPropsWithVariable(
'host',
{
key: 'model',
value,
},
{}
);
expect(result).toEqual({
targets: {
A: {
alias: '$tag_host [[tag_host]] $col $host',
query:
'SELECT mean(total) AS "total" FROM "disk" WHERE "host" =~ /$host$/ AND $timeFilter GROUP BY time($interval), "host", "path"',
},
},
});
});
it('when called with an id that is part of valid alias names it should return the correct graph', () => {
const value = {
targets: [
{
id: 'A',
description: '[[tag_host1]]',
description2: '$tag_host1',
query:
'SELECT mean(total) AS "total" FROM "disk" WHERE "host" =~ /$host$/ AND $timeFilter GROUP BY time($interval), "host", "path"',
alias: '[[tag_host1]] $tag_host1 $col $host',
},
],
};
const tagHostResult = getPropsWithVariable(
'tag_host1',
{
key: 'model',
value,
},
{}
);
expect(tagHostResult).toEqual({
targets: {
A: {
description: '[[tag_host1]]',
description2: '$tag_host1',
},
},
});
});
it('when called with an id that is part of valid query names it should return the correct graph', () => {
const value = {
targets: [
{
id: 'A',
description: '[[timeFilter]]',
description2: '$timeFilter',
query:
'SELECT mean(total) AS "total" FROM "disk" WHERE "host" =~ /$host$/ AND $timeFilter GROUP BY time($interval), "host", "path"',
alias: '[[timeFilter]] $timeFilter $col $host',
},
],
};
const tagHostResult = getPropsWithVariable(
'timeFilter',
{
key: 'model',
value,
},
{}
);
expect(tagHostResult).toEqual({
targets: {
A: {
description: '[[timeFilter]]',
description2: '$timeFilter',
alias: '[[timeFilter]] $timeFilter $col $host',
},
},
});
});
});

View File

@ -65,6 +65,16 @@ export const toVisNetworkEdges = (edges: GraphEdge[]): any[] => {
return new vis.DataSet(edgesWithStyle);
};
function getVariableName(expression: string) {
variableRegex.lastIndex = 0;
const match = variableRegex.exec(expression);
if (!match) {
return null;
}
const variableName = match.slice(1).find((match) => match !== undefined);
return variableName;
}
export const getUnknownVariableStrings = (variables: VariableModel[], model: any) => {
const unknownVariableNames: string[] = [];
const modelAsString = safeStringifyValue(model, 2);
@ -89,7 +99,7 @@ export const getUnknownVariableStrings = (variables: VariableModel[], model: any
continue;
}
const variableName = match.slice(1);
const variableName = getVariableName(match);
if (variables.some((variable) => variable.id === variableName)) {
// ignore defined variables
@ -100,16 +110,32 @@ export const getUnknownVariableStrings = (variables: VariableModel[], model: any
continue;
}
unknownVariableNames.push(variableName);
if (variableName) {
unknownVariableNames.push(variableName);
}
}
return unknownVariableNames;
};
const validVariableNames: Record<string, RegExp[]> = {
alias: [/^m$/, /^measurement$/, /^col$/, /^tag_\w+|\d+$/],
query: [/^timeFilter$/],
};
export const getPropsWithVariable = (variableId: string, parent: { key: string; value: any }, result: any) => {
const stringValues = Object.keys(parent.value).reduce((all, key) => {
const value = parent.value[key];
if (value && typeof value === 'string' && containsVariable(value, variableId)) {
if (!value || typeof value !== 'string') {
return all;
}
const isValidName = validVariableNames[key]
? validVariableNames[key].find((regex: RegExp) => regex.test(variableId))
: undefined;
const hasVariable = containsVariable(value, variableId);
if (!isValidName && hasVariable) {
all = {
...all,
[key]: value,
@ -146,10 +172,15 @@ export const getPropsWithVariable = (variableId: string, parent: { key: string;
return result;
};
export interface VariableUsageTree {
variable: VariableModel;
tree: any;
}
export interface VariableUsages {
unUsed: VariableModel[];
unknown: Array<{ variable: VariableModel; tree: any }>;
usages: Array<{ variable: VariableModel; tree: any }>;
unknown: VariableUsageTree[];
usages: VariableUsageTree[];
}
export const createUsagesNetwork = (variables: VariableModel[], dashboard: DashboardModel | null): VariableUsages => {
@ -158,8 +189,8 @@ export const createUsagesNetwork = (variables: VariableModel[], dashboard: Dashb
}
const unUsed: VariableModel[] = [];
let usages: Array<{ variable: VariableModel; tree: any }> = [];
let unknown: Array<{ variable: VariableModel; tree: any }> = [];
let usages: VariableUsageTree[] = [];
let unknown: VariableUsageTree[] = [];
const model = dashboard.getSaveModelClone();
const unknownVariables = getUnknownVariableStrings(variables, model);
@ -220,7 +251,7 @@ export const traverseTree = (usage: UsagesToNetwork, parent: { id: string; value
return usage;
};
export const transformUsagesToNetwork = (usages: Array<{ variable: VariableModel; tree: any }>): UsagesToNetwork[] => {
export const transformUsagesToNetwork = (usages: VariableUsageTree[]): UsagesToNetwork[] => {
const results: UsagesToNetwork[] = [];
for (const usage of usages) {
@ -249,12 +280,7 @@ const countLeaves = (object: any): number => {
return (total as unknown) as number;
};
export const getVariableUsages = (
variableId: string,
variables: VariableModel[],
dashboard: DashboardModel | null
): number => {
const { usages } = createUsagesNetwork(variables, dashboard);
export const getVariableUsages = (variableId: string, usages: VariableUsageTree[]): number => {
const usage = usages.find((usage) => usage.variable.id === variableId);
if (!usage) {
return 0;

View File

@ -4,12 +4,14 @@ import { variableEditorReducer, VariableEditorState } from '../editor/reducer';
import { variablesReducer } from './variablesReducer';
import { VariableModel } from '../types';
import { transactionReducer, TransactionState } from './transactionReducer';
import { variableInspectReducer, VariableInspectState } from '../inspect/reducer';
export interface TemplatingState {
variables: Record<string, VariableModel>;
optionsPicker: OptionsPickerState;
editor: VariableEditorState;
transaction: TransactionState;
inspect: VariableInspectState;
}
export const templatingReducers = combineReducers({
@ -17,6 +19,7 @@ export const templatingReducers = combineReducers({
variables: variablesReducer,
optionsPicker: optionsPickerReducer,
transaction: transactionReducer,
inspect: variableInspectReducer,
});
export default {