Skip to content

[ml] synchronously register embeddables and actions to fix flaky add panel test#267325

Merged
nreese merged 4 commits into
elastic:mainfrom
nreese:issue_259576
May 5, 2026
Merged

[ml] synchronously register embeddables and actions to fix flaky add panel test#267325
nreese merged 4 commits into
elastic:mainfrom
nreese:issue_259576

Conversation

@nreese
Copy link
Copy Markdown
Contributor

@nreese nreese commented May 1, 2026 •
edited
Loading

closes #259576 and #259443

Image below shows failure case where Dashboard's "add panel" menu only contains 18 items instead of the expected 21. The reason for this failure is that the ml plugin registers actions asynchronously. This introduces a race condition where ml ADD_PANEL_TRIGGER actions are not registered before Dashboard's "add panel" menu is rendered.

PR resolves the race condition by registering embeddables and actions synchronously. The license check is moved from registration time to the action's isCompatible method and embeddable's buildEmbeddable method.

Screenshot 2026-05-01 at 11 08 30 AM

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 1, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 4, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 4, 2026

/ci

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented May 4, 2026

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
ml 4266 4268 +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 5.6MB 5.6MB +1.1KB

Page load bundle

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

id before after diff
ml 88.6KB 88.6KB -38.0B

History

@nreese nreese marked this pull request as ready for review May 4, 2026 15:42
@nreese nreese requested a review from a team as a code owner May 4, 2026 15:42
@nreese nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes Team:ML Team label for ML (also use :ml) t// backport:version Backport to applied version labels v9.5.0 labels May 4, 2026
@infra-vault-gh-plugin-prod
Copy link
Copy Markdown

infra-vault-gh-plugin-prod Bot commented May 4, 2026

Pinging @elastic/kibana-presentation (Team:Presentation)

@rbrtj rbrtj self-requested a review May 4, 2026 20:40
jgowdyelastic
jgowdyelastic approved these changes May 5, 2026
View reviewed changes
Copy link
Copy Markdown
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
Thanks for making these changes

@nreese nreese merged commit 3795d4b into elastic:main May 5, 2026
65 checks passed
@kibanamachine kibanamachine added backport:skip This PR does not require backporting and removed backport:version Backport to applied version labels labels May 5, 2026
@nreese nreese mentioned this pull request May 5, 2026
Closed
@github-actions github-actions Bot mentioned this pull request May 5, 2026
Closed
@nreese nreese mentioned this pull request May 6, 2026
Merged
nreese added a commit that referenced this pull request May 6, 2026
@nreese @kibanamachine
unskip add panel flyout tests (#267973)
2886e3b 
Closes #259443 and
#259576

### Problem

#267325 broke tests
`dashboard_panel_listing.spec.ts` and
`dashboard_panel_listing_obs_group.spec.ts`. At the time
#267325 merged, there was a bug in
CI where CI did not run dashboard tests when code changes were not in
dependency chain yet code changes affected registries and had a direct
impact on dashboard runtime behavior.

The reason the tests broke is that
#267325 changed the order in which
"Add panel" actions are registered. Before
#267325, ml actions where
registered async - so they got registered last. After
#267325, ml actions where
registered sync, so they go registered before some other actions.

Registration order mattered because there was a bug in
`dashboard_panel_listing.spec.ts` and
`dashboard_panel_listing_obs_group.spec.ts` tests. The logic to extract
`groups` used a map with `order` as the key. When there was a collision
on `order`, only the last value would be preserved. Changing the
registration order changed which value survived the order collision.

Code snippet showing how values where put into a map and a collision on
`order` would lose values.
```
    const panelGroupByOrder = new Map<string, string>();
    panelGroupData
      .filter((item): item is { order: string; groupTitle: string } =>
        Boolean(item.order && item.groupTitle)
      )
      .forEach((item) => panelGroupByOrder.set(item.order, item.groupTitle));

    return [...panelGroupByOrder.values()];
```

Image showing how 2 groups have the same order, thus altering test
results after action registration race condition resolved. Notice values
for `data-group-sort-order`.
<img height="400" alt="Screenshot 2026-05-05 at 2 09 51 PM"
src="https://github.com/user-attachments/assets/da8299c2-a080-4750-849b-f20235c1e4b5"
/>

### Solution

PR resolves tests by replacing `getPanelGroupOrder` with
`getAddPanelFlyoutGroups` and fixing `order` collision bug.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jgowdyelastic jgowdyelastic jgowdyelastic approved these changes

@rbrtj rbrtj Awaiting requested review from rbrtj

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:ML Team label for ML (also use :ml) t// Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v9.5.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Failing test: Dashboard panel listing (includes observability group) - renders panel groups and panel count

3 participants

@nreese @kibanamachine @jgowdyelastic