Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Use anomaly score explanation for chart tooltip multi-bucket impact #146866

Merged

Conversation

peteharverson
Copy link
Contributor

@peteharverson peteharverson commented Dec 2, 2022

Summary

Follow-up to #142999, using the multi_bucket_impact field from the anomaly_score_explanation data to populate the multi-bucket information in the anomaly chart tooltip using the same visualization style as used in the expanded table row.

image

The anomaly charts in the Anomaly Explorer and Single Metric Viewer, and the severity cell in the anomalies table now use a new isMultiBucketAnomaly check in ml/common/util/anomaly_utils.tsm to determine whether to indicate the anomaly as multi-bucket with the plus-shaped symbol. Note that this will result in some changes to whether anomalies are plotted as multi-bucket anomalies. Specifically anomalies with a 'moderate' multi-bucket impact are now likely to be marked with a plus symbol where before they were not:

Before:
image

After:
image

Checklist

Delete any items that are not applicable to this PR.

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@@ -27,7 +27,7 @@ export const TimeSeriesExplorerHelpPopover = () => {
<p>
<FormattedMessage
id="xpack.ml.timeSeriesExplorer.popoverAnomalyExplanation"
defaultMessage="An anomaly score is calculated for each bucket time interval, with a value from 0 to 100. Anomalous events are highlighted in colors that indicate their severity. If an anomaly is depicted with a cross symbol instead of a dot, it has a medium or high multi-bucket impact. This extra analysis can catch anomalies even when they fall within the bounds of expected behavior."
defaultMessage="An anomaly score is calculated for each bucket time interval, with a value from 0 to 100. Anomalous events are highlighted in colors that indicate their severity. If an anomaly is depicted with a cross symbol instead of a dot, it has a moderate, significant or high multi-bucket impact. This extra analysis can catch anomalies even when they fall within the bounds of expected behavior."
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if there should be another comma after 'significant' or not for clarity 🤔 To signify there are three levels of impact.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the Oxford comma after consultation with @szabosteve in aecb00b

Copy link
Contributor

@szabosteve szabosteve Dec 7, 2022

Choose a reason for hiding this comment

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

Yeah, it might be a good idea to use the Oxford comma here, so it's easier to identify the three levels. Even if the sentence is grammatically correct without the second comma, too.
EDIT: Sorry, didn't see Pete's comment above.

}

if (sb !== undefined && mb > sb) {
return (((mb - sb) * mb) / sb) * 1.7 >= 2;
Copy link
Member

Choose a reason for hiding this comment

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

Can we add in a comment to add some context for this formula? E.g. why are we multiplying by 1.7?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It isn't easy coming up with a clear explanation of this calculation, but I added a comment which hopefully provides some help in aecb00b.

@qn895
Copy link
Member

qn895 commented Dec 6, 2022

Tested and LGTM 🎉 Just left a few nit comments.

@peteharverson peteharverson force-pushed the ml-multi-bucket-chart-tooltip branch from eb64ad6 to 0df33ad Compare December 7, 2022 10:36
tooltip += '\u25A1 '; // Unicode hollow square
}

return tooltip;
Copy link
Member

@jgowdyelastic jgowdyelastic Dec 7, 2022

Choose a reason for hiding this comment

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

there are probably 100 different ways to do this, but here's a suggestion that uses fill:

return new Array(5).fill('\u25A0 ', 0, numFilledSquares).fill('\u25A1 ', numFilledSquares).join('')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 6a4a0ba

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
ml 1578 1577 -1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 3.4MB 3.4MB +209.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
ml 42.4KB 42.2KB -188.0B
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 109 115 +6
securitySolution 445 451 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 68 74 +6
osquery 110 117 +7
securitySolution 521 527 +6
total +21

History

  • 💚 Build #92271 succeeded eb64ad6f4c2c8086c263d81e387a83860cb4f189
  • 💔 Build #92265 failed 4a31edcde91d9bbd0c4f785a85fe360620e6c42e

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @peteharverson

@peteharverson peteharverson merged commit 9209d50 into elastic:main Dec 7, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Dec 7, 2022
@peteharverson peteharverson deleted the ml-multi-bucket-chart-tooltip branch December 7, 2022 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants