Skip to content

fix: 慢查询/错误 SQL 日志参数化 - #6493

Merged
Calcium-Ion merged 4 commits into
QuantumNous:mainfrom
feitianbubu:fix/parameterized-sql-log
Jul 27, 2026
Merged

fix: 慢查询/错误 SQL 日志参数化#6493
Calcium-Ion merged 4 commits into
QuantumNous:mainfrom
feitianbubu:fix/parameterized-sql-log

Conversation

@feitianbubu

@feitianbubu feitianbubu commented Jul 27, 2026

Copy link
Copy Markdown
Member

📝 变更描述 / Description

gorm 未显式配置 Logger 时用默认 logger,慢查询和出错的 SQL 会把参数值内联后整句打进日志

🚀 变更类型 / Type of change

  • 🐛 Bug 修复 (Bug fix)
  • ✨ 新功能 (New feature)
  • ⚡ 性能优化 / 重构 (Refactor) - 日志加固
  • 📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

✅ 提交前检查项 / Checklist

  • 人工确认: 我已亲自整理并撰写此描述,没有直接粘贴未经处理的 AI 输出。
  • 非重复提交: 我已搜索现有的 Issues 与 PRs,确认不是重复提交。
  • Bug fix 说明: 不适用(归类为重构/日志加固)。
  • 变更理解: 我已理解这些更改的工作原理及可能影响。
  • 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
  • 本地验证: 已在本地运行并通过测试或手动验证,维护者可以据此复核结果。
  • 安全合规: 代码中无敏感凭据,且符合项目代码规范。

📸 运行证明 / Proof of Work

慢查询测试:

修复前:

[42.673ms] [rows:0] SELECT * FROM `users` WHERE access_token = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwib1116IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.KMUFsID222yG3nMiGM6H9FNFUROf3wh7SmqJp-QV30' AND `users`.`deleted_at` IS NULL AND `users`.`id` = 1 ORDER BY `users`.`id` LIMIT 1

修复后:

[51.453ms] [rows:0] SELECT * FROM `users` WHERE access_token = ? AND `users`.`deleted_at` IS NULL AND `users`.`id` = ? ORDER BY `users`.`id` LIMIT 1

错误sql测试:

修复前:

Table 'newapi.users' doesn't exist
[21.489ms] [rows:0] SELECT * FROM `users` WHERE (username = 'root' OR email = 'sky@qq.com') AND `users`.`deleted_at` IS NULL ORDER BY `users`.`id` LIMIT 1

修复后:

mysql error 1146
[24.602ms] [rows:0] SELECT * FROM `users` WHERE (username = ? OR email = ?) AND `users`.`deleted_at` IS NULL ORDER BY `users`.`id` LIMIT 1

SQL_SLOW_THRESHOLD_MS 范围限制(0, 3600000)

如果排查问题需要显示参数值,可临时开环境变量 DEBUG=true

Summary by CodeRabbit

  • New Features

    • Added configurable slow-query logging for database operations, including an option to disable it with a value of 0.
    • Enhanced database logging to automatically sanitize driver error details when not in debug mode.
  • Bug Fixes

    • Standardized database connection/log configuration across supported database systems.
  • Documentation

    • Updated the environment example with clear guidance for SQL_SLOW_THRESHOLD_MS, including valid range and default fallback behavior.
  • Tests

    • Added unit tests covering error sanitization and sanitized trace logging behavior.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

The PR centralizes GORM configuration, adds slow-query threshold validation and driver-error sanitization, applies shared configuration across database initialization branches, updates dependency declarations, and adds logger tests.

GORM logging and configuration

Layer / File(s) Summary
Logger configuration and error sanitization
model/gorm_logger.go, .env.example
Adds configurable GORM logging, slow-query threshold handling, and debug-sensitive sanitization for supported database driver errors.
Database initialization wiring
model/main.go, go.mod
Replaces inline GORM configurations across database branches with the shared helper and declares database drivers directly.
Logger sanitization and trace validation
model/gorm_logger_test.go
Tests driver-specific, wrapped, SQLite, non-driver, and debug-dependent logging behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GORM
  participant sanitizedLogWriter
  participant sanitizeDBError
  participant OutputWriter
  GORM->>sanitizedLogWriter: Printf formatted trace output
  sanitizedLogWriter->>sanitizeDBError: Sanitize errors when debug is disabled
  sanitizeDBError-->>sanitizedLogWriter: Return reduced driver error
  sanitizedLogWriter->>OutputWriter: Write configured log output
Loading

Poem

I’m a rabbit, logging with care,
Hiding secret errors from the air.
Slow queries hop in line,
Debug reveals the full design,
While every database joins the lair.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了本次将慢查询和错误 SQL 日志参数化、避免参数值内联的主要变更。
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@feitianbubu feitianbubu changed the title fix: 慢查询/错误 SQL 日志参数化,避免 access_token、渠道 key 明文进日志 fix: 慢查询/错误 SQL 日志参数化 Jul 27, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@model/gorm_logger.go`:
- Line 26: Update the GORM logger configuration around ParameterizedQueries and
traceErrStr so enabled parameterization also prevents sensitive bound values or
tokens from appearing through separately logged driver errors. Add failing-query
coverage for every supported database, and sanitize or omit the error message
while retaining the existing SQL redaction behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a2fca72-5c0a-4027-948b-434fc63418c1

📥 Commits

Reviewing files that changed from the base of the PR and between f3ab2cf and f7fa82a.

📒 Files selected for processing (2)
  • model/gorm_logger.go
  • model/main.go

Comment thread model/gorm_logger.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@model/gorm_logger_test.go`:
- Around line 85-98: Update the test around newGormLoggerWithWriter and Trace to
assert that the returned gorm.ParamsFilter.ParamsFilter contains "secret-value"
in both DEBUG-disabled and DEBUG-enabled modes before invoking Trace, ensuring
the actual parameter-filter contract is exercised rather than relying only on
the redacted SQL placeholder.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1791f8af-4637-4977-bcd5-8747e3a0534a

📥 Commits

Reviewing files that changed from the base of the PR and between dbb91cc and a419779.

📒 Files selected for processing (3)
  • go.mod
  • model/gorm_logger.go
  • model/gorm_logger_test.go

Comment thread model/gorm_logger_test.go Outdated
Comment on lines +85 to +98
fc := func() (string, int64) { return "SELECT * FROM t WHERE k = ?", 0 }

common.DebugEnabled = false
var buf bytes.Buffer
newGormLoggerWithWriter(&buf).Trace(context.Background(), time.Now(), fc, driverErr)
out := buf.String()
assert.Contains(t, out, "mysql error 1062")
assert.Contains(t, out, "k = ?")
assert.NotContains(t, out, "secret-value")

common.DebugEnabled = true
buf.Reset()
newGormLoggerWithWriter(&buf).Trace(context.Background(), time.Now(), fc, driverErr)
assert.Contains(t, buf.String(), "secret-value")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Exercise the actual parameter-filter contract.

fc already returns k = ?, so this test passes even if ParameterizedQueries or ParamsFilter forwarding regresses. Assert gorm.ParamsFilter.ParamsFilter with "secret-value" in both DEBUG modes before calling Trace.

Suggested coverage
 common.DebugEnabled = false
 var buf bytes.Buffer
-newGormLoggerWithWriter(&buf).Trace(context.Background(), time.Now(), fc, driverErr)
+gormLogger := newGormLoggerWithWriter(&buf)
+filter, ok := gormLogger.(gorm.ParamsFilter)
+require.True(t, ok)
+sql, params := filter.ParamsFilter(context.Background(), "SELECT * FROM t WHERE k = ?", "secret-value")
+assert.Equal(t, "SELECT * FROM t WHERE k = ?", sql)
+assert.Empty(t, params)
+gormLogger.Trace(context.Background(), time.Now(), func() (string, int64) { return sql, 0 }, driverErr)

As per coding guidelines, backend tests must protect real behavior and regression paths.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model/gorm_logger_test.go` around lines 85 - 98, Update the test around
newGormLoggerWithWriter and Trace to assert that the returned
gorm.ParamsFilter.ParamsFilter contains "secret-value" in both DEBUG-disabled
and DEBUG-enabled modes before invoking Trace, ensuring the actual
parameter-filter contract is exercised rather than relying only on the redacted
SQL placeholder.

Source: Coding guidelines

@Calcium-Ion
Calcium-Ion merged commit c3db414 into QuantumNous:main Jul 27, 2026
1 check passed
lizhongyi1209 pushed a commit to lizhongyi1209/new-api that referenced this pull request Jul 28, 2026
* fix: parameterize slow/error SQL logs to avoid leaking credentials

* fix: validate SQL_SLOW_THRESHOLD_MS range

* fix: sanitize database driver error messages in SQL logs

* refactor: sanitize at gorm log writer seam to keep caller attribution
IQZZ020501 pushed a commit to IQZZ020501/new-api that referenced this pull request Jul 30, 2026
* fix: parameterize slow/error SQL logs to avoid leaking credentials

* fix: validate SQL_SLOW_THRESHOLD_MS range

* fix: sanitize database driver error messages in SQL logs

* refactor: sanitize at gorm log writer seam to keep caller attribution
0401lucky pushed a commit to 0401lucky/new-api that referenced this pull request Aug 2, 2026
* fix: parameterize slow/error SQL logs to avoid leaking credentials

* fix: validate SQL_SLOW_THRESHOLD_MS range

* fix: sanitize database driver error messages in SQL logs

* refactor: sanitize at gorm log writer seam to keep caller attribution
refeiner pushed a commit to wuqiang44444444/new-api that referenced this pull request Aug 17, 2026
* fix: parameterize slow/error SQL logs to avoid leaking credentials

* fix: validate SQL_SLOW_THRESHOLD_MS range

* fix: sanitize database driver error messages in SQL logs

* refactor: sanitize at gorm log writer seam to keep caller attribution
330079598 pushed a commit to 330079598/new-api that referenced this pull request Aug 19, 2026
* fix: parameterize slow/error SQL logs to avoid leaking credentials

* fix: validate SQL_SLOW_THRESHOLD_MS range

* fix: sanitize database driver error messages in SQL logs

* refactor: sanitize at gorm log writer seam to keep caller attribution
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