Repository navigation
feat(log-db): support ClickHouse LOG_SQL_DSN - #29
Conversation
📝 WalkthroughWalkthroughChangesClickHouse log database support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SetupScript
participant ConfigLoader
participant DatabaseManager
participant LogServices
SetupScript->>SetupScript: Detect ClickHouse DSN and default port
SetupScript->>ConfigLoader: Provide LOG_SQL_DSN
ConfigLoader->>DatabaseManager: Select ClickHouse log engine
DatabaseManager->>LogServices: Expose ClickHouse dialect helpers
LogServices->>DatabaseManager: Build ClickHouse-compatible queries
DatabaseManager-->>LogServices: Execute queries and return results
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@backend/internal/config/config.go`:
- Around line 237-243: Update detectLogEngine in
backend/internal/config/config.go and the ClickHouse DSN handling at deploy.sh
lines 93-94, 566-577, and 611-613, plus setup-log-db.sh lines 116-125, 300-304,
and 339-342, to recognize http:// and https:// schemes. Preserve the original
scheme and query parameters, including secure, skip_verify, and tls_server_name,
when rewriting LOG_SQL_DSN, and use port 8123 for HTTP and 8443 for HTTPS
instead of native protocol defaults.
In `@backend/internal/service/user_management.go`:
- Around line 731-732: Update the uint64 conversion case in the relevant
value-conversion function to handle values greater than math.MaxInt64 before
converting to int64. Apply an explicit overflow policy, such as returning an
error or another established safe fallback, so oversized ClickHouse unsigned
values are never silently converted into negative results.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: da69dc7c-d0a7-4e41-98a3-536299fcfc1c
⛔ Files ignored due to path filters (1)
backend/go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
README.mdbackend/go.modbackend/internal/config/config.gobackend/internal/config/config_test.gobackend/internal/database/database.gobackend/internal/database/database_test.gobackend/internal/database/indexes.gobackend/internal/service/abuse_broadcast.gobackend/internal/service/ai_auto_ban.gobackend/internal/service/dashboard.gobackend/internal/service/ip_monitoring.gobackend/internal/service/log_analytics.gobackend/internal/service/risk_monitoring.gobackend/internal/service/token.gobackend/internal/service/user_management.godeploy.shsetup-log-db.sh
| // detectLogEngine extends main database detection with ClickHouse log stores. | ||
| func detectLogEngine(dsn string) DatabaseEngine { | ||
| if strings.HasPrefix(strings.ToLower(dsn), "clickhouse://") { | ||
| return ClickHouse | ||
| } | ||
| return detectEngine(dsn) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For github.com/ClickHouse/clickhouse-go/v2 v2.47.0, which DSN schemes are supported by the database/sql driver, and which options must be retained for HTTP/HTTPS and TLS connections?
💡 Result:
For github.com/ClickHouse/clickhouse-go/v2 (version 2.47.0), the supported Data Source Name (DSN) schemes for the database/sql driver are clickhouse:// (which uses the Native TCP protocol) and http:// (or https://) for HTTP-based communication [1][2][3]. When configuring connections for HTTP/HTTPS or TLS, the following options must be considered: 1. Protocol Selection: - Use http:// or https:// in your DSN string to trigger HTTP transport [4][2][3]. - Alternatively, you can specify Protocol: clickhouse.HTTP in an Options struct if using the non-sql.Open connection methods [5][2]. 2. TLS Connections (HTTPS/Native+TLS): - For secure connections, you can use the https:// scheme in your DSN or set secure=true as a query parameter [4][2]. - When secure=true is used, a minimal TLS configuration is created [4]. - To manage TLS verification, use the skip_verify=true DSN parameter to set InsecureSkipVerify to true (strongly discouraged for production) [1][5][2]. - For advanced TLS configurations (e.g., custom CA, client certificates, or specific cipher suites), you must pass a fully configured tls.Config object via the Options struct rather than relying solely on DSN string parameters [5][4]. - Use the tls_server_name parameter in the DSN to set the TLS SNI (Server Name Indication) or verification hostname [6][7]. 3. HTTP-Specific Settings: - http_proxy: Used to specify an HTTP proxy address [6][1]. - http_path: Used for custom URL paths (e.g., for proxies or load balancers requiring specific endpoints) [6][1]. Note that when using an https:// scheme, the driver automatically handles the secure connection, making the explicit secure=true parameter redundant but often used interchangeably [4]. Always ensure you are using the correct ports: 9000/9440 for Native (TCP) and 8123/8443 for HTTP [2][8].
Citations:
- 1: https://github.com/ClickHouse/clickhouse-docs/blob/main/docs/integrations/language-clients/go/config-reference.md
- 2: https://clickhouse.com/docs/integrations/language-clients/go/config-reference
- 3: https://clickhouse.com/integrations/go
- 4: https://pkg.go.dev/github.com/ClickHouse/clickhouse-go/v2@v2.34.0
- 5: https://clickhouse.com/docs/integrations/language-clients/go/configuration
- 6: https://pkg.go.dev/github.com/ClickHouse/clickhouse-go/v2
- 7: https://github.com/ClickHouse/clickhouse-go?tab=readme-ov-file
- 8: https://github.com/ClickHouse/clickhouse-docs/blob/main/docs/integrations/language-clients/go/index.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(config\.go|deploy\.sh|setup-log-db\.sh)$' || true
echo "== config.go relevant =="
sed -n '200,260p' backend/internal/config/config.go 2>/dev/null || true
echo "== deploy.sh relevant =="
sed -n '80,105p' deploy.sh 2>/dev/null || true
sed -n '550,585p' deploy.sh 2>/dev/null || true
sed -n '600,620p' deploy.sh 2>/dev/null || true
echo "== setup-log-db.sh relevant =="
sed -n '105,130p' setup-log-db.sh 2>/dev/null || true
sed -n '290,310p' setup-log-db.sh 2>/dev/null || true
sed -n '330,350p' setup-log-db.sh 2>/dev/null || true
echo "== search ClickHouse/log/db engine detection =="
rg -n 'detectLogEngine|LOG_SQL_DSN|clickhouse|LOG_SQL_DSN_FINAL|dsn_engine|final_dsn|detectEngine' backend deploy.sh setup-log-db.sh 2>/dev/null || trueRepository: james-6-23/new_api_tools
Length of output: 13325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== clickhouse-go DSN handling =="
go mod download github.com/ClickHouse/clickhouse-go/v2@v2.47.0 >/dev/null
CACHE="$(go env GOCACHE)"
FILE="$(go env GOPATH)/pkg/mod/github.com/!click!house/clickhouse-go/v2@v2.47.0/conn.go"
if [ -f "$FILE" ]; then
rg -n 'http://|https://|clickhouse://|ParseDSN|Protocol|InsecureSkipVerify|secure|skip_verify|tls_server_name|Port' "$FILE" -C 2
else
echo "conn.go not found"
fi
echo "== deploy/sh build_clickhouse_dsn =="
sed -n '130,200p' setup-log-db.sh 2>/dev/null || true
echo "== static parser for DSN rewriting sites =="
python3 - <<'PY'
import re
from pathlib import Path
files = {
'config.go': Path('backend/internal/config/config.go'),
'deploy.sh': Path('deploy.sh'),
'setup-log-db.sh': Path('setup-log-db.sh'),
}
cases = [
("http://user:pass@clickhouse:8123/db?secure=true", "http"),
("https://user:pass@clickhouse:8443/db?skip_verify=true", "https"),
("clickhouse://user:pass@clickhouse:9000/db?secure=true", "clickhouse"),
("clickhouse://user:pass@clickhouse:9440/db?secure=true", "clickhouse"),
]
for path in files.values():
text = path.read_text()
print(f"-- {path} --")
for dsn, expected in cases:
detected = None
if "detectLogEngine" in text or "dsn_engine" in text:
detected = "clickhouse" if re.search(r'(?:^[^#]*?\b(?:detectLogEngine|dsn_engine)\b[^#]*?)?elif\s+(?:\[\[|"|\'|`)?\$\{?dsn\}?~\s*\^\s*(http[s]?://|https://|clickhouse:)//', text) \
or re.search(r'elif\s+\[\[\s*"?'\$$dsn\??[^\]]*~\^\s*(http[s]?://|https://|clickhouse:)//', text) \
or 'http://user:pass@clicklass:8123/db?secure=true"'.startswith(expected) else None
# just no-op placeholder
PY
PYRepository: james-6-23/new_api_tools
Length of output: 3849
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== clickhouse-go DSN parsing =="
CACHE="$(go env GOCACHE)"
find "$CACHE" -path 'github.com/!click!house/clickhouse-go/v2@v2.47.0/*' \( -name '*.go' -o -path '*dsn*' -o -path '*conn*' \) -maxdepth 4 | head -50 || true
PKG="$(go env GOPATH)/pkg/mod/github.com/!click!house/clickhouse-go/v2@v2.47.0"
echo "pkg=$PKG"
find "$PKG" -maxdepth 3 -type f | sed "s#$PKG/##" | head -100
for f in "$PKG/conn.go" "$PKG/dsn.go" "$PKG/options.go" "$PKG/go.mod"; do
if [ -f "$f" ]; then
echo "-- $f --"
rg -n 'http(s)?://|clickhouse://|ParseDSN|InsecureSkipVerify|secure|skip_verify|tls_server_name|http_path|http_proxy|Port|NewTLS|Dial|Open' "$f" -C 2 || true
fi
done
echo "== all dsn_engine and clickhouse builders =="
rg -n 'dsn_engine|build_clickhouse_dsn|extract_dsn_engine|build_clickhouse_dsn|9000|9440|8123|8443|LOG_SQL_DSN_FINAL|LOG_SQL_DSN|detectLogEngine|detectEngine|host=|dbname' backend/internal/config/config.go deploy.sh setup-log-db.sh backend/internal/config/config_test.go 2>/dev/null || trueRepository: james-6-23/new_api_tools
Length of output: 3219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== clickhouse options files =="
for f in backend/internal/config/config.go backend/internal/config/config_test.go deploy.sh setup-log-db.sh \
backend/go.mod backend/go.sum; do
echo "-- $f --"
rg -n 'http(s)?://|clickhouse://|detectLogEngine|dsn_engine|build_clickhouse_dsn|extract_dsn_engine|LOG_SQL_DSN|LOG_SQL_DSN_FINAL|8123|8443|9000|9440|secure|skip_verify|tls_server_name|http_path|Protocol|Driver_name' "$f" -C 2 || true
done
echo "== clickhouse-go option parser strings in installed module =="
PKG="$(go env GOPATH)/pkg/mod/github.com/!click!house/clickhouse-go/v2@v2.47.0"
for f in "$PKG/clickhouse_options.go" "$PKG/clickhouse_std.go" "$PKG/dsn.go" "$PKG/conn.go"; do
if [ -f "$f" ]; then
echo "-- $f --"
rg -n 'http(s)?://|clickhouse://|parseDSN|Scheme|Protocol|clickhouse.HTTP|InsecureSkipVerify|secure|skip_verify|tls_server_name|http_path|http_proxy|Port|NewTLS|Dial' "$f" -C 2
fi
doneRepository: james-6-23/new_api_tools
Length of output: 14840
Handle ClickHouse HTTP(S) log DSNs consistently.
ClickHouse DSNs can use http:// or https:// for HTTP transport, but these scripts only recognize native clickhouse:// and rewire them into clickhouse://${user}:${pass}@${host}:${port}/${db}, dropping the original scheme and query parameters. secure defaults to true even for HTTP, so HTTPS/TLS options are not preserved.
- Keep HTTP
8123/HTTPS8443defaults instead of9000/9440. - Preserve the original scheme and ClickHouse DSN parameters such as
secure,skip_verify, andtls_server_namewhen rewritingLOG_SQL_DSN.
📍 Affects 3 files
backend/internal/config/config.go#L237-L243(this comment)deploy.sh#L93-L94deploy.sh#L566-L577deploy.sh#L611-L613setup-log-db.sh#L116-L125setup-log-db.sh#L300-L304setup-log-db.sh#L339-L342
🤖 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 `@backend/internal/config/config.go` around lines 237 - 243, Update
detectLogEngine in backend/internal/config/config.go and the ClickHouse DSN
handling at deploy.sh lines 93-94, 566-577, and 611-613, plus setup-log-db.sh
lines 116-125, 300-304, and 339-342, to recognize http:// and https:// schemes.
Preserve the original scheme and query parameters, including secure,
skip_verify, and tls_server_name, when rewriting LOG_SQL_DSN, and use port 8123
for HTTP and 8443 for HTTPS instead of native protocol defaults.
| case uint64: | ||
| return int64(val) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent uint64 overflow from becoming negative.
Line 732 wraps values above MaxInt64 into negative int64s. Define an overflow policy before conversion so ClickHouse unsigned values cannot silently corrupt IDs or metrics.
Proposed fix
case uint64:
+ if val > uint64(^uint64(0)>>1) {
+ return int64(^uint64(0) >> 1)
+ }
return int64(val)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case uint64: | |
| return int64(val) | |
| case uint64: | |
| if val > uint64(^uint64(0)>>1) { | |
| return int64(^uint64(0) >> 1) | |
| } | |
| return int64(val) |
🤖 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 `@backend/internal/service/user_management.go` around lines 731 - 732, Update
the uint64 conversion case in the relevant value-conversion function to handle
values greater than math.MaxInt64 before converting to int64. Apply an explicit
overflow policy, such as returning an error or another established safe
fallback, so oversized ClickHouse unsigned values are never silently converted
into negative results.
2 participants
背景
部分 NewAPI 部署会把
logs写入 ClickHouse。现有日志分库连接只支持 MySQL/PostgreSQL，导致仪表盘、风控和 IP 分析无法读取这类实例的实时日志。Related to #23.
改动
LOG_SQL_DSN识别clickhouse://，主库仍限定为 MySQL/PostgreSQLchannel_name时，从主库批量补齐渠道名称created_at, request_id排序，避免兼容id恒为 0 时顺序失真deploy.sh与setup-log-db.sh支持解析、改写和写入 ClickHouse 日志库 DSNLOG_SQL_DSN时回落主库的原有行为，并补充配置与方言单元测试验证
cd backend && go test ./...bash -n deploy.sh setup-log-db.shhealthy/api/analytics/sync-status返回的日志总数与 ClickHouseSELECT count()一致/api/dashboard/usage、/api/risk/leaderboards、/api/risk/users/1/analysis均成功读取 ClickHouse 日志数据Summary by CodeRabbit
New Features
Documentation
LOG_SQL_DSNconfiguration guidance with supported database types, fallback behavior, generation recommendations, and examples.Tests