[ML] Explain log rate spikes: Fix data out of date when brush selection changes #137791

alvarezmelissa87 · 2022-08-01T21:30:25Z

Summary

Related meta issue: #136265

This PR updates the run analysis button for the spike analysis table when the brush selection is updated.

Button will go orange and have a warning icon with a tooltip informing the user that the data is out of date and they should rerun the analysis.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

elasticmachine · 2022-08-01T21:30:37Z

Pinging @elastic/ml-ui (:ml)

...plugins/aiops/public/components/explain_log_rate_spikes/explain_log_rate_spikes_analysis.tsx

x-pack/packages/ml/aiops_components/src/dual_brush/dual_brush.tsx

x-pack/packages/ml/aiops_components/src/progress_controls/progress_controls.tsx

x-pack/packages/ml/aiops_utils/src/get_window_parameters.ts

walterra · 2022-08-02T11:01:23Z

...plugins/aiops/public/components/explain_log_rate_spikes/explain_log_rate_spikes_analysis.tsx

@@ -82,6 +99,7 @@ export const ExplainLogRateSpikesAnalysis: FC<ExplainLogRateSpikesAnalysisProps>
    if (onSelectedChangePoint) {
      onSelectedChangePoint(null);
    }
+    setShouldRerunAnalysis(false);


This should be replaced with setCurrentAnalysisWindowParameters(windowParameters); similar to the call in the useEffect that trigger the analysis on load.

Good catch - updated in 3f2a5c5

It looks like that fix wasn't commited/pushed properly, I cannot see it being changed in the latest version, can you try again?

walterra · 2022-08-02T13:59:45Z

...plugins/aiops/public/components/explain_log_rate_spikes/explain_log_rate_spikes_analysis.tsx

+      currentAnalysisWindowParameters !== undefined &&
+      !isEqual(currentAnalysisWindowParameters, windowParameters)
+    ) {
+      console.log('STALE!!'); // remove


This can be removed now.

Removed in 3f2a5c5

…-ref HEAD~1..HEAD --fix'

alvarezmelissa87 · 2022-08-02T16:52:15Z

This has been updated and is ready for a final look when you get a chance cc @walterra, @darnautov 🙏

qn895 · 2022-08-02T17:08:36Z

x-pack/packages/ml/aiops_components/src/dual_brush/dual_brush.tsx

@@ -169,8 +169,9 @@ export function DualBrush({
          };

          if (
-            baselineSelection[0] !== newBrushPx.baselineMin ||
-            baselineSelection[1] !== newBrushPx.baselineMax
+            id === 'baseline' &&


Small suggestion to have 'baseline' and 'deviation' as constants, which can be used in x-pack/packages/ml/aiops_components/src/dual_brush/dual_brush.tsx as well

qn895 · 2022-08-02T17:11:23Z

Tested latest changes (as of fe8fa3b) and functionally LGTM. Just left a small comment.

Also, out of scope of this PR, but let's say if I have a bucket/time range selected, clicking on another bucket would not do anything unless I click Clear selection. Might be good to improve on this user experience in a future PR.

…-ref HEAD~1..HEAD --fix'

walterra

Latest update with useMemo works for me! LGTM.

darnautov

Code LGTM

alvarezmelissa87 · 2022-08-03T14:37:50Z

@elasticmachine merge upstream

…-ref HEAD~1..HEAD --fix'

kibana-ci · 2022-08-03T16:08:24Z

💚 Build Succeeded

Buildkite Build
Commit: 738e6cb

Metrics [docs]

Async chunks

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

id	before	after	diff
`aiops`	474.6KB	476.2KB	+1.6KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/aiops-utils`	46	49	+3

History

💔 Build #62823 failed 914ccba
💔 Build #62598 failed 96d8f4e
💔 Build #62510 failed fe8fa3b
💔 Build #62435 failed 3f2a5c5
💔 Build #62238 failed 81e75ae14c6d2f1a3d290807d9db542173d6d7a3

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

cc @alvarezmelissa87

kibanamachine · 2022-08-03T16:15:04Z

💚 All backports created successfully

Status	Branch	Result
✅	8.4

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

…on changes (#137791) * update run analysis button content when selection changges * fix brush overlap causing endless rerender * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * fix resize triggering rerun analysis prompt * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * add comments to getSnappedWindowParameters function * use memo instead of using component state * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * fix eslint error and simplify usememo callback * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit b17579a)

…on changes (#137791) (#138009) * update run analysis button content when selection changges * fix brush overlap causing endless rerender * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * fix resize triggering rerun analysis prompt * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * add comments to getSnappedWindowParameters function * use memo instead of using component state * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * fix eslint error and simplify usememo callback * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit b17579a) Co-authored-by: Melissa Alvarez <[email protected]>

alvarezmelissa87 added bug Fixes for quality problems that affect the customer experience :ml release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.4.0 v8.5.0 labels Aug 1, 2022

alvarezmelissa87 requested review from walterra and qn895 August 1, 2022 21:30

alvarezmelissa87 self-assigned this Aug 1, 2022

alvarezmelissa87 requested a review from a team as a code owner August 1, 2022 21:30

darnautov reviewed Aug 2, 2022

View reviewed changes

walterra reviewed Aug 2, 2022

View reviewed changes

update run analysis button content when selection changges

3f2a5c5

alvarezmelissa87 force-pushed the log-rate-spikes-stale-analysis-text branch from 81e75ae to 3f2a5c5 Compare August 2, 2022 14:31

alvarezmelissa87 and others added 4 commits August 2, 2022 11:58

fix brush overlap causing endless rerender

ef2c6c7

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

63e3d6e

…-ref HEAD~1..HEAD --fix'

fix resize triggering rerun analysis prompt

a228751

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

fe8fa3b

…-ref HEAD~1..HEAD --fix'

qn895 reviewed Aug 2, 2022

View reviewed changes

alvarezmelissa87 and others added 4 commits August 2, 2022 16:25

add comments to getSnappedWindowParameters function

96d8f4e

use memo instead of using component state

4449f49

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

5df301a

…-ref HEAD~1..HEAD --fix'

fix eslint error and simplify usememo callback

914ccba

walterra approved these changes Aug 3, 2022

View reviewed changes

darnautov approved these changes Aug 3, 2022

View reviewed changes

kibanamachine and others added 3 commits August 3, 2022 10:37

Merge branch 'main' into log-rate-spikes-stale-analysis-text

f0acd86

Merge branch 'main' into log-rate-spikes-stale-analysis-text

e623b81

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

738e6cb

…-ref HEAD~1..HEAD --fix'

alvarezmelissa87 merged commit b17579a into elastic:main Aug 3, 2022

alvarezmelissa87 deleted the log-rate-spikes-stale-analysis-text branch August 3, 2022 16:11

alvarezmelissa87 mentioned this pull request Aug 3, 2022

[ML] Explain log rate spikes: MVP 8.4 improvements. #136265

Closed

34 tasks

kibanamachine mentioned this pull request Aug 3, 2022

[8.4] [ML] Explain log rate spikes: Fix data out of date when brush selection changes (#137791) #138009

Merged

This was referenced Aug 4, 2022

[Observability][SecuritySolution] Fix to prevent observability style conflict in flyout across plugins #138091

Merged

[SecuritySolution][Bug] Fix to add empty values to timeline #138510

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Explain log rate spikes: Fix data out of date when brush selection changes #137791

[ML] Explain log rate spikes: Fix data out of date when brush selection changes #137791

alvarezmelissa87 commented Aug 1, 2022 •

edited

Loading

elasticmachine commented Aug 1, 2022

walterra Aug 2, 2022

alvarezmelissa87 Aug 2, 2022

walterra Aug 3, 2022

walterra Aug 2, 2022

alvarezmelissa87 Aug 2, 2022

alvarezmelissa87 commented Aug 2, 2022

qn895 Aug 2, 2022

qn895 commented Aug 2, 2022

walterra left a comment

darnautov left a comment

alvarezmelissa87 commented Aug 3, 2022

kibana-ci commented Aug 3, 2022

API count

kibanamachine commented Aug 3, 2022

[ML] Explain log rate spikes: Fix data out of date when brush selection changes #137791

[ML] Explain log rate spikes: Fix data out of date when brush selection changes #137791

Conversation

alvarezmelissa87 commented Aug 1, 2022 • edited Loading

Summary

Checklist

elasticmachine commented Aug 1, 2022

walterra Aug 2, 2022

Choose a reason for hiding this comment

alvarezmelissa87 Aug 2, 2022

Choose a reason for hiding this comment

walterra Aug 3, 2022

Choose a reason for hiding this comment

walterra Aug 2, 2022

Choose a reason for hiding this comment

alvarezmelissa87 Aug 2, 2022

Choose a reason for hiding this comment

alvarezmelissa87 commented Aug 2, 2022

qn895 Aug 2, 2022

Choose a reason for hiding this comment

qn895 commented Aug 2, 2022

walterra left a comment

Choose a reason for hiding this comment

darnautov left a comment

Choose a reason for hiding this comment

alvarezmelissa87 commented Aug 3, 2022

kibana-ci commented Aug 3, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

API count

History

kibanamachine commented Aug 3, 2022

💚 All backports created successfully

Questions ?

alvarezmelissa87 commented Aug 1, 2022 •

edited

Loading