Support multiple category fields #66
Conversation
…s in charts when cell selected
…g in HC preview component
…tmap; Fix bug of entity details wrong when filtering by indiv entity
…s in detector list
ohltyler
force-pushed
the
multi-category-rebased
branch
from
5f284f0 to
de4d6a9
Compare
|
All of the changes look good to me. Wait for you to resolve the PR build failure or add any new commit. |
Seems identical issue with the CI is happening in main now - likely due to the version bumps in AD backend plugin, and not related to the changes in this PR. Will work on a separate fix for that. |
|
@kaituo I've re-ran again and checks are passing now. No changes were made that I know of; could be flaky tests due to low resources in GitHub runners or changes in dependent packages. |
| @@ -215,9 +229,25 @@ export const AnomalyHeatmapChart = React.memo( | |||
| return false; | |||
| }; | |||
|
|
|||
| // Custom hook to refresh all of the heatmap data when running a historical task | |||
There was a problem hiding this comment.
Will it cause heavy load if query too much? Maybe we can increase interval between refresh query for HC?
There was a problem hiding this comment.
I think it's a fair point. I'm ok to increase the timeout - how about 30s?
There was a problem hiding this comment.
Sounds good, maybe confirm with UX design to check if this is reasonable and should we add some tooltip
There was a problem hiding this comment.
Sure, confirming with UX.
There was a problem hiding this comment.
Confirmed 30s is ok - no extra notices/wording is necessary.
| @@ -80,10 +83,19 @@ export const FeatureBreakDown = React.memo((props: FeatureBreakDownProps) => { | |||
| const filteredFeatureData = []; | |||
| for (let i = 0; i < anomaliesFound.length; i++) { | |||
| const currentAnomalyData = anomaliesResult.anomalies[i]; | |||
| const dataEntityList = get( | |||
There was a problem hiding this comment.
What's the difference between dataEntityList and cellEntityList?
There was a problem hiding this comment.
dataEntityList is the associated entity list with the current anomaly data in the for loop. cellEntityList is the entity list pulled from the selected heatmap cell. Note that no logic changes were made here. Instead, entityList used to be get(currentAnomalyData, 'entity', [] as EntityData[])[0].value and cellEntityList used to be props.selectedHeatmapCell.entityValue.
There was a problem hiding this comment.
cellEntityList is the entity list pulled from the selected heatmap cell
Does that mean user can choose multiple heatmap cells?
There was a problem hiding this comment.
No, only one can be selected at a time. But the cell itself can contain a list of entities if it is a multi-category detector
Signed-off-by: Tyler Ohlsen <[email protected]>
Signed-off-by: Tyler Ohlsen <[email protected]>
Description
Adds support for detectors to have multiple category fields. This is already supported in the backend plugin - see here. Supports both real-time & historical detectors.
Main changes include:
CategoryFieldcomponent to support multiple selections in the combo box (up to 2)1 - In the 'View by' title at the top of the anomalies chart
2 - On y-axis (each field delimited by a newline)
3 - When hovering over a heatmap cell. This involved adding a
customdatafield in the plot data in order to show anotherline of text (see here for more details)
4- In chart titles
5 - In anomaly table
AdditionalSettingsin detector config page to support showing multiple category fieldsNote that the bulk of these changes are related to changing the key of a single category field value that was used before, to making it more generalizable and supporting a list of category fields as a key. This way, we can easily support 3,4,5, etc. category fields without making code changes.
Some changes had to be made to the queries to fetch anomaly results for high-cardinality detectors. Similar to what's mentioned in the paragraph above, the queries were usually contingent on using a single category field as a key for fetching data, which will fetch incorrect results in the multi-category-field scenario. To handle this, several queries had to change:
getTopAnomalousEntitiesQuery(link): to support BWC, we still will fetch single-category-field results the same way as before, and perform a terms aggregation on a single category field. To support multi-category-field results, we use a new query that performs a terms aggregation on a newmodel_idfield, such that each unique set of category field values will be mapped to a single model id. This was introduced in the backend plugin as part of support for multi-category detectors.getAnomalySummaryQuery(link),getBucketizedAnomalyResultsQuery(link),getEntityAnomalySummariesQuery(link): similar togetTopAnomalousEntitiesQuery, for these functions, the filtering has been factored out into a helper functiongetEntityFilterswhich is used to determine if the results should be filtered out by a category field (if only one category field), or model id (if multiple category fields)getAnomalyResults(link), which is used for fetching the raw results when a heatmap cell is selected, now uses an optionally-passed-in entity list to dynamically generate filters for the category fields before making the call to OpenSearch. Note thatmodel_idis not used here, since we don't store/know the individual model ID when making the call. Also, with future plans to support results using a subset of category fields, this implementation leaves it more flexible for future changes.Screenshots:
Category field component, with wording changes indicating up to 2 can now be selected:
Entity list when hovering over heatmap cell:
Additional settings component:
Full results page (changes highlighted in red):
Check List
--signoffBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.