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

[ObsUX] Enable Hosts advanced setting on serverless #170035

Merged
merged 2 commits into from
Oct 30, 2023
Merged

[ObsUX] Enable Hosts advanced setting on serverless #170035

merged 2 commits into from
Oct 30, 2023

Conversation

mykolaharmash
Copy link
Contributor

@mykolaharmash mykolaharmash commented Oct 27, 2023 •
edited
Loading

Closes #169198

Summary

  • Adds Hosts advanced setting to the observability settings allowlist
  • Adds tests for Hosts sidenav item visibility
CleanShot.2023-10-27.at.16.09.14.mp4

How to test

  • Run locally in serverless mode
  • Make sure "Observability Hosts view" toggle is visible on the Advanced Settings screen
  • Toggle the setting on and off, make sure sidenav properly reacts to the changes
  • Go to Infra settings screen
  • Toggle the same setting on and off, and also make sure sidenav properly reacts to the changes

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@mykolaharmash mykolaharmash added release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Oct 27, 2023
@mykolaharmash mykolaharmash marked this pull request as ready for review October 27, 2023 14:06
@mykolaharmash mykolaharmash requested review from a team as code owners October 27, 2023 14:06
@mykolaharmash mykolaharmash added the Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services label Oct 27, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@mykolaharmash mykolaharmash requested a review from a team October 27, 2023 14:12
Dosant
Dosant reviewed Oct 27, 2023
View reviewed changes
x-pack/plugins/serverless_observability/public/components/side_navigation/index.tsx Outdated
return pathNameSerialized.startsWith(prepend('/app/metrics/hosts'));
{
link: 'metrics:hosts',
sideNavStatus: settings.hasInfrastructureHosts ? 'visible' : 'hidden',
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if you have tried it, but I think it might be simpler if we'd keep this static, but hide/show the link by updating the metrics:hosts navLinkStatus in core:

kibana/x-pack/plugins/infra/public/plugin.ts

Line 270 in 9a8277e

navLinkStatus: AppNavLinkStatus.visible,
, it should be possible to update this visibility

The serverless side nav should hide the link from side nav based on that visibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, you're right, thank you!. And it actually already works because of this, not sure how I missed this 🙈

@mykolaharmash
Copy link
Contributor Author

Reverted the changes in the sidenav component as they turned out to be redundant based on the comment from @Dosant 🙌

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

@mykolaharmash mykolaharmash changed the title [ObsUX] Toggle Hosts sidenav item in serverless [ObsUX] Enable Hosts advanced setting on serverless Oct 30, 2023
@crespocarlos crespocarlos self-requested a review October 30, 2023 09:17
crespocarlos
crespocarlos approved these changes Oct 30, 2023
View reviewed changes
Copy link
Contributor

@crespocarlos crespocarlos left a comment

Choose a reason for hiding this comment

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

LGTM!

ElenaStoeva
ElenaStoeva approved these changes Oct 30, 2023
View reviewed changes
Copy link
Contributor

@ElenaStoeva ElenaStoeva left a comment

Choose a reason for hiding this comment

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

LGTM

@mykolaharmash mykolaharmash merged commit e804ef2 into elastic:main Oct 30, 2023
@kibanamachine kibanamachine added v8.12.0 backport:skip This commit does not require backporting labels Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dosant Dosant Dosant left review comments

@ElenaStoeva ElenaStoeva ElenaStoeva approved these changes

@crespocarlos crespocarlos crespocarlos approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting 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 Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ObsUX] Make serverless sidenav show/hide Hosts depending on settings toggle
8 participants
@mykolaharmash @apmmachine @elasticmachine @kibana-ci @crespocarlos @Dosant @ElenaStoeva @kibanamachine