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

[Uptime] Waterfall Chart - enable timings for local files by leveraging total time #87424

Merged
merged 10 commits into from
Jan 11, 2021
Merged

[Uptime] Waterfall Chart - enable timings for local files by leveraging total time #87424

merged 10 commits into from
Jan 11, 2021

Conversation

dominiqueclarke
Copy link
Contributor

@dominiqueclarke dominiqueclarke commented Jan 6, 2021 •
edited
Loading

Fixes #86331

Summary

This PR creates an entry for a resource in the waterfall chart, even when individual timing values are unavailable, as is the case with local files. We first loop through checking the specific timings. If an entry is not found for specific timing values, in the case where all the specific named timings equal -1, then we move on and use the total time.

Screen Shot 2021-01-11 at 11 47 18 AM

Test cases

  • Create a synthetic monitor that hits local files on your file system and verify waterfall chart is displaying the timings accurately
  • Create a synthetic monitor that hits server hosted content, for regression testing.

@dominiqueclarke dominiqueclarke added bug Fixes for quality problems that affect the customer experience v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Jan 6, 2021
@dominiqueclarke dominiqueclarke requested a review from a team as a code owner January 6, 2021 02:26
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@dominiqueclarke dominiqueclarke force-pushed the fix/86331-uptime-waterfall-enable-timings-for-static-files branch from 71b24c6 to 117ef60 Compare January 6, 2021 02:35
@dominiqueclarke dominiqueclarke force-pushed the fix/86331-uptime-waterfall-enable-timings-for-static-files branch from 117ef60 to 67fc179 Compare January 6, 2021 02:38
shahzad31
shahzad31 reviewed Jan 7, 2021
View reviewed changes
...plugins/uptime/public/components/monitor/synthetics/step_detail/waterfall/data_formatting.ts Outdated
acc.push({
x: index,
y0: currentOffset,
y: currentOffset + item.timings.total,
Copy link
Contributor

Choose a reason for hiding this comment

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

so total will always be there?

Copy link
Contributor Author

@dominiqueclarke dominiqueclarke Jan 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Vignesh believes that it will be available for static files.

I've added additional logic to account for scenarios in which total is unavailable, as well as scenarios in which the entire timings object is unavailable.

I noticed that this is the case for when the connection is refused. All items, including the total, are listed as -1. With my new logic, this is what the behavior would be. There is no hover state.

This is when there is nothing up and running on localhost:8000
Screen Shot 2021-01-07 at 2 47 48 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correction: I'm able to remove the tooltip but not the background hover state, which seems to come bundled with passing a custom tooltip to the BarSeries from Elastic Charts. I've yet to discover a way to either use a custom tooltip or specify the tooltip as none on a per datum basis. I can continue the conversation in the elastic charts chat on Monday.

Screen Shot 2021-01-10 at 9 23 53 PM

Screen Shot 2021-01-10 at 8 55 05 PM

Copy link
Member

Choose a reason for hiding this comment

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

Total would always be there for all requests. We need to just use them as fallback when we don't have timings for other parts of the request.

@dominiqueclarke
Copy link
Contributor Author

@elasticmachine merge upstream

@dominiqueclarke
Copy link
Contributor Author

@elasticmachine merge upstream

shahzad31
shahzad31 approved these changes Jan 11, 2021
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.

LGTM, Great work and improvement !!

@dominiqueclarke
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
uptime 1.0MB 1.1MB +1.1KB

History

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

@dominiqueclarke dominiqueclarke merged commit 02f7956 into elastic:master Jan 11, 2021
@dominiqueclarke dominiqueclarke deleted the fix/86331-uptime-waterfall-enable-timings-for-static-files branch January 11, 2021 21:35
dominiqueclarke added a commit to dominiqueclarke/kibana that referenced this pull request Jan 11, 2021
@dominiqueclarke @kibanamachine
[Uptime] Waterfall Chart - enable timings for local files by leveragi…
bd8cd75 
…ng total time (elastic#87424)

* uptime waterfall enable timings for static files by leveraging total time

* update cases for when their is no waterfall timing available

* add showTooltip propertier to WaterfallDataSeriesConfigProperties

* remove content downloading from the legend

* add mime type to content downloading label

Co-authored-by: Kibana Machine <[email protected]>
@dominiqueclarke dominiqueclarke mentioned this pull request Jan 11, 2021
Merged
@dominiqueclarke dominiqueclarke mentioned this pull request Jan 11, 2021
Merged
dominiqueclarke added a commit to dominiqueclarke/kibana that referenced this pull request Jan 11, 2021
@dominiqueclarke @kibanamachine
[Uptime] Waterfall Chart - enable timings for local files by leveragi…
0be20da 
…ng total time (elastic#87424)

* uptime waterfall enable timings for static files by leveraging total time

* update cases for when their is no waterfall timing available

* add showTooltip propertier to WaterfallDataSeriesConfigProperties

* remove content downloading from the legend

* add mime type to content downloading label

Co-authored-by: Kibana Machine <[email protected]>
dominiqueclarke added a commit that referenced this pull request Jan 12, 2021
@dominiqueclarke @kibanamachine
[Uptime] Waterfall Chart - enable timings for local files by leveragi…
aa42cee 
…ng total time (#87424) (#87930)

* uptime waterfall enable timings for static files by leveraging total time

* update cases for when their is no waterfall timing available

* add showTooltip propertier to WaterfallDataSeriesConfigProperties

* remove content downloading from the legend

* add mime type to content downloading label

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
dominiqueclarke added a commit that referenced this pull request Jan 12, 2021
@dominiqueclarke @kibanamachine
[Uptime] Waterfall Chart - enable timings for local files by leveragi…
60c0bac 
…ng total time (#87424) (#87931)

* uptime waterfall enable timings for static files by leveraging total time

* update cases for when their is no waterfall timing available

* add showTooltip propertier to WaterfallDataSeriesConfigProperties

* remove content downloading from the legend

* add mime type to content downloading label

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vigneshshanmugam vigneshshanmugam vigneshshanmugam left review comments

@shahzad31 shahzad31 shahzad31 approved these changes

Assignees
No one assigned
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.11.0 v7.12.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Uptime] Waterfall view is squished
5 participants
@dominiqueclarke @elasticmachine @kibanamachine @shahzad31 @vigneshshanmugam