Skip to content

feat(ourlogs): Add infinite table and auto refresh #92059

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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

k-fish
Copy link
Member

@k-fish k-fish commented May 21, 2025

Summary

This adds a virtualized table behind a new feature flag (ourlogs-live-refresh). This has a max number of pages (10, ~0-10k log lines stored) and a virtualization that likely needs to be tuned for render performance before turning it on for users.

The logs infinite table is currently a completely separate table only used behind the feature flag, using tanstack virtual and tanstack infinite query. The auto-refreshing refreshes the page behind the ingest delay (a period in which the data may be incomplete) to avoid having to collate results, fetches large pages (up to 1000 resultss) and a virtual time is moved forwards at ~3fps to simulate streaming of that data on the frontend.

Virtual Streaming

Screenshot 2025-05-21 at 4 11 59 PM

Other

  • Changed logs from using the overly nested hooks (eg. useOurlogs -> useWrappedDiscoverQuery -> useGenericDiscoverQuery) to a basic hook useLogsQuery with the new useApiQuery, which might introduce some subtle differences, but so far removes strange bugs like the enabled: tri-state boolean bug. useInfiniteQuery does basically the same and can re-use the query-key to reduce bugs between the two.
  • The data provider now provides either the infinite data or the finite data, log pages other than the main explore view will break until they're ported over (needs to be done before flag is enabled).
  • Page params includes some auto-refresh related params, changing them or changing the search will disable auto-refresh.
  • Added a LogsFixture to correctly create a log item in tests
  • Added a bunch of tests, although they still aren't comprehensive enough for auto-refresh and scrolling behaviour

Future work

  • Needs better querying logic to start showing results streaming in immediately, vs. the small pause due to virtual time / page results mismatch
  • Need loading indicators when scrolling past the edge of results and running into an infinite pageload (this is separate from the rendering issue where you see blank lines)
  • Improving per-log line performance so we see less blank lines when scrolling quickly. Currently lines are around 5-10ms, which multiplied by a full page of 50+ lines ends up causing jank. This will also improve UI jank around turning on and off auto-refresh
  • Better auto-refresh logic around when to turn off auto-refresh (visibility, mouse move, etc.)

This adds a virtualized table behind a new feature flag (ourlogs-live-refresh). This has a max number of pages (10, ~0-10k log lines stored) and a virtualization that likely needs to be tuned for render performance before turning it on for users.

The logs infinite table is currently a completely separate table only used behind the feature flag, using tanstack virtual and tanstack infinite query.
@k-fish k-fish requested review from a team as code owners May 21, 2025 20:15
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 21, 2025
Copy link

codecov bot commented May 22, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
10377 1 10376 9
View the top 1 failed test(s) by shortest run time
LogsTable should not be interactable on embedded views
Stack Traces | 0.222s run time
Error: Expected test not to call console.error().

If the error is expected, test for it explicitly by mocking it out using jest.spyOn(console, 'error').mockImplementation() and test that the warning occurs.



Error: No mocked response found for request: GET .../project-slug/trace-items/0196a1bc007c7dfbbe099b6328e41d12/
    at processTimers (node:internal/timers:529:7)
    at console.captureMessage [as error] (.../node_modules/jest-fail-on-console/index.js:83:25)
    at Object.<anonymous> (.../app/__mocks__/api.tsx:46:15)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusHook (.../jest-circus/build/run.js:281:40)
    at _runTest (.../jest-circus/build/run.js:254:5)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)
    at flushUnexpectedConsoleCalls (.../node_modules/jest-fail-on-console/index.js:48:13)
    at Object.<anonymous> (.../node_modules/jest-fail-on-console/index.js:139:7)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusHook (.../jest-circus/build/run.js:281:40)
    at _runTest (.../jest-circus/build/run.js:254:5)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants