[ML] Decouple open-in-anomaly-explorer action #178729

darnautov · 2024-03-14T13:45:09Z

Summary

Decouples open-in-anomaly-explorer UI action from embeddable framework.

Modifies and exports helper utils from the embeddable plugin to convert embeddable inputs and outputs to APIs
Updates anomaly swim lane and anomaly charts embeddables to expose required API for the "Open in Anomaly Explorer" action

elasticmachine · 2024-03-14T13:45:15Z

Pinging @elastic/ml-ui (:ml)

nreese

As discussed offline, in x-pack/plugins/ml/public/ui_actions/open_in_anomaly_explorer_action.tsx, the action can no longer use embeddable.getInput and embeddable.getOutput. Instead, action will have to read values directly from embeddable. For example embeddable.jobIds. All embeddables that support this action will have to expose jobIds.

nreese · 2024-03-14T21:26:14Z

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


-        const { jobIds, timeRange, viewBy } = embeddable.getInput();
-        const { perPage, fromPage } = embeddable.getOutput();
+        const { localTimeRange, viewBy, jobIds, perPage, fromPage } = embeddable;


update from main, localTimeRange has been renamed to timeRange$

One more thing. embeddable.getInput().timeRange combined dashboard time range and any custom time range stored with the panel. I think when reading this you will need
const timeRange = embeddable.timeRange$?.value ?? embeddable.parentApi?.timeRange$?.value

Fixed in 4211c14

nreese · 2024-03-14T21:30:17Z

x-pack/plugins/ml/public/embeddables/anomaly_swimlane/anomaly_swimlane_embeddable.tsx

  constructor(
    initialInput: AnomalySwimlaneEmbeddableInput,
    public services: [CoreStart, MlDependencies, AnomalySwimlaneServices],
    parent?: IContainer
  ) {
    super(initialInput, services[2].anomalyDetectorService, services[1].data.dataViews, parent);
+
+    this.apiSubscriptions.add(


I think you will want to add distinctUntilKeyChanged so that BehaviorSubject is only updated when the key changes, and not everytime the output or input changes. You should just be able to use embeddableInputToSubject and embeddableOutputToSubject utility methods from src/plugins/embeddable/public/lib/embeddables/compatibility/embeddable_compatibility_utils.ts

Updated in fd352ad

…omaly-explorer-action

ThomThomson

Nice work! The actions are decoupled as expected, and the changes to the embeddable plugin look good.

Left a few small nits and cleanups. LGTM!

ThomThomson · 2024-03-18T20:42:03Z

x-pack/plugins/ml/public/embeddables/anomaly_swimlane/anomaly_swimlane_embeddable.tsx

@@ -41,12 +42,37 @@ export class AnomalySwimlaneEmbeddable extends AnomalyDetectionEmbeddable<
  private reload$ = new Subject<void>();
  public readonly type: string = ANOMALY_SWIMLANE_EMBEDDABLE_TYPE;

+  // API
+  public viewBy = new BehaviorSubject<string | undefined>(this.getInput().viewBy);


nit. If you're initializing these in the constructor via embeddableInputToSubject or embeddableOutputToSubject, you don't need to initialize them via new here. The helpers will handle that.

Good spot, leftovers from the original implementation, updated in 48d9c50

ThomThomson · 2024-03-18T20:42:21Z

x-pack/plugins/ml/public/embeddables/anomaly_swimlane/anomaly_swimlane_embeddable.tsx

+  public perPage = new BehaviorSubject<number | undefined>(this.getOutput().perPage);
+  public fromPage = new BehaviorSubject<number | undefined>(this.getOutput().fromPage);
+
+  private apiSubscriptions = new Subscription();


Is this used?

Yes, I pass it to embeddableInputToSubject and embeddableOutputToSubject to populate subscriptions and unsubscribe on destroy

ThomThomson · 2024-03-18T20:42:47Z

x-pack/plugins/ml/public/embeddables/anomaly_charts/anomaly_charts_embeddable.tsx

@@ -40,12 +41,25 @@ export class AnomalyChartsEmbeddable extends AnomalyDetectionEmbeddable<
  private reload$ = new Subject<void>();
  public readonly type: string = ANOMALY_EXPLORER_CHARTS_EMBEDDABLE_TYPE;

+  // API
+  public entityFields = new BehaviorSubject<MlEntityField[] | undefined>(


Why not use embeddableOutputToSubject?

Made it for one embeddable and forgot the other 🤦🏻 Updated in d4debca

ThomThomson · 2024-03-18T20:46:04Z

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

+}
+
+export interface SwimLaneDrilldownApi {
+  // Props from embeddable input


Do we need these comments? Ideally soon these won't have any connection to the legacy embeddable's input / output.

Did the cleanup in 22df822

ThomThomson · 2024-03-18T20:49:11Z

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

-      embeddable,
-    }: SwimLaneDrilldownContext | AnomalyChartsFieldSelectionContext) {
+    async isCompatible({ embeddable }: EmbeddableApiContext) {
+      if (!isApiCompatible(embeddable)) return false;


nit: Here, wouldn't it be simpler to call your more specific type guards like:

return isSwimLaneEmbeddableContext(context) || isAnomalyChartsEmbeddableContext(context)? That way you wouldn't need to have the separate isApiCompatible method which checks the same things?

I first wrote this isCompatible method looking at the example and added custom type guards later. Updated in dfaef99

kibana-ci · 2024-03-19T10:03:57Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: dfaef99

Failed CI Steps

FTR Configs #84

Test Failures

[job] [logs] FTR Configs #84 / serverless observability UI Dataset Quality Dataset quality summary updates active datasets and estimated data KPIs

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`embeddable`	451	460	+9

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	+1.4KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`embeddable`	9	11	+2

Page load bundle

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

id	before	after	diff
`embeddable`	65.6KB	66.1KB	+436.0B
`ml`	78.4KB	78.5KB	+139.0B
total			+575.0B

Unknown metric groups

API count

id	before	after	diff
`embeddable`	553	562	+9

History

💛 Build #197959 was flaky b2130b3
💔 Build #197941 failed 4211c14
💔 Build #197810 failed 0673c47
💛 Build #197686 was flaky 00fb5db

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

cc @darnautov

peteharverson

Tested the 'Open in Anomaly Explorer' action with embedded swim lane and anomaly charts in dashboards and cases and LGTM

decoupling open-in-anomaly-explorer action

00fb5db

darnautov requested a review from a team as a code owner March 14, 2024 13:45

darnautov added the :ml label Mar 14, 2024

darnautov self-assigned this Mar 14, 2024

darnautov added Feature:UIActions UI actions. These are client side only, not related to the server side actions.. Team:ML Team label for ML (also use :ml) v8.14.0 labels Mar 14, 2024

darnautov requested review from nreese, peteharverson and ThomThomson March 14, 2024 13:45

darnautov added the release_note:skip Skip the PR/issue when compiling release notes label Mar 14, 2024

darnautov marked this pull request as draft March 14, 2024 15:32

nreese reviewed Mar 14, 2024

View reviewed changes

darnautov added 3 commits March 14, 2024 21:28

update api

5f0c122

publishing subjects

f2886b1

publishing subject for anomaly charts

0673c47

darnautov marked this pull request as ready for review March 14, 2024 21:23

darnautov requested a review from nreese March 14, 2024 21:23

nreese reviewed Mar 14, 2024

View reviewed changes

darnautov added 3 commits March 18, 2024 11:48

Merge remote-tracking branch 'origin/main' into ml-migrate-open-in-an…

d291b52

…omaly-explorer-action

get time range

4211c14

use embeddable input and output utils

fd352ad

darnautov requested a review from a team as a code owner March 18, 2024 11:45

botelastic bot added the Feature:Embedding Embedding content via iFrame label Mar 18, 2024

darnautov requested a review from nreese March 18, 2024 11:46

fix ts issue, revert types

b2130b3

ThomThomson approved these changes Mar 18, 2024

View reviewed changes

ThomThomson added the project:embeddableRebuild label Mar 18, 2024

use embeddableOutputToSubject for chart embeddable

d4debca

darnautov added 3 commits March 19, 2024 09:35

fix initializers for APIs

48d9c50

cleanup comments

22df822

update isCompatible

dfaef99

darnautov mentioned this pull request Mar 19, 2024

[Embeddables Rebuild] Migrate Anomaly Swim Lane #174967

Closed

peteharverson approved these changes Mar 19, 2024

View reviewed changes

darnautov merged commit a9968cb into elastic:main Mar 19, 2024
17 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Mar 19, 2024

darnautov deleted the ml-migrate-open-in-anomaly-explorer-action branch March 19, 2024 13:26

ThomThomson mentioned this pull request Mar 22, 2024

[Embeddables Rebuild] Decouple other actions from Embeddable framework #175138

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Decouple open-in-anomaly-explorer action #178729

[ML] Decouple open-in-anomaly-explorer action #178729

darnautov commented Mar 14, 2024 •

edited

Loading

elasticmachine commented Mar 14, 2024

nreese left a comment

nreese Mar 14, 2024

nreese Mar 14, 2024 •

edited

Loading

darnautov Mar 18, 2024

nreese Mar 14, 2024

darnautov Mar 18, 2024

ThomThomson left a comment

ThomThomson Mar 18, 2024

darnautov Mar 19, 2024

ThomThomson Mar 18, 2024

darnautov Mar 19, 2024

ThomThomson Mar 18, 2024

darnautov Mar 19, 2024

ThomThomson Mar 18, 2024

darnautov Mar 19, 2024

ThomThomson Mar 18, 2024

darnautov Mar 19, 2024

kibana-ci commented Mar 19, 2024

API count

peteharverson left a comment

[ML] Decouple open-in-anomaly-explorer action #178729

[ML] Decouple open-in-anomaly-explorer action #178729

Conversation

darnautov commented Mar 14, 2024 • edited Loading

Summary

elasticmachine commented Mar 14, 2024

nreese left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nreese Mar 14, 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

ThomThomson 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

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

kibana-ci commented Mar 19, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

peteharverson left a comment

Choose a reason for hiding this comment

darnautov commented Mar 14, 2024 •

edited

Loading

nreese Mar 14, 2024 •

edited

Loading