Skip to content

feat(logging): add request payload retention - #164

Open
stoner-byte wants to merge 2 commits into
astaxie:mainfrom
keepcrazy:codex/request-payload-retention
Open

feat(logging): add request payload retention#164
stoner-byte wants to merge 2 commits into
astaxie:mainfrom
keepcrazy:codex/request-payload-retention

Conversation

@stoner-byte

Copy link
Copy Markdown

Summary

Add an opt-in retention policy for request and response payload logs so operators can bound sensitive payload storage and prevent unbounded SQLite growth without removing request metadata, usage, routing, or billing records.

The new TOKENHUB_REQUEST_PAYLOAD_RETENTION_DAYS setting defaults to 0, preserving the existing no-cleanup behavior. A positive value retains that many days of payload logs.

Related Issue

N/A.

Changes

  • Add and validate TOKENHUB_REQUEST_PAYLOAD_RETENTION_DAYS; reject negative and non-numeric values at startup.
  • Run payload cleanup asynchronously at startup and every 24 hours, coordinated by UTC date through the existing cluster task mechanism.
  • Delete only request_payload_logs older than the cutoff in independently committed batches of 500 rows.
  • Add an index on request_payload_logs.created_at, using concurrent creation on PostgreSQL.
  • Stop the cleanup scheduler during graceful shutdown and log failures without preventing the service from running.
  • Keep VACUUM as an explicit operator action so deleted SQLite pages remain available for reuse.
  • Document and propagate the setting through environment examples, Compose variants, start.sh, and English, Simplified Chinese, and Japanese deployment docs.

Type of Change

  • Bug fix
  • New feature
  • Refactor or maintenance
  • Documentation
  • Deployment or configuration

Verification

  • go test ./...
  • go vet ./...
  • Focused race tests for request payload cleanup and configuration
  • node --test tools/*.test.mjs (103/103 passed)
  • node tools/check-doc-translations.mjs --base upstream/main --head HEAD
  • node tools/check-ui-translations.mjs
  • node tools/check-env-contract.mjs
  • node tools/check-source-lines.mjs
  • git diff --check
  • Manual SQLite deployment validation with a two-day retention window: startup deleted 19,483 expired payload rows, zero expired rows remained, and the cleanup index and daily task state were present.
  • Compose rendering was not run because the local Docker daemon was unavailable; all three Compose YAML files are covered by the repository environment contract checks.

Compatibility, Security, and Operations

The default value is 0, so upgrades retain all payload logs unless an operator opts in. The cleanup changes no API and does not modify the existing 64 KiB payload capture limits. It deletes only request_payload_logs; request metadata, usage, route attempts, and billing data are unaffected.

A positive retention value permanently deletes expired payload bodies. Rollback is immediate by setting the value back to 0, but already deleted payloads require a database backup to recover. SQLite reuses freed pages but does not return space to the operating system until an operator performs VACUUM. Initial index creation and historical cleanup may take time on a large database and should be deployed during a low-traffic period.

Checklist

  • Tests were added or updated for behavior changes, or the reason they are unnecessary is documented.
  • No credentials, local .env files, databases, backups, or runtime logs are included.
  • Environment variable changes are synchronized across examples, Compose, start.sh, and deployment documentation where applicable.
  • Shared user-facing behavior is documented consistently in English, Simplified Chinese, and Japanese where applicable.
  • data/model-catalog.yaml remains tracked and catalog changes were reviewed where applicable.
  • git diff --check passes.

@astaxie

astaxie commented Aug 8, 2026

Copy link
Copy Markdown
Owner

please fix the conflicts

@astaxie astaxie left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer summary for current head 970ae452: the implementation remains blocked by merge conflicts with current main, even though the focused cleanup tests, go vet, repository gates, and all three Compose renders passed in the prior isolated review of this unchanged head.

A fresh git merge-tree against current main reproduces content conflicts in deploy/.env.example, deploy/docker-compose.yml, deploy/docker-compose.postgres.yml, and deploy/docker-compose.remote-postgres.yml. Please rebase or merge current main, reconcile the environment contract consistently across all Compose variants, then rerun the full deployment/environment checks and CI. No PR branch changes or workflow runs were triggered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants