Skip to content

[8.x] [Embeddable] Avoid rerendering loop due to search context reload (#203150)#203818

Merged
kibanamachine merged 1 commit intoelastic:8.xfrom
kibanamachine:backport/8.x/pr-203150
Dec 11, 2024
Merged

[8.x] [Embeddable] Avoid rerendering loop due to search context reload (#203150)#203818
kibanamachine merged 1 commit intoelastic:8.xfrom
kibanamachine:backport/8.x/pr-203150

Conversation

@kibanamachine
Copy link
Contributor

Backport

This will backport the following commits from main to 8.x:

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

…stic#203150)

## Summary

Fixes elastic#202266

This PR fixes the underline rerendering issue at the `useSearchApi` hook
level, so any embeddable component who uses this hook would benefit from
the fix.

Ideally the props passed to the Lens component should be memoized, but
this assumption would break many integrations as the previous embeddable
component did take care of filtering duplicates.
To test this:
* Go to `Observability > Alerts > Manage rules` and `Add Rule`, pick the
`Custom threshold` option and verify the infinite reload does not happen

Once fixed this, another problem bubbled up with the brushing use case:
when brushing a chart the chart was always one time range step behind.
The other bug was due to the `fetch$(api)` function propagating a stale
`data` search context who the Lens embeddable was relying on.
To solve this other problem the following changes have been applied:
* read the `searchSessionId` from the `api` directly (used for
`autoRefresh`)
* make sure to test the `Refresh` feature with both relative and
absolute time ranges
* read the `search context` from the `parentApi` directly if implements
the `unifiedSearch` API
* to test this, brush and check that the final time range matches the
correct time range

**Note**: the fundamental issue for the latter is `fetch$` not emitting
the up-to-date `data` when the parentApi search context updates. The
retrieved `data` is stale and one step behind, so it is not reliable. cc
@elastic/kibana-presentation

As @ThomThomson noticed in his test failure investigation another issue
was found in this PR due to the wrong handling of the searchSessionId
within the Observability page (for more details see [his
analysis](elastic#203150 (comment))).
@markov00 and @crespocarlos helped risolve this problem with some
additional changes on the Observability side of things: this will lead
to some extra searchSessionId to be created, which will be eventually
solved by Observability team [shortly moving away from the
`searchSessionId`
mechanism](elastic#203412)

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com>
(cherry picked from commit d4194ba)
@kibanamachine kibanamachine added the backport This PR is a backport of another PR label Dec 11, 2024
@kibanamachine kibanamachine enabled auto-merge (squash) December 11, 2024 15:12
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. labels Dec 11, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@github-actions
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Contributor

elasticmachine commented Dec 11, 2024

�� Build Succeeded

  • Buildkite Build
  • Commit: 8c265bd
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-203818-8c265bde68e0

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
aiops 615.1KB 615.3KB +169.0B
apm 3.5MB 3.5MB +6.0B
canvas 1.1MB 1.1MB +183.0B
cases 492.7KB 492.7KB +6.0B
controls 459.6KB 459.8KB +187.0B
dataVisualizer 613.8KB 613.8KB +6.0B
discover 813.4KB 813.4KB +12.0B
imageEmbeddable 69.1KB 69.2KB +6.0B
infra 1.7MB 1.7MB +180.0B
lens 1.5MB 1.5MB +163.0B
links 52.6KB 52.6KB +6.0B
ml 4.7MB 4.7MB +12.0B
observability 481.4KB 481.4KB +2.0B
presentationUtil 74.1KB 74.1KB +4.0B
securitySolution 13.5MB 13.5MB +24.0B
slo 836.8KB 836.8KB +6.0B
synthetics 1.1MB 1.1MB +17.0B
total +989.0B

Page load bundle

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

id before after diff
dashboard 51.6KB 51.6KB +6.0B
dashboardEnhanced 17.9KB 17.9KB +10.0B
discoverEnhanced 7.6KB 7.1KB -470.0B
embeddable 70.5KB 70.5KB +6.0B
embeddableEnhanced 8.9KB 9.0KB +60.0B
imageEmbeddable 5.9KB 6.0KB +54.0B
infra 56.1KB 56.1KB +6.0B
inputControlVis 8.3KB 8.3KB +7.0B
lens 50.0KB 50.2KB +217.0B
maps 54.4KB 54.5KB +173.0B
presentationPanel 43.3KB 43.3KB +6.0B
presentationUtil 30.9KB 30.9KB +60.0B
reporting 52.7KB 52.7KB +60.0B
synthetics 37.6KB 37.7KB +54.0B
urlDrilldown 17.4KB 17.5KB +60.0B
visualizations 70.9KB 70.9KB +12.0B
total +321.0B

History

cc @dej611

@kibanamachine kibanamachine merged commit 8eb5049 into elastic:8.x Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR ci:project-deploy-observability Create an Observability project Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics.

3 participants