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.

Sign up for GitHub

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

Jump to bottom

[Stack Monitoring ] Migrate kibana instances page to react #113874

Merged
merged 4 commits into from
Oct 5, 2021
Merged

[Stack Monitoring ] Migrate kibana instances page to react #113874

merged 4 commits into from
Oct 5, 2021

Conversation

klacabane
Copy link
Contributor

@klacabane klacabane commented Oct 4, 2021 •
edited
Loading

Summary

Migrates the Kibana Instances page to react.

There's some noise in the diff (contained in b69038c) due to removing SetupModeProps duplicates, will make it easier to add types when that happens.

Testing

  • pagination and sorting works
  • normal and setup mode render properly
  • tried the metricbeat migration pane by setting up a legacy-monitored kibana and hot reloading metricbeat to start monitoring that instance. The pane never detected the data. Quick investigation reveals angular app polls an additional api /api/monitoring/v1/setup/collection/cluster/:id when in setup mode but react only calls it once and continuously renders stale data. Will track the issue in [Stack Monitoring] SetupMode pane renders stale data #113873

Screenshot 2021-10-04 at 23 38 32

Screenshot 2021-10-04 at 23 29 31

@klacabane klacabane requested a review from a team as a code owner October 4, 2021 23:37
@klacabane klacabane requested review from a team October 4, 2021 23:37
@klacabane klacabane added auto-backport Deprecated - use backport:version if exact versions are needed Epic: Stack Monitoring de-angularization release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.16.0 v8.0.0 labels Oct 4, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

matschaffer
matschaffer approved these changes Oct 5, 2021
View reviewed changes
Copy link
Contributor

@matschaffer matschaffer left a comment

Choose a reason for hiding this comment

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

Looks solid 👍🏻 I was wondering about that refactoring myself. Good call.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
monitoring 630 631 +1

Async chunks

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

id before after diff
monitoring 1007.3KB 1011.0KB +3.7KB

History

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

@klacabane klacabane merged commit cf6bb10 into elastic:master Oct 5, 2021
@klacabane klacabane deleted the react-kibana-instances branch October 5, 2021 12:44
@klacabane klacabane mentioned this pull request Oct 5, 2021
Closed
3 tasks
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
❌ 7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 113874

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 7, 2021
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 113874 or prevent reminders by adding the backport:skip label.

3 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 113874 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 113874 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 113874 or prevent reminders by adding the backport:skip label.

@klacabane klacabane mentioned this pull request Oct 13, 2021
Merged
klacabane added a commit to klacabane/kibana that referenced this pull request Oct 13, 2021
@klacabane
[Stack Monitoring ] Migrate kibana instances page to react (elastic#1…
5d1a827 
…13874)

* kibana instances page component

* kibana instances route

* extract SetupModeProps to remove duplication
# Conflicts:
#	x-pack/plugins/monitoring/public/application/index.tsx
#	x-pack/plugins/monitoring/public/application/pages/cluster/overview_page.tsx
#	x-pack/plugins/monitoring/public/application/pages/elasticsearch/index_advanced_page.tsx
#	x-pack/plugins/monitoring/public/application/pages/elasticsearch/index_page.tsx
#	x-pack/plugins/monitoring/public/application/pages/elasticsearch/node_page.tsx
#	x-pack/plugins/monitoring/public/application/pages/kibana/instances.tsx
#	x-pack/plugins/monitoring/public/application/setup_mode/setup_mode_renderer.d.ts
@kibanamachine
Copy link
Contributor

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.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 13, 2021
klacabane added a commit that referenced this pull request Oct 13, 2021
@klacabane
[Stack Monitoring ] Migrate kibana instances page to react (#113874) (#…
04a4809 
…114788)

* kibana instances page component

* kibana instances route

* extract SetupModeProps to remove duplication
# Conflicts:
#	x-pack/plugins/monitoring/public/application/index.tsx
#	x-pack/plugins/monitoring/public/application/pages/cluster/overview_page.tsx
#	x-pack/plugins/monitoring/public/application/pages/elasticsearch/index_advanced_page.tsx
#	x-pack/plugins/monitoring/public/application/pages/elasticsearch/index_page.tsx
#	x-pack/plugins/monitoring/public/application/pages/elasticsearch/node_page.tsx
#	x-pack/plugins/monitoring/public/application/pages/kibana/instances.tsx
#	x-pack/plugins/monitoring/public/application/setup_mode/setup_mode_renderer.d.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matschaffer matschaffer matschaffer approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Epic: Stack Monitoring de-angularization release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.16.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@klacabane @elasticmachine @kibanamachine @matschaffer