-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Fleet][Logs UI] Prevent double loading of entries in <LogStream />
component.
#102980
[Fleet][Logs UI] Prevent double loading of entries in <LogStream />
component.
#102980
Conversation
Pinging @elastic/fleet (Feature:Fleet) |
Pinging @elastic/fleet (Team:Fleet) |
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Firefox XPack UI Functional Tests.x-pack/test/functional/apps/grok_debugger/grok_debugger·js.logstash grok debugger app "before all" hook in "grok debugger app"Standard Out
Stack Trace
Metrics [docs]Async chunks
To update your PR or re-run it, just comment with: cc @afgomez |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good fix and very helpful write-up of the problem. Looks good to me! 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes the issue 👍
… component. (elastic#102980) * Use better loading indicator for `useLogSource` * Use clearer name for the loading entries flag * Reuse query object if its value persists
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
Hi @EricDavisX
Screen Recording: Agents.-.Fleet.-.Elastic.-.Google.Chrome.2021-07-12.12-22-04.mp4Build details:
Thanks |
Summary
Closes #99618
The
<LogStream />
has a problem where it double-loads the entries.Recording.1.mp4
This is caused by changes in the value of the internal variable
derivedIndexPattern
. When the component loads,derivedIndexPattern
has a default uninitialized value. Later this variable changes to its resolved value. This makes the following code trigger twice:https://github.com/afgomez/kibana/blob/b8e633f2bb00b1f7537f71697afc532b73137742/x-pack/plugins/infra/public/components/log_stream/log_stream.tsx#L124-L130
Even if the value of the generated
parsedQuery
object is the same, is not the same object. This triggers a chain of react hook dependencies to be rebuilt, ending up with new entries being fetch.To prevent this we keep a
cachedQuery
ref inside theuseLogStream
container. This ref ensures that the identity of the query stays the same if its value doesn't change, preventing the double re-render.