Refactor AnomalyHistory Chart to improve performance for HC detector #350
Conversation
yizheliu-amazon
added
the
enhancement
| AnomalyHeatmapSortType.SEVERITY, | ||
| COMBINED_OPTIONS.options[0].value | ||
| ) | ||
| props.showAlerts |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Issue #, if available:
#313
Description of changes:
Currently, AnomalyHistory chart loads all anomaly only data and pass it to heatmap chart to do the filtering/sorting for rendering anomaly summaries data on heatmap. The downside of it is 1) we have to manually aggregate/sort raw anomaly data, and it can cause performance issue if there are too many anomalies in specified date range; 2) we miss the feature data of anomaly grade 0 data points, such that it has less friendly UX compared to all feature data is rendered
This PR changes to 1) find anomalous entities via ES query; 2) get anomaly summaries for each entities via ES query;3) get anomaly/feature data only when heatmap cell is selected.
This moves the heavy aggregation/sorting work to ES from front-end, and has better UX with all feature data is rendered.
After the change:
Preview: same as before
DetectorResult page: anomaly data points with anomaly grade 0 is also included
DetectorResult page: feature data points with anomaly grade 0 is also included
UT will be added in following commit.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.