Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Onboarding] Stack - update index management breadcrumbs #209599

Open
wants to merge 53 commits into
base: main
Choose a base branch
from

Conversation

saarikabhasi
Copy link
Member

@saarikabhasi saarikabhasi commented Feb 4, 2025

Summary

Requirement:
In stack and when its search solution space, we need to update search index details breadcrumbs, when navigated via Content -> Index Management :

  • Index management list page - Content / Index Management / Indices
  • Index list page - Content / Index Management / indices / <index_name>
  • drop Stack management from the breadcrumb

In Classic nav, index management index details page breadcrumbs will have no change

Solutions

Currently, Index management app is rendered from management_app. The management app sets breadcrumbs for all the dependant apps. The easiest way to implement is to set breadcrumbs based on active solution type - es but this would alter breadcrumbs when index management app is rendered from side nav footer ( management -> index management) and other related management apps as well.

Other options is to modify setBreadcrumbs in ManagementAppMountParams but the setBreadcrumbs is used by multiple other apps.

In this PR, index management app is mounted via search indices plugin. In this way we can customize breadcrumbs for index management when rendered from search_indices plugin. When its search solution type, index management app will work independently from management app.

Screenshots

Search solution Nav - Changed breadcrumb ( dropped stack management & added index name)

Screenshot 2025-02-04 at 1 29 08 PM

Serverless

Note: No change in functionality from this PR. Added for additional info

index details page breadcrumbs should be Data/ Index Management / Indices/<index_name>
index list page breadcrumbs should be Data/ Index Management / Indices/

Serverless Details page
Screenshot 2025-02-04 at 1 23 14 PM

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

@saarikabhasi saarikabhasi force-pushed the onboarding/update-breadcrumbs-indexmanagement branch from 15172fd to 42e6684 Compare February 4, 2025 17:34
@saarikabhasi saarikabhasi changed the title Onboarding/update breadcrumbs indexmanagement [Onboarding] Stack - update index management breadcrumbs Feb 4, 2025
@saarikabhasi saarikabhasi added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) Team:Search labels Feb 4, 2025
@saarikabhasi saarikabhasi marked this pull request as ready for review February 4, 2025 17:39
@saarikabhasi saarikabhasi requested review from a team as code owners February 4, 2025 17:39
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

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

[❌] x-pack/test/functional_search/config.ts: 0/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group1.ts: 25/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/search/config.ts: 25/25 tests passed.

see run history

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

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

[❌] x-pack/test/functional_search/config.ts: 0/25 tests passed.
[❌] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group1.ts: 24/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/search/config.ts: 25/25 tests passed.

see run history

@saarikabhasi saarikabhasi force-pushed the onboarding/update-breadcrumbs-indexmanagement branch from a73ede5 to b5ed052 Compare February 27, 2025 20:28
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

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

[❌] x-pack/test/functional_solution_sidenav/config.ts: 0/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group1.ts: 25/25 tests passed.
[❌] x-pack/test_serverless/functional/test_suites/search/config.ts: 0/25 tests passed.
[✅] x-pack/test/functional_search/config.ts: 25/25 tests passed.

see run history

@saarikabhasi
Copy link
Member Author

@mattkime I have updated this PR and is ready for review. In this solution, search_indices plugin registers a new application for index management app and the breadcrumbs logic is handled via search indices plugin. Could you please take a look again, thanks!

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

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

[✅] x-pack/test/functional_solution_sidenav/config.ts: 25/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/search/config.ts: 25/25 tests passed.
[✅] x-pack/test/functional_search/config.ts: 25/25 tests passed.
[❌] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group1.ts: 3/25 tests passed.

see run history

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
searchIndices 338 340 +2

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/deeplinks-search 19 20 +1
@kbn/index-management-shared-types 126 133 +7
indexManagement 239 241 +2
searchIndices 18 19 +1
total +11

Async chunks

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

id before after diff
enterpriseSearch 1.3MB 1.3MB -20.0B
searchIndices 180.8KB 182.8KB +2.1KB
total +2.1KB

Page load bundle

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

id before after diff
indexManagement 29.4KB 29.7KB +349.0B
searchIndices 8.6KB 9.0KB +503.0B
searchNavigation 4.8KB 4.8KB +34.0B
serverlessSearch 25.5KB 25.6KB +6.0B
total +892.0B
Unknown metric groups

API count

id before after diff
@kbn/deeplinks-search 19 20 +1
@kbn/index-management-shared-types 126 133 +7
indexManagement 244 246 +2
searchIndices 18 19 +1
total +11

async chunk count

id before after diff
searchIndices 4 6 +2

History

@sphilipse sphilipse added backport:version Backport to applied version labels and removed v9.0.0 backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) labels Mar 4, 2025
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

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

[❌] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group1.ts: 1/25 tests passed.
[❌] x-pack/test/functional_search/config.ts: 24/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/search/config.ts: 25/25 tests passed.
[✅] x-pack/test/functional_solution_sidenav/config.ts: 25/25 tests passed.

see run history

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

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

[❌] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group1.ts: 22/25 tests passed.
[❌] x-pack/test/functional_search/config.ts: 24/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/search/config.ts: 25/25 tests passed.
[✅] x-pack/test/functional_solution_sidenav/config.ts: 25/25 tests passed.

see run history

@saarikabhasi
Copy link
Member Author

Flaky Test Runner Stats

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

[❌] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group1.ts: 22/25 tests passed.
[❌] x-pack/test/functional_search/config.ts: 24/25 tests passed.
[✅] x-pack/test_serverless/functional/test_suites/search/config.ts: 25/25 tests passed.
[✅] x-pack/test/functional_solution_sidenav/config.ts: 25/25 tests passed.

This seems to be unrelated failures, retrying build succeeds failed FTRs and I cannot reproduce the errors locally.

Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

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

I'm mostly curious about the relationship between ManagementAppMountParams and SearchIndicesAppMountParams types. I think they should be the same unless there's something I'm missing.

@@ -129,7 +130,7 @@ export async function mountManagementSection({
}: {
coreSetup: CoreSetup<StartDependencies>;
usageCollection: UsageCollectionSetup;
params: ManagementAppMountParams;
params: ManagementAppMountParams | SearchIndicesAppMountParams;
Copy link
Contributor

Choose a reason for hiding this comment

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

I left another similar comment before, got a bit confused and deleted it BUT - I'm confused why there needs to be different types here. It seems like they should be the same or at least extremely similar.

Copy link
Member Author

Choose a reason for hiding this comment

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

SearchIndicesAppMountParams is a subset of ManagementAppMountParams. The ManagementAppMountParams has few required field which would be undefined when mounting from search_indices plugin. This would cause type_check error.
Also from the code, I understand those fields are not used by mount_management_section in index management.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think other way to simplify, is to create a new type IndexManagementAppMountParams which is a subset of ManagementAppMountParams and replace mount_management_section to use this type instead of ManagementAppMountParams. I will try that and confirm nothing breaks and update this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Search v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants