[ML] Single Metric Viewer embeddable: add open and edit panel actions #179364

alvarezmelissa87 · 2024-03-25T16:36:11Z

Summary

Part of #176651

This PR:

adds the 'Open in Single Metric Viewer' and 'Edit' panel actions to the Single Metric Viewer embeddable for dashboards.
detector index fix moved to separate PR

Screen shots

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

walterra

Code LGTM, also tested this with some datasets.

walterra · 2024-03-26T13:46:10Z

/ci

elasticmachine · 2024-03-28T01:25:54Z

Pinging @elastic/ml-ui (:ml)

alvarezmelissa87 · 2024-03-28T01:26:07Z

@elasticmachine merge upstream

x-pack/plugins/ml/public/application/jobs/new_job/pages/job_type/page.tsx

x-pack/plugins/ml/public/application/util/index_utils.ts

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

x-pack/plugins/ml/public/embeddables/types.ts

x-pack/plugins/ml/public/ui_actions/open_in_single_metric_viewer_action.tsx

… jobs

alvarezmelissa87 · 2024-03-28T23:53:33Z

Did a bit of refactoring to fully decouple actions from old embeddable system and fixed a bug where the selectedDetectorIndex wasn't being passed correctly in a separate PR - I'll update this PR after the fix has gone in. This is ready for another test and look when you get a chance 🙏
cc @darnautov, @walterra

darnautov · 2024-04-02T09:02:27Z

x-pack/plugins/ml/public/application/jobs/new_job/pages/job_type/page.tsx

+  const isTimeBasedIndex: boolean = selectedDataView.isTimeBased();
+
+  useEffect(() => {
+    if (!isTimeBasedIndex) {
+      toasts.addWarning({
+        title: i18n.translate('xpack.ml.dataViewNotBasedOnTimeSeriesNotificationTitle', {
+          defaultMessage: 'The data view {dataViewIndexPattern} is not based on a time series',
+          values: { dataViewIndexPattern: selectedDataView.getIndexPattern() },
+        }),
+        text: i18n.translate('xpack.ml.dataViewNotBasedOnTimeSeriesNotificationDescription', {
+          defaultMessage: 'Anomaly detection only runs over time-based indices',
+        }),
+      });
+    }
+  }, [isTimeBasedIndex, selectedDataView, toasts]);
+


nit: can be simplified even further, preventing access to the isTImeBased method on each render!

Suggested change

const isTimeBasedIndex: boolean = selectedDataView.isTimeBased();

useEffect(() => {

if (!isTimeBasedIndex) {

toasts.addWarning({

title: i18n.translate('xpack.ml.dataViewNotBasedOnTimeSeriesNotificationTitle', {

defaultMessage: 'The data view {dataViewIndexPattern} is not based on a time series',

values: { dataViewIndexPattern: selectedDataView.getIndexPattern() },

}),

text: i18n.translate('xpack.ml.dataViewNotBasedOnTimeSeriesNotificationDescription', {

defaultMessage: 'Anomaly detection only runs over time-based indices',

}),

});

}

}, [isTimeBasedIndex, selectedDataView, toasts]);

useEffect(function warnNonTimeSeriesData() {

if (!selectedDataView.isTimeBased()) {

toasts.addWarning({

title: i18n.translate('xpack.ml.dataViewNotBasedOnTimeSeriesNotificationTitle', {

defaultMessage: 'The data view {dataViewIndexPattern} is not based on a time series',

values: { dataViewIndexPattern: selectedDataView.getIndexPattern() },

}),

text: i18n.translate('xpack.ml.dataViewNotBasedOnTimeSeriesNotificationDescription', {

defaultMessage: 'Anomaly detection only runs over time-based indices',

}),

});

}

}, [selectedDataView, toasts]);

There's a reliance on the isTimeBasedIndex variable a few lines down - I considered wrapping in a useEffect but I think the addition of the function would negate any small optimization we'd get from that, in my opinion. Happy to discuss further if you feel strongly. 😄

darnautov · 2024-04-02T09:08:00Z

x-pack/plugins/ml/public/application/timeseriesexplorer/get_controls_for_detector.ts

@@ -14,7 +14,7 @@ import type { JobId, CombinedJob } from '../../../common/types/anomaly_detection
 */
 export function getControlsForDetector(
  selectedDetectorIndex: number,
-  selectedEntities: Record<string, any>,
+  selectedEntities: Record<string, any> | undefined,


can you please use the MlEntityField type instead of any

Went back to double check and the original Record<string, any> type is correct - went back to ensure the types are correct. All types should now be correct. Updated in c39b883

I don't think any is correct. If you don't know the incoming type - it should be unknown and covered with a type guard. Otherwise you should be aware of the expected type.

darnautov · 2024-04-02T09:11:59Z

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

+    this.jobId = embeddableInputToSubject<JobId, SingleMetricViewerEmbeddableInput>(
+      this.apiSubscriptions,
+      this,
+      'jobId'


With the already released embeddable version in 8.13, you need to provide backward compatibility.
The input prop used to be named jobIds and contained an array of strings instead of a string.

This PR has a change in single_metric_viewer_setup_flyout.tsx which resolves the array to a string when setting up the panel jobId: jobIds[0],.

Good call! Going to keep the original API for now to prevent any issues c39b883

darnautov · 2024-04-02T09:13:37Z

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

+    this.title = embeddableInputToSubject<string, SingleMetricViewerEmbeddableInput>(
+      this.apiSubscriptions,
+      this,
+      'title'
+    );


title is a common prop and is handled by the embeddable framework

Suggested change

this.title = embeddableInputToSubject<string, SingleMetricViewerEmbeddableInput>(

this.apiSubscriptions,

this,

'title'

);

Yes, it can be handled via Panel settings but is also editable when using the Edit panel action that has been added.
When converting to the new framework, this will be removed, but right now it's still handled by the edit action.
I will remove this when refactoring to the new framework in a follow up PR to ensure full functionality at every step.

Anomaly swim lane has exactly the same - editable via Panel settings and Edit panel actions, but it does not duplicate title API.

Good call - removed in c39b883

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

alvarezmelissa87 · 2024-04-02T19:54:42Z

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

alvarezmelissa87 · 2024-04-02T19:54:57Z

@elasticmachine merge upstream

...lugins/ml/public/embeddables/single_metric_viewer/single_metric_viewer_embeddable_factory.ts

x-pack/plugins/ml/public/ui_actions/open_in_single_metric_viewer_action.tsx

peteharverson

Tested latest changes and LGTM

.../application/timeseriesexplorer/components/plot_function_controls/plot_function_controls.tsx

...gins/ml/public/application/timeseriesexplorer/components/series_controls/series_controls.tsx

darnautov · 2024-04-04T14:46:11Z

x-pack/plugins/ml/public/application/timeseriesexplorer/get_controls_for_detector.ts

@@ -14,7 +14,7 @@ import type { JobId, CombinedJob } from '../../../common/types/anomaly_detection
 */
 export function getControlsForDetector(
  selectedDetectorIndex: number,
-  selectedEntities: Record<string, any>,
+  selectedEntities: Record<string, any> | undefined,


I don't think any is correct. If you don't know the incoming type - it should be unknown and covered with a type guard. Otherwise you should be aware of the expected type.

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

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

darnautov

Latest changes LGTM, thanks for improving types!

kibana-ci · 2024-04-04T17:24:24Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 968eb17

Failed CI Steps

Jest Tests #10

Test Failures

[job] [logs] Jest Tests #10 / useGetCaseUsers calls the api when invoked with the correct parameters

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1974	1976	+2

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.7MB	3.7MB	+3.1KB

History

💚 Build #200976 succeeded 26dbdce
💚 Build #200918 succeeded 6b6025a
💚 Build #200865 succeeded 902197b
💚 Build #200685 succeeded 4c0191b
💚 Build #200359 succeeded 120d0f2
💚 Build #200119 succeeded fc585a5

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

cc @alvarezmelissa87

… set correctly for 'Open' action (#180086) ## Summary Follow up to #179364 This PR ensures the detector index is set correctly the url in the `Open in single metric viewer` action to link from dashboards to ML > Single metric viewer. ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

alvarezmelissa87 added 2 commits March 25, 2024 08:27

add open in single metric viewer action to smv embeddable

24e4b4d

wip: add edit smv panel action

db9a4fd

alvarezmelissa87 changed the title ~~Ml smv embeddable enhancements~~ [ML] Single Metric Viewer embeddable: add open and edit panel actions Mar 25, 2024

fix chart description not updating correctly on edit

da7983d

alvarezmelissa87 self-assigned this Mar 25, 2024

alvarezmelissa87 added :ml Feature:Anomaly Detection ML anomaly detection v8.14.0 release_note:enhancement labels Mar 25, 2024

alvarezmelissa87 requested review from walterra and darnautov March 25, 2024 22:16

update types

c14ca22

walterra approved these changes Mar 26, 2024

View reviewed changes

alvarezmelissa87 marked this pull request as ready for review March 28, 2024 01:25

alvarezmelissa87 requested a review from a team as a code owner March 28, 2024 01:25

Merge branch 'main' into ml-smv-embeddable-enhancements

6647978

darnautov reviewed Mar 28, 2024

View reviewed changes

alvarezmelissa87 added 3 commits March 28, 2024 12:39

finish decoupling actions from old system

bc6fb2c

only keep track of single jobId as smv will only ever need one

a7d5a1e

Update types. Ensure detectorIndex is passed and reset when switching…

fc585a5

… jobs

alvarezmelissa87 requested a review from peteharverson March 28, 2024 23:52

move fix to separate PR

120d0f2

darnautov reviewed Apr 2, 2024

View reviewed changes

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

Fix types, revert to old api to prevent backport issues

c39b883

Merge branch 'main' into ml-smv-embeddable-enhancements

4c0191b

peteharverson reviewed Apr 3, 2024

View reviewed changes

darnautov reviewed Apr 3, 2024

View reviewed changes

x-pack/plugins/ml/public/ui_actions/open_in_single_metric_viewer_action.tsx Show resolved Hide resolved

alvarezmelissa87 added 3 commits April 3, 2024 08:32

ensure action icon is correct

902197b

ensure panelTitle correct when editing

6b6025a

remove incorrect MlEntityField type

26dbdce

peteharverson approved these changes Apr 4, 2024

View reviewed changes

darnautov reviewed Apr 4, 2024

View reviewed changes

fix entities type

968eb17

darnautov approved these changes Apr 4, 2024

View reviewed changes

alvarezmelissa87 merged commit 339e24f into elastic:main Apr 4, 2024
17 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Apr 4, 2024

alvarezmelissa87 deleted the ml-smv-embeddable-enhancements branch April 4, 2024 17:27

This was referenced Apr 4, 2024

[ML] Anomaly Detection: Single Metric Viewer embeddable - 8.14 enhancements #176651

Closed

[ML] Single Metric Viewer in dashboards: ensure the detector index is set correctly for 'Open' action #180086

Merged

thomheymann mentioned this pull request Apr 8, 2024

[Observability Onboarding] Removes hover effect from use case selection #180303

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Single Metric Viewer embeddable: add open and edit panel actions #179364

[ML] Single Metric Viewer embeddable: add open and edit panel actions #179364

alvarezmelissa87 commented Mar 25, 2024 •

edited

Loading

walterra left a comment

walterra commented Mar 26, 2024

elasticmachine commented Mar 28, 2024

alvarezmelissa87 commented Mar 28, 2024

alvarezmelissa87 commented Mar 28, 2024 •

edited

Loading

darnautov Apr 2, 2024 •

edited

Loading

alvarezmelissa87 Apr 2, 2024

darnautov Apr 2, 2024

alvarezmelissa87 Apr 2, 2024 •

edited

Loading

darnautov Apr 4, 2024

darnautov Apr 2, 2024

alvarezmelissa87 Apr 2, 2024

alvarezmelissa87 Apr 2, 2024

darnautov Apr 2, 2024

alvarezmelissa87 Apr 2, 2024 •

edited

Loading

darnautov Apr 2, 2024

alvarezmelissa87 Apr 2, 2024

alvarezmelissa87 commented Apr 2, 2024

alvarezmelissa87 commented Apr 2, 2024

peteharverson left a comment

darnautov Apr 4, 2024

darnautov left a comment

kibana-ci commented Apr 4, 2024

[ML] Single Metric Viewer embeddable: add open and edit panel actions #179364

[ML] Single Metric Viewer embeddable: add open and edit panel actions #179364

Conversation

alvarezmelissa87 commented Mar 25, 2024 • edited Loading

Summary

Screen shots

Checklist

walterra left a comment

Choose a reason for hiding this comment

walterra commented Mar 26, 2024

elasticmachine commented Mar 28, 2024

alvarezmelissa87 commented Mar 28, 2024

alvarezmelissa87 commented Mar 28, 2024 • edited Loading

darnautov Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alvarezmelissa87 Apr 2, 2024 • 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

alvarezmelissa87 Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alvarezmelissa87 commented Apr 2, 2024

alvarezmelissa87 commented Apr 2, 2024

peteharverson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

darnautov left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 4, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

History

alvarezmelissa87 commented Mar 25, 2024 •

edited

Loading

alvarezmelissa87 commented Mar 28, 2024 •

edited

Loading

darnautov Apr 2, 2024 •

edited

Loading

alvarezmelissa87 Apr 2, 2024 •

edited

Loading

alvarezmelissa87 Apr 2, 2024 •

edited

Loading