Skip to content

[Refactor][Performance] Migrate addTriggerAction to addTriggerActionAsync#227593

Merged
mbondyra merged 22 commits intoelastic:mainfrom
mbondyra:presentation/migrate_add_trigger_action
Jul 16, 2025
Merged

[Refactor][Performance] Migrate addTriggerAction to addTriggerActionAsync#227593
mbondyra merged 22 commits intoelastic:mainfrom
mbondyra:presentation/migrate_add_trigger_action

Conversation

@mbondyra
Copy link
Copy Markdown
Contributor

@mbondyra mbondyra commented Jul 11, 2025

Summary

Just a few migrations to lower the page load. Part of #227743

@mbondyra mbondyra added Feature:Input Control Input controls visualization Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// labels Jul 11, 2025
@mbondyra mbondyra requested a review from a team as a code owner July 11, 2025 09:42
@mbondyra mbondyra added the release_note:skip Skip the PR/issue when compiling release notes label Jul 11, 2025
@mbondyra mbondyra requested review from a team as code owners July 11, 2025 09:42
@mbondyra mbondyra requested a review from a team as a code owner July 11, 2025 09:42
@mbondyra mbondyra added backport:version Backport to applied version labels v9.2.0 labels Jul 11, 2025
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@mbondyra mbondyra force-pushed the presentation/migrate_add_trigger_action branch from 42d456b to 07e4ba4 Compare July 11, 2025 09:42
@mbondyra mbondyra requested a review from a team as a code owner July 11, 2025 09:42
Copy link
Copy Markdown
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kibana-presentation changes LGTM. Does this migrate all actions? Could deprecated addTriggerAction and registerAction methods be removed?
code review only

@mbondyra
Copy link
Copy Markdown
Contributor Author

mbondyra commented Jul 11, 2025

@nreese Unfortunately not yet — there are still quite a few in ML and other areas. I only removed the ones from the Analyst Experience team.
For our team, some dev examples still use the sync version. I’ll check with Ola if she wants to pick up the rest — it’s a good opportunity for her to learn more about uiActions.
I’ve also created an umbrella issue to ask the other teams to handle their rewrites — in the end, everyone wins with these changes.

@nreese
Copy link
Copy Markdown
Contributor

nreese commented Jul 11, 2025

Unfortunately not yet — there are still quite a few in ML and other areas. I only removed the ones from the Analyst Experience team.

Thanks for checking.

@mbondyra mbondyra force-pushed the presentation/migrate_add_trigger_action branch from 6347f63 to 9ea265b Compare July 12, 2025 13:28
@mbondyra mbondyra requested a review from a team as a code owner July 15, 2025 08:53
@mbondyra mbondyra requested a review from a team as a code owner July 15, 2025 09:11
Copy link
Copy Markdown
Contributor

@janmonschke janmonschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kibana-cases changes lgtm

@darnautov darnautov self-requested a review July 15, 2025 10:10
Copy link
Copy Markdown
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AIOps changes tested and LGTM, but I wonder if it'd be better to create an aggregated export file to reduce a number of asynchronous chunks?

@mbondyra
Copy link
Copy Markdown
Contributor Author

AIOps changes tested and LGTM, but I wonder if it'd be better to create an aggregated export file to reduce a number of asynchronous chunks?

Hey @darnautov it crossed my mind tbh. I think they are so small that it's ok we'll create just one chunk where all actions will be loaded after one is requested, but opted for simplicity here. I'll add a commit to fix it. Thanks!

Copy link
Copy Markdown
Member

@umbopepato umbopepato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactored actions work correctly, RO code changes LGTM!
Added a small suggestion 🙂

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
aiops 599 600 +1
cases 1140 1141 +1
data 536 537 +1
total +3

Async chunks

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

id before after diff
aiops 504.4KB 512.6KB +8.2KB
cases 1.3MB 1.3MB +1.3KB
data 51.6KB 61.5KB +9.8KB
discoverEnhanced 58.0KB 62.8KB +4.8KB
inputControlVis 98.1KB 100.5KB +2.3KB
lens 1.5MB 1.5MB +1.3KB
reporting 160.7KB 166.2KB +5.5KB
total +33.3KB

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
data 29 30 +1

Page load bundle

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

id before after diff
aiops 17.6KB 13.4KB -4.2KB
cases 136.0KB 135.3KB -752.0B
data 402.8KB 395.2KB -7.6KB
discoverEnhanced 9.1KB 5.1KB -4.0KB
inputControlVis 8.7KB 6.8KB -1.9KB
lens 59.3KB 58.5KB -780.0B
reporting 54.0KB 49.1KB -4.9KB
total -24.1KB
Unknown metric groups

async chunk count

id before after diff
aiops 39 40 +1
cases 37 38 +1
data 3 5 +2
discoverEnhanced 1 2 +1
inputControlVis 4 5 +1
lens 18 19 +1
reporting 7 8 +1
total +8

References to deprecated APIs

id before after diff
aiops 6 1 -5
cases 32 31 -1
data 32 29 -3
discoverEnhanced 2 0 -2
inputControlVis 11 10 -1
lens 76 75 -1
reporting 9 8 -1
total -14

History

@mbondyra mbondyra merged commit 9a1b3f0 into elastic:main Jul 16, 2025
12 checks passed
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 18, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 227593 locally
cc: @mbondyra

1 similar comment
@kibanamachine
Copy link
Copy Markdown
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 227593 locally
cc: @mbondyra

@mbondyra mbondyra added the backport:skip This PR does not require backporting label Jul 21, 2025
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 21, 2025
@mbondyra mbondyra removed the backport:version Backport to applied version labels label Jul 21, 2025
Bluefinger pushed a commit to Bluefinger/kibana that referenced this pull request Jul 22, 2025
# Summary
Hi Operations team! 👋

Recently I've worked on a
[few](elastic#227593)
[PRs](elastic#226052) that reduce our
page load bundle size, and while doing so I noticed that many of our
limits are much higher than the actual page load sizes. It made me
think: these limits should be realistic, because they’re one of the main
ways we protect page load performance.
Right now, based on the current limits, we theoretically allow up to
11,452,104 bytes (~11 MB) to load upfront. But when we check the real
page load, it’s only 5.192.979 bytes — about half of that.

We already have a `--update-limits` flag for the `node
scripts/build_kibana_platform_plugins.js --update-limits` command, but
it only bumps limits up when the size grows above the limit, adding a
flat 15 KB buffer to the bundle size.
I’d like to propose:
1. Allowing the `--update-limits` to allow folks to also lower limits
when the bundle shrinks drastically
2. Replacing the flat 15 KB buffer with 10% of the plugin’s size, so the
buffer scales realistically. Right now, with ~197 plugins,
`--update-limits` allows for an extra (15KB*197=) 3 MB above the total
page size — which is way too much in my opinion!

## What’s in this PR
✅ Adds logic to let us lower limits automatically, not just bump them up
with `node scripts/build_kibana_platform_plugins.js --update-limits`
✅ Replaces the flat +15 KB bump with a 10% buffer relative to each
plugin’s size when using `node scripts/build_kibana_platform_plugins.js
--update-limits`
✅ Updated the limits.yml with the result from the above script

## Why it matters
1. Keeps bundle sizes tight
2. Protects us from accidental regressions
Bluefinger pushed a commit to Bluefinger/kibana that referenced this pull request Jul 22, 2025
…sync (elastic#227593)

## Summary

Just a few migrations to lower the page load. Part of
elastic#227743

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
kertal pushed a commit to kertal/kibana that referenced this pull request Jul 25, 2025
# Summary
Hi Operations team! 👋

Recently I've worked on a
[few](elastic#227593)
[PRs](elastic#226052) that reduce our
page load bundle size, and while doing so I noticed that many of our
limits are much higher than the actual page load sizes. It made me
think: these limits should be realistic, because they’re one of the main
ways we protect page load performance.
Right now, based on the current limits, we theoretically allow up to
11,452,104 bytes (~11 MB) to load upfront. But when we check the real
page load, it’s only 5.192.979 bytes — about half of that.

We already have a `--update-limits` flag for the `node
scripts/build_kibana_platform_plugins.js --update-limits` command, but
it only bumps limits up when the size grows above the limit, adding a
flat 15 KB buffer to the bundle size.
I’d like to propose:
1. Allowing the `--update-limits` to allow folks to also lower limits
when the bundle shrinks drastically
2. Replacing the flat 15 KB buffer with 10% of the plugin’s size, so the
buffer scales realistically. Right now, with ~197 plugins,
`--update-limits` allows for an extra (15KB*197=) 3 MB above the total
page size — which is way too much in my opinion!

## What’s in this PR
✅ Adds logic to let us lower limits automatically, not just bump them up
with `node scripts/build_kibana_platform_plugins.js --update-limits`
✅ Replaces the flat +15 KB bump with a 10% buffer relative to each
plugin’s size when using `node scripts/build_kibana_platform_plugins.js
--update-limits`
✅ Updated the limits.yml with the result from the above script

## Why it matters
1. Keeps bundle sizes tight
2. Protects us from accidental regressions
kertal pushed a commit to kertal/kibana that referenced this pull request Jul 25, 2025
…sync (elastic#227593)

## Summary

Just a few migrations to lower the page load. Part of
elastic#227743

---------

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

Labels

backport:skip This PR does not require backporting Feature:Input Control Input controls visualization Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants