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

[Infra][Hosts] Display N/A badge for APM hosts without system metrics #191181

Conversation

iblancof
Copy link
Contributor

@iblancof iblancof commented Aug 23, 2024

Summary

Closes #190516

This PR introduces an "N/A" badge for hosts that are returned by the metrics/infra/host API with hasSystemMetrics set to false. This enhancement will help users quickly identify and address issues with APM hosts that are not monitored by the system integration. The badge will provide an explanation of the problem and suggest troubleshooting steps.

Screen Recording 2024-08-23 at 11 58 29

Automated tests
Upon reviewing the code, I noticed that this screen is covered by functional tests.

My initial idea was to add some functional tests to verify the badge display as well as the button and link redirections. However, I found that this is quite complex, as it involves modifying the data used in the tests, and I’m still not very familiar with that process.

Given the deadline for this epic, I’ve decided not to include tests in this issue. If we believe they are really necessary, I would need to spend some time understanding how data is managed in our functional testing layer. Any feedback on this would be greatly appreciated.

@iblancof iblancof added release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.16.0 labels Aug 23, 2024
@iblancof iblancof self-assigned this Aug 23, 2024
@iblancof iblancof requested a review from a team as a code owner August 23, 2024 11:49
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Aug 23, 2024
@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

Copy link
Member

@jennypavlova jennypavlova left a comment

Choose a reason for hiding this comment

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

Well done 👏 I checked the code and left some nits and I will test it later.

@jennypavlova
Copy link
Member

I tested the popover and it works as expected and has the correct links, thanks for adding it 💯

@iblancof iblancof force-pushed the 190516-infra-display-na-badge-for-apm-hosts-with-metrics-with-null-value branch from 808bd3d to e44500c Compare August 26, 2024 08:57
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! Thanks for this PR.

Copy link
Member

@jennypavlova jennypavlova left a comment

Choose a reason for hiding this comment

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

Thank you for all the changes! I added a question, apart from that it LGTM!

@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 27, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 8eb701d
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-191181-8eb701d20658

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1577 1578 +1

Async chunks

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

id before after diff
infra 1.5MB 1.5MB +2.0KB

History

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

cc @iblancof

@iblancof iblancof merged commit 1e428ad into elastic:main Aug 27, 2024
25 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 27, 2024
@iblancof iblancof deleted the 190516-infra-display-na-badge-for-apm-hosts-with-metrics-with-null-value branch August 27, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Infra] Display N/A badge for APM hosts with metrics with null value
7 participants