search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
20.16k stars 8.28k forks source link

[Stack Monitoring] Visual refresh changes #204258

Closed weronikaolejniczak closed 2 weeks ago

weronikaolejniczak commented 1 month ago

Summary

This PR introduces changes to x-pack/plugins/monitoring necessary for the Visual Refresh project (https://github.com/elastic/kibana/issues/199715):

Additionally:

closes #8228

QA

We need to test the critical paths in the Stack monitoring, paying close attention to:

Specific paths:

Checklist

consulthys commented 1 month ago

Thank you everyone for helping us on this, truly appreciated!

I've made a first test run with Amsterdam to verify that everything was still looking "as usual". I've been through all screens and I have nothing to report, everything looks great!

I'm not able to start Kibana with the Borealis theme (errors out), I've shared some details here so I'll do another test run with Borealis once we get that sorted out.

weronikaolejniczak commented 1 month ago

/ci

patpscal commented 1 month ago

All LGTM -- I just noticed a badge (Replica) that I'm not sure should be success as a positive reinforcement , or secondary accent - leaving it here!

Screenshot 2024-12-17 at 11 50 32
consulthys commented 1 month ago

All LGTM -- I just noticed a badge (Replica) that I'm not sure should be success as a positive reinforcement , or secondary accent - leaving it here!

Totally agree with @patpscal it can definitely be secondary accent

weronikaolejniczak commented 1 month ago

@patpscal @consulthys great catch! Just fyi, accentSecondary is removed for the EuiBadge component. We can pass in the color prop anything that we want, it doesn't have to be danger | warning | accent | default | primary | success.

Screenshot 2024-12-17 at 13 36 50

consulthys commented 1 month ago

@patpscal @consulthys great catch! Just fyi, accentSecondary is removed for the EuiBadge component. We can pass in the color prop anything that we want, it doesn't have to be danger | warning | accent | default | primary | success.

Thanks @weronikaolejniczak I think instead of success we just need a green-ish color

weronikaolejniczak commented 1 month ago

@andreadelrio @mgadewoll @patpscal @consulthys thank you so much for the testing and suggestions! 💚 I updated the PR with the changes.

Let me know the final decision on those legend badge colors and I'd appreciate a double-check of the paths I defined in the PR description (where I made riskier style changes).

Testing:

- ✅ Removing wrapping <div> (addressing: https://github.com/elastic/kibana/pull/204258#discussion_r1886698189) File: x-pack/plugins/monitoring/public/components/beats/listing/listing.js Path: Menu bar > Under "Management" > Stack Monitoring > Under "Beats" > Beats (localhost url)

Amsterdam Borealis
Screenshot 2024-12-17 at 14 18 30

- ✅ Removing wrapping <div> (addressing: https://github.com/elastic/kibana/pull/204258#discussion_r1886698189) File: x-pack/plugins/monitoring/public/components/kibana/instances/instances.tsx Path: Menu bar > Under "Management" > Stack Monitoring > Under "Kibana" > Instances (localhost url)

Amsterdam Borealis
Screenshot 2024-12-17 at 14 16 45

[!WARNING] The issue with the Badge overlapping the next column is reproducible regardless of the changes made in this PR. If you have a ready solution, I'd appreciate it! Otherwise, let's leave it like so.

Untested paths (fyi @consulthys):

- Removing wrapping <div> (addressing: https://github.com/elastic/kibana/pull/204258#discussion_r1886698189) File: x-pack/plugins/monitoring/public/components/logstash/listing/listing.js Path: Menu bar > Under "Management" > Stack Monitoring > Under "Logstash" > Instances

- Removing wrapping <div> (addressing: https://github.com/elastic/kibana/pull/204258#discussion_r1886698189) File: x-pack/plugins/monitoring/public/components/apm/instances/instances.tsx Path: Menu bar > Under "Management" > Stack Monitoring > Under "APM server" > APM servers (localhost url)

- Removing !important flag (addressing: https://github.com/elastic/kibana/pull/204258#discussion_r1886753978) File: x-pack/plugins/monitoring/public/components/logstash/pipeline_viewer/views/metric.tsx Path: Menu bar > Under "Management" > Stack Monitoring > Under "Logstash" > Pipeline viewer

andreadelrio commented 1 month ago

@patpscal @consulthys great catch! Just fyi, accentSecondary is removed for the EuiBadge component. We can pass in the color prop anything that we want, it doesn't have to be danger | warning | accent | default | primary | success.

Thanks @weronikaolejniczak I think instead of success we just need a green-ish color

I think sticking with success is our best option here. No harm in using it.

weronikaolejniczak commented 1 month ago

/ci

weronikaolejniczak commented 3 weeks ago

@elasticmachine merge upstream

weronikaolejniczak commented 3 weeks ago

/ci

weronikaolejniczak commented 2 weeks ago

@elasticmachine merge upstream

elasticmachine commented 2 weeks ago

:green_heart: Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
monitoring 478 396 -82

Async chunks

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

id before after diff
monitoring 532.1KB 514.8KB -17.3KB

Page load bundle

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

id before after diff
monitoring 25.5KB 25.3KB -217.0B
Unknown metric groups #### async chunk count | id | [before](https://github.com/elastic/kibana/commit/301f91da2be03532508da05777e2db84701c8bc2) | [after](https://github.com/elastic/kibana/commit/972ab09a60c570624eac52d48c1449f0a56f1bee) | diff | | --- | --- | --- | --- | | `monitoring` | 6 | 7 | +1 | #### ESLint disabled line counts | id | [before](https://github.com/elastic/kibana/commit/301f91da2be03532508da05777e2db84701c8bc2) | [after](https://github.com/elastic/kibana/commit/972ab09a60c570624eac52d48c1449f0a56f1bee) | diff | | --- | --- | --- | --- | | `monitoring` | 25 | 26 | +1 | #### Total ESLint disabled count | id | [before](https://github.com/elastic/kibana/commit/301f91da2be03532508da05777e2db84701c8bc2) | [after](https://github.com/elastic/kibana/commit/972ab09a60c570624eac52d48c1449f0a56f1bee) | diff | | --- | --- | --- | --- | | `monitoring` | 32 | 33 | +1 |

History