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

[Dashboards][Telemetry] Dashboard loaded event is emitted before the dashboard is rendered for some journeys #193907

Closed
thomasneirynck opened this issue Sep 24, 2024 · 1 comment · Fixed by #194349
Assignees
Labels
impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@thomasneirynck
Copy link
Contributor

Kibana version:

main

Describe the bug:

The kibana_loaded event is fired multiple times for the web_logs_dashboard_esql and flight_dashboard journeys. The first time it is emitted much too early. This is a bug.

This was likely introduced on Sept 10 with #192221.

e.g. note how after 9/10, the event is starting to get collected twice consistently. (the jittery peaks and values are due to test runs)

Image

Steps to reproduce:

  1. Run the web_logs_dashboard_esql or flight_dashboard journey dashboard
  2. Note the kibana_loaded event is fired twice

Expected behavior:

The event should only be emitted when the dashboard has completed rendering.

@thomasneirynck thomasneirynck added the bug Fixes for quality problems that affect the customer experience label Sep 24, 2024
@botelastic botelastic bot added the needs-team Issues missing a team label label Sep 24, 2024
@thomasneirynck thomasneirynck removed bug Fixes for quality problems that affect the customer experience needs-team Issues missing a team label labels Sep 24, 2024
@botelastic botelastic bot added the needs-team Issues missing a team label label Sep 24, 2024
@thomasneirynck thomasneirynck added the impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. label Sep 24, 2024
@thomasneirynck thomasneirynck self-assigned this Sep 24, 2024
@thomasneirynck thomasneirynck added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Sep 24, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Sep 24, 2024
thomasneirynck added a commit that referenced this issue Oct 11, 2024
…ls have been added to a dashboard (#194349)

## Summary

Closes #193907.

The existing implementation suffers from a race condition where the
performamce-tracker would start to evaluate the render-state of a
dashboard before all panels have been added.

When a panel-type (such as a markdown viz) would already have a
completed render-state (ie last Phase-event === `rendered`), it would
consider the entire dashboard as rendered.

This is a spot fix which delays evaluating the dashboard-render state
until all panels have been added to the dashboard (ie. `getPanelCount`
matches the number of panels that are being observed by the performance
tracker).

I slightly edited some of the Observable-piping to a series that I
believe is a little bit more readable.

This has opened a larger discussion of whether Phase-events are the
right way to check for completeness, and whether they should juse use
the `RenderCompleteDispatcher` instead. However, given the size of the
impact, a fuller investigation will come after this fix.

### Checklist

### Risk Matrix
### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 11, 2024
…ls have been added to a dashboard (elastic#194349)

## Summary

Closes elastic#193907.

The existing implementation suffers from a race condition where the
performamce-tracker would start to evaluate the render-state of a
dashboard before all panels have been added.

When a panel-type (such as a markdown viz) would already have a
completed render-state (ie last Phase-event === `rendered`), it would
consider the entire dashboard as rendered.

This is a spot fix which delays evaluating the dashboard-render state
until all panels have been added to the dashboard (ie. `getPanelCount`
matches the number of panels that are being observed by the performance
tracker).

I slightly edited some of the Observable-piping to a series that I
believe is a little bit more readable.

This has opened a larger discussion of whether Phase-events are the
right way to check for completeness, and whether they should juse use
the `RenderCompleteDispatcher` instead. However, given the size of the
impact, a fuller investigation will come after this fix.

### Checklist

### Risk Matrix
### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit e822571)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
2 participants