Add feature attribution #296

jackiehanyang · 2022-08-05T09:27:28Z

Description

[Describe what this change achieves]
UI changes:

in anomaly result chart, show feature contribution of this anomaly with RectAnnotation component
in feature break down chart, show expected value when there's an anomaly and current value when there's no anomaly as an additional on the chart
swap feature break down tab with anomaly occurrence tab to make it the default selected tab

Issues Resolved

opensearch-project/anomaly-detection#299

[List any issues this PR will resolve]

Check List

New functionality includes testing. Tested on localhost
- All tests pass. will add more tests in the upcoming pr
New functionality has been documented.
Commits are signed per the DCO using --signoff

By 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.

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

sean-zheng-amazon

overall looks good. sync'ed offline, may need to make minor adjustments to improve consistency.

ohltyler · 2022-08-05T16:03:00Z

public/pages/AnomalyCharts/components/FeatureChart/FeatureChart.tsx

+                      <LineSeries
+                        id={"ExpectedValue"}
+                        name={"Expected Value"}
+                        color={"#0475a2"}


What is this color? Can you add a comment in the code about it somewhere? If supposed to be invisible, may need to find a way to always keep it consistent with the chart background, including if Dashboards is in dark mode or not

It's a bluish color and shouldn't be invisible. I need to attend the UX office hour next Tuesday to get this color finalized.

Ok - is there a way for you to work with them sooner? Concern is that 8/9 is scheduled as the final sanity testing day - see timeline opensearch-project/opensearch-build#2271

sure, will reach out to them offline to draw an conclusion on this

public/pages/AnomalyCharts/components/FeatureChart/FeatureChart.tsx

public/pages/AnomalyCharts/containers/AnomalyDetailsChart.tsx

public/pages/AnomalyCharts/utils/constants.ts

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

public/pages/utils/anomalyResultUtils.ts

public/models/interfaces.ts

public/pages/utils/anomalyResultUtils.ts

public/pages/DetectorResults/containers/AnomalyHistory.tsx

public/pages/AnomalyCharts/containers/AnomalyDetailsChart.tsx

ohltyler

Overall ok with the current changes - have a couple days buffer for finishing up. Thanks for adding this!

ohltyler · 2022-08-05T17:04:09Z

Also, if you can clean up the PR description and add some screenshots of the changes in the chart that would be great.

Or could rename this PR to specify that this is just adding the expected value and feature contribution to the anomaly result schema, and future PR to enable it in the charts.

ohltyler · 2022-08-05T17:12:29Z

There's also some integ test failures - I'm assuming it is related to switching the default anomaly history tabs

       Spec                                              Tests  Passing  Failing  Pending  Skipped  
  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ ✔  plugins/anomaly-detection-dashboard      01:04        1        1        -        -        - │
  │    s-plugin/create_detector_spec.js                                                            │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  plugins/anomaly-detection-dashboard      01:07        8        8        -        -        - │
  │    s-plugin/dashboard_spec.js                                                                  │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✖  plugins/anomaly-detection-dashboard      01:57        5        4        1        -        - │
  │    s-plugin/detector_configuration_spe                                                         │
  │    c.js                                                                                        │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  plugins/anomaly-detection-dashboard      01:35       11       11        -        -        - │
  │    s-plugin/detector_list_spec.js                                                              │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✖  plugins/anomaly-detection-dashboard      04:37        6        2        4        -        - │
  │    s-plugin/historical_analysis_spec.j                                                         │
  │    s                                                                                           │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✖  plugins/anomaly-detection-dashboard      02:31        6        4        2        -        - │
  │    s-plugin/overview_spec.js                                                                   │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✖  plugins/anomaly-detection-dashboard      01:24        3        -        1        -        2 │
  │    s-plugin/real_time_results_spec.js                                                          │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✖  plugins/anomaly-detection-dashboard      03:41        3        -        3        -        - │
  │    s-plugin/sample_detector_spec.js                                                            │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘

public/models/interfaces.ts

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

ohltyler · 2022-08-05T21:34:23Z

public/pages/AnomalyCharts/containers/AnomalyDetailsChart.tsx


+      const featureData = get(anomaly, `features`, {}) 
+      let featureAttributionList = [] as any[];
+      if (Array.isArray(contributionData)) {


when would contributions not be an array?

when it's getting response by calling search result api route in stead of get detector result api route

Not sure what you mean about get detector result api route. I see the parsing you've done in getAnomalyResults(). Where does the parsing happen where it is an array instead?

there's no parsing for that route because it's processed at receiving response stage. See line 964 in ad.ts file, a type was defined and returned with response data

I'm still unsure when contributions would be an array. Even AnomalyResult defines it as a map here

it's from here - https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/blob/main/public/pages/DetectorResults/containers/AnomalyHistory.tsx#L664

the data has been flattened before passing in - https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/pull/296/files#diff-45683a46376000d3a5181f2[…]457590f0212f08b208877beaR602

server/routes/ad.ts

public/pages/AnomalyCharts/containers/AnomalyDetailsChart.tsx

server/routes/ad.ts

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

server/routes/ad.ts

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

server/routes/ad.ts

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

server/models/types.ts

public/pages/utils/anomalyResultUtils.ts

…t have this field Signed-off-by: Jackie Han <jkhanjob@gmail.com>

amitgalitz

LGTM on my end, made sure to keep track of what needs to be refactored/cleaned up in next PR. Will check out if I need to review/approve again if you need to address more of Tyler's comments

ohltyler

Overall LGTM, thanks for making the changes! Let's follow up on #296 (comment)

Signed-off-by: Jackie Han <jkhanjob@gmail.com> (cherry picked from commit d973b8e)

jackiehanyang added 2 commits August 5, 2022 02:23

Add feature attributions

09e74ca

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

Remove unused code

42892f2

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

jackiehanyang requested a review from a team August 5, 2022 09:27

opensearch-trigger-bot bot added the backport 2.x label Aug 5, 2022

sean-zheng-amazon previously approved these changes Aug 5, 2022

View reviewed changes

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/AnomalyCharts/components/FeatureChart/FeatureChart.tsx Outdated Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/AnomalyCharts/containers/AnomalyDetailsChart.tsx Outdated Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/AnomalyCharts/containers/AnomalyDetailsChart.tsx Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/AnomalyCharts/containers/AnomalyDetailsChart.tsx Outdated Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/AnomalyCharts/utils/constants.ts Outdated Show resolved Hide resolved

clean code and address comments

b884116

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

jackiehanyang dismissed sean-zheng-amazon’s stale review via b884116 August 5, 2022 16:28

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/utils/anomalyResultUtils.ts Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/models/interfaces.ts Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/utils/anomalyResultUtils.ts Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/utils/anomalyResultUtils.ts Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/DetectorResults/containers/AnomalyHistory.tsx Outdated Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/AnomalyCharts/containers/AnomalyDetailsChart.tsx Outdated Show resolved Hide resolved

ohltyler previously approved these changes Aug 5, 2022

View reviewed changes

ohltyler requested a review from sean-zheng-amazon August 5, 2022 17:00

amitgalitz reviewed Aug 5, 2022

View reviewed changes

public/models/interfaces.ts Show resolved Hide resolved

Correct data for anomlay result chart

f457bcb

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

jackiehanyang dismissed ohltyler’s stale review via f457bcb August 5, 2022 20:23

Adjust feature break down chart data

dd182b7

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

ohltyler reviewed Aug 5, 2022

View reviewed changes

amitgalitz reviewed Aug 5, 2022

View reviewed changes

server/routes/ad.ts Outdated Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/AnomalyCharts/containers/AnomalyDetailsChart.tsx Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

server/routes/ad.ts Outdated Show resolved Hide resolved

amitgalitz reviewed Aug 5, 2022

View reviewed changes

server/routes/ad.ts Outdated Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

server/routes/ad.ts Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

server/routes/ad.ts Outdated Show resolved Hide resolved

remove unnecessary debugging code

c6a46b3

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

amitgalitz reviewed Aug 5, 2022

View reviewed changes

server/routes/ad.ts Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

server/routes/ad.ts Outdated Show resolved Hide resolved

amitgalitz reviewed Aug 5, 2022

View reviewed changes

server/routes/ad.ts Outdated Show resolved Hide resolved

clean up code

629f128

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

ohltyler reviewed Aug 5, 2022

View reviewed changes

server/routes/ad.ts Outdated Show resolved Hide resolved

avoid npe while parsing search detector result

9d18af3

Signed-off-by: Jackie Han <jkhanjob@gmail.com>

ohltyler reviewed Aug 5, 2022

View reviewed changes

server/models/types.ts Outdated Show resolved Hide resolved

ohltyler reviewed Aug 5, 2022

View reviewed changes

public/pages/utils/anomalyResultUtils.ts Show resolved Hide resolved

make contribution field optional since older anomlay results might no…

06da1f1

…t have this field Signed-off-by: Jackie Han <jkhanjob@gmail.com>

jackiehanyang added the backport 2.1 label Aug 5, 2022

amitgalitz approved these changes Aug 5, 2022

View reviewed changes

ohltyler approved these changes Aug 5, 2022

View reviewed changes

ohltyler merged commit d973b8e into opensearch-project:main Aug 5, 2022

opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 5, 2022

Add feature attribution (#296)

c0f73b4

Signed-off-by: Jackie Han <jkhanjob@gmail.com> (cherry picked from commit d973b8e)

opensearch-trigger-bot bot mentioned this pull request Aug 5, 2022

[Backport 2.x] Add feature attribution #297

Merged

opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 5, 2022

Add feature attribution (#296)

993a7cf

Signed-off-by: Jackie Han <jkhanjob@gmail.com> (cherry picked from commit d973b8e)

opensearch-trigger-bot bot mentioned this pull request Aug 5, 2022

[Backport 2.2] Add feature attribution #298

Closed

ohltyler added backport 2.2 and removed backport 2.1 labels Aug 5, 2022

opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 5, 2022

Add feature attribution (#296)

41efe79

Signed-off-by: Jackie Han <jkhanjob@gmail.com> (cherry picked from commit d973b8e)

opensearch-trigger-bot bot mentioned this pull request Aug 5, 2022

[Backport 2.2] Add feature attribution #299

Merged

ohltyler pushed a commit that referenced this pull request Aug 5, 2022

Add feature attribution (#296)

3c47e2c

Signed-off-by: Jackie Han <jkhanjob@gmail.com> (cherry picked from commit d973b8e)

ohltyler pushed a commit that referenced this pull request Aug 5, 2022

Add feature attribution (#296)

ede45f3

Signed-off-by: Jackie Han <jkhanjob@gmail.com> (cherry picked from commit d973b8e)

ohltyler added the enhancement New feature or request label Aug 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add feature attribution #296

Add feature attribution #296

jackiehanyang commented Aug 5, 2022 •

edited

Loading

sean-zheng-amazon left a comment

ohltyler Aug 5, 2022

jackiehanyang Aug 5, 2022

ohltyler Aug 5, 2022

jackiehanyang Aug 5, 2022

ohltyler left a comment

ohltyler commented Aug 5, 2022

ohltyler commented Aug 5, 2022

ohltyler Aug 5, 2022

jackiehanyang Aug 5, 2022

ohltyler Aug 5, 2022

jackiehanyang Aug 5, 2022

ohltyler Aug 5, 2022 •

edited

Loading

jackiehanyang Aug 5, 2022

jackiehanyang Aug 5, 2022

amitgalitz left a comment

ohltyler left a comment

Add feature attribution #296

Add feature attribution #296

Conversation

jackiehanyang commented Aug 5, 2022 • edited Loading

Description

Issues Resolved

Check List

sean-zheng-amazon left a 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 left a comment

Choose a reason for hiding this comment

ohltyler commented Aug 5, 2022

ohltyler commented Aug 5, 2022

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 Aug 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amitgalitz left a comment

Choose a reason for hiding this comment

ohltyler left a comment

Choose a reason for hiding this comment

jackiehanyang commented Aug 5, 2022 •

edited

Loading

ohltyler Aug 5, 2022 •

edited

Loading