Skip to content

fix(tracelog): avoid growing truncated query arguments - #2657

Open
sb123sb123 wants to merge 1 commit into
jackc:masterfrom
sb123sb123:fix/tracelog-configurable-arg-limit
Open

sb123sb123 wants to merge 1 commit into
jackc:masterfrom
sb123sb123:fix/tracelog-configurable-arg-limit

Conversation

@sb123sb123

Copy link
Copy Markdown

Fixes #2095

Why

tracelog used a fixed 64-byte prefix and then appended the truncation notice. A 65-byte string was therefore logged as 84 bytes, making the logged value longer than the original. The same issue was more pronounced for []byte, which is logged as hexadecimal. This also makes the truncation length configurable as requested in #998.

What changed

  • Add TraceLogConfig.MaxLogArgsLen, defaulting to 64 bytes while preserving the existing behavior for zero-value or non-positive settings.
  • Apply the configured limit to query and batch-query arguments.
  • Retain the number of omitted bytes while reducing the retained prefix when necessary so the normal truncation output is shorter than the original logged value.
  • Preserve UTF-8 rune boundaries and cover both strings and hexadecimal []byte arguments with regression tests.

Tests

  • go test ./tracelog -run '^TestLogQueryArgs(MaxLength|DefaultDoesNotGrow)$' -count=1
  • go vet ./tracelog
  • gofmt -l tracelog/tracelog.go tracelog/tracelog_test.go (no output)
  • git diff --check
  • go test ./... -run '^$' -count=1 (compile-only pass)
  • Reproduced the upstream behavior before the change: a 65-byte string produced an 84-byte logged value.

The database-backed TestLogQueryArgsHandlesUTF8 was also attempted, but the remote Windows host has no PostgreSQL listening on localhost:5432, so all query-execution modes failed at connection setup with connection refused. Dependency downloads on that host used goproxy.cn because proxy.golang.org timed out.

AI assistance

This PR was prepared with AI assistance from GPT-5.6 Luna for issue and PR research, implementation drafting, and test review. I read the repository guidance, understand the change, reviewed the complete final diff, and verified the behavior with a baseline reproduction and focused tests.

@jackc

jackc commented Sep 26, 2026

Copy link
Copy Markdown
Owner

This addresses both points from my feedback on #2588: configurable truncation and retaining the omitted-byte count.

Before merging, please fix TestLogQueryArgsHandlesUTF8, which fails in all five query modes. Its first assertion still expects 63 zeros plus 😊 unchanged, but the new implementation truncates it. Please reconcile that expectation and retain explicit UTF-8 validity coverage.

Please also clarify the intended behavior for small limits: with MaxLogArgsLen: 8, a 9-byte string becomes a 20-byte truncation notice. Any adjustment should preserve the omitted-byte count.

Review performed by Codex and checked by me.

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.

Tracelog truncation can make output longer.

2 participants