Repository navigation
feat: offload log payloads to object storage #3301
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,11 @@ import { | |
| toolResults, | ||
| tools, | ||
| } from "@llmgateway/db"; | ||
| import { logger } from "@llmgateway/logger"; | ||
| import { | ||
| getLogPayload, | ||
| isPayloadStorageEnabled, | ||
| } from "@llmgateway/shared/payload-storage"; | ||
| import { buildSignedGatewayVideoLogContentUrl } from "@llmgateway/shared/video-access"; | ||
|
|
||
| import type { ServerTypes } from "@/vars.js"; | ||
|
|
@@ -56,16 +61,44 @@ const { | |
| ...publicLogColumns | ||
| } = getTableColumns(tables.log); | ||
|
|
||
| // Detail-only payload columns: rendered nowhere in list views and, on legacy | ||
| // rows written before payload offloading, still multi-MB inline — the list | ||
| // query skips them entirely. payloadRef is transport-internal (resolved into | ||
| // the payload fields by the detail handler) and never leaves the API. | ||
| const { | ||
| rawRequest: _rawRequest, | ||
| rawResponse: _rawResponse, | ||
| upstreamRequest: _upstreamRequest, | ||
| upstreamResponse: _upstreamResponse, | ||
| responsesApiData: _responsesApiData, | ||
| payloadRef: _payloadRef, | ||
| ...listLogColumns | ||
| } = publicLogColumns; | ||
|
|
||
| const logSelection = { | ||
| ...publicLogColumns, | ||
| organizationName: tables.organization.name, | ||
| projectName: tables.project.name, | ||
| apiKeyName: tables.apiKey.description, | ||
| }; | ||
|
|
||
| async function enrichLogsWithVideoContentUrls<T extends LogRecord>( | ||
| logEntries: T[], | ||
| ): Promise<T[]> { | ||
| const logListSelection = { | ||
| ...listLogColumns, | ||
| organizationName: tables.organization.name, | ||
| projectName: tables.project.name, | ||
| apiKeyName: tables.apiKey.description, | ||
| }; | ||
|
|
||
| async function enrichLogsWithVideoContentUrls< | ||
| T extends Pick< | ||
| LogRecord, | ||
| | "id" | ||
| | "content" | ||
| | "videoOutputCost" | ||
| | "videoDownloadCount" | ||
| | "lastVideoDownloadedAt" | ||
| >, | ||
| >(logEntries: T[]): Promise<T[]> { | ||
| const hasVideoLogState = (log: T) => | ||
| log.videoOutputCost !== null || | ||
| log.videoDownloadCount > 0 || | ||
|
|
@@ -234,6 +267,10 @@ const logSchema = z.object({ | |
| gatewayContentFilterResponse: gatewayContentFilterResponseSchema | ||
| .nullable() | ||
| .optional(), | ||
| // True when the row's offloaded payload blob could not be fetched (e.g. | ||
| // already deleted by the bucket lifecycle rule); the payload fields then | ||
| // carry the stored previews instead of the full data. | ||
| payloadUnavailable: z.boolean().optional(), | ||
| }); | ||
|
|
||
| // GET /logs/:id - Fetch a single log by ID | ||
|
|
@@ -710,7 +747,7 @@ logs.openapi(get, async (c) => { | |
|
|
||
| // Execute the query using select | ||
| let dbQuery = db | ||
| .select(logSelection) | ||
| .select(logListSelection) | ||
| .from(tables.log) | ||
| .leftJoin( | ||
| tables.organization, | ||
|
|
@@ -986,5 +1023,34 @@ logs.openapi(getById, async (c) => { | |
| }); | ||
| } | ||
|
|
||
| return c.json({ log }); | ||
| // Hydrate offloaded payload fields from object storage. The blob wins over | ||
| // the truncated previews stored in the row; a missing or unreadable blob | ||
| // degrades to the previews with payloadUnavailable set rather than failing | ||
| // the whole detail view. | ||
| let payloadUnavailable = false; | ||
| if (log.payloadRef && isPayloadStorageEnabled()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Mark the payload unavailable when storage is disabled. If a row has Handle every Proposed fix- if (log.payloadRef && isPayloadStorageEnabled()) {
+ if (log.payloadRef) {
+ if (!isPayloadStorageEnabled()) {
+ payloadUnavailable = true;
+ } else {
let payload: Record<string, unknown> | null = null;
try {
payload = await getLogPayload(log.payloadRef);
} catch (error) {
logger.warn("Failed to fetch log payload from storage", {
payloadRef: log.payloadRef,
error,
});
}
if (payload) {
log = { ...log, ...(payload as Partial<typeof log>) };
} else {
payloadUnavailable = true;
}
+ }
}🤖 Prompt for AI Agents |
||
| let payload: Record<string, unknown> | null = null; | ||
| try { | ||
| payload = await getLogPayload(log.payloadRef); | ||
| } catch (error) { | ||
| logger.warn("Failed to fetch log payload from storage", { | ||
| payloadRef: log.payloadRef, | ||
| error, | ||
| }); | ||
| } | ||
| if (payload) { | ||
| log = { ...log, ...(payload as Partial<typeof log>) }; | ||
| } else { | ||
| payloadUnavailable = true; | ||
| } | ||
| } | ||
|
Comment on lines
+1026
to
+1046
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🌐 Web query:
💡 Result: In the AWS SDK for JavaScript v3, the NodeHttpHandler default values for both Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'logs\.ts|payload-storage\.ts' . | sed 's#^\./##'
echo "== relevant logs.ts section =="
if [ -f apps/api/src/routes/logs.ts ]; then
nl -ba apps/api/src/routes/logs.ts | sed -n '920,1010p'
fi
echo "== payload storage files =="
if [ -f packages/shared/src/payload-storage.ts ]; then
nl -ba packages/shared/src/payload-storage.ts | sed -n '1,260p'
fi
echo "== S3Client / getLogPayload references =="
rg -n "new S3Client|getLogPayload|NodeHttpHandler|requestTimeout|connectionTimeout|maxAttempts|payloadRef|payloadStorage|isPayloadStorageEnabled" . \
-t ts -t tsx -t js -tjsx \
--glob '!node_modules' --glob '!dist' --glob '!build' \
|| trueRepository: theopenco/llmgateway Length of output: 722 🌐 Web query:
💡 Result: In the AWS SDK for JavaScript v3, the NodeHttpHandler default values for timeout-related options are set to 0, which signifies that no timeout is applied [1][2][3]. Specifically, the following settings in NodeHttpHandlerOptions default to 0: - connectionTimeout: The maximum time in milliseconds allowed to establish a connection. A value of 0 disables this timeout [1][4]. - requestTimeout: The maximum number of milliseconds the total request and response cycle may take. A value of 0 disables this timeout [1][2][4]. - socketTimeout: The maximum time in milliseconds a socket may remain idle before being closed. A value of 0 means there is no maximum (no timeout) [1][4]. Because these default to 0, the SDK will not automatically terminate requests or connections based on these time constraints unless you explicitly configure a non-zero value [1][2]. If you intend to enforce specific time limits, you should provide these options when instantiating the NodeHttpHandler [5][6]. Citations:
🏁 Script executed: #!/bin/bash
set -u
echo "== relevant logs.ts section =="
if [ -f apps/api/src/routes/logs.ts ]; then
awk '{printf "%8d\t%s\n", NR, $0}' apps/api/src/routes/logs.ts | sed -n '920,1010p'
fi
echo "== payload storage files =="
if [ -f packages/shared/src/payload-storage.ts ]; then
awk '{printf "%8d\t%s\n", NR, $0}' packages/shared/src/payload-storage.ts | sed -n '1,260p'
fi
echo "== S3Client / getLogPayload references =="
rg -n "new S3Client|getLogPayload|NodeHttpHandler|requestTimeout|connectionTimeout|maxAttempts|payloadRef|payloadStorage|isPayloadStorageEnabled" . \
--glob '!node_modules' --glob '!dist' --glob '!build' --glob '!*.lock' \
|| true
echo "== imports in logs.ts around logs.ts =="
awk '{printf "%8d\t%s\n", NR, $0}' apps/api/src/routes/logs.ts | sed -n '1,60p'Repository: theopenco/llmgateway Length of output: 23228 Bound the payload blob fetch on the shared S3 client.
🤖 Prompt for AI Agents |
||
|
|
||
| const { payloadRef: _logPayloadRef, ...logForResponse } = log; | ||
|
|
||
| return c.json({ | ||
| log: { | ||
| ...logForResponse, | ||
| ...(payloadUnavailable ? { payloadUnavailable: true } : {}), | ||
| }, | ||
| }); | ||
| }); | ||
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.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Exclude
toolResultsfrom list queries.toolResultsremains inlistLogColumns. Legacy and storage-disabled rows can therefore return the complete tool output in every list response. This defeats the detail-only projection and can retain a large payload in the query.Add
toolResultsto this destructuring block. Add it to the omission assertions inapps/api/src/routes/logs.spec.ts.Proposed fix
const { rawRequest: _rawRequest, rawResponse: _rawResponse, upstreamRequest: _upstreamRequest, upstreamResponse: _upstreamResponse, responsesApiData: _responsesApiData, + toolResults: _toolResults, payloadRef: _payloadRef, ...listLogColumns } = publicLogColumns;📝 Committable suggestion
🤖 Prompt for AI Agents