From 96758f0899e926f102c6feac5d970e8f377e3334 Mon Sep 17 00:00:00 2001 From: Andy Pickering Date: Fri, 21 Feb 2020 11:30:57 +0900 Subject: [PATCH 1/4] Monitoring Dashboards: Add `Board` type --- .../monitoring/dashboards/index.tsx | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/frontend/public/components/monitoring/dashboards/index.tsx b/frontend/public/components/monitoring/dashboards/index.tsx index 78de4e3d86a..9707f7b5abb 100644 --- a/frontend/public/components/monitoring/dashboards/index.tsx +++ b/frontend/public/components/monitoring/dashboards/index.tsx @@ -379,7 +379,7 @@ const MonitoringDashboardsPage_: React.FC = ({ patchVariable, }) => { const [board, setBoard] = React.useState(); - const [boards, setBoards] = React.useState([]); + const [boards, setBoards] = React.useState([]); const [error, setError] = React.useState(); const [isLoading, , , setLoaded] = useBoolean(true); @@ -394,7 +394,7 @@ const MonitoringDashboardsPage_: React.FC = ({ setLoaded(); setError(undefined); - const getBoardData = (item) => ({ + const getBoardData = (item): Board => ({ data: JSON.parse(_.values(item?.data)[0]), name: item.metadata.name, }); @@ -421,7 +421,7 @@ const MonitoringDashboardsPage_: React.FC = ({ const data = _.find(boards, { name: newBoard })?.data; - _.each(data?.templating?.list as TemplateVariable[], (v) => { + _.each(data?.templating?.list, (v) => { if (v.type === 'query' || v.type === 'interval') { patchVariable(v.name, { isHidden: v.hide !== 0, @@ -491,6 +491,22 @@ type TemplateVariable = { type: string; }; +type Row = { + panels: Panel[]; +}; + +type Board = { + data: { + panels: Panel[]; + rows: Row[]; + templating: { + list: TemplateVariable[]; + }; + title: string; + }; + name: string; +}; + type Variable = { isHidden?: boolean; isLoading?: boolean; @@ -521,7 +537,7 @@ type SingleVariableDropdownProps = { }; type BoardProps = { - rows: Panel[]; + rows: Row[]; }; type AllVariableDropdownsProps = { From 8c6eccfec192aa4a16a4eb80464f75c4e0cd3571 Mon Sep 17 00:00:00 2001 From: Andy Pickering Date: Fri, 21 Feb 2020 12:53:13 +0900 Subject: [PATCH 2/4] Monitoring Dashboards: Fix Refresh Interval when `Off` is selected The `Off` text was not being displayed. --- frontend/public/components/monitoring/dashboards/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/public/components/monitoring/dashboards/index.tsx b/frontend/public/components/monitoring/dashboards/index.tsx index 9707f7b5abb..6b292845b4a 100644 --- a/frontend/public/components/monitoring/dashboards/index.tsx +++ b/frontend/public/components/monitoring/dashboards/index.tsx @@ -254,7 +254,7 @@ const PollIntervalDropdown_: React.FC = ({ items={pollIntervalOptions} label="Refresh Interval" onChange={onChange} - selectedKey={formatPrometheusDuration(pollInterval)} + selectedKey={pollInterval === null ? pollOffText : formatPrometheusDuration(pollInterval)} /> ); }; From 2b39800944955b7e43d9994307802ecce9e4b209 Mon Sep 17 00:00:00 2001 From: Andy Pickering Date: Fri, 21 Feb 2020 11:53:58 +0900 Subject: [PATCH 3/4] Monitoring Dashboards: Don't change state during render & performance Call `changeBoard()` from `useEffect` hook instead of during render. This is a bug fix since `changeBoard()` changes the component state, so it shouldn't be called during render. Speed up dashboard switching by - Adding a Redux action for updating all dashboard variables at once - Setting the `isLoading` flag to `true` immediately for `query` variables, which prevents that flag being quickly switched from `false` to `true` and triggering unnecessary renders --- frontend/public/actions/ui.ts | 4 ++ .../monitoring/dashboards/index.tsx | 66 ++++++++++--------- frontend/public/reducers/ui.ts | 6 ++ 3 files changed, 45 insertions(+), 31 deletions(-) diff --git a/frontend/public/actions/ui.ts b/frontend/public/actions/ui.ts index 24f4a75f876..7fc75cb4476 100644 --- a/frontend/public/actions/ui.ts +++ b/frontend/public/actions/ui.ts @@ -28,6 +28,7 @@ export enum ActionType { SetCurrentLocation = 'setCurrentLocation', MonitoringDashboardsClearVariables = 'monitoringDashboardsClearVariables', MonitoringDashboardsPatchVariable = 'monitoringDashboardsPatchVariable', + MonitoringDashboardsPatchAllVariables = 'monitoringDashboardsPatchAllVariables', MonitoringDashboardsSetPollInterval = 'monitoringDashboardsSetPollInterval', MonitoringDashboardsSetTimespan = 'monitoringDashboardsSetTimespan', MonitoringDashboardsVariableOptionsLoaded = 'monitoringDashboardsVariableOptionsLoaded', @@ -269,6 +270,8 @@ export const monitoringDashboardsClearVariables = () => action(ActionType.MonitoringDashboardsClearVariables); export const monitoringDashboardsPatchVariable = (key: string, patch: any) => action(ActionType.MonitoringDashboardsPatchVariable, { key, patch }); +export const monitoringDashboardsPatchAllVariables = (variables: any) => + action(ActionType.MonitoringDashboardsPatchAllVariables, { variables }); export const monitoringDashboardsSetPollInterval = (pollInterval: number) => action(ActionType.MonitoringDashboardsSetPollInterval, { pollInterval }); export const monitoringDashboardsSetTimespan = (timespan: number) => @@ -346,6 +349,7 @@ const uiActions = { updateOverviewFilterValue, monitoringDashboardsClearVariables, monitoringDashboardsPatchVariable, + monitoringDashboardsPatchAllVariables, monitoringDashboardsSetPollInterval, monitoringDashboardsSetTimespan, monitoringDashboardsVariableOptionsLoaded, diff --git a/frontend/public/components/monitoring/dashboards/index.tsx b/frontend/public/components/monitoring/dashboards/index.tsx index 6b292845b4a..31f81dd10b2 100644 --- a/frontend/public/components/monitoring/dashboards/index.tsx +++ b/frontend/public/components/monitoring/dashboards/index.tsx @@ -180,7 +180,7 @@ const SingleVariableDropdown = connect( const AllVariableDropdowns_: React.FC = ({ variables }) => ( <> - {_.map(_.keys(variables.toJS()), (name) => ( + {variables.keySeq().map((name) => ( ))} @@ -373,10 +373,9 @@ const Board: React.FC = ({ rows }) => ( ); const MonitoringDashboardsPage_: React.FC = ({ - clearVariables, deleteAll, match, - patchVariable, + patchAllVariables, }) => { const [board, setBoard] = React.useState(); const [boards, setBoards] = React.useState([]); @@ -415,31 +414,38 @@ const MonitoringDashboardsPage_: React.FC = ({ boards, ]); - const changeBoard = (newBoard: string) => { - if (newBoard !== board) { - clearVariables(); - - const data = _.find(boards, { name: newBoard })?.data; + const changeBoard = React.useCallback( + (newBoard: string) => { + if (newBoard !== board) { + const data = _.find(boards, { name: newBoard })?.data; + + const allVariables = {}; + _.each(data?.templating?.list, (v) => { + if (v.type === 'query' || v.type === 'interval') { + allVariables[v.name] = ImmutableMap({ + isHidden: v.hide !== 0, + isLoading: v.type === 'query', + options: _.map(v.options, 'value'), + query: v.type === 'query' ? v.query : undefined, + value: _.find(v.options, { selected: true })?.value || v.options?.[0]?.value, + }); + } + }); + patchAllVariables(allVariables); - _.each(data?.templating?.list, (v) => { - if (v.type === 'query' || v.type === 'interval') { - patchVariable(v.name, { - isHidden: v.hide !== 0, - options: _.map(v.options, 'value'), - query: v.type === 'query' ? v.query : undefined, - value: _.find(v.options, { selected: true })?.value || v.options?.[0]?.value, - }); - } - }); + setBoard(newBoard); + history.replace(`/monitoring/dashboards/${newBoard}`); + } + }, + [board, boards, patchAllVariables], + ); - setBoard(newBoard); - history.replace(`/monitoring/dashboards/${newBoard}`); + // Default to displaying the first board + React.useEffect(() => { + if (!board && !_.isEmpty(boards)) { + changeBoard(match.params.board || boards?.[0]?.name); } - }; - - if (!board && !_.isEmpty(boards)) { - changeBoard(match.params.board || boards?.[0]?.name); - } + }, [board, boards, changeBoard, match.params.board]); if (error) { return ; @@ -478,9 +484,8 @@ const MonitoringDashboardsPage_: React.FC = ({ ); }; const MonitoringDashboardsPage = connect(null, { - clearVariables: UIActions.monitoringDashboardsClearVariables, deleteAll: UIActions.queryBrowserDeleteAllQueries, - patchVariable: UIActions.monitoringDashboardsPatchVariable, + patchAllVariables: UIActions.monitoringDashboardsPatchAllVariables, })(MonitoringDashboardsPage_); type TemplateVariable = { @@ -541,7 +546,7 @@ type BoardProps = { }; type AllVariableDropdownsProps = { - variables: ImmutableMap; + variables: ImmutableMap>; }; type TimespanDropdownProps = { @@ -557,7 +562,7 @@ type PollIntervalDropdownProps = { type CardBodyProps = { panel: Panel; pollInterval: null | number; - variables: ImmutableMap; + variables: ImmutableMap>; }; type CardProps = { @@ -565,12 +570,11 @@ type CardProps = { }; type MonitoringDashboardsPageProps = { - clearVariables: () => undefined; deleteAll: () => undefined; match: { params: { board: string }; }; - patchVariable: (key: string, patch: Variable) => undefined; + patchAllVariables: (variables: VariablesMap) => undefined; }; export default withFallback(MonitoringDashboardsPage, ErrorBoundaryFallback); diff --git a/frontend/public/reducers/ui.ts b/frontend/public/reducers/ui.ts index f6fc4c84ae7..0cce3c8ea72 100644 --- a/frontend/public/reducers/ui.ts +++ b/frontend/public/reducers/ui.ts @@ -150,6 +150,12 @@ export default (state: UIState, action: UIAction): UIState => { ImmutableMap(action.payload.patch), ); + case ActionType.MonitoringDashboardsPatchAllVariables: + return state.setIn( + ['monitoringDashboards', 'variables'], + ImmutableMap(action.payload.variables), + ); + case ActionType.MonitoringDashboardsSetPollInterval: return state.setIn(['monitoringDashboards', 'pollInterval'], action.payload.pollInterval); From 97ffadeab40e622eb9b651c450db2ab190da5d9a Mon Sep 17 00:00:00 2001 From: Andy Pickering Date: Tue, 25 Feb 2020 14:34:03 +0900 Subject: [PATCH 4/4] Monitoring Dashboards: Remove broken bar chart links The bar chart panels were linking to the Query Browser page, but the links were broken. Fixed by removing the links since the other panels are not links and we should be consistent. --- frontend/public/components/graphs/bar.tsx | 45 ++++++++++--------- .../monitoring/dashboards/bar-chart.tsx | 9 +++- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/frontend/public/components/graphs/bar.tsx b/frontend/public/components/graphs/bar.tsx index 54dd6dfd91e..7c4f7899efe 100644 --- a/frontend/public/components/graphs/bar.tsx +++ b/frontend/public/components/graphs/bar.tsx @@ -23,13 +23,14 @@ const PADDING_RATIO = 1 / 3; export const BarChart: React.FC = ({ barSpacing = 15, barWidth = DEFAULT_BAR_WIDTH, - title, - query, data = [], + isLink = true, + LabelComponent, + loading = false, + query, theme = getCustomTheme(ChartThemeColor.blue, ChartThemeVariant.light, barTheme), + title, titleClassName, - loading = false, - LabelComponent, }) => { const [containerRef, width] = useRefWidth(); @@ -46,7 +47,7 @@ export const BarChart: React.FC = ({ return ( {data.length ? ( - + {data.map((datum, index) => (
@@ -80,16 +81,17 @@ export const BarChart: React.FC = ({ }; export const Bar: React.FC = ({ + barSpacing, + barWidth, delay = undefined, humanize = humanizeNumber, + isLink = true, + LabelComponent, metric, namespace, - barSpacing, - barWidth, - theme, query, + theme, title, - LabelComponent, }) => { const [response, , loading] = usePrometheusPoll({ delay, @@ -101,14 +103,15 @@ export const Bar: React.FC = ({ return ( ); }; @@ -119,25 +122,27 @@ type LabelComponentProps = { }; type BarChartProps = { - LabelComponent?: React.ComponentType; barSpacing?: number; barWidth?: number; + data?: DataPoint[]; + isLink?: boolean; + LabelComponent?: React.ComponentType; + loading?: boolean; query?: string; theme?: any; // TODO figure out the best way to import VictoryThemeDefinition title?: string; - data?: DataPoint[]; titleClassName?: string; - loading?: boolean; }; type BarProps = { - LabelComponent?: React.ComponentType; + barSpacing?: number; + barWidth?: number; delay?: number; humanize?: Humanize; + isLink?: boolean; + LabelComponent?: React.ComponentType; metric: string; namespace?: string; - barSpacing?: number; - barWidth?: number; query: string; theme?: any; // TODO figure out the best way to import VictoryThemeDefinition title?: string; diff --git a/frontend/public/components/monitoring/dashboards/bar-chart.tsx b/frontend/public/components/monitoring/dashboards/bar-chart.tsx index ed5122e3701..6b9d5fa5942 100644 --- a/frontend/public/components/monitoring/dashboards/bar-chart.tsx +++ b/frontend/public/components/monitoring/dashboards/bar-chart.tsx @@ -6,7 +6,14 @@ import { Bar } from '../../graphs'; const Label = ({ metric }) => <>{_.values(metric).join()}; const BarChart: React.FC = ({ pollInterval, query }) => ( - + ); type BarChartProps = {