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

[Dashboard] Cleanup services #193644

Merged

Conversation

Heenawter
Copy link
Contributor

@Heenawter Heenawter commented Sep 20, 2024

Closes #167437

Summary

This PR refactors the Dashboard services to no longer use the PluginServiceProvider from the PresentationUtil plugin.

Checklist

For maintainers

@Heenawter Heenawter added Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:medium Medium Level of Effort technical debt Improvement of the software architecture and operational architecture release_note:skip Skip the PR/issue when compiling release notes impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Sep 20, 2024
Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM

@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 26, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: c80fb1e
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-193644-c80fb1ea643d

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dashboard 562 530 -32

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
dashboard 123 126 +3

Async chunks

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

id before after diff
dashboard 490.5KB 473.7KB -16.8KB

Page load bundle

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

id before after diff
dashboard 26.9KB 27.7KB +870.0B
Unknown metric groups

API count

id before after diff
dashboard 128 131 +3

async chunk count

id before after diff
dashboard 15 16 +1

References to deprecated APIs

id before after diff
dashboard 40 36 -4

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@Heenawter Heenawter requested a review from nreese September 26, 2024 17:14
Copy link
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
code review only

@Heenawter Heenawter merged commit ce08d4e into elastic:main Sep 26, 2024
25 checks passed
@Heenawter Heenawter deleted the cleanup-dashboard-services_2024-09-19 branch September 26, 2024 18:10
Heenawter added a commit to Heenawter/kibana that referenced this pull request Sep 26, 2024
Closes elastic#167437

## Summary

This PR refactors the Dashboard services to no longer use the
`PluginServiceProvider` from the `PresentationUtil` plugin.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit ce08d4e)

# Conflicts:
#	src/plugins/dashboard/public/services/saved_objects_tagging/saved_objects_tagging.stub.ts
#	src/plugins/dashboard/public/services/saved_objects_tagging/saved_objects_tagging_service.ts
#	src/plugins/dashboard/public/services/saved_objects_tagging/types.ts
@Heenawter
Copy link
Contributor Author

💚 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

Heenawter added a commit that referenced this pull request Sep 26, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Dashboard] Cleanup services
(#193644)](#193644)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Hannah
Mudge","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-26T18:10:33Z","message":"[Dashboard]
Cleanup services (#193644)\n\nCloses
https://github.com/elastic/kibana/issues/167437\r\n\r\n##
Summary\r\n\r\nThis PR refactors the Dashboard services to no longer use
the\r\n`PluginServiceProvider` from the `PresentationUtil`
plugin.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"ce08d4e37343d425580a74f6e8a95ff443971a95","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Dashboard","Team:Presentation","loe:medium","technical
debt","release_note:skip","impact:high","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services","apm:review"],"number":193644,"url":"https://github.com/elastic/kibana/pull/193644","mergeCommit":{"message":"[Dashboard]
Cleanup services (#193644)\n\nCloses
https://github.com/elastic/kibana/issues/167437\r\n\r\n##
Summary\r\n\r\nThis PR refactors the Dashboard services to no longer use
the\r\n`PluginServiceProvider` from the `PresentationUtil`
plugin.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"ce08d4e37343d425580a74f6e8a95ff443971a95"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193644","number":193644,"mergeCommit":{"message":"[Dashboard]
Cleanup services (#193644)\n\nCloses
https://github.com/elastic/kibana/issues/167437\r\n\r\n##
Summary\r\n\r\nThis PR refactors the Dashboard services to no longer use
the\r\n`PluginServiceProvider` from the `PresentationUtil`
plugin.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"ce08d4e37343d425580a74f6e8a95ff443971a95"}}]}]
BACKPORT-->
Heenawter added a commit that referenced this pull request Oct 7, 2024
Closes #194733

## Summary

In #193644, I forgot to remove
references to the old `servicesReady` promise - this caused an issue
where it never resolved `true`, so anywhere that depended on this would
be stuck in a loading state. This PR fixes this by replacing all
instances of `servicesReady` with the new
`untilPluginStartServicesReady` promise.

Specifically, this fixes the exported `DashboardListingTable` that the
Security page uses:

- **Before:**


https://github.com/user-attachments/assets/78fc8ad8-7bff-43bf-95ec-d52f4da91371

- **After:**



https://github.com/user-attachments/assets/af1be9d3-9af5-4a30-9b5d-bc4352214a97


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 7, 2024
)

Closes elastic#194733

## Summary

In elastic#193644, I forgot to remove
references to the old `servicesReady` promise - this caused an issue
where it never resolved `true`, so anywhere that depended on this would
be stuck in a loading state. This PR fixes this by replacing all
instances of `servicesReady` with the new
`untilPluginStartServicesReady` promise.

Specifically, this fixes the exported `DashboardListingTable` that the
Security page uses:

- **Before:**

https://github.com/user-attachments/assets/78fc8ad8-7bff-43bf-95ec-d52f4da91371

- **After:**

https://github.com/user-attachments/assets/af1be9d3-9af5-4a30-9b5d-bc4352214a97

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 10271a2)
kibanamachine added a commit that referenced this pull request Oct 7, 2024
…) (#195301)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Dashboard] Await new services in exported listing table
(#195277)](#195277)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Hannah
Mudge","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-07T17:36:27Z","message":"[Dashboard]
Await new services in exported listing table (#195277)\n\nCloses
https://github.com/elastic/kibana/issues/194733\r\n\r\n##
Summary\r\n\r\nIn #193644, I
forgot to remove\r\nreferences to the old `servicesReady` promise - this
caused an issue\r\nwhere it never resolved `true`, so anywhere that
depended on this would\r\nbe stuck in a loading state. This PR fixes
this by replacing all\r\ninstances of `servicesReady` with the
new\r\n`untilPluginStartServicesReady` promise.\r\n\r\nSpecifically,
this fixes the exported `DashboardListingTable` that the\r\nSecurity
page uses:\r\n\r\n-
**Before:**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/78fc8ad8-7bff-43bf-95ec-d52f4da91371\r\n\r\n-
**After:**\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/af1be9d3-9af5-4a30-9b5d-bc4352214a97\r\n\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"10271a2e1fb5860a8a6d3d3e3f072d5b67a3f63f","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Dashboard","regression","release_note:fix","Team:Presentation","loe:small","impact:medium","v9.0.0","backport:prev-minor"],"title":"[Dashboard]
Await new services in exported listing
table","number":195277,"url":"https://github.com/elastic/kibana/pull/195277","mergeCommit":{"message":"[Dashboard]
Await new services in exported listing table (#195277)\n\nCloses
https://github.com/elastic/kibana/issues/194733\r\n\r\n##
Summary\r\n\r\nIn #193644, I
forgot to remove\r\nreferences to the old `servicesReady` promise - this
caused an issue\r\nwhere it never resolved `true`, so anywhere that
depended on this would\r\nbe stuck in a loading state. This PR fixes
this by replacing all\r\ninstances of `servicesReady` with the
new\r\n`untilPluginStartServicesReady` promise.\r\n\r\nSpecifically,
this fixes the exported `DashboardListingTable` that the\r\nSecurity
page uses:\r\n\r\n-
**Before:**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/78fc8ad8-7bff-43bf-95ec-d52f4da91371\r\n\r\n-
**After:**\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/af1be9d3-9af5-4a30-9b5d-bc4352214a97\r\n\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"10271a2e1fb5860a8a6d3d3e3f072d5b67a3f63f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195277","number":195277,"mergeCommit":{"message":"[Dashboard]
Await new services in exported listing table (#195277)\n\nCloses
https://github.com/elastic/kibana/issues/194733\r\n\r\n##
Summary\r\n\r\nIn #193644, I
forgot to remove\r\nreferences to the old `servicesReady` promise - this
caused an issue\r\nwhere it never resolved `true`, so anywhere that
depended on this would\r\nbe stuck in a loading state. This PR fixes
this by replacing all\r\ninstances of `servicesReady` with the
new\r\n`untilPluginStartServicesReady` promise.\r\n\r\nSpecifically,
this fixes the exported `DashboardListingTable` that the\r\nSecurity
page uses:\r\n\r\n-
**Before:**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/78fc8ad8-7bff-43bf-95ec-d52f4da91371\r\n\r\n-
**After:**\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/af1be9d3-9af5-4a30-9b5d-bc4352214a97\r\n\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"10271a2e1fb5860a8a6d3d3e3f072d5b67a3f63f"}}]}]
BACKPORT-->

Co-authored-by: Hannah Mudge <[email protected]>
nreese added a commit that referenced this pull request Oct 31, 2024
Closes #198517

#193644 refactored Dashboard
services. Part of this refactor moved `fieldFormats` from
`data.fieldFormats` to getting `fieldFormats` directly from the
fieldFormats plugin. That is because `data.fieldFormats` is marked as
deprecated. The problem is that the fieldFormats plugin was not defined
under `requiredPlugins` and thus was undefined at runtime.

### Test instructions
1) install web logs sample data
2) open web logs sample data dashboard
3) hover over "[Logs] Response Codes Over Time + Annotations" panel and
select "three dots" icon. Then select "Download CSV". Verify there are
no errors in web browser console and CSV is downloaded.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 31, 2024
Closes elastic#198517

elastic#193644 refactored Dashboard
services. Part of this refactor moved `fieldFormats` from
`data.fieldFormats` to getting `fieldFormats` directly from the
fieldFormats plugin. That is because `data.fieldFormats` is marked as
deprecated. The problem is that the fieldFormats plugin was not defined
under `requiredPlugins` and thus was undefined at runtime.

### Test instructions
1) install web logs sample data
2) open web logs sample data dashboard
3) hover over "[Logs] Response Codes Over Time + Annotations" panel and
select "three dots" icon. Then select "Download CSV". Verify there are
no errors in web browser console and CSV is downloaded.

(cherry picked from commit 0cd2d92)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 31, 2024
Closes elastic#198517

elastic#193644 refactored Dashboard
services. Part of this refactor moved `fieldFormats` from
`data.fieldFormats` to getting `fieldFormats` directly from the
fieldFormats plugin. That is because `data.fieldFormats` is marked as
deprecated. The problem is that the fieldFormats plugin was not defined
under `requiredPlugins` and thus was undefined at runtime.

### Test instructions
1) install web logs sample data
2) open web logs sample data dashboard
3) hover over "[Logs] Response Codes Over Time + Annotations" panel and
select "three dots" icon. Then select "Download CSV". Verify there are
no errors in web browser console and CSV is downloaded.

(cherry picked from commit 0cd2d92)
nreese added a commit to nreese/kibana that referenced this pull request Nov 1, 2024
Closes elastic#198517

elastic#193644 refactored Dashboard
services. Part of this refactor moved `fieldFormats` from
`data.fieldFormats` to getting `fieldFormats` directly from the
fieldFormats plugin. That is because `data.fieldFormats` is marked as
deprecated. The problem is that the fieldFormats plugin was not defined
under `requiredPlugins` and thus was undefined at runtime.

### Test instructions
1) install web logs sample data
2) open web logs sample data dashboard
3) hover over "[Logs] Response Codes Over Time + Annotations" panel and
select "three dots" icon. Then select "Download CSV". Verify there are
no errors in web browser console and CSV is downloaded.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:review backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) ci:project-deploy-observability Create an Observability project Feature:Dashboard Dashboard related features impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas technical debt Improvement of the software architecture and operational architecture v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard] Simplify module services
7 participants