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

[Synthetics] Waterfall view #84821

Merged
merged 15 commits into from
Dec 14, 2020
Merged

[Synthetics] Waterfall view #84821

merged 15 commits into from
Dec 14, 2020

Conversation

Kerry350
Copy link
Contributor

@Kerry350 Kerry350 commented Dec 2, 2020 •
edited
Loading

Summary

This PR implements the majority of #80162, #80168 (note: the popover is yet to come, I had missed this on the ticket, but don't want to hold up this PR), and #80170.

waterfall

Notes

  • There is a new API for retrieving network events.

  • The journey API has been slightly extended to return a details object along with the steps, this contains details on the timestamp of the run, and next / previous run details. Whilst this information isn't used directly where we list the steps yet, the refactors that are due in the future should yield this a useful addition there too. Regardless, the performance impact shouldn't be large here.

  • Filtering will follow (if there's time before FF)

Testing

  • ⚠️ You will need to be using the latest version of the agent, as the start / end fields have been renamed and are used here
  • You will need to be shipping synthetics data
  • This page is currently linked to from the step name within the current executed step list

@Kerry350 Kerry350 force-pushed the 80162-step-detail-page branch 2 times, most recently from e94dfaf to 091f610 Compare December 6, 2020 20:40
@Kerry350 Kerry350 force-pushed the 80162-step-detail-page branch from 091f610 to a06bfb1 Compare December 7, 2020 17:02
@Kerry350 Kerry350 requested a review from a team December 7, 2020 17:14
@Kerry350 Kerry350 self-assigned this Dec 7, 2020
@Kerry350 Kerry350 added release_note:enhancement Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.11.0 v8.0.0 labels Dec 7, 2020
@Kerry350 Kerry350 marked this pull request as ready for review December 7, 2020 17:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@Kerry350 Kerry350 mentioned this pull request Dec 9, 2020
Closed
@shahzad31
Copy link
Contributor

@elasticmachine merge upstream

shahzad31
shahzad31 reviewed Dec 10, 2020
View reviewed changes
x-pack/plugins/uptime/public/components/common/step_detail_link.tsx
Comment on lines +25 to +31
return (
<EuiLink>
<Link data-test-subj={`step-detail-link`} to={to}>
{children}
</Link>
</EuiLink>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

This kills the purpose of EuiLink, we have a small utility for these kind of use cases,

can you please use that https://github.com/elastic/kibana/blob/master/x-pack/plugins/uptime/public/components/common/react_router_helpers/link_for_eui.tsx#L55

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can change it. I'd actually copied this style from here: https://github.com/elastic/kibana/blob/master/x-pack/plugins/uptime/public/components/common/monitor_page_link.tsx#L22, so maybe that needs changing too?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah we should change it there as well. i guess we can create a follow up issue to refactor all places in uptime.

shahzad31
shahzad31 reviewed Dec 10, 2020
View reviewed changes
Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

Looks great, code wise look ok. Just need to figure out the timing stuff and why some events are missing. I am open to getting those resolved in agent or wait for those to be get fixed.

@Kerry350
Copy link
Contributor Author

Looks great, code wise look ok. Just need to figure out the timing stuff and why some events are missing. I am open to getting those resolved in agent or wait for those to be get fixed.

Thanks for the review. I'll be switching the UI code over to use @vigneshshanmugam fix from the Agent side for the timings. I'm not sure what's happening on the missing events front though.

@shahzad31
Copy link
Contributor

i am hitting this error
image

@Kerry350
Copy link
Contributor Author

^ We spoke on Zoom and the above was an outdated Agent version so the new timings from the agent were missing.

shahzad31
shahzad31 approved these changes Dec 14, 2020
View reviewed changes
Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

Works well, just need to fix padding maring for sidenav and remove refresh button and date picker from this view

image

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
uptime 543 565 +22

Async chunks

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

id before after diff
uptime 1007.4KB 1.0MB +53.7KB

Distributable file count

id before after diff
default 47129 47894 +765

History

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

@Kerry350 Kerry350 merged commit dd7bbb8 into elastic:master Dec 14, 2020
@Kerry350 Kerry350 mentioned this pull request Dec 14, 2020
Merged
Kerry350 added a commit to Kerry350/kibana that referenced this pull request Dec 14, 2020
@Kerry350
[Synthetics] Waterfall view (elastic#84821)
53df452 
* Add a new synthetics step detail page for displaying waterfall data
Kerry350 added a commit that referenced this pull request Dec 15, 2020
@Kerry350
[7.x] [Synthetics] Waterfall view (#84821) (#85875)
604aca4 
* [Synthetics] Waterfall view (#84821)

* Add a new synthetics step detail page for displaying waterfall data

* Amend test for 7.x index
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shahzad31 shahzad31 shahzad31 approved these changes

Assignees

@Kerry350 Kerry350

Labels
release_note:enhancement Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Kerry350 @elasticmachine @shahzad31 @kibanamachine