Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request redesigns the request logging system, transitioning from a multi-file session layout to a unified artifact format where each request is stored as a single JSON file on disk and indexed via SQLite. It introduces split retention policies for application and call logs, replaces legacy environment variables with a new configuration model, and implements a startup migration to archive deprecated log layouts. Review feedback highlights an issue where final payloads might be omitted if intermediate pipeline steps aren't recorded, suggests optimizing the log saving process into a single database transaction, and recommends using date-based directory parsing for more reliable log rotation.
Comment on lines
+536
to
+553
| const pipelinePayloads = detailedLoggingEnabled ? reqLogger?.getPipelinePayloads?.() : null; | ||
|
|
||
| if (pipelinePayloads) { | ||
| if (providerResponse !== undefined) { | ||
| pipelinePayloads.providerResponse = providerResponse as Record<string, unknown>; | ||
| } | ||
| if (clientResponse !== undefined) { | ||
| pipelinePayloads.clientResponse = clientResponse as Record<string, unknown>; | ||
| } | ||
| if (error) { | ||
| pipelinePayloads.error = { | ||
| ...(typeof pipelinePayloads.error === "object" && pipelinePayloads.error | ||
| ? (pipelinePayloads.error as Record<string, unknown>) | ||
| : {}), | ||
| message: error, | ||
| }; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The pipelinePayloads object is only updated if it's non-null. If the request logger hasn't recorded any intermediate steps yet, getPipelinePayloads() returns null, causing the final provider/client responses and errors to be omitted from the pipeline capture even when detailed logging is enabled. This logic should be adjusted to ensure the final payloads are captured if detailed logging is active.
let pipelinePayloads = detailedLoggingEnabled ? reqLogger?.getPipelinePayloads?.() : null;
if (detailedLoggingEnabled) {
if (providerResponse !== undefined || clientResponse !== undefined || error) {
pipelinePayloads = pipelinePayloads || {};
if (providerResponse !== undefined) {
pipelinePayloads.providerResponse = providerResponse as Record<string, unknown>;
}
if (clientResponse !== undefined) {
pipelinePayloads.clientResponse = clientResponse as Record<string, unknown>;
}
if (error) {
pipelinePayloads.error = {
...(typeof pipelinePayloads.error === "object" && pipelinePayloads.error
? (pipelinePayloads.error as Record<string, unknown>)
: {}),
message: error,
};
}
}
}
Comment on lines
337
to
374
| const db = getDbInstance(); | ||
| db.prepare( | ||
| ` | ||
| INSERT INTO call_logs (id, timestamp, method, path, status, model, requested_model, provider, | ||
| account, connection_id, duration, tokens_in, tokens_out, request_type, source_format, target_format, | ||
| api_key_id, api_key_name, combo_name, request_body, response_body, error) | ||
| VALUES (@id, @timestamp, @method, @path, @status, @model, @requestedModel, @provider, | ||
| @account, @connectionId, @duration, @tokensIn, @tokensOut, @requestType, @sourceFormat, @targetFormat, | ||
| @apiKeyId, @apiKeyName, @comboName, @requestBody, @responseBody, @error) | ||
| INSERT INTO call_logs ( | ||
| id, timestamp, method, path, status, model, requested_model, provider, | ||
| account, connection_id, duration, tokens_in, tokens_out, request_type, source_format, | ||
| target_format, api_key_id, api_key_name, combo_name, request_body, response_body, error, | ||
| artifact_relpath, has_pipeline_details | ||
| ) | ||
| VALUES ( | ||
| @id, @timestamp, @method, @path, @status, @model, @requestedModel, @provider, | ||
| @account, @connectionId, @duration, @tokensIn, @tokensOut, @requestType, @sourceFormat, | ||
| @targetFormat, @apiKeyId, @apiKeyName, @comboName, @requestBody, @responseBody, @error, | ||
| NULL, 0 | ||
| ) | ||
| ` | ||
| ).run(logEntry); | ||
|
|
||
| // 2. Trim old entries beyond max | ||
| const maxLogs = await getMaxCallLogs(); | ||
| const countRow = asRecord(db.prepare("SELECT COUNT(*) as cnt FROM call_logs").get()); | ||
| const count = toNumber(countRow.cnt); | ||
| if (count > maxLogs) { | ||
| db.prepare( | ||
| ` | ||
| DELETE FROM call_logs WHERE id IN ( | ||
| SELECT id FROM call_logs ORDER BY timestamp ASC LIMIT ? | ||
| ) | ||
| ` | ||
| ).run(count - maxLogs); | ||
| } | ||
|
|
||
| // 3. Write full payload to disk file (untruncated) | ||
| // Disabled when no-log is active or payload mode is metadata/none. | ||
| if ( | ||
| shouldLogPayloadOnDisk && | ||
| !noLogEnabled && | ||
| (protectedRequestBody !== null || protectedResponseBody !== null) | ||
| ) { | ||
| writeCallLogToDisk( | ||
| { ...logEntry, tokens: { in: logEntry.tokensIn, out: logEntry.tokensOut } }, | ||
| if (!noLogEnabled) { | ||
| const artifact = buildArtifact( | ||
| logEntry, | ||
| protectedRequestBody, | ||
| protectedResponseBody | ||
| protectedResponseBody, | ||
| protectedError, | ||
| protectedPipelinePayloads | ||
| ); | ||
| const artifactRelPath = writeCallArtifact(artifact); | ||
|
|
||
| if (artifactRelPath) { | ||
| db.prepare( | ||
| ` | ||
| UPDATE call_logs | ||
| SET artifact_relpath = ?, has_pipeline_details = ? | ||
| WHERE id = ? | ||
| ` | ||
| ).run(artifactRelPath, protectedPipelinePayloads ? 1 : 0, logEntry.id); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The saveCallLog function performs an INSERT followed by an UPDATE to store the artifact path. This can be optimized into a single INSERT by calculating the artifact path and preparing the artifact before the database operation.
const db = getDbInstance();
let artifactRelPath: string | null = null;
let hasPipelineDetails = 0;
if (!noLogEnabled) {
const artifact = buildArtifact(
logEntry,
protectedRequestBody,
protectedResponseBody,
protectedError,
protectedPipelinePayloads
);
artifactRelPath = writeCallArtifact(artifact);
hasPipelineDetails = protectedPipelinePayloads ? 1 : 0;
}
db.prepare(
`
INSERT INTO call_logs (
id, timestamp, method, path, status, model, requested_model, provider,
account, connection_id, duration, tokens_in, tokens_out, request_type, source_format,
target_format, api_key_id, api_key_name, combo_name, request_body, response_body, error,
artifact_relpath, has_pipeline_details
)
VALUES (
@id, @timestamp, @method, @path, @status, @model, @requestedModel, @provider,
@account, @connectionId, @duration, @tokensIn, @tokensOut, @requestType, @sourceFormat,
@targetFormat, @apiKeyId, @apiKeyName, @comboName, @requestBody, @responseBody, @error,
@artifactRelPath, @hasPipelineDetails
)
`
).run({ ...logEntry, artifactRelPath, hasPipelineDetails });
Comment on lines
380
to
398
| export function rotateCallLogs() { | ||
| if (!CALL_LOGS_DIR || !fs.existsSync(CALL_LOGS_DIR)) return; | ||
|
|
||
| try { | ||
| const entries = fs.readdirSync(CALL_LOGS_DIR); | ||
| const now = Date.now(); | ||
| const retentionMs = LOG_RETENTION_DAYS * 24 * 60 * 60 * 1000; | ||
| const retentionMs = getCallLogRetentionDays() * 24 * 60 * 60 * 1000; | ||
|
|
||
| for (const entry of entries) { | ||
| const entryPath = path.join(CALL_LOGS_DIR, entry); | ||
| const stat = fs.statSync(entryPath); | ||
| if (stat.isDirectory() && now - stat.mtimeMs > retentionMs) { | ||
| fs.rmSync(entryPath, { recursive: true, force: true }); | ||
| console.log(`[callLogs] Rotated old logs: ${entry}`); | ||
| } | ||
| } | ||
| } catch (err: any) { | ||
| console.error("[callLogs] Failed to rotate logs:", err.message); | ||
| } catch (error) { | ||
| console.error("[callLogs] Failed to rotate request artifacts:", (error as Error).message); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
rotateCallLogs relies on the directory's mtimeMs to determine if it should be deleted. Directory modification times can be updated by various filesystem operations (like adding/removing files) and are not a reliable indicator of the log's age. Since directories are named by date (YYYY-MM-DD), it's safer to parse the date from the directory name.
export function rotateCallLogs() {
if (!CALL_LOGS_DIR || !fs.existsSync(CALL_LOGS_DIR)) return;
try {
const entries = fs.readdirSync(CALL_LOGS_DIR);
const now = new Date();
now.setHours(0, 0, 0, 0);
const retentionDays = getCallLogRetentionDays();
for (const entry of entries) {
const entryPath = path.join(CALL_LOGS_DIR, entry);
if (!fs.statSync(entryPath).isDirectory()) continue;
// Parse YYYY-MM-DD from directory name
const match = entry.match(/^(\d{4})-(\d{2})-(\d{2})$/);
if (!match) continue;
const folderDate = new Date(entry);
if (Number.isNaN(folderDate.getTime())) continue;
const diffDays = (now.getTime() - folderDate.getTime()) / (1000 * 60 * 60 * 24);
if (diffDays > retentionDays) {
fs.rmSync(entryPath, { recursive: true, force: true });
}
}
} catch (error) {
console.error("[callLogs] Failed to rotate request artifacts:", (error as Error).message);
}
}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewThe PR implements a unified request logging system with:
Code Quality
Files Reviewed
Existing Comments (3)Three existing inline comments on Observations Outside Diff
This is a well-executed refactoring that properly handles the breaking change with migration logic. The code is clean, tests are comprehensive, and the approach correctly addresses the fragmentation issues described in the PR. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates OmniRoute’s fragmented request logging into a unified SQLite + single JSON artifact per request model, with split retention controls for application/audit logs vs request/call logs, plus a one-time upgrade archive of legacy layouts.
Changes:
- Replaced legacy
DATA_DIR/logs/*sessions +DATA_DIR/log.txtwith per-request JSON artifacts underDATA_DIR/call_logs/YYYY-MM-DD/and an updatedcall_logsSQLite schema. - Introduced new
APP_LOG_*andCALL_LOG_RETENTION_DAYSenv model viasrc/lib/logEnv.ts, updating log rotation + console log viewer plumbing accordingly. - Added one-time startup archiving of legacy request log layouts into
DATA_DIR/log_archives/*.zip, plus updated API/UI/docs and unit tests.
Reviewed changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/usage-analytics.test.mjs | Updates usage analytics test to derive recent request summaries from SQLite call_logs. |
| tests/unit/request-log-migration.test.mjs | Adds coverage for legacy request log archival + failure behavior. |
| tests/unit/log-retention.test.mjs | Adds coverage for split APP vs CALL retention cleanup behavior. |
| tests/unit/console-log-levels.test.mjs | Updates tests for APP_LOG_FILE_PATH env rename. |
| tests/unit/call-log-cap.test.mjs | Replaces max-count cap expectations with unified artifact + retention rotation behavior. |
| tests/unit/batch-b-final.test.mjs | Updates compliance retention expectation and reformats cases. |
| src/shared/validation/settingsSchemas.ts | Removes deprecated request-log related settings fields from validation schema. |
| src/shared/validation/schemas.ts | Mirrors removal of deprecated logging settings fields. |
| src/shared/utils/structuredLogger.ts | Switches structured logger envs to APP_LOG_* via logEnv. |
| src/shared/utils/logger.ts | Switches pino logger level source to APP_LOG_LEVEL. |
| src/shared/schemas/validation.ts | Removes deprecated settings properties from shared schema. |
| src/shared/components/RequestLoggerV2.tsx | Updates UI copy to reflect unified artifact + pipeline capture behavior. |
| src/shared/components/RequestLoggerDetail.tsx | Expands pipeline payload section mapping for the unified artifact keys. |
| src/shared/components/ConsoleLogViewer.tsx | Updates user-facing guidance to APP_LOG_TO_FILE. |
| src/server-init.ts | Ensures usage migrations (including archive) run before serving, expands cleanup log conditions. |
| src/lib/usage/usageHistory.ts | Removes log.txt writer; getRecentLogs now queries call_logs. |
| src/lib/usage/migrations.ts | Adds legacy request log archival to zip + marker; adjusts call log schema migration writes. |
| src/lib/usage/callLogs.ts | Writes unified per-request artifact; adds artifact_relpath + pipeline flag updates; uses CALL_LOG_RETENTION_DAYS. |
| src/lib/proxyLogger.ts | Removes count-based DB trimming; caps only in-memory entries. |
| src/lib/logRotation.ts | Switches to logEnv getters for app log rotation configuration. |
| src/lib/logEnv.ts | New centralized env parsing for app/call log configuration. |
| src/lib/db/migrations/013_unified_log_artifacts.sql | Migrates settings key detailed_logs_enabled → call_log_pipeline_enabled and removes deprecated keys. |
| src/lib/db/migrations/001_initial_schema.sql | Adds artifact_relpath + has_pipeline_details columns to call_logs. |
| src/lib/db/detailedLogs.ts | Retains as legacy reader; toggles now read call_log_pipeline_enabled. |
| src/lib/db/core.ts | Ensures new call_logs columns exist via runtime schema checks. |
| src/lib/consoleInterceptor.ts | Switches interceptor envs to APP_LOG_* via logEnv. |
| src/lib/compliance/index.ts | Splits retention windows (APP vs CALL) and cleans additional tables. |
| src/instrumentation-node.ts | Imports usage migrations early; expands cleanup log conditions. |
| src/domain/types.ts | Updates settings type docs to reflect pipeline toggle key. |
| src/app/api/storage/health/route.ts | Returns split retention windows instead of a single constant. |
| src/app/api/settings/route.ts | Removes deprecated enableRequestLogs and maxCallLogs behaviors. |
| src/app/api/logs/export/route.ts | Treats request-logs exports as call_logs exports. |
| src/app/api/logs/detail/route.ts | POST toggles pipeline capture setting (call_log_pipeline_enabled) and updates messaging. |
| src/app/api/logs/console/route.ts | Uses getAppLogFilePath() instead of LOG_FILE_PATH. |
| src/app/(dashboard)/dashboard/settings/components/SystemStorageTab.tsx | Updates Storage UI to show split retention windows and removes max-call-log limit controls. |
| README.md | Documents breaking change and new logging env vars + artifact locations. |
| package.json | Adds yazl dependency for zip archival. |
| package-lock.json | Locks yazl + transitive deps. |
| open-sse/utils/usageTracking.ts | Removes DB side effects in streaming usage helper; keeps legacy summary hook. |
| open-sse/utils/requestLogger.ts | Replaces filesystem session logging with in-memory pipeline payload capture + chunk buffers. |
| open-sse/utils/logger.ts | Switches Open-SSE logger to APP_LOG_LEVEL / APP_LOG_FORMAT. |
| open-sse/handlers/chatCore.ts | Embeds pipeline payloads into unified call log artifact; removes request_detail_logs writes. |
| CHANGELOG.md | Adds breaking change notes + highlights unified logging/retention changes. |
Comments suppressed due to low confidence (1)
src/lib/proxyLogger.ts:153
- The SQLite
proxy_logstable is no longer trimmed on insert (the old count-based delete was removed), so it can grow without bound between retention cleanups. SincecleanupExpiredLogs()currently runs only at startup, a long-running instance can accumulate a very largeproxy_logstable within the retention window. Consider adding a bounded pruning strategy (by age and/or max row count) duringlogProxyEvent, or scheduling periodic compliance cleanup.
// 2. Persist to SQLite
if (shouldPersistToDisk) {
try {
const db = getDbInstance();
db.prepare(
`INSERT INTO proxy_logs (id, timestamp, status, proxy_type, proxy_host, proxy_port,
level, level_id, provider, target_url, public_ip, latency_ms, error,
connection_id, combo_id, account, tls_fingerprint)
VALUES (@id, @timestamp, @status, @proxyType, @proxyHost, @proxyPort,
@level, @levelId, @provider, @targetUrl, @publicIp, @latencyMs, @error,
@connectionId, @comboId, @account, @tlsFingerprint)`
).run({
id: log.id,
timestamp: log.timestamp,
status: log.status,
proxyType: log.proxy?.type || null,
proxyHost: log.proxy?.host || null,
proxyPort: log.proxy?.port ? Number(log.proxy.port) : null,
level: log.level,
levelId: log.levelId,
provider: log.provider,
targetUrl: log.targetUrl,
publicIp: log.publicIp,
latencyMs: log.latencyMs,
error: log.error,
connectionId: log.connectionId,
comboId: log.comboId,
account: log.account,
tlsFingerprint: log.tlsFingerprint ? 1 : 0,
});
} catch (err: any) {
console.warn("[proxyLogger] Failed to persist:", err.message);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+138
to
212
| const streamChunks = createEmptyStreamChunks(); | ||
| const payloads: RequestPipelinePayloads = { | ||
| streamChunks, | ||
| }; | ||
|
|
||
| return { | ||
| get sessionPath() { | ||
| return sessionPath; | ||
| }, | ||
| sessionPath: null, | ||
|
|
||
| // 1. Log client raw request (before all conversion steps) | ||
| logClientRawRequest(endpoint, body, headers = {}) { | ||
| writeJsonFile(sessionPath, "1_req_client.json", { | ||
| payloads.clientRawRequest = { | ||
| timestamp: new Date().toISOString(), | ||
| endpoint, | ||
| headers: maskSensitiveHeaders(headers), | ||
| body, | ||
| }); | ||
| }; | ||
| }, | ||
|
|
||
| // 2. Log raw request from client (after initial conversion like responsesApi) | ||
| logRawRequest(body, headers = {}) { | ||
| writeJsonFile(sessionPath, "2_req_source.json", { | ||
| payloads.sourceRequest = { | ||
| timestamp: new Date().toISOString(), | ||
| headers: maskSensitiveHeaders(headers), | ||
| body, | ||
| }); | ||
| }; | ||
| }, | ||
|
|
||
| // 3. Log OpenAI intermediate format (source → openai) | ||
| logOpenAIRequest(body) { | ||
| writeJsonFile(sessionPath, "3_req_openai.json", { | ||
| payloads.openaiRequest = { | ||
| timestamp: new Date().toISOString(), | ||
| body, | ||
| }); | ||
| }; | ||
| }, | ||
|
|
||
| // 4. Log target format request (openai → target) | ||
| logTargetRequest(url, headers, body) { | ||
| writeJsonFile(sessionPath, "4_req_target.json", { | ||
| payloads.providerRequest = { | ||
| timestamp: new Date().toISOString(), | ||
| url, | ||
| headers: maskSensitiveHeaders(headers), | ||
| body, | ||
| }); | ||
| }; | ||
| }, | ||
|
|
||
| // 5. Log provider response (for non-streaming or error) | ||
| logProviderResponse(status, statusText, headers, body) { | ||
| const filename = "5_res_provider.json"; | ||
| writeJsonFile(sessionPath, filename, { | ||
| payloads.providerResponse = { | ||
| timestamp: new Date().toISOString(), | ||
| status, | ||
| statusText, | ||
| headers: headers | ||
| ? typeof headers.entries === "function" | ||
| ? Object.fromEntries(headers.entries()) | ||
| : headers | ||
| : {}, | ||
| headers: maskSensitiveHeaders(headers), | ||
| body, | ||
| }); | ||
| }; | ||
| }, | ||
|
|
||
| // 5. Append streaming chunk to provider response (async) | ||
| appendProviderChunk(chunk) { | ||
| if (!fs || !sessionPath) return; | ||
| const filePath = path.join(sessionPath, "5_res_provider.txt"); | ||
| fs.promises.appendFile(filePath, chunk).catch(() => {}); | ||
| if (typeof chunk === "string" && chunk.length > 0) { | ||
| streamChunks.provider.push(chunk); | ||
| } | ||
| }, | ||
|
|
||
| // 6. Append OpenAI intermediate chunks (async) | ||
| appendOpenAIChunk(chunk) { | ||
| if (!fs || !sessionPath) return; | ||
| const filePath = path.join(sessionPath, "6_res_openai.txt"); | ||
| fs.promises.appendFile(filePath, chunk).catch(() => {}); | ||
| if (typeof chunk === "string" && chunk.length > 0) { | ||
| streamChunks.openai.push(chunk); | ||
| } | ||
| }, | ||
|
|
||
| // 7. Log converted response to client (for non-streaming) | ||
| logConvertedResponse(body) { | ||
| writeJsonFile(sessionPath, "7_res_client.json", { | ||
| payloads.clientResponse = { | ||
| timestamp: new Date().toISOString(), | ||
| body, | ||
| }); | ||
| }; | ||
| }, | ||
|
|
||
| // 7b. Append streaming chunk to converted response (async) | ||
| appendConvertedChunk(chunk) { | ||
| if (!fs || !sessionPath) return; | ||
| const filePath = path.join(sessionPath, "7_res_client.txt"); | ||
| fs.promises.appendFile(filePath, chunk).catch(() => {}); | ||
| if (typeof chunk === "string" && chunk.length > 0) { | ||
| streamChunks.client.push(chunk); | ||
| } | ||
| }, |
There was a problem hiding this comment.
createRequestLogger now buffers every streaming chunk into in-memory arrays (streamChunks.*). These arrays grow without bounds during long/large streams, and they are populated even when pipeline capture is disabled (since chunk append calls still run). This is an OOM risk; consider gating chunk accumulation behind an explicit enabled flag (e.g., only collect when call_log_pipeline_enabled is true) and/or enforcing a max chunk count/byte budget with truncation metadata.
Comment on lines
533
to
+538
| claudeCacheUsageMeta?: Record<string, unknown>; | ||
| }) => { | ||
| const callLogId = generateRequestId(); | ||
| const pipelinePayloads = detailedLoggingEnabled ? reqLogger?.getPipelinePayloads?.() : null; | ||
|
|
||
| if (pipelinePayloads) { |
There was a problem hiding this comment.
persistAttemptLogs references reqLogger before reqLogger is declared/initialized later in the same scope. This is fragile (TDZ risk if the function is ever invoked earlier) and commonly triggers TS “used before declaration” diagnostics depending on build settings. Consider declaring let reqLogger before persistAttemptLogs (assigning it later), or moving persistAttemptLogs below the reqLogger initialization / passing the pipeline payloads into persistAttemptLogs explicitly.
Comment on lines
380
to
406
| export function rotateCallLogs() { | ||
| if (!CALL_LOGS_DIR || !fs.existsSync(CALL_LOGS_DIR)) return; | ||
|
|
||
| try { | ||
| const entries = fs.readdirSync(CALL_LOGS_DIR); | ||
| const now = Date.now(); | ||
| const retentionMs = LOG_RETENTION_DAYS * 24 * 60 * 60 * 1000; | ||
| const retentionMs = getCallLogRetentionDays() * 24 * 60 * 60 * 1000; | ||
|
|
||
| for (const entry of entries) { | ||
| const entryPath = path.join(CALL_LOGS_DIR, entry); | ||
| const stat = fs.statSync(entryPath); | ||
| if (stat.isDirectory() && now - stat.mtimeMs > retentionMs) { | ||
| fs.rmSync(entryPath, { recursive: true, force: true }); | ||
| console.log(`[callLogs] Rotated old logs: ${entry}`); | ||
| } | ||
| } | ||
| } catch (err: any) { | ||
| console.error("[callLogs] Failed to rotate logs:", err.message); | ||
| } catch (error) { | ||
| console.error("[callLogs] Failed to rotate request artifacts:", (error as Error).message); | ||
| } | ||
| } | ||
|
|
||
| // Run rotation on startup | ||
| if (shouldPersistToDisk) { | ||
| try { | ||
| rotateCallLogs(); | ||
| } catch {} | ||
| } catch { | ||
| // Best-effort startup cleanup. | ||
| } | ||
| } |
There was a problem hiding this comment.
rotateCallLogs() (artifact retention) is only invoked once at module load/startup. On long-running instances that rarely restart, artifact directories older than CALL_LOG_RETENTION_DAYS will never be removed, which undermines the retention guarantees described in the PR. Consider scheduling periodic rotation (e.g., daily timer) and/or invoking rotation opportunistically (e.g., every N calls) so retention is enforced during runtime.
Collaborator
|
Thanks @rdself for this great contribution! 🎉 The improvements have been integrated into the |
diegosouzapw
added a commit
that referenced
this pull request
Mar 31, 2026
This was referenced Apr 30, 2026
Merged
Merged
Merged
Poid-ZA
pushed a commit
to Poid-ZA/OmniRoute
that referenced
this pull request
Aug 5, 2026
…ntion and artifacts
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…ntion and artifacts
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
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.
Why this change
The current logging layout is fragmented across multiple storage paths and formats: SQLite rows,
DATA_DIR/log.txt, multi-file session folders underDATA_DIR/logs/, and payload archives underDATA_DIR/call_logs/.That fragmentation creates three practical problems:
DATA_DIR/logs/kept accumulating multi-file request sessions without automatic cleanup or file rotation.DATA_DIR/log.txtonly truncated to a fixed line count and did not have a proper size/time-based rotation policy. Disk usage can grow very quickly and fill the data volume with low-value log files.This PR consolidates request logging into one predictable structure with one retention model and one per-request artifact format.
Breaking Change
This PR redesigns OmniRoute's request logging, retention, and logging configuration model.
Upgrade impact
DATA_DIR/call_logs/YYYY-MM-DD/.DATA_DIR/logs/multi-file session layout andDATA_DIR/log.txtsummary file are removed.DATA_DIR/logs/, legacyDATA_DIR/call_logs/, andDATA_DIR/log.txtintoDATA_DIR/log_archives/*.zipbefore removing the deprecated layout.APP_LOG_TO_FILEAPP_LOG_FILE_PATHAPP_LOG_MAX_FILE_SIZEAPP_LOG_RETENTION_DAYSAPP_LOG_LEVELAPP_LOG_FORMATCALL_LOG_RETENTION_DAYSdetailed_logs_enabledis replaced bycall_log_pipeline_enabled.What changed
DATA_DIR/call_logs/.APP_LOG_RETENTION_DAYS) and request/call (CALL_LOG_RETENTION_DAYS) windows.APP_LOG_*namespace.usage_historyrow that previously wrote an extra record with empty status metadata.Validation
npm run typecheck:corenpm run test:unit(triggered by the commit hook, 1170 tests passed)data/directory:streamChunks