Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 45 additions & 9 deletions src/sentry/src/Tracing/Aspect/DbAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,45 @@

namespace FriendsOfHyperf\Sentry\Tracing\Aspect;

use FriendsOfHyperf\Sentry\Constants;
use FriendsOfHyperf\Sentry\Feature;
use FriendsOfHyperf\Sentry\Util\SqlParser;
use Hyperf\DB\DB;
use Hyperf\Context\Context;
use Hyperf\DB\Pool\PoolFactory;
use Hyperf\Di\Aop\AbstractAspect;
use Hyperf\Di\Aop\ProceedingJoinPoint;
use PDO;
use Psr\Container\ContainerInterface;
use Sentry\State\Scope;
use Sentry\Tracing\SpanContext;
use WeakMap;

use function FriendsOfHyperf\Sentry\trace;
use function Hyperf\Tappable\tap;

/**
* @property string $poolName
* @property PDO $connection
* @property array $config
*/
class DbAspect extends AbstractAspect
{
public array $classes = [
DB::class . '::__call',
'Hyperf\DB\MySQLConnection::reconnect',
'Hyperf\DB\DB::getConnection',
'Hyperf\DB\DB::__call',
];

/**
* @var WeakMap<\Hyperf\DB\AbstractConnection,array>
*/
private WeakMap $serverCache;

public function __construct(
protected ContainerInterface $container,
protected Feature $feature
) {
$this->serverCache = new WeakMap();
}

public function process(ProceedingJoinPoint $proceedingJoinPoint)
Expand All @@ -44,24 +58,46 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)
return $proceedingJoinPoint->process();
}

if ($proceedingJoinPoint->methodName === 'reconnect') {
return tap($proceedingJoinPoint->process(), function () use ($proceedingJoinPoint) {
/** @var \Hyperf\DB\AbstractConnection $connection */
$connection = $proceedingJoinPoint->getInstance();
$this->serverCache[$connection] = $connection->getConfig();
});
}
Comment on lines +61 to +67
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

验证初次连接时的缓存填充逻辑。

当前实现仅在 reconnect 时填充 serverCache。如果连接池返回的是已建立的连接且未触发重连,serverCache 可能不包含该连接的配置信息,导致 Line 72 获取不到服务器地址。

请验证以下场景是否正常工作:

  1. 应用启动后首次调用 DB::getConnection() 获取连接
  2. 该连接来自连接池且无需重连
  3. 此时 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 -50

Length 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.


if ($proceedingJoinPoint->methodName === 'getConnection') {
return tap($proceedingJoinPoint->process(), function ($connection) {
/** @var \Hyperf\DB\AbstractConnection $connection */
$server = $this->serverCache[$connection] ?? null;
if ($server !== null) {
Context::set(Constants::TRACE_DB_SERVER_ADDRESS, $server['host'] ?? 'localhost');
Context::set(Constants::TRACE_DB_SERVER_PORT, $server['port'] ?? 3306);
}
});
}

$arguments = $proceedingJoinPoint->arguments['keys'];
$poolName = (fn () => $this->poolName)->call($proceedingJoinPoint->getInstance());
/** @var \Hyperf\Pool\Pool $pool */
$pool = $this->container->get(PoolFactory::class)->getPool($poolName);
$operation = $arguments['name'];
$database = '';
$driver = 'unknown';
$table = '';

/** @var \Hyperf\DB\DB $instance */
$instance = $proceedingJoinPoint->getInstance();
$poolName = (fn () => $this->poolName)->call($instance);
/** @var \Hyperf\Pool\Pool $pool */
$pool = $this->container->get(PoolFactory::class)->getPool($poolName);
if ($pool instanceof \Hyperf\DB\Pool\Pool) {
$config = $pool->getConfig();
$database = $config['database'] ?? '';
$driver = $config['driver'] ?? 'unknown';
$driver = $config['driver'] ?? $driver;
}

$sql = $arguments['arguments']['query'] ?? '';
$sqlParse = SqlParser::parse($sql);
$table = $sqlParse['table'];
$operation = $sqlParse['operation'];

$data = [
'db.system' => $driver,
'db.name' => $database,
Expand All @@ -72,8 +108,8 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint)
'db.pool.max_idle_time' => $pool->getOption()->getMaxIdleTime(),
'db.pool.idle' => $pool->getConnectionsInChannel(),
'db.pool.using' => $pool->getCurrentConnections(),
// 'server.host' => '',
// 'server.port' => '',
'server.host' => Context::get(Constants::TRACE_DB_SERVER_ADDRESS, 'localhost'),
'server.port' => Context::get(Constants::TRACE_DB_SERVER_PORT, 3306),
];

if ($this->feature->isTracingTagEnabled('db.sql.bindings', true)) {
Expand Down