[ML] Single Metric Viewer embeddable in dashboards: move all config to flyout #182756

alvarezmelissa87 · 2024-05-06T21:02:52Z

Summary

Part of #182042
This PR also fixes the issue in the swimlane embeddable that fails to close the flyout if navigating into the main dashboards page.

Related meta issue: #181272
Item: https://github.com/elastic/kibana/issues/181272

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
Flaky Test Runner was used on any tests changed
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 · 2024-05-06T21:02:55Z

Pinging @elastic/ml-ui (:ml)

...ugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_embeddable_factory.tsx

x-pack/plugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_initializer.tsx

darnautov · 2024-05-08T14:17:30Z

x-pack/plugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_initializer.tsx

+        }
+      }
+    },
+    [isMounted, jobIds, mlApiServices, panelTitle, initialValuesRef.current.isNewJob, job?.job_id]


Just realized, both here and in the anomaly swim lane initializer, we need to introduce a debounce on job IDs change.

🤔 I'm not sure we do in this case since we only do the fetch if a new job has been selected (that hasn't already been loaded). Also - are people rapidly clicking around a ton on job ids? I think the dropdown closes after each selection (at least on single selection) so it would be difficult for a user to be changing job ids extremely quickly.

x-pack/plugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_initializer.tsx

darnautov · 2024-05-08T14:22:42Z

x-pack/plugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_setup_flyout.tsx

+        {
+          type: 'push',
+          ownFocus: true,
+          size: 's',
+          onClose: () => {
+            flyoutSession.close();
+            reject();
+          },
+        }
      );
+      // Close the flyout when user navigates out of the current plugin
+      currentAppId$
+        .pipe(skip(1), takeUntil(from(flyoutSession.onClose)), distinctUntilChanged())
+        .subscribe(() => {
+          flyoutSession.close();


as this logic has been copied from the anomaly swim lane initializer, shall we consolidate it in a single function, e.g. initMlEmbeddalbeFlyout?

Agreed - this is going to be very similar boiler plate code for all the embeddables 👍
Though I'm okay leaving it until the rest of the embeddables are converted to make sure we account for all potential cases.

It's definitely the case for every embeddable, i.e. for anomaly charts as well.

@alvarezmelissa87 important update! we need to track overlay a bit differently, you can look up in the recent change point detection PR, where I also update the swim lane edit and add flow

kibana/x-pack/plugins/ml/public/embeddables/anomaly_swimlane/anomaly_swimlane_setup_flyout.tsx

Line 64 in 4419370

if (tracksOverlays(parentApi)) {

it will automatically track if the user navigates away from the dashboard app and disables all controls on the active dashboard.

Added in 7603fd4 🙏

x-pack/plugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_initializer.tsx

...ugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_embeddable_factory.tsx

peteharverson · 2024-05-09T10:31:26Z

Testing this I also found an issue when changing the selected job (also happens on main). I think this occurs when switching from a job with 1 detector to one with multiple detectors and selecting the detector at an index which doesn't exist in the first job. Here I select a job with 2 detectors and pick the second detector.

Looks like buildConfigFromDetector in chart_config_builder.ts is called several times, and at one point is passed the config for the previously selected job but the detector index from the new selection:

Screen.Recording.2024-05-09.at.11.22.06.mov

alvarezmelissa87 · 2024-05-09T17:47:06Z

Created a separate PR for the fix for the issue you found, @peteharverson so we can backport to 8.14. 👍

alvarezmelissa87 · 2024-05-09T22:05:38Z

This is ready for another look when you get a chance cc @darnautov, @peteharverson

peteharverson

Latest changes LGTM

…fferent job works as expected (#183086) ## Summary Fixes #182756 (comment) This PR ensures the loaded job and the selected job id are synced to ensure the chart loads correctly. After: https://github.com/elastic/kibana/assets/6446462/4c80bdd6-54cd-4404-8fa4-8930aca1b3f9

x-pack/plugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_initializer.tsx

darnautov

LGTM

alvarezmelissa87 · 2024-05-10T21:04:50Z

@elasticmachine merge upstream

alvarezmelissa87 · 2024-05-14T17:34:45Z

@elasticmachine merge upstream

kibanamachine · 2024-05-14T17:34:49Z

merge conflict between base and head

…-fix'

kibana-ci · 2024-05-14T19:56:21Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 7603fd4

Failed CI Steps

FTR Configs #44

Test Failures

[job] [logs] FTR Configs #44 / Lists plugin Lists API "before all" hook for "should return a 400 if an endpoint exception item with a list-based entry is provided"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	2013	2012	-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`	4.2MB	4.2MB	+519.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`	77.0KB	77.0KB	-2.0B

Unknown metric groups

async chunk count

id	before	after	diff
`ml`	98	99	+1

History

💛 Build #209322 was flaky 1580223
💔 Build #209303 failed aedae88
💔 Build #209255 failed 8775a1a
💔 Build #209065 failed 5dd4765
💔 Build #209019 failed 7e3b004
💔 Build #208770 failed 1853517

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

cc @alvarezmelissa87

alvarezmelissa87 added :ml v8.15.0 labels May 6, 2024

alvarezmelissa87 self-assigned this May 6, 2024

alvarezmelissa87 requested a review from a team as a code owner May 6, 2024 21:02

alvarezmelissa87 added release_note:enhancement Feature:Anomaly Detection ML anomaly detection labels May 6, 2024

alvarezmelissa87 requested review from darnautov and peteharverson May 6, 2024 21:06

move all embeddable config to flyout

778ebf2

alvarezmelissa87 force-pushed the ml-smv-embeddable-dash-config-update branch from 4d3ca40 to 778ebf2 Compare May 6, 2024 21:12

alvarezmelissa87 mentioned this pull request May 6, 2024

[ML] Anomaly Detection: Single Metric Viewer embeddable enhancements #181272

Open

10 tasks

alvarezmelissa87 added 2 commits May 6, 2024 16:02

fix translation label

e3c8cd4

let jobselector handle job fetch error

4765fed

peteharverson reviewed May 7, 2024

View reviewed changes

...ugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_embeddable_factory.tsx Show resolved Hide resolved

ensure edit works as expected

eef2233

darnautov reviewed May 8, 2024

View reviewed changes

Merge branch 'main' into ml-smv-embeddable-dash-config-update

1853517

peteharverson reviewed May 9, 2024

View reviewed changes

x-pack/plugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_initializer.tsx Outdated Show resolved Hide resolved

...ugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_embeddable_factory.tsx Outdated Show resolved Hide resolved

fix isMounted and change name

7e3b004

alvarezmelissa87 mentioned this pull request May 9, 2024

[ML] Single Metric Viewer embeddable in dashboards: ensure edit to different job works as expected #183086

Merged

simplify job selection handling

5dd4765

peteharverson approved these changes May 10, 2024

View reviewed changes

alvarezmelissa87 closed this in #183086 May 10, 2024

alvarezmelissa87 reopened this May 10, 2024

fix swimlane flyout and update functional tests

8775a1a

darnautov reviewed May 10, 2024

View reviewed changes

x-pack/plugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_initializer.tsx Outdated Show resolved Hide resolved

simplify to use jobId as it will always be just one job

aedae88

darnautov approved these changes May 10, 2024

View reviewed changes

Merge branch 'main' into ml-smv-embeddable-dash-config-update

1580223

alvarezmelissa87 and others added 3 commits May 14, 2024 11:40

Merge branch 'main' into ml-smv-embeddable-dash-config-update

e09f55a

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

ab2174e

…-fix'

update with trackOverlay

7603fd4

alvarezmelissa87 merged commit f0f2ca0 into elastic:main May 14, 2024
19 checks passed

kibanamachine added the backport:skip This commit does not require backporting label May 14, 2024

alvarezmelissa87 deleted the ml-smv-embeddable-dash-config-update branch May 14, 2024 20:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Single Metric Viewer embeddable in dashboards: move all config to flyout #182756

[ML] Single Metric Viewer embeddable in dashboards: move all config to flyout #182756

alvarezmelissa87 commented May 6, 2024 •

edited

elasticmachine commented May 6, 2024

darnautov May 8, 2024

alvarezmelissa87 May 9, 2024

darnautov May 8, 2024

alvarezmelissa87 May 8, 2024 •

edited

darnautov May 10, 2024

darnautov May 13, 2024

alvarezmelissa87 May 14, 2024

peteharverson commented May 9, 2024

alvarezmelissa87 commented May 9, 2024

alvarezmelissa87 commented May 9, 2024 •

edited

peteharverson left a comment

darnautov left a comment

alvarezmelissa87 commented May 10, 2024

alvarezmelissa87 commented May 14, 2024

kibanamachine commented May 14, 2024

kibana-ci commented May 14, 2024

async chunk count

[ML] Single Metric Viewer embeddable in dashboards: move all config to flyout #182756

[ML] Single Metric Viewer embeddable in dashboards: move all config to flyout #182756

Conversation

alvarezmelissa87 commented May 6, 2024 • edited

Summary

Checklist

elasticmachine commented May 6, 2024

darnautov May 8, 2024

Choose a reason for hiding this comment

alvarezmelissa87 May 9, 2024

Choose a reason for hiding this comment

darnautov May 8, 2024

Choose a reason for hiding this comment

alvarezmelissa87 May 8, 2024 • edited

Choose a reason for hiding this comment

darnautov May 10, 2024

Choose a reason for hiding this comment

darnautov May 13, 2024

Choose a reason for hiding this comment

alvarezmelissa87 May 14, 2024

Choose a reason for hiding this comment

peteharverson commented May 9, 2024

alvarezmelissa87 commented May 9, 2024

alvarezmelissa87 commented May 9, 2024 • edited

peteharverson left a comment

Choose a reason for hiding this comment

darnautov left a comment

Choose a reason for hiding this comment

alvarezmelissa87 commented May 10, 2024

alvarezmelissa87 commented May 14, 2024

kibanamachine commented May 14, 2024

kibana-ci commented May 14, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

Page load bundle

async chunk count

History

alvarezmelissa87 commented May 6, 2024 •

edited

alvarezmelissa87 May 8, 2024 •

edited

alvarezmelissa87 commented May 9, 2024 •

edited