Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Refactor AnomalyHistory Chart to improve performance for HC detector #350

Merged
merged 8 commits into from
Dec 31, 2020
Merged

Refactor AnomalyHistory Chart to improve performance for HC detector #350

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f7dcc40
Use anomaly summaries instead of raw anomalies for heatmap
yizheliu-amazon Dec 17, 2020
98e3fed
Fetch anomaly result data only when heatmap cell is selected
yizheliu-amazon Dec 18, 2020
2ea4946
fix a few minor bugs with view option change
yizheliu-amazon Dec 21, 2020
15d18e4
Address comments
yizheliu-amazon Dec 22, 2020
f8d83ad
Make isNotSample optional for heatmap chart
yizheliu-amazon Dec 22, 2020
314e25a
Add UT for HC related charts
yizheliu-amazon Dec 25, 2020
0dd71b5
Add comment to explain the hasValidHCProps() method
yizheliu-amazon Dec 29, 2020
f9b7217
Addressed comments
yizheliu-amazon Dec 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion public/pages/AnomalyCharts/containers/AnomaliesChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
import { get } from 'lodash';
import moment, { DurationInputArg2 } from 'moment';
import React, { useState } from 'react';
import { EntityAnomalySummaries } from '../../../../server/models/interfaces';
import ContentPanel from '../../../components/ContentPanel/ContentPanel';
import { useDelayedLoader } from '../../../hooks/useDelayedLoader';
import {
Expand All @@ -38,6 +39,7 @@ import { AnomalyDetailsChart } from '../containers/AnomalyDetailsChart';
import {
AnomalyHeatmapChart,
HeatmapCell,
HeatmapDisplayOption,
} from '../containers/AnomalyHeatmapChart';
import {
getAnomalyGradeWording,
Expand Down Expand Up @@ -71,10 +73,13 @@ interface AnomaliesChartProps {
isHCDetector?: boolean;
detectorCategoryField?: string[];
onHeatmapCellSelected?(heatmapCell: HeatmapCell): void;
onDisplayOptionChanged?(heatmapDisplayOption: HeatmapDisplayOption): void;
selectedHeatmapCell?: HeatmapCell;
newDetector?: Detector;
zoomRange?: DateRange;
anomaliesResult: Anomalies | undefined;
heatmapDisplayOption?: HeatmapDisplayOption;
entityAnomalySummaries?: EntityAnomalySummaries[];
}

export const AnomaliesChart = React.memo((props: AnomaliesChartProps) => {
Expand Down Expand Up @@ -183,7 +188,11 @@ export const AnomaliesChart = React.memo((props: AnomaliesChartProps) => {
<EuiFlexGroup direction="column">
{props.isHCDetector &&
props.onHeatmapCellSelected &&
props.detectorCategoryField ? (
props.detectorCategoryField &&
(props.showAlerts !== true ||
(props.showAlerts &&
props.onDisplayOptionChanged &&
props.entityAnomalySummaries)) ? (
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
<EuiFlexGroup style={{ padding: '20px' }}>
<EuiFlexItem style={{ margin: '0px' }}>
<div
Expand Down Expand Up @@ -222,6 +231,9 @@ export const AnomaliesChart = React.memo((props: AnomaliesChartProps) => {
'detectionInterval.period.unit'
)}
onHeatmapCellSelected={props.onHeatmapCellSelected}
entityAnomalySummaries={props.entityAnomalySummaries}
onDisplayOptionChanged={props.onDisplayOptionChanged}
heatmapDisplayOption={props.heatmapDisplayOption}
/>,
props.showAlerts !== true
? [
Expand Down
164 changes: 118 additions & 46 deletions public/pages/AnomalyCharts/containers/AnomalyHeatmapChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import React, { useState } from 'react';
import moment from 'moment';
import Plotly, { PlotData } from 'plotly.js-dist';
import plotComponentFactory from 'react-plotly.js/factory';
import { get, isEmpty } from 'lodash';
import { get, isEmpty, uniq } from 'lodash';
import {
EuiFlexItem,
EuiFlexGroup,
Expand All @@ -41,8 +41,10 @@ import {
AnomalyHeatmapSortType,
sortHeatmapPlotData,
filterHeatmapPlotDataByY,
getEnitytAnomaliesHeatmapData,
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
} from '../utils/anomalyChartUtils';
import { MIN_IN_MILLI_SECS } from '../../../../server/utils/constants';
import { EntityAnomalySummaries } from '../../../../server/models/interfaces';

interface AnomalyHeatmapChartProps {
title: string;
Expand All @@ -56,13 +58,35 @@ interface AnomalyHeatmapChartProps {
detectorInterval?: number;
unit?: string;
onHeatmapCellSelected(cell: HeatmapCell | undefined): void;
onDisplayOptionChanged?(option: HeatmapDisplayOption | undefined): void;
heatmapDisplayOption?: HeatmapDisplayOption;
entityAnomalySummaries?: EntityAnomalySummaries[];
}

export interface HeatmapCell {
dateRange: DateRange;
entityValue: string;
}

export interface HeatmapDisplayOption {
sortType: AnomalyHeatmapSortType;
entityOption: { label: string; value: number };
}

const COMBINED_OPTIONS = {
label: 'Combined options',
options: [
{ label: 'Top 10', value: 10 },
{ label: 'Top 20', value: 20 },
{ label: 'Top 30', value: 30 },
],
};

export const INITIAL_HEATMAP_DISPLAY_OPTION = {
sortType: AnomalyHeatmapSortType.SEVERITY,
entityOption: COMBINED_OPTIONS.options[0],
} as HeatmapDisplayOption;

export const AnomalyHeatmapChart = React.memo(
(props: AnomalyHeatmapChartProps) => {
const showLoader = useDelayedLoader(props.isLoading);
Expand All @@ -80,15 +104,6 @@ export const AnomalyHeatmapChart = React.memo(
},
];

const COMBINED_OPTIONS = {
label: 'Combined options',
options: [
{ label: 'Top 10', value: 10 },
{ label: 'Top 20', value: 20 },
{ label: 'Top 30', value: 30 },
],
};

const PlotComponent = plotComponentFactory(Plotly);

const getViewEntityOptions = (inputHeatmapData: PlotData[]) => {
Expand All @@ -109,28 +124,57 @@ export const AnomalyHeatmapChart = React.memo(
});

return [
COMBINED_OPTIONS,
getCombindeOptions(
COMBINED_OPTIONS,
props.heatmapDisplayOption?.entityOption
),
{
label: 'Individual entities',
options: individualEntityOptions,
options: individualEntityOptions.reverse(),
kaituo marked this conversation as resolved.
Show resolved Hide resolved
},
];
};

const getCombindeOptions = (
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
existingOptions: any,
selectedCombinedOption: any | undefined
) => {
if (!selectedCombinedOption) {
return existingOptions;
}
return {
label: existingOptions.label,
options: uniq([selectedCombinedOption, ...existingOptions.options]),
kaituo marked this conversation as resolved.
Show resolved Hide resolved
};
};

const [originalHeatmapData, setOriginalHeatmapData] = useState(
getAnomaliesHeatmapData(
props.anomalies,
props.dateRange,
AnomalyHeatmapSortType.SEVERITY,
COMBINED_OPTIONS.options[0].value
)
props.showAlerts
Copy link
Contributor

@ohltyler ohltyler Dec 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add another prop to determine if we are using actual vs. sample results, rather than reusing the showAlerts one? One reason for this is that those may not always coincide with each other. For example, with historical detector results, props.showAlerts is false since alerts don't apply, but it is also not sample results.

I'm reusing the AnomalyHistory component in the historical detector results page, and have actually added a prop to handle this (as well as for all of the necessary children components) here. I'm wondering if you can maybe do something similar here. Will help with possible merge conflicts down the road as well. Let me know what you think.

Copy link
Contributor Author

@yizheliu-amazon yizheliu-amazon Dec 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was thinking about the same thing. Sure. I can add same prop as you did, isNotSample. For my PR, I will have isNotSample in Heatmap Chart Props, but keep using props.showAlert for input of heatmap chart so that we can have as less conflict as possible. I will change to use isNotSample once your change is merged.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, sounds good.

? // use anomaly summary data in case of realtime result
getEnitytAnomaliesHeatmapData(
props.dateRange,
props.entityAnomalySummaries,
props.heatmapDisplayOption.entityOption.value
)
: // use anomalies data in case of sample result
getAnomaliesHeatmapData(
props.anomalies,
kaituo marked this conversation as resolved.
Show resolved Hide resolved
props.dateRange,
AnomalyHeatmapSortType.SEVERITY,
COMBINED_OPTIONS.options[0].value
)
);

const [heatmapData, setHeatmapData] = useState<PlotData[]>(
originalHeatmapData
);

const [sortByFieldValue, setSortByFieldValue] = useState(
SORT_BY_FIELD_OPTIONS[0].value
const [sortByFieldValue, setSortByFieldValue] = useState<
AnomalyHeatmapSortType
>(
props.showAlerts
? props.heatmapDisplayOption.sortType
: SORT_BY_FIELD_OPTIONS[0].value
);

const [currentViewOptions, setCurrentViewOptions] = useState([
Expand Down Expand Up @@ -193,14 +237,14 @@ export const AnomalyHeatmapChart = React.memo(
);
setHeatmapData([transparentHeatmapData, ...selectedHeatmapData]);

const selectedEndDate = moment(
const selectedStartDate = moment(
//@ts-ignore
heatmapData[0].x[selectedCellIndices[1]],
HEATMAP_X_AXIS_DATE_FORMAT
).valueOf();

const selectedStartDate =
selectedEndDate -
const selectedEndDate =
selectedStartDate +
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
get(selectedHeatmapData, '[0].cellTimeInterval', MIN_IN_MILLI_SECS);
props.onHeatmapCellSelected({
dateRange: {
Expand Down Expand Up @@ -228,18 +272,25 @@ export const AnomalyHeatmapChart = React.memo(
if (isEmpty(selectedViewOptions)) {
// when `clear` is hit for combo box
setCurrentViewOptions([COMBINED_OPTIONS.options[0]]);
const displayTopEntityNum = get(COMBINED_OPTIONS.options[0], 'value');

const updateHeatmapPlotData = getAnomaliesHeatmapData(
props.anomalies,
props.dateRange,
sortByFieldValue,
displayTopEntityNum
);
setOriginalHeatmapData(updateHeatmapPlotData);
setHeatmapData(updateHeatmapPlotData);
setNumEntities(updateHeatmapPlotData[0].y.length);
setEntityViewOptions(getViewEntityOptions(updateHeatmapPlotData));
if (props.showAlerts && props.onDisplayOptionChanged) {
props.onDisplayOptionChanged({
sortType: sortByFieldValue,
entityOption: COMBINED_OPTIONS.options[0],
});
} else {
const displayTopEntityNum = get(COMBINED_OPTIONS.options[0], 'value');
const updateHeatmapPlotData = getAnomaliesHeatmapData(
props.anomalies,
props.dateRange,
sortByFieldValue,
displayTopEntityNum
);
setOriginalHeatmapData(updateHeatmapPlotData);
setHeatmapData(updateHeatmapPlotData);
setNumEntities(updateHeatmapPlotData[0].y.length);
setEntityViewOptions(getViewEntityOptions(updateHeatmapPlotData));
}
return;
}
const nonCombinedOptions = [] as any[];
Expand All @@ -253,17 +304,26 @@ export const AnomalyHeatmapChart = React.memo(
if (isCombinedViewEntityOption(option)) {
// only allow 1 combined option
setCurrentViewOptions([option]);
const displayTopEntityNum = get(option, 'value');
const updateHeatmapPlotData = getAnomaliesHeatmapData(
props.anomalies,
props.dateRange,
sortByFieldValue,
displayTopEntityNum
);
setOriginalHeatmapData(updateHeatmapPlotData);
setHeatmapData(updateHeatmapPlotData);
setNumEntities(updateHeatmapPlotData[0].y.length);
setEntityViewOptions(getViewEntityOptions(updateHeatmapPlotData));
if (props.showAlerts && props.onDisplayOptionChanged) {
props.onDisplayOptionChanged({
sortType: sortByFieldValue,
entityOption: option,
});
} else {
const displayTopEntityNum = get(option, 'value');
const updateHeatmapPlotData = getAnomaliesHeatmapData(
props.anomalies,
props.dateRange,
sortByFieldValue,
displayTopEntityNum
);

setOriginalHeatmapData(updateHeatmapPlotData);
setHeatmapData(updateHeatmapPlotData);
setNumEntities(updateHeatmapPlotData[0].y.length);
setEntityViewOptions(getViewEntityOptions(updateHeatmapPlotData));
}

return;
} else {
nonCombinedOptions.push(option);
Expand Down Expand Up @@ -296,6 +356,19 @@ export const AnomalyHeatmapChart = React.memo(

const handleSortByFieldChange = (value: any) => {
setSortByFieldValue(value);
props.onHeatmapCellSelected(undefined);
if (
props.showAlerts &&
props.onDisplayOptionChanged &&
currentViewOptions.length === 1 &&
isCombinedViewEntityOption(currentViewOptions[0])
) {
props.onDisplayOptionChanged({
sortType: value,
entityOption: currentViewOptions[0],
});
return;
}
const sortedHeatmapData = sortHeatmapPlotData(
heatmapData[0],
value,
Expand All @@ -305,7 +378,6 @@ export const AnomalyHeatmapChart = React.memo(
opacity: 1,
});
setHeatmapData([updatedHeatmapData]);
props.onHeatmapCellSelected(undefined);
};

return (
Expand Down
Loading