feat: enhance DB connection tracing with real-time server address detection #1028
Conversation
…ection This change improves database tracing by capturing and tracking the actual server addresses for database connections. Key improvements include: - Implement WeakMap-based caching to store connection configurations - Intercept reconnect and getConnection methods to capture server details - Track server host and port information in tracing context - Optimize aspect to capture connection details at the right lifecycle point This enables more accurate distributed tracing with actual database server addresses instead of placeholder values.
|
Caution Review failedThe pull request is closed. WalkthroughDbAspect 增强了对 Hyperf DB 的拦截点，新增对 Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant DbAspect
participant ServerCache as WeakMap
participant Context
participant DB as Hyperf\DB
Caller->>DbAspect: 调用 reconnect(...)
DbAspect->>DB: proceed reconnect(...)
DB-->>DbAspect: 返回 Connection 实例
DbAspect->>ServerCache: cache(connection -> config)
DbAspect-->>Caller: 返回结果
Caller->>DbAspect: 调用 getConnection(...)
DbAspect->>DB: proceed getConnection(...)
DB-->>DbAspect: 返回 Connection 实例
DbAspect->>ServerCache: read config by instance
DbAspect->>Context: set TRACE_DB_SERVER_ADDRESS / TRACE_DB_SERVER_PORT
DbAspect-->>Caller: 返回 Connection 实例
Note over DbAspect, Context: 构建 DB span 时
DbAspect->>Context: read server.host / server.port (默认 localhost:3306)
DbAspect-->>Caller: 输出包含 db.collection.name、db.operation.name、db.pool.* 与 server 信息 的追踪数据
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分钟
Possibly related PRs
诗歌
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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. 🔧 PHPStan (2.1.32)At least one path must be specified to analyse. 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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Aspect/DbAspect.php(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Aspect/DbAspect.php (4)
src/sentry/src/Constants.php (1)
Constants(14-43)src/sentry/src/Util/SqlParser.php (1)
SqlParser(20-65)src/sentry/src/Tracing/Tracer.php (1)
trace(70-93)src/sentry/src/Function.php (1)
trace(48-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
🔇 Additional comments (4)
src/sentry/src/Tracing/Aspect/DbAspect.php (4)
43-50: LGTM! WeakMap 使用得当。使用 WeakMap 来缓存连接配置是一个很好的设计选择，它可以避免阻止连接对象的垃圾回收，确保内存使用效率。
58-64: LGTM! reconnect 拦截逻辑清晰。使用
tap函数在 reconnect 后缓存连接配置是一个优雅的实现，不会影响原有的返回值。
81-86: LGTM! 使用闭包访问私有属性是合适的方法。通过闭包绑定访问实例的
$poolName私有属性是 PHP 中的标准模式，实现简洁且有效。
107-108: 执行顺序验证已完成——代码是安全的。通过验证发现，当
DB::__call被调用时，Hyperf 的连接解析器会先检查 Context 中是否存在连接。若不存在，会从连接池获取并隐式调用getConnection()，这会触发 Context 值的设置。虽然在事务或连接重用的场景中，getConnection()可能不会再次被调用，导致 Context 值可能不被更新，但代码通过提供合理的默认值（'localhost' 和 3306）已妥善处理此情况。在绝大多数场景下，尤其是首次数据库操作时，getConnection()会在__call之前被调用并正确设置 Context 值。此代码实现是安全的。
…ection (#1028) * feat: enhance DB connection tracing with real-time server address detection This change improves database tracing by capturing and tracking the actual server addresses for database connections. Key improvements include: - Implement WeakMap-based caching to store connection configurations - Intercept reconnect and getConnection methods to capture server details - Track server host and port information in tracing context - Optimize aspect to capture connection details at the right lifecycle point This enables more accurate distributed tracing with actual database server addresses instead of placeholder values. * feat: improve server address tracing in DbAspect with default values * feat: improve driver detection logic in DbAspect for database connections --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Aspect/DbAspect.php(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Aspect/DbAspect.php (3)
src/sentry/src/Constants.php (1)
Constants(14-43)src/sentry/src/Feature.php (2)
Feature(18-75)__construct(20-22)src/sentry/src/Util/SqlParser.php (1)
SqlParser(20-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
🔇 Additional comments (7)
src/sentry/src/Tracing/Aspect/DbAspect.php (7)
14-28: 导入语句正确，支持新增的追踪功能。所有新增的导入都在代码中被使用：
Constants和Context用于追踪上下文，WeakMap用于缓存连接配置，tap用于副作用处理。实现合理。
30-34: 文档注解增强了类型提示。新增的
@property注解为动态访问的属性提供了类型信息，有助于 IDE 自动补全和静态分析。
37-41: 拦截点更新支持新的追踪逻辑。增加了对
reconnect和getConnection的拦截，使得能够在连接生命周期的关键节点捕获服务器信息。实现思路清晰。
43-52: WeakMap 的使用有效避免了内存泄漏。使用
WeakMap缓存连接配置是一个很好的设计选择，它能够自动管理连接对象的生命周期，当连接对象被垃圾回收时，缓存条目也会自动清理，不会造成内存泄漏。
69-78: 空值合并保护已正确应用。代码已使用
??操作符为host和port提供了默认值，避免了未定义键的访问警告。Line 73 的null检查进一步确保了只有成功获取到缓存配置时才会写入上下文。实现安全可靠。
85-89: 通过闭包访问私有属性的方式简洁高效。使用
(fn () => $this->poolName)->call($instance)访问DB实例的私有属性是一个优雅的解决方案，避免了使用反射的开销。后续的类型检查确保了池对象的类型安全。
101-113: 追踪数据更加丰富，有助于问题诊断。新增的元数据字段（表名、操作类型、连接池指标、服务器地址）显著提升了可观测性。从
Context获取服务器信息的方式与getConnection拦截器中设置的逻辑一致，确保了数据的完整性。
| if ($proceedingJoinPoint->methodName === 'reconnect') { | ||
| return tap($proceedingJoinPoint->process(), function () use ($proceedingJoinPoint) { | ||
| /** @var \Hyperf\DB\AbstractConnection $connection */ | ||
| $connection = $proceedingJoinPoint->getInstance(); | ||
| $this->serverCache[$connection] = $connection->getConfig(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
验证初次连接时的缓存填充逻辑。
当前实现仅在 reconnect 时填充 serverCache。如果连接池返回的是已建立的连接且未触发重连，serverCache 可能不包含该连接的配置信息，导致 Line 72 获取不到服务器地址。
请验证以下场景是否正常工作：
- 应用启动后首次调用
DB::getConnection()获取连接 - 该连接来自连接池且无需重连
- 此时
serverCache中是否有该连接的配置
运行以下脚本验证连接生命周期中 reconnect 的调用时机：
🏁 Script executed:
#!/bin/bash
# 描述：查找 reconnect 方法的调用位置，确认何时会触发重连
# 搜索 reconnect 方法的定义和调用
rg -n --type php -C 5 'function reconnect\(\)|->reconnect\(\)'
# 搜索连接池如何获取和管理连接
rg -n --type php -C 5 'class.*Connection.*Pool' -g '!vendor/**'
# 查找 getConnection 的实现
ast-grep --pattern $'function getConnection($$$) {
$$$
}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Verify file existence and examine DbAspect.php content
echo "=== Checking file existence ==="
find . -name "DbAspect.php" -type f 2>/dev/null | head -20
echo "=== Viewing DbAspect.php content ==="
cat src/sentry/src/Tracing/Aspect/DbAspect.php 2>/dev/null || echo "File not found at specified path"
echo "=== Searching for reconnect without type filter ==="
rg -n 'reconnect' -C 3 2>/dev/null | head -50
echo "=== Searching for getConnection without type filter ==="
rg -n 'getConnection' -C 2 2>/dev/null | head -50Length of output: 11210
[/web_search]
🌐 Web query:
Hyperf database connection pool reconnect lifecycle when new connection
💡 Result:
Short answer — what happens when the pool needs a "new"/reconnected DB connection in Hyperf:
-
When a connection is borrowed the Connection->getActiveConnection() first calls check(). check() compares now against connection->lastUseTime + pool.maxIdleTime; if the connection is considered expired/invalid, getActiveConnection() calls reconnect(). If reconnect() fails an exception is thrown. [1]
-
If there are no idle connections available and the pool hasn't reached its configured max connections, the pool will create a new connection (up to max). If the pool is at capacity, callers wait up to wait_timeout (or get a "pool exhausted" error). Pool options that control this are min/max connections, maxIdleTime, maxLifetime/heartbeat, connect_timeout and wait_timeout. [2][3]
Practical implications:
- Tuning maxIdleTime / maxLifetime avoids returning stale sockets and forces reconnection before DB-side timeouts.
- If reconnect() can be slow, increase connect_timeout or min_connections to avoid latency spikes.
- Monitor pool exhaustion (wait_timeout) and adjust max_connections or reduce connection hold time. [2][3]
Sources: Hyperf Connection implementation showing check()/reconnect() and lastUseTime logic, and Hyperf/third‑party docs explaining pool options and behavior. [1][2][3]
References
[1] Source showing Connection::getActiveConnection() / check() logic.
[2] Hyperf connection‑pool configuration and lifecycle documentation.
[3] Community posts/examples on pool exhaustion and pool option effects.
getConnection() 处理器应同时缓存新连接的配置信息
验证确认了原始问题的有效性。根据 Hyperf 连接池生命周期：reconnect() 仅在连接超时失效时调用，新创建的连接不会触发重连。因此 serverCache 仅在第 64-65 行的 reconnect() 处理器中填充。
对于从连接池获取的新连接：
- 第 71 行的
$server = $this->serverCache[$connection] ?? null返回 null - 第 72-75 行 Context 值不被设置，使用默认值 ('localhost', 3306)
- 实际的数据库服务器配置未被捕获
修复：在 getConnection() 处理器（第 69-76 行）中，当缓存未命中时也应捕获配置：
if ($proceedingJoinPoint->methodName === 'getConnection') {
return tap($proceedingJoinPoint->process(), function ($connection) {
$server = $this->serverCache[$connection] ?? null;
if ($server === null) {
$server = $connection->getConfig();
$this->serverCache[$connection] = $server;
}
Context::set(Constants::TRACE_DB_SERVER_ADDRESS, $server['host'] ?? 'localhost');
Context::set(Constants::TRACE_DB_SERVER_PORT, $server['port'] ?? 3306);
});
}这确保新连接和重连连接的配置都被正确缓存和上下文化。
🤖 Prompt for AI Agents
In src/sentry/src/Tracing/Aspect/DbAspect.php around lines 61-76, the
reconnect() handler caches connection config but the getConnection() handler
does not populate serverCache for newly created connections, causing Context to
use defaults; update the getConnection() branch so that after calling
$proceedingJoinPoint->process() you inspect the returned connection, and if
$this->serverCache[$connection] is null fetch $connection->getConfig() and store
it in $this->serverCache[$connection], then set
Context::set(Constants::TRACE_DB_SERVER_ADDRESS, $server['host'] ?? 'localhost')
and Context::set(Constants::TRACE_DB_SERVER_PORT, $server['port'] ?? 3306) so
both new and reconnected connections populate the cache and Context.
Summary
reconnectandgetConnectionmethods to capture actual server detailsTechnical Details
This PR enhances the
DbAspectto capture and track actual database server addresses during connection lifecycle events:reconnectto cache config andgetConnectionto populate contextBenefits
Test Plan
Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.