Skip to content

[System] rework metric dashboards#6743

Merged
drewdaemon merged 23 commits intomainfrom
update-system-overview-dashboards
Oct 9, 2023
Merged

[System] rework metric dashboards#6743
drewdaemon merged 23 commits intomainfrom
update-system-overview-dashboards

Conversation

@drewdaemon
Copy link
Copy Markdown
Contributor

@drewdaemon drewdaemon commented Jun 28, 2023 •
edited
Loading

What does this PR do?

Reworks the metric system overview and host overview dashboards to use Lens.

Also leverages Lens features to represent current system state more reliably and clearly (especially, uses Lens Last value function instead of TSVB "Last value" mode to resolve #1437).

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

How to test this PR locally

Related issues

Screenshots

Overview

Before

overview_before

After

system-overview-light

Host overview

Before

host_overview_before

After

host-overview-light

@drewdaemon drewdaemon added enhancement New feature or request Team:Elastic-Agent-Data-Plane Agent Data Plane team [elastic/elastic-agent-data-plane] labels Jun 28, 2023
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Jun 28, 2023 •
edited
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-10-09T13:32:22.350+0000

  • Duration: 18 min 21 sec

Test stats 🧪

Test Results
Failed 0
Passed 151
Skipped 0
Total 151

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@P1llus
Copy link
Copy Markdown
Member

P1llus commented Jul 10, 2023

/test

@nchaulet nchaulet mentioned this pull request Jul 14, 2023
Merged
@botelastic
Copy link
Copy Markdown

botelastic bot commented Aug 12, 2023

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic bot added the Stalled label Aug 12, 2023
@drewdaemon
Copy link
Copy Markdown
Contributor Author

drewdaemon commented Aug 14, 2023

Let's keep this open!

@botelastic botelastic bot removed the Stalled label Aug 14, 2023
@botelastic
Copy link
Copy Markdown

botelastic bot commented Sep 13, 2023

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic bot added the Stalled label Sep 13, 2023
@drewdaemon
Copy link
Copy Markdown
Contributor Author

drewdaemon commented Sep 13, 2023

Keep this open

@botelastic botelastic bot removed the Stalled label Sep 13, 2023
@drewdaemon drewdaemon mentioned this pull request Sep 27, 2023
Closed
4 tasks
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Oct 2, 2023 •
edited
Loading

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (3/3) 💚
Files 100.0% (4/4) 💚
Classes 100.0% (4/4) 💚
Methods 63.415% (52/82) 👎 -36.585
Lines 99.863% (2924/2928) 👍 0.859
Conditionals 100.0% (0/0) 💚

@drewdaemon drewdaemon marked this pull request as ready for review October 3, 2023 01:26
@drewdaemon drewdaemon requested review from a team as code owners October 3, 2023 01:26
tommyers-elastic
tommyers-elastic reviewed Oct 3, 2023
View reviewed changes
packages/system/changelog.yml Outdated
@@ -1,4 +1,9 @@
# newer versions go on top
- version: "1.41.0"
Copy link
Copy Markdown
Contributor

@tommyers-elastic tommyers-elastic Oct 3, 2023

Choose a reason for hiding this comment

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

wonder if this warrants a bump to 2.0.0?

Copy link
Copy Markdown
Contributor Author

@drewdaemon drewdaemon Oct 3, 2023

Choose a reason for hiding this comment

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

Happy to change this (not sure what the criteria are for a major bump).

FWIW, I will be submitting a second PR to finish converting all the visualizations in the package to Lens. Does that change anything?

tommyers-elastic
tommyers-elastic approved these changes Oct 3, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@tommyers-elastic tommyers-elastic left a comment

Choose a reason for hiding this comment

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

this looks great, thanks! without reviewing the reams of JSON and just looking at the screenshots, this gets a +1 from me.

fearful-symmetry
fearful-symmetry approved these changes Oct 4, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@fearful-symmetry fearful-symmetry left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, but the one thing that worries me is the Load metric. This is only available on non-Windows OSes, so Windows users just get a blank UI item. However, I'm not sure we have the capability to just say "remove the visualization if there's no data," hence why I'm approving this. If we can do that and no one told me, we definitely should.

@drewdaemon
Copy link
Copy Markdown
Contributor Author

drewdaemon commented Oct 4, 2023 •
edited
Loading

I'm not sure we have the capability to just say "remove the visualization if there's no data," hence why I'm approving this. If we can do that and no one told me, we definitely should.

We don't have that capability. It's an interesting feature request. I think our answer today is: create two dashboards. But, that doesn't always make sense (this case for example) so I'm going to raise this with the team to see if we are tracking an issue.

@drewdaemon drewdaemon requested a review from dej611 October 4, 2023 22:57
efd6
efd6 approved these changes Oct 4, 2023
View reviewed changes
@drewdaemon drewdaemon requested a review from muthu-mps October 5, 2023 17:44
muthu-mps
muthu-mps approved these changes Oct 6, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@muthu-mps muthu-mps left a comment

Choose a reason for hiding this comment

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

LGTM!

@drewdaemon drewdaemon force-pushed the update-system-overview-dashboards branch from 5dbe535 to 7cb2edf Compare October 6, 2023 14:00
@drewdaemon
Copy link
Copy Markdown
Contributor Author

drewdaemon commented Oct 6, 2023

The screenshots I added here are quite large. Do we have any guidelines on screenshot size?

cc @efd6 @muthu-mps

@drewdaemon drewdaemon merged commit 7dd75d0 into main Oct 9, 2023
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Oct 9, 2023

Package system - 1.42.0 containing this change is available at https://epr.elastic.co/search?package=system

@mbondyra mbondyra deleted the update-system-overview-dashboards branch October 9, 2023 14:47
@markov00 markov00 mentioned this pull request Apr 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@efd6 efd6 efd6 approved these changes

@dej611 dej611 dej611 approved these changes

@fearful-symmetry fearful-symmetry fearful-symmetry approved these changes

@muthu-mps muthu-mps muthu-mps approved these changes

@tommyers-elastic tommyers-elastic tommyers-elastic approved these changes

@rdner rdner Awaiting requested review from rdner rdner was automatically assigned from elastic/elastic-agent-data-plane

Assignees

No one assigned

Labels

enhancement New feature or request Integration:system System Team:Elastic-Agent-Data-Plane Agent Data Plane team [elastic/elastic-agent-data-plane]

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[System] Issue with short time frame (5min)

9 participants

@drewdaemon @elasticmachine @P1llus @dej611 @fearful-symmetry @efd6 @muthu-mps @tommyers-elastic @andrewkroh