Repository navigation
Refine pipeline logging and add retention caps #880
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ import { | |
| parseStoredPayload, | ||
| serializePayloadForStorage, | ||
| } from "../logPayloads"; | ||
| import { getCallLogMaxEntries, getCallLogRetentionDays } from "../logEnv"; | ||
|
|
||
| type JsonRecord = Record<string, unknown>; | ||
|
|
||
|
|
@@ -230,6 +230,7 @@ function writeCallArtifact(artifact: CallLogArtifact): string | null { | |
| try { | ||
| fs.mkdirSync(path.dirname(absPath), { recursive: true }); | ||
| fs.writeFileSync(absPath, JSON.stringify(artifact, null, 2)); | ||
| rotateCallLogs(); | ||
|
There was a problem hiding this comment. WARNING: Performance concern - rotation and overflow cleanup runs synchronously after every artifact write. This includes retention-based directory cleanup and file count pruning, which could cause latency spikes under high request throughput. Consider deferring the cleanup to a scheduled job or batching it (e.g., run cleanup every N writes or every X seconds).
Contributor
There was a problem hiding this comment. Executing Consider moving this maintenance task to a background interval or a periodic schedule (e.g., once an hour) instead of running it on the hot path of every request. |
||
| return relPath; | ||
|
Comment on lines
230
to
234
There was a problem hiding this comment.
|
||
| } catch (error) { | ||
| console.error("[callLogs] Failed to write request artifact:", (error as Error).message); | ||
|
|
@@ -293,6 +294,69 @@ function readLegacyLogFromDisk(entry: { | |
| return null; | ||
| } | ||
|
|
||
| function cleanupEmptyCallLogDirs() { | ||
| if (!CALL_LOGS_DIR || !fs.existsSync(CALL_LOGS_DIR)) return; | ||
|
|
||
| try { | ||
| for (const entry of fs.readdirSync(CALL_LOGS_DIR)) { | ||
| const entryPath = path.join(CALL_LOGS_DIR, entry); | ||
| const stat = fs.statSync(entryPath); | ||
| if (!stat.isDirectory()) continue; | ||
| if (fs.readdirSync(entryPath).length === 0) { | ||
| fs.rmSync(entryPath, { recursive: true, force: true }); | ||
| } | ||
| } | ||
| } catch { | ||
| // Best effort only. | ||
| } | ||
| } | ||
|
|
||
| export function cleanupOverflowCallLogFiles(baseDir = CALL_LOGS_DIR, maxEntries?: number) { | ||
| if (!baseDir || !fs.existsSync(baseDir)) return; | ||
|
|
||
| const limit = maxEntries ?? getCallLogMaxEntries(); | ||
| if (!Number.isInteger(limit) || limit < 1) return; | ||
|
|
||
| try { | ||
| const files = fs | ||
| .readdirSync(baseDir) | ||
| .flatMap((entry) => { | ||
| const entryPath = path.join(baseDir, entry); | ||
| try { | ||
| const stat = fs.statSync(entryPath); | ||
| if (!stat.isDirectory()) return []; | ||
|
|
||
| return fs | ||
| .readdirSync(entryPath) | ||
| .filter((file) => file.endsWith(".json")) | ||
| .map((file) => { | ||
| const filePath = path.join(entryPath, file); | ||
| const fileStat = fs.statSync(filePath); | ||
| return { filePath, mtimeMs: fileStat.mtimeMs }; | ||
| }); | ||
| } catch { | ||
| return []; | ||
| } | ||
| }) | ||
| .sort((a, b) => b.mtimeMs - a.mtimeMs); | ||
|
Comment on lines
+321
to
+341
Contributor
There was a problem hiding this comment. The implementation of |
||
|
|
||
| for (const file of files.slice(limit)) { | ||
| try { | ||
| fs.rmSync(file.filePath, { force: true }); | ||
| } catch { | ||
| // Best effort only. | ||
| } | ||
| } | ||
|
|
||
| cleanupEmptyCallLogDirs(); | ||
| } catch (error) { | ||
| console.error( | ||
| "[callLogs] Failed to prune overflow request artifacts:", | ||
| (error as Error).message | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export async function saveCallLog(entry: any) { | ||
| if (!shouldPersistToDisk) return; | ||
|
|
||
|
|
@@ -392,6 +456,7 @@ export function rotateCallLogs() { | |
| fs.rmSync(entryPath, { recursive: true, force: true }); | ||
| } | ||
| } | ||
| cleanupOverflowCallLogFiles(CALL_LOGS_DIR, getCallLogMaxEntries()); | ||
| } catch (error) { | ||
| console.error("[callLogs] Failed to rotate request artifacts:", (error as Error).message); | ||
| } | ||
|
|
||
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.
This section documents the new rotation behavior, but other repo docs still describe the old request-logger pipeline/file layout (including
2_req_source.jsonandENABLE_REQUEST_LOGS=true), e.g.docs/ARCHITECTURE.mdanddocs/CODEBASE_DOCUMENTATION.md. That drift can confuse operators upgrading to the unified logging model; please update or remove those older references (and any localized copies if applicable) to match the new pipeline stages and env var model.