Skip to content

FTR - optimize service initialization#212421

Merged
pheyos merged 6 commits intoelastic:mainfrom
pheyos:ftr_optimize_service_init
Feb 27, 2025
Merged

FTR - optimize service initialization#212421
pheyos merged 6 commits intoelastic:mainfrom
pheyos:ftr_optimize_service_init

Conversation

@pheyos
Copy link
Member

@pheyos pheyos commented Feb 25, 2025 •
edited by kibanamachine
Loading

Summary

This PR optimizes the FTR service initialization by not loading UI service for API tests and by removing retries during test user setup

Changes

  • Remove loading of common UI services from common services (UI services should not be loaded for API tests)
  • Move security service from @kbn/ftr-common-functional-ui-services to @kbn/ftr-common-functional-services as it should be available to API tests as well
  • Only try once to delete testUser during init (this user usually does not exist on a fresh deployment - and if it does, a single delete request is enough to get rid of it)

Benchmark results

These changes will reduce FTR CI runtime overall by ~100 minutes 🚀
Due to parallel workers in CI, the effective runtime of the whole CI job will be less than that.

  • The removal of UI service loading (which includes starting a browser instance) for API tests reduces init time by ~0.5 seconds. With 313 API configs that are started on CI, this reduces the runtime overall by ~156 seconds / ~2.6 minutes.
  • The removal of test user delete retries reduces init time by ~10 seconds. With 589 FTR configs that are started on CI, this reduces the runtime overall by ~5890 seconds / ~98 minutes.
  • These numbers have been taken on a local machine and since CI workers are usually slower, we should see at least this amount of improvement if not more in CI.

@pheyos pheyos self-assigned this Feb 25, 2025
pheyos
pheyos commented Feb 26, 2025
View reviewed changes
src/platform/packages/shared/kbn-ftr-common-functional-services/services/security/test_user.ts
private readonly browser =
// browser service is not normally available in common.
this.ctx.hasService('browser') ? (this.ctx.getService('browser' as any) as Browser) : undefined;
this.ctx.hasService('browser') ? this.ctx.getService('browser' as any) : undefined;
Copy link
Member Author

@pheyos pheyos Feb 26, 2025

Choose a reason for hiding this comment

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

The construction here was not ideal: the service tries to determine if it's loaded for a UI suite and in that case adds steps in the UI after the test user role has been updated. Importing the types from the UI services would introduce a cyclic dependency. I've decided to remove the typing here as it's only a few lines of code making use of it. The alternative would probably be to move the browser and test subject services to yet another package so we can import from this common package as well as from the UI package, but to me that seemed too much effort for the gained benefit.

@pheyos pheyos added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 backport:version Backport to applied version labels v9.1.0 v8.19.0 labels Feb 26, 2025
@pheyos pheyos marked this pull request as ready for review February 26, 2025 14:56
@pheyos pheyos requested a review from a team February 26, 2025 14:56
@pheyos pheyos requested review from a team as code owners February 26, 2025 14:56
@pheyos pheyos requested a review from machadoum February 26, 2025 14:56
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Feb 26, 2025
@elasticmachine
Copy link
Contributor

elasticmachine commented Feb 26, 2025

Pinging @elastic/fleet (Team:Fleet)

machadoum
machadoum approved these changes Feb 27, 2025
View reviewed changes
Copy link
Member

@machadoum machadoum left a comment

Choose a reason for hiding this comment

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

Entity Analytics team changes LGTM!

@elasticmachine
Copy link
Contributor

elasticmachine commented Feb 27, 2025

💚 Build Succeeded

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
@kbn/ftr-common-functional-services 89 98 +9
@kbn/ftr-common-functional-ui-services 524 515 -9
total -0

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
@kbn/ftr-common-functional-services 1 5 +4
@kbn/ftr-common-functional-ui-services 7 3 -4
total -0
Unknown metric groups

API count

id before after diff
@kbn/ftr-common-functional-services 114 123 +9
@kbn/ftr-common-functional-ui-services 564 555 -9
total -0

History

cc @pheyos

@pheyos pheyos merged commit 7a381af into elastic:main Feb 27, 2025
10 checks passed
@pheyos pheyos deleted the ftr_optimize_service_init branch February 27, 2025 10:35
@kibanamachine
Copy link
Contributor

kibanamachine commented Feb 27, 2025

Starting backport for target branches: 8.x, 9.0

https://github.com/elastic/kibana/actions/runs/13564395702

@kibanamachine
Copy link
Contributor

kibanamachine commented Feb 27, 2025

💔 All backports failed

Status Branch Result
❌ 8.x Backport failed because of merge conflicts

You might need to backport the following PRs to 8.x:
- SKA: Relocate "platform" packages that remain on /packages (#208704)
❌ 9.0 Backport failed because of merge conflicts

You might need to backport the following PRs to 9.0:
- [Data View Field] Fix popularity score bugs (#211201)
- Upgrade EUI to v99.3.0-borealis.0 (#211671)
- Update langchain (main) (#205553)
- [refactoring] Distinguish User Controls from Risk Engine in DashboardEnablementPanel (#212441)
- Move Findings Flyout to Security Solution or Shared Package Phase 1 (#212053)
- [Perfomance] Add is_initial_load meta (#206645)
- [Dashboard Navigation] Unskip tests (#211660)

Manual backport

To create the backport manually run:

node scripts/backport --pr 212421

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine mentioned this pull request Feb 28, 2025
Merged
7 tasks
@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 3, 2025

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 212421 locally

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 3, 2025
@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 4, 2025

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 212421 locally

@pheyos pheyos mentioned this pull request Mar 5, 2025
Merged
@pheyos
Copy link
Member Author

pheyos commented Mar 5, 2025

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

pheyos added a commit to pheyos/kibana that referenced this pull request Mar 5, 2025
@pheyos
FTR - optimize service initialization (elastic#212421)
626ffca 
## Summary

This PR optimizes the FTR service initialization by not loading UI
service for API tests and by removing retries during test user setup

## Changes

- Remove loading of common UI services from common services (UI services
should not be loaded for API tests)
- Move `security` service from `@kbn/ftr-common-functional-ui-services`
to `@kbn/ftr-common-functional-services` as it should be available to
API tests as well
- Only try once to delete `testUser` during init (this user usually does
not exist on a fresh deployment - and if it does, a single delete
request is enough to get rid of it)

## Benchmark results

**These changes will reduce FTR CI runtime overall by ~100 minutes**
:rocket:
Due to parallel workers in CI, the effective runtime of the whole CI job
will be less than that.

- The removal of UI service loading (which includes starting a browser
instance) for API tests reduces init time by ~0.5 seconds. With 313 API
configs that are started on CI, this reduces the runtime overall by ~156
seconds / ~2.6 minutes.
- The removal of test user delete retries reduces init time by ~10
seconds. With 589 FTR configs that are started on CI, this reduces the
runtime overall by ~5890 seconds / ~98 minutes.
- These numbers have been taken on a local machine and since CI workers
are usually slower, we should see at least this amount of improvement if
not more in CI.

(cherry picked from commit 7a381af)

# Conflicts:
#	src/platform/packages/shared/kbn-ftr-common-functional-services/index.ts
@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 5, 2025

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

pheyos added a commit that referenced this pull request Mar 5, 2025
@pheyos
[8.x] FTR - optimize service initialization (#212421) (#213213)
33b3901 
# Backport

This will backport the following commits from `main` to `8.x`:
- [FTR - optimize service initialization
(#212421)](#212421)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Robert
Oskamp","email":"robert.oskamp@elastic.co"},"sourceCommit":{"committedDate":"2025-02-27T10:35:47Z","message":"FTR
- optimize service initialization (#212421)\n\n## Summary\n\nThis PR
optimizes the FTR service initialization by not loading UI\nservice for
API tests and by removing retries during test user setup\n\n##
Changes\n\n- Remove loading of common UI services from common services
(UI services\nshould not be loaded for API tests)\n- Move `security`
service from `@kbn/ftr-common-functional-ui-services`\nto
`@kbn/ftr-common-functional-services` as it should be available to\nAPI
tests as well\n- Only try once to delete `testUser` during init (this
user usually does\nnot exist on a fresh deployment - and if it does, a
single delete\nrequest is enough to get rid of it)\n\n## Benchmark
results\n\n**These changes will reduce FTR CI runtime overall by ~100
minutes**\n:rocket:\nDue to parallel workers in CI, the effective
runtime of the whole CI job\nwill be less than that.\n\n- The removal of
UI service loading (which includes starting a browser\ninstance) for API
tests reduces init time by ~0.5 seconds. With 313 API\nconfigs that are
started on CI, this reduces the runtime overall by ~156\nseconds / ~2.6
minutes.\n- The removal of test user delete retries reduces init time by
~10\nseconds. With 589 FTR configs that are started on CI, this reduces
the\nruntime overall by ~5890 seconds / ~98 minutes.\n- These numbers
have been taken on a local machine and since CI workers\nare usually
slower, we should see at least this amount of improvement if\nnot more
in
CI.","sha":"7a381afb177c81ed4d0704cfcec49e6da7179376","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport
missing","Team:Fleet","v9.0.0","backport:version","v9.1.0","v8.19.0"],"title":"FTR
- optimize service
initialization","number":212421,"url":"https://github.com/elastic/kibana/pull/212421","mergeCommit":{"message":"FTR
- optimize service initialization (#212421)\n\n## Summary\n\nThis PR
optimizes the FTR service initialization by not loading UI\nservice for
API tests and by removing retries during test user setup\n\n##
Changes\n\n- Remove loading of common UI services from common services
(UI services\nshould not be loaded for API tests)\n- Move `security`
service from `@kbn/ftr-common-functional-ui-services`\nto
`@kbn/ftr-common-functional-services` as it should be available to\nAPI
tests as well\n- Only try once to delete `testUser` during init (this
user usually does\nnot exist on a fresh deployment - and if it does, a
single delete\nrequest is enough to get rid of it)\n\n## Benchmark
results\n\n**These changes will reduce FTR CI runtime overall by ~100
minutes**\n:rocket:\nDue to parallel workers in CI, the effective
runtime of the whole CI job\nwill be less than that.\n\n- The removal of
UI service loading (which includes starting a browser\ninstance) for API
tests reduces init time by ~0.5 seconds. With 313 API\nconfigs that are
started on CI, this reduces the runtime overall by ~156\nseconds / ~2.6
minutes.\n- The removal of test user delete retries reduces init time by
~10\nseconds. With 589 FTR configs that are started on CI, this reduces
the\nruntime overall by ~5890 seconds / ~98 minutes.\n- These numbers
have been taken on a local machine and since CI workers\nare usually
slower, we should see at least this amount of improvement if\nnot more
in
CI.","sha":"7a381afb177c81ed4d0704cfcec49e6da7179376"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/212421","number":212421,"mergeCommit":{"message":"FTR
- optimize service initialization (#212421)\n\n## Summary\n\nThis PR
optimizes the FTR service initialization by not loading UI\nservice for
API tests and by removing retries during test user setup\n\n##
Changes\n\n- Remove loading of common UI services from common services
(UI services\nshould not be loaded for API tests)\n- Move `security`
service from `@kbn/ftr-common-functional-ui-services`\nto
`@kbn/ftr-common-functional-services` as it should be available to\nAPI
tests as well\n- Only try once to delete `testUser` during init (this
user usually does\nnot exist on a fresh deployment - and if it does, a
single delete\nrequest is enough to get rid of it)\n\n## Benchmark
results\n\n**These changes will reduce FTR CI runtime overall by ~100
minutes**\n:rocket:\nDue to parallel workers in CI, the effective
runtime of the whole CI job\nwill be less than that.\n\n- The removal of
UI service loading (which includes starting a browser\ninstance) for API
tests reduces init time by ~0.5 seconds. With 313 API\nconfigs that are
started on CI, this reduces the runtime overall by ~156\nseconds / ~2.6
minutes.\n- The removal of test user delete retries reduces init time by
~10\nseconds. With 589 FTR configs that are started on CI, this reduces
the\nruntime overall by ~5890 seconds / ~98 minutes.\n- These numbers
have been taken on a local machine and since CI workers\nare usually
slower, we should see at least this amount of improvement if\nnot more
in
CI.","sha":"7a381afb177c81ed4d0704cfcec49e6da7179376"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 5, 2025
@pheyos pheyos removed the v9.0.0 label Mar 6, 2025
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Mar 22, 2025
@pheyos @CAWilson94
FTR - optimize service initialization (elastic#212421)
2823959 
## Summary

This PR optimizes the FTR service initialization by not loading UI
service for API tests and by removing retries during test user setup

## Changes

- Remove loading of common UI services from common services (UI services
should not be loaded for API tests)
- Move `security` service from `@kbn/ftr-common-functional-ui-services`
to `@kbn/ftr-common-functional-services` as it should be available to
API tests as well
- Only try once to delete `testUser` during init (this user usually does
not exist on a fresh deployment - and if it does, a single delete
request is enough to get rid of it)

## Benchmark results

**These changes will reduce FTR CI runtime overall by ~100 minutes**
:rocket:
Due to parallel workers in CI, the effective runtime of the whole CI job
will be less than that.

- The removal of UI service loading (which includes starting a browser
instance) for API tests reduces init time by ~0.5 seconds. With 313 API
configs that are started on CI, this reduces the runtime overall by ~156
seconds / ~2.6 minutes.
- The removal of test user delete retries reduces init time by ~10
seconds. With 589 FTR configs that are started on CI, this reduces the
runtime overall by ~5890 seconds / ~98 minutes.
- These numbers have been taken on a local machine and since CI workers
are usually slower, we should see at least this amount of improvement if
not more in CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@machadoum machadoum machadoum approved these changes

@crespocarlos crespocarlos crespocarlos approved these changes

@jbudz jbudz jbudz approved these changes

@jloleysens jloleysens jloleysens approved these changes

@JordanSh JordanSh JordanSh approved these changes

@juliaElastic juliaElastic juliaElastic approved these changes

+1 more reviewer

@wayneseymour wayneseymour wayneseymour approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@pheyos pheyos

Labels

backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.19.0 v9.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@pheyos @elasticmachine @kibanamachine @machadoum @crespocarlos @jbudz @wayneseymour @jloleysens @JordanSh @juliaElastic