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

[UX] Add core web vitals in obsv homepage #78976

Merged
merged 28 commits into from
Oct 5, 2020

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Sep 30, 2020

Summary

Fixes: #78974

Added core vital view in observability homepage, basically moved component from UX app and reused api.

image

@cauemarcondes
Copy link
Contributor

@shahzad31 maybe you've already done it, but I think you want to add an empty state for the User Experience panel. You can do it by adding a new entry in this file https://github.com/elastic/kibana/blob/master/x-pack/plugins/observability/public/pages/overview/empty_section.ts#L53:L53


interface UXHasDataResponse {
hasData: boolean;
serviceName: string | number | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know about this, I feel hasData has been given way more responsibility than only checking if there is any data available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

with separation of hooks this will improve

@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Oct 5, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@cauemarcondes
Copy link
Contributor

@shahzad31 @formgeist I'm not sure if you want to add an empty state section for the UX too like:
Screenshot 2020-10-05 at 10 38 16

@formgeist
Copy link
Contributor

@cauemarcondes @shahzad31 Yes, good call - I think we should promote here too. The button link would go to the RUM agent setup instructions.

I'll try and find some copy that we can use for the message.

@cauemarcondes
Copy link
Contributor

I think we should promote here too. The button link would go to the RUM agent setup instructions.

@formgeist maybe we should consider adding a new app in the /landing page too?!

@formgeist
Copy link
Contributor

@cauemarcondes Perhaps, but I think this is also a silent release, so I'll follow up with the UX PMs to see how they want to promote it.

@shahzad31
Copy link
Contributor Author

shahzad31 commented Oct 5, 2020

@shahzad31 I'm not sure if you want to add an empty state section for the UX too like:

@cauemarcondes added empty state, i am not sure, what happened there, i am adding this second time, i guess i deleted that while i was deleting loading improvement refactor :)

Copy link
Contributor

@cauemarcondes cauemarcondes 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 the refactorings 🎉

@formgeist
Copy link
Contributor

I reckon we need to make the "view in app" link read the service name value so we link to the right service within the app. I think right now it doesn't pass anything into the URL params. @shahzad31 is it selecting the app with the most traffic in the UX app as well? Hence why I think it's important that you don't land on another service than is specified in the overview page.

Screenshot 2020-10-05 at 16 25 15

@shahzad31
Copy link
Contributor Author

shahzad31 commented Oct 5, 2020

I reckon we need to make the "view in app" link read the service name value so we link to the right service within the app. I think right now it doesn't pass anything into the URL params. @shahzad31 is it selecting the app with the most traffic in the UX app as well? Hence why I think it's important that you don't land on another service than is specified in the overview page.

Screenshot 2020-10-05 at 16 25 15

@formgeist I think i will have to check, but i think it make sense to align this in UX and make sure it selects the service with most traffic, i will do the change in UX app.

Copy link
Contributor

@formgeist formgeist left a comment

Choose a reason for hiding this comment

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

LGTM from design - only need to make the view in app link use the service name

@formgeist
Copy link
Contributor

The UX app will find the service with the most traffic as default selected service as well, so we don't need to specify it in the URL for "view in app".

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id before after diff
apm 1276 1273 -3
observability 96 106 +10
total +7

async chunks size

id before after diff
apm 4.2MB 4.1MB -4.1KB
observability 163.2KB 166.8KB +3.7KB
total -416.0B

distributable file count

id before after diff
default 47107 47111 +4

page load bundle size

id before after diff
apm 44.3KB 46.3KB +2.0KB
observability 52.4KB 71.2KB +18.8KB
total +20.8KB

History

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

@shahzad31 shahzad31 merged commit de130ab into elastic:master Oct 5, 2020
@shahzad31 shahzad31 deleted the ux-app-in-obsv-homepage branch October 5, 2020 17:15
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Oct 6, 2020
@sorenlouv sorenlouv removed the Team:APM All issues that need APM UI Team support label Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:enhancement Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UX] Add web core vital panel to observability homepage
6 participants