[cueweb] Add optional Loki backend for frame log viewing#2447
Draft
ramonfigueiredo wants to merge 1 commit into
Draft
[cueweb] Add optional Loki backend for frame log viewing#2447ramonfigueiredo wants to merge 1 commit into
ramonfigueiredo wants to merge 1 commit into
Conversation
When NEXT_PUBLIC_LOKI_URL is set, the frame log viewer queries Loki by frame_id instead of reading the on-disk .rqlog file, mirroring CueGUI's LokiViewPlugin. Falls back to the file-based viewer when unset. - lib/loki.ts: Loki HTTP API client (label values for frame attempts, query_range for log lines) - frames/[frame-name]: branch on the env flag; new LokiLogView reuses the same Monaco editor, version dropdown, and empty/loading states - Document NEXT_PUBLIC_LOKI_URL in README.md and .env.example - Add lib/loki.ts unit tests
Contributor
📝 WalkthroughWalkthroughAdds an optional Loki log backend to CueWeb's frame log viewer. A new TypeScript client ( ChangesLoki Log Backend Integration
Sequence Diagram(s)sequenceDiagram
actor User
participant FramePage as page.tsx
participant LokiLogView as LokiLogView
participant LokiClient as lib/loki.ts
participant LokiServer as Loki HTTP API
User->>FramePage: navigate to frame
FramePage->>LokiClient: isLokiEnabled()
LokiClient-->>FramePage: true
FramePage->>LokiLogView: render with frameId, startTime
LokiLogView->>LokiClient: getFrameLogVersions(frameId, startTime)
LokiClient->>LokiServer: GET /label/session_start_time/values?query={frame_id=...}
LokiServer-->>LokiClient: string[] of session timestamps
LokiClient-->>LokiLogView: LokiLogVersion[] sorted newest-first
LokiLogView->>LokiClient: getFrameLogLines(frameId, sessionStartTime, startTime)
LokiClient->>LokiServer: GET /query_range?query={session_start_time=...,frame_id=...}
LokiServer-->>LokiClient: LokiStream[] with log values
LokiClient-->>LokiLogView: concatenated log text
LokiLogView-->>User: Monaco editor with log lines
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cueweb/app/__tests__/loki.test.ts (1)
102-153: ⚡ Quick winAdd a regression test for cross-stream timestamp ordering.
The current happy-path asserts one stream only, so it won’t catch incorrect ordering when Loki returns multiple streams.
Suggested test case
describe('getFrameLogLines', () => { + it('orders lines chronologically across multiple streams', async () => { + const loki = loadLoki('http://loki:3100'); + mockFetch(() => ({ + status: 'success', + data: { + result: [ + { stream: { src: 'stderr' }, values: [['1700000002000000000', 'second']] }, + { stream: { src: 'stdout' }, values: [['1700000001000000000', 'first']] }, + ], + }, + })); + + await expect( + loki.getFrameLogLines('frame-123', '1700000000'), + ).resolves.toBe('first\nsecond'); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cueweb/app/__tests__/loki.test.ts` around lines 102 - 153, The current test for getFrameLogLines only validates ordering within a single stream. Add a new test case within the describe block that mocks Loki to return multiple streams with interleaved timestamps and verify that getFrameLogLines correctly orders the log lines by timestamp across all streams rather than just concatenating them in stream order. The test should set up at least two stream objects in the result array, each with values that have timestamps interspersed with the other stream's timestamps, and assert that the returned text reflects the correct chronological ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cueweb/lib/loki.ts`:
- Around line 175-198: The `getFrameLogLines` function uses a fixed start time
with direction=forward and a single query with a limit, which causes it to miss
recent logs when they exceed the limit boundary, and directly concatenating
lines from multiple streams scrambles their chronological order. To fix this,
implement pagination by tracking the last timestamp returned from each query and
using it as the start time for the next query to fetch all available logs, and
sort all collected lines chronologically before joining them together to ensure
proper ordering across streams like stdout and stderr.
---
Nitpick comments:
In `@cueweb/app/__tests__/loki.test.ts`:
- Around line 102-153: The current test for getFrameLogLines only validates
ordering within a single stream. Add a new test case within the describe block
that mocks Loki to return multiple streams with interleaved timestamps and
verify that getFrameLogLines correctly orders the log lines by timestamp across
all streams rather than just concatenating them in stream order. The test should
set up at least two stream objects in the result array, each with values that
have timestamps interspersed with the other stream's timestamps, and assert that
the returned text reflects the correct chronological ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c8a1620-9523-4ff1-9c44-6738901b0569
📒 Files selected for processing (6)
cueweb/.env.examplecueweb/README.mdcueweb/app/__tests__/loki.test.tscueweb/app/frames/[frame-name]/loki-log-view.tsxcueweb/app/frames/[frame-name]/page.tsxcueweb/lib/loki.ts
Comment on lines
+175
to
+198
| const params = new URLSearchParams({ | ||
| query: selector, | ||
| direction: "forward", | ||
| limit: String(LOKI_QUERY_LIMIT), | ||
| }); | ||
| // Prefer the explicit start time, otherwise derive it from the selected | ||
| // session so Loki doesn't reject the query for too-wide a range. | ||
| const start = | ||
| toUnixNano(startTime) ?? | ||
| (sessionStartTime ? toUnixNano(parseFloat(sessionStartTime)) : undefined); | ||
| if (start) params.set("start", start); | ||
|
|
||
| const json = await lokiGet<LokiQueryResponse>( | ||
| `/loki/api/v1/query_range?${params.toString()}`, | ||
| ); | ||
|
|
||
| const streams = json.data?.result ?? []; | ||
| const lines: string[] = []; | ||
| for (const stream of streams) { | ||
| for (const [, line] of stream.values ?? []) { | ||
| lines.push(line); | ||
| } | ||
| } | ||
| return lines.join("\n"); |
Contributor
There was a problem hiding this comment.
getFrameLogLines can miss recent logs and return lines out of chronological order.
With fixed start, direction=forward, and a single limited query, refresh can keep returning the same earliest window once logs exceed the limit. Also, concatenating stream blocks directly can scramble global ordering across streams (e.g., stdout/stderr).
Proposed fix
const params = new URLSearchParams({
query: selector,
- direction: "forward",
+ // Prefer latest window so refresh surfaces recent output.
+ direction: "backward",
limit: String(LOKI_QUERY_LIMIT),
});
@@
const streams = json.data?.result ?? [];
- const lines: string[] = [];
- for (const stream of streams) {
- for (const [, line] of stream.values ?? []) {
- lines.push(line);
- }
- }
- return lines.join("\n");
+ const entries = streams.flatMap((stream) =>
+ (stream.values ?? []).map(([ts, line]) => ({ ts, line })),
+ );
+ entries.sort((a, b) => {
+ const at = BigInt(a.ts);
+ const bt = BigInt(b.ts);
+ return at < bt ? -1 : at > bt ? 1 : 0;
+ });
+ return entries.map((e) => e.line).join("\n");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const params = new URLSearchParams({ | |
| query: selector, | |
| direction: "forward", | |
| limit: String(LOKI_QUERY_LIMIT), | |
| }); | |
| // Prefer the explicit start time, otherwise derive it from the selected | |
| // session so Loki doesn't reject the query for too-wide a range. | |
| const start = | |
| toUnixNano(startTime) ?? | |
| (sessionStartTime ? toUnixNano(parseFloat(sessionStartTime)) : undefined); | |
| if (start) params.set("start", start); | |
| const json = await lokiGet<LokiQueryResponse>( | |
| `/loki/api/v1/query_range?${params.toString()}`, | |
| ); | |
| const streams = json.data?.result ?? []; | |
| const lines: string[] = []; | |
| for (const stream of streams) { | |
| for (const [, line] of stream.values ?? []) { | |
| lines.push(line); | |
| } | |
| } | |
| return lines.join("\n"); | |
| const params = new URLSearchParams({ | |
| query: selector, | |
| // Prefer latest window so refresh surfaces recent output. | |
| direction: "backward", | |
| limit: String(LOKI_QUERY_LIMIT), | |
| }); | |
| // Prefer the explicit start time, otherwise derive it from the selected | |
| // session so Loki doesn't reject the query for too-wide a range. | |
| const start = | |
| toUnixNano(startTime) ?? | |
| (sessionStartTime ? toUnixNano(parseFloat(sessionStartTime)) : undefined); | |
| if (start) params.set("start", start); | |
| const json = await lokiGet<LokiQueryResponse>( | |
| `/loki/api/v1/query_range?${params.toString()}`, | |
| ); | |
| const streams = json.data?.result ?? []; | |
| const entries = streams.flatMap((stream) => | |
| (stream.values ?? []).map(([ts, line]) => ({ ts, line })), | |
| ); | |
| entries.sort((a, b) => { | |
| const at = BigInt(a.ts); | |
| const bt = BigInt(b.ts); | |
| return at < bt ? -1 : at > bt ? 1 : 0; | |
| }); | |
| return entries.map((e) => e.line).join("\n"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cueweb/lib/loki.ts` around lines 175 - 198, The `getFrameLogLines` function
uses a fixed start time with direction=forward and a single query with a limit,
which causes it to miss recent logs when they exceed the limit boundary, and
directly concatenating lines from multiple streams scrambles their chronological
order. To fix this, implement pagination by tracking the last timestamp returned
from each query and using it as the start time for the next query to fetch all
available logs, and sort all collected lines chronologically before joining them
together to ensure proper ordering across streams like stdout and stderr.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
Summarize your change.
[cueweb] Add optional Loki backend for frame log viewing
When NEXT_PUBLIC_LOKI_URL is set, the frame log viewer queries Loki by frame_id instead of reading the on-disk .rqlog file, mirroring CueGUI's LokiViewPlugin. Falls back to the file-based viewer when unset.
Summary by CodeRabbit
New Features
NEXT_PUBLIC_LOKI_URLenvironment variable, the frame viewer queries Loki instead of reading local files.Documentation
.env.examplewith Loki configuration guidance.Tests