Add other changes related to historical workbench #352

ohltyler · 2020-12-22T20:53:39Z

Issue #, if available:

Description of changes:

This PR is 4 of 4 related to adding the historical detectors to the plugin. Once all 4 PRs are merged into the development branch, there will be a follow up PR to merge the development branch into master.

This PR adds several changes to existing pages, server-side code, redux store, and data models to accommodate the addition of the historical detector workbench.

Changes include:

Filtering out historical detectors on existing real-time pages (dashboard and detector list)
Adding side nav hierarchy and the additional historical detector list page
Wording changes proposed by the tech writer that affected both real-time and historical portions of the plugin
Minor formatting changes proposed by UX reviewers that affected both real-time and historical portions of the plugin
Updating and adding some data models
Adding an isNotSample prop and historical-detector-related props to various chart components to differentiate between real-time results, sample results, and historical results
Adds historical detector list in redux store and various functions to retrieve them
Adds some checks in getDetector() server function to retrieve task-related info if the detector is historical
Adds a Finished detector state that is only applicable to historical detectors
Adds and removes some stats in the anomaly history chart, also removes the confidence line chart entirely for the historical detector case

Screenshot of updated anomaly history chart for the historical case.

Removes confidence and last anomaly occurrence fields,
Removes confidence in line chart,
Adds avg anomaly grade stat

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

ylwu-amzn · 2020-12-23T01:33:19Z

public/pages/DetectorResults/containers/AnomalyHistory.tsx

@@ -343,7 +370,7 @@ export const AnomalyHistory = (props: AnomalyHistoryProps) => {
        {isLoading || isLoadingAnomalyResults ? (
          <EuiFlexGroup
            justifyContent="spaceAround"
-            style={{ height: '200px', paddingTop: '100px' }}
+            style={{ height: '500px', paddingTop: '100px' }}


Why change to 500px here ? Will it impact realtime detector?

This change was to prevent the page to look as jerky when in the loading state, since it would often cause the screen to add & remove the scrollbar. It does affect the realtime detector, but improves that as well since it is just an aesthetic improvement.

Got it, will it impact the realtime detector's AD result chart?

Yes, please see above comment. It's an improvement for both real-time and historical.

ylwu-amzn · 2020-12-23T01:35:29Z

public/utils/utils.tsx

+  if (typeof timestamp === 'string') {
+    return timestamp;
+  }
+  return moment(timestamp).format();


Should we add date format here ? Like "MM/DD/YYYY hh:mm A"

Good catch, will add the enforced output format.

Fixed in latest commit.

public/utils/utils.tsx

server/routes/ad.ts

server/routes/utils/adHelpers.ts

yizheliu-amazon · 2020-12-23T20:00:26Z

public/models/interfaces.ts

@@ -184,7 +197,20 @@ export type AnomalySummary = {
  lastAnomalyOccurrence: string;
 };

+export type HistoricalDetectorAnomalySummary = {


Is this being used? Looks like AnomalySummary can be reused.

This is a good catch. And actually, the reason for having a separate data model here is because the lastAnomalyOccurrence field is not applicable for the historical detector case, and is not part of the mockups.

Looks like currently, the HistoricalDetectorAnomalySummary type is not being used properly in the AnomalyHistory component. I will need to add a check to show the appropriate summary based on the detector type.

After looking more, seems it will be easier to not add a new HistoricalDetectorAnomalySummary here at all like you suggested. I've synced up with UX and for the historical detector case, the last anomaly occurrence and the confidence fields are actually removed, and an average anomaly grade field is added. Will adjust accordingly and remove the unnecessary HistoricalDetectorAnomalySummary type.

Per the mockup, have also removed the confidence showing on the chart entirely for the historical detector case. I've added a screenshot showing the changes.

yizheliu-amazon · 2020-12-23T20:02:09Z

public/models/interfaces.ts

 export type DateRange = {
  startDate: number;
  endDate: number;
 };
+
+export type DetectionDateRange = {


I think re-using DateRange should be sufficient.

The reason for adding this DetectionDateRange is because the fields understood by the backend take startTime and endTime fields instead of startDate and endDate fields used by the DateRange. I didn't want to add more converting when parsing existing detectors or creating/updating new detectors. Let me know what you think.

ylwu-amzn · 2020-12-28T18:30:07Z

server/routes/ad.ts

-            default_operator: 'OR',
-            query: `*${indices.trim().split(' ').join('* *')}*`,
+            default_operator: 'AND',
+            query: `*${indices.trim().split('-').join('* *')}*`,


Will it work just splitting by "-" ? Have you tested with " "?

Yes, this is another part of the bug that needed to be fixed. Note that the search field also splits by - in order to properly search for detectors with a - in them. A similar fix needs to be done for detectors that are using indices with - in them. See here for more details.

ohltyler · 2020-12-28T20:39:33Z

Regarding the changes for the anomaly history chart, please see the added screenshot in the description and the latest commit.

Add other changes related to workbench

bd46a0c

ohltyler requested review from yizheliu-amazon and ylwu-amzn December 22, 2020 20:53

ylwu-amzn reviewed Dec 23, 2020

View reviewed changes

public/utils/utils.tsx Show resolved Hide resolved

ylwu-amzn reviewed Dec 23, 2020

View reviewed changes

server/routes/ad.ts Show resolved Hide resolved

ylwu-amzn reviewed Dec 23, 2020

View reviewed changes

server/routes/ad.ts Show resolved Hide resolved

ylwu-amzn reviewed Dec 23, 2020

View reviewed changes

server/routes/utils/adHelpers.ts Show resolved Hide resolved

yizheliu-amazon reviewed Dec 23, 2020

View reviewed changes

ylwu-amzn reviewed Dec 28, 2020

View reviewed changes

Remove confidence from anomaly results chart and add avg grade stat

f358a26

yizheliu-amazon approved these changes Dec 29, 2020

View reviewed changes

ylwu-amzn approved these changes Jan 5, 2021

View reviewed changes

ohltyler merged commit aa20f09 into opendistro-for-elasticsearch:historical-workbench-dev Jan 5, 2021

ohltyler added the feature new feature label Jan 21, 2021

ohltyler mentioned this pull request Jan 22, 2021

Add historical detectors #359

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add other changes related to historical workbench #352

Add other changes related to historical workbench #352

ohltyler commented Dec 22, 2020 •

edited

Loading

ylwu-amzn Dec 23, 2020

ohltyler Dec 28, 2020

ylwu-amzn Dec 28, 2020

ohltyler Dec 28, 2020

ylwu-amzn Dec 23, 2020

ohltyler Dec 28, 2020

ohltyler Dec 28, 2020

yizheliu-amazon Dec 23, 2020

ohltyler Dec 28, 2020

ohltyler Dec 28, 2020 •

edited

Loading

ohltyler Dec 28, 2020 •

edited

Loading

yizheliu-amazon Dec 23, 2020

ohltyler Dec 28, 2020

ylwu-amzn Dec 28, 2020 •

edited

Loading

ohltyler Dec 28, 2020

ohltyler commented Dec 28, 2020

Add other changes related to historical workbench #352

Add other changes related to historical workbench #352

Conversation

ohltyler commented Dec 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ohltyler Dec 28, 2020 • edited Loading

Choose a reason for hiding this comment

ohltyler Dec 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ylwu-amzn Dec 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ohltyler commented Dec 28, 2020

ohltyler commented Dec 22, 2020 •

edited

Loading

ohltyler Dec 28, 2020 •

edited

Loading

ohltyler Dec 28, 2020 •

edited

Loading

ylwu-amzn Dec 28, 2020 •

edited

Loading