Skip to content

synchronously register embeddables and actions to fix race condition causing flaky add panel test#268218

Merged
nreese merged 29 commits into
elastic:mainfrom
nreese:aiops_sync_registration
May 11, 2026
Merged

synchronously register embeddables and actions to fix race condition causing flaky add panel test#268218
nreese merged 29 commits into
elastic:mainfrom
nreese:aiops_sync_registration

Conversation

@nreese
Copy link
Copy Markdown
Contributor

@nreese nreese commented May 7, 2026

Closes #268101

Flaky test runner https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/12180

The reason for flaky tests is that plugins are registering actions asynchronously. This introduces a race condition where ADD_PANEL_TRIGGER actions may not be registered before Dashboard's "add panel" menu is rendered.

PR resolves the race condition by registering embeddables and actions synchronously.

PR tries to prevent issue like this from being re-introduced by throwing on embeddable registration after start method is run.

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 7, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 7, 2026

/ci

'annotation-and-navigationGroup',
'mlGroup',
'logs-aiopsGroup',
'mlGroup',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated expects because aiops is rendered before ml once race condition is resolved. These 2 groups both specify order of 0

Image

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 7, 2026

/ci

@kibanamachine
Copy link
Copy Markdown
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#12168

[❌] src/platform/plugins/shared/dashboard/test/scout/ui/parallel.playwright.config.ts (--arch stateful --domain classic): 0/50 tests passed.
[❌] src/platform/plugins/shared/dashboard/test/scout/ui/parallel.playwright.config.ts (--arch serverless --domain search): 0/50 tests passed.
[❌] src/platform/plugins/shared/dashboard/test/scout/ui/parallel.playwright.config.ts (--arch serverless --domain security_complete): 0/50 tests passed.
[❌] src/platform/plugins/shared/dashboard/test/scout/ui/parallel.playwright.config.ts (--arch serverless --domain observability_complete): 0/50 tests passed.
[✅] src/platform/plugins/shared/dashboard/test/scout/ui/parallel.playwright.config.ts (--arch serverless --domain observability_logs_essentials): 50/50 tests passed.

see run history

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 7, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 7, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 7, 2026

/ci

@nreese nreese changed the title [aiops] synchronously register embeddables and actions to fix flaky add panel test [aiops][slo] synchronously register embeddables and actions to fix flaky add panel test May 7, 2026
loadTestFile(require.resolve('./dashboard_settings'));
loadTestFile(require.resolve('./data_shared_attributes'));
loadTestFile(require.resolve('./dashboard_back_button'));
loadTestFile(require.resolve('./dashboard_panel_listing'));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed dashboard_panel_listing because it tests the same features covered by src/platform/plugins/shared/dashboard/test/scout/ui/parallel_tests/dashboard_add_panel_flyout.spec.ts

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 7, 2026

/ci

@nreese nreese added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting v9.5.0 labels May 8, 2026
@kibanamachine
Copy link
Copy Markdown
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#12180

[✅] src/platform/plugins/shared/dashboard/test/scout/ui/parallel.playwright.config.ts (--arch stateful --domain classic): 25/25 tests passed.
[✅] src/platform/plugins/shared/dashboard/test/scout/ui/parallel.playwright.config.ts (--arch serverless --domain search): 25/25 tests passed.
[✅] src/platform/plugins/shared/dashboard/test/scout/ui/parallel.playwright.config.ts (--arch serverless --domain security_complete): 25/25 tests passed.
[✅] src/platform/plugins/shared/dashboard/test/scout/ui/parallel.playwright.config.ts (--arch serverless --domain observability_complete): 25/25 tests passed.
[✅] src/platform/plugins/shared/dashboard/test/scout/ui/parallel.playwright.config.ts (--arch serverless --domain observability_logs_essentials): 25/25 tests passed.

see run history

@infra-vault-gh-plugin-prod
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Contributor

@baileycash-elastic baileycash-elastic left a comment

Choose a reason for hiding this comment

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

actionable obs changes lgtm, tested locally

@botelastic botelastic Bot added Feature:Drilldowns Embeddable panel Drilldowns Feature:Embedding Embedding content via iFrame Team:obs-presentation Focus: APM UI, Infra UI, Hosts UI, Universal Profiling, Obs Overview and left Navigation labels May 8, 2026
@infra-vault-gh-plugin-prod
Copy link
Copy Markdown

Pinging @elastic/obs-presentation-team (Team:obs-presentation)

Copy link
Copy Markdown
Contributor

@iblancof iblancof left a comment

Choose a reason for hiding this comment

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

obs-exploration code changesLGTM

@jgowdyelastic
Copy link
Copy Markdown
Member

I had already created a very similar PR for ML's AIOps plugin earlier this week which has now been merged.
Apologies for conflicts. I believe all of the AIOps changes in this PR can be removed.

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 8, 2026

I had already created a very similar #268001 for ML's AIOps plugin earlier this week which has now been merged.
Apologies for conflicts. I believe all of the AIOps changes in this PR can be removed.

Thanks for the heads up. I have removed all aiops changes

@nreese nreese requested a review from a team as a code owner May 8, 2026 14:30
alerting: 22371
alertingVTwo: 76778
apm: 27979
apm: 28027
Copy link
Copy Markdown
Contributor Author

@nreese nreese May 8, 2026

Choose a reason for hiding this comment

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

Increase expected because plugin was async importing ./embeddable/register_embeddables during setup method. This PR replaced the async import with a sync import to better reflect the actual page load metrics

Copy link
Copy Markdown
Contributor

@rmyz rmyz left a comment

Choose a reason for hiding this comment

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

obs-presentation changes LGTM

Copy link
Copy Markdown
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Presentation team Changes LGTM! Code only review

constructor() {}

public setup(core: CoreSetup<LinksStartDependencies>, plugins: LinksSetupDependencies) {
core.getStartServices().then(([_, deps]) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great to see this removed. No reason to get start contracts when they aren't used.

Copy link
Copy Markdown
Contributor

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

kbn/scout changes LGTM

@TamerlanG
Copy link
Copy Markdown
Contributor

Approved, but you would need to have to update the RSPack limits.yml too

@jgowdyelastic jgowdyelastic removed the request for review from a team May 11, 2026 08:21
@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented May 11, 2026

Approved, but you would need to have to update the RSPack limits.yml too

Asked in slack about how to determine RSPack limits and received the answer

"However, I would probably not require those updates, because that’s currently out of band, it’s an alternative, so we can’t keep prompting devs to update it"

With this prompt, I will leave RSPack limits unchanged

@kibanamachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Scout Lane #10 - stateful-classic / default / local-stateful-classic - Observability Landing Page (discover.isEsqlDefault enabled) - redirects to onboarding when no logs data exists
  • [job] [logs] Scout Lane #3 - stateful-classic / default / local-stateful-classic - Tags management — accessibility - tags management page a11y
  • [job] [logs] Jest Integration Tests #7 / workflow with wait step when duration is short should have correct workflow duration

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
slo 1344 1346 +2

Async chunks

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

id before after diff
apm 2.8MB 2.7MB -2.3KB
slo 1.2MB 1.2MB +631.0B
total -1.7KB

Page load bundle

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

id before after diff
apm 25.3KB 27.4KB +2.1KB
embeddable 17.7KB 17.9KB +231.0B
links 8.1KB 8.0KB -79.0B
presentationUtil 8.0KB 8.0KB +32.0B
slo 37.0KB 37.1KB +108.0B
total +2.4KB
Unknown metric groups

API count

id before after diff
presentationUtil 87 91 +4

async chunk count

id before after diff
apm 87 86 -1

ESLint disabled line counts

id before after diff
presentationUtil 7 6 -1

References to deprecated APIs

id before after diff
controls 1 4 +3
slo 19 20 +1
synthetics 44 46 +2
total +6

Total ESLint disabled count

id before after diff
presentationUtil 7 6 -1

Unreferenced deprecated APIs

id before after diff
controls 1 4 +3
slo 19 20 +1
synthetics 44 46 +2
total +6

History

@nreese nreese merged commit dac65d8 into elastic:main May 11, 2026
31 checks passed
clintandrewhall pushed a commit that referenced this pull request May 12, 2026
…causing flaky add panel test (#268218)

Closes #268101

Flaky test runner
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/12180

The reason for flaky tests is that plugins are registering actions
asynchronously. This introduces a race condition where
`ADD_PANEL_TRIGGER` actions may not be registered before Dashboard's
"add panel" menu is rendered.

PR resolves the race condition by registering embeddables and actions
synchronously.

PR tries to prevent issue like this from being re-introduced by throwing
on embeddable registration after start method is run.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 13, 2026
…causing flaky add panel test (elastic#268218)

Closes elastic#268101

Flaky test runner
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/12180

The reason for flaky tests is that plugins are registering actions
asynchronously. This introduces a race condition where
`ADD_PANEL_TRIGGER` actions may not be registered before Dashboard's
"add panel" menu is rendered.

PR resolves the race condition by registering embeddables and actions
synchronously.

PR tries to prevent issue like this from being re-introduced by throwing
on embeddable registration after start method is run.

---------

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:Drilldowns Embeddable panel Drilldowns Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes Team:obs-presentation Focus: APM UI, Infra UI, Hosts UI, Universal Profiling, Obs Overview and left Navigation Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failing test: Dashboard add panel flyout - renders add panel flyout

9 participants