From 030511c4649441f001ba671ae8097151f9e84a5c Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:10:06 +0800 Subject: [PATCH 1/8] feat: add RedisCommandExecutedListener for enhanced Redis command tracking --- src/sentry/src/Aspect/RedisAspect.php | 9 +- .../Listener/RedisCommandExecutedListener.php | 68 ++++++++++ src/sentry/src/Tracing/Aspect/RedisAspect.php | 8 +- .../Listener/RedisCommandExecutedListener.php | 117 ++++++++++++++++++ 4 files changed, 200 insertions(+), 2 deletions(-) create mode 100644 src/sentry/src/Listener/RedisCommandExecutedListener.php create mode 100644 src/sentry/src/Tracing/Listener/RedisCommandExecutedListener.php diff --git a/src/sentry/src/Aspect/RedisAspect.php b/src/sentry/src/Aspect/RedisAspect.php index 2c32e729d..cff3593eb 100644 --- a/src/sentry/src/Aspect/RedisAspect.php +++ b/src/sentry/src/Aspect/RedisAspect.php @@ -15,11 +15,15 @@ use FriendsOfHyperf\Sentry\Switcher; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; +use Hyperf\Redis\Event\CommandExecuted; use Hyperf\Redis\RedisConnection; use Sentry\Breadcrumb; use function Hyperf\Tappable\tap; +/** + * @deprecated since v3.1, will be removed in v3.2. + */ class RedisAspect extends AbstractAspect { public array $classes = [ @@ -36,7 +40,10 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint) $startTime = microtime(true); return tap($proceedingJoinPoint->process(), function ($result) use ($arguments, $startTime) { - if (! $this->switcher->isBreadcrumbEnable('redis')) { + if ( + class_exists(CommandExecuted::class) + || ! $this->switcher->isBreadcrumbEnable('redis') + ) { return; } diff --git a/src/sentry/src/Listener/RedisCommandExecutedListener.php b/src/sentry/src/Listener/RedisCommandExecutedListener.php new file mode 100644 index 000000000..f9b764646 --- /dev/null +++ b/src/sentry/src/Listener/RedisCommandExecutedListener.php @@ -0,0 +1,68 @@ +setRedisEventEnable(); + } + + public function listen(): array + { + return [ + CommandExecuted::class, + ]; + } + + /** + * @param object|CommandExecuted $event + */ + public function process(object $event): void + { + if ( + ! $this->switcher->isBreadcrumbEnable('redis') + || ! $event instanceof CommandExecuted + ) { + return; + } + + Integration::addBreadcrumb(new Breadcrumb( + Breadcrumb::LEVEL_INFO, + Breadcrumb::TYPE_DEFAULT, + 'redis', + $event->command, + [ + 'result' => $event->result, + 'arguments' => $event->parameters, + 'timeMs' => $event->time * 1000, + ] + )); + } + + private function setRedisEventEnable() + { + foreach ((array) $this->config->get('redis', []) as $connection => $_) { + $this->config->set('redis.' . $connection . '.event.enable', true); + } + } +} diff --git a/src/sentry/src/Tracing/Aspect/RedisAspect.php b/src/sentry/src/Tracing/Aspect/RedisAspect.php index 16e84f013..a619b78a7 100644 --- a/src/sentry/src/Tracing/Aspect/RedisAspect.php +++ b/src/sentry/src/Tracing/Aspect/RedisAspect.php @@ -16,6 +16,7 @@ use Hyperf\Coroutine\Coroutine; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; +use Hyperf\Redis\Event\CommandExecuted; use Hyperf\Redis\Pool\PoolFactory; use Hyperf\Redis\Redis; use Psr\Container\ContainerInterface; @@ -23,6 +24,8 @@ use Throwable; /** + * @deprecated since v3.1, will be removed in v3.2. + * * @property string $poolName * @method array getConfig() * @property array $config @@ -43,7 +46,10 @@ public function __construct( public function process(ProceedingJoinPoint $proceedingJoinPoint) { - if (! $this->switcher->isTracingSpanEnable('redis')) { + if ( + class_exists(CommandExecuted::class) + || ! $this->switcher->isTracingSpanEnable('redis') + ) { return $proceedingJoinPoint->process(); } diff --git a/src/sentry/src/Tracing/Listener/RedisCommandExecutedListener.php b/src/sentry/src/Tracing/Listener/RedisCommandExecutedListener.php new file mode 100644 index 000000000..85ea9a084 --- /dev/null +++ b/src/sentry/src/Tracing/Listener/RedisCommandExecutedListener.php @@ -0,0 +1,117 @@ +setRedisEventEnable(); + } + + public function listen(): array + { + return [ + CommandExecuted::class, + ]; + } + + /** + * @param object|CommandExecuted $event + */ + public function process(object $event): void + { + if (! $this->switcher->isTracingSpanEnable('redis')) { + return; + } + + $arguments = $event->parameters; + + $poolName = $event->connectionName; + $pool = $this->container->get(PoolFactory::class)->getPool($poolName); + $config = $this->config->get('redis.' . $poolName, []); + + $data = [ + 'coroutine.id' => Coroutine::id(), + 'db.system' => 'redis', + 'db.redis.connection' => $poolName, + 'db.redis.database_index' => $config['db'] ?? 0, + 'db.redis.parameters' => $arguments['arguments'], + // 'db.statement' => strtoupper($arguments['name']) . implode(' ', $arguments['arguments']), + 'db.redis.pool.name' => $poolName, + 'db.redis.pool.max' => $pool->getOption()->getMaxConnections(), + 'db.redis.pool.max_idle_time' => $pool->getOption()->getMaxIdleTime(), + 'db.redis.pool.idle' => $pool->getConnectionsInChannel(), + 'db.redis.pool.using' => $pool->getCurrentConnections(), + ]; + + // rule: operation db.table + // $op = sprintf('%s %s', $arguments['name'], $arguments['arguments']['key'] ?? ''); + // $description = sprintf('%s::%s()', $proceedingJoinPoint->className, $arguments['name']); + $key = $arguments['arguments'][0] ?? ''; + $op = 'db.redis'; + $description = sprintf( + '%s %s', + strtoupper($arguments['name'] ?? ''), + is_array($key) ? implode(',', $key) : $key + ); + $span = $this->startSpan($op, $description); + + if (! $span) { + return; + } + + if ($this->switcher->isTracingExtraTagEnable('redis.result')) { + $data['redis.result'] = $event->result; + } + + if ($exception = $event->throwable) { + $span->setStatus(SpanStatus::internalError()); + $span->setTags([ + 'error' => true, + 'exception.class' => $exception::class, + 'exception.message' => $exception->getMessage(), + 'exception.code' => $exception->getCode(), + ]); + if ($this->switcher->isTracingExtraTagEnable('exception.stack_trace')) { + $data['exception.stack_trace'] = (string) $exception; + } + + throw $exception; + } + + $span->setData($data); + $span->finish(); + } + + private function setRedisEventEnable() + { + foreach ((array) $this->config->get('redis', []) as $connection => $_) { + $this->config->set('redis.' . $connection . '.event.enable', true); + } + } +} From 5ca73bfb89106d8e3b8dba12e7912933d8c1576a Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:14:25 +0800 Subject: [PATCH 2/8] feat: replace RedisCommandExecutedListener with TracingRedisListener for improved Redis command tracing --- src/sentry/src/ConfigProvider.php | 2 ++ ...edisCommandExecutedListener.php => TracingRedisListener.php} | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) rename src/sentry/src/Tracing/Listener/{RedisCommandExecutedListener.php => TracingRedisListener.php} (98%) diff --git a/src/sentry/src/ConfigProvider.php b/src/sentry/src/ConfigProvider.php index 79f5f9da5..087c9b79e 100644 --- a/src/sentry/src/ConfigProvider.php +++ b/src/sentry/src/ConfigProvider.php @@ -56,6 +56,7 @@ public function __invoke(): array Listener\CrontabExceptionListener::class, Listener\DbQueryListener::class, Listener\KafkaExceptionListener::class, + Listener\RedisCommandExecutedListener::class, Listener\RequestExceptionListener::class, Listener\SetRequestLifecycleListener::class, Crons\Listener\CronEventListener::class, @@ -65,6 +66,7 @@ public function __invoke(): array Tracing\Listener\TracingCrontabListener::class, Tracing\Listener\TracingDbQueryListener::class, Tracing\Listener\TracingKafkaListener::class, + Tracing\Listener\TracingRedisListener::class, Tracing\Listener\TracingRequestListener::class, ], 'annotations' => [ diff --git a/src/sentry/src/Tracing/Listener/RedisCommandExecutedListener.php b/src/sentry/src/Tracing/Listener/TracingRedisListener.php similarity index 98% rename from src/sentry/src/Tracing/Listener/RedisCommandExecutedListener.php rename to src/sentry/src/Tracing/Listener/TracingRedisListener.php index 85ea9a084..5440054ea 100644 --- a/src/sentry/src/Tracing/Listener/RedisCommandExecutedListener.php +++ b/src/sentry/src/Tracing/Listener/TracingRedisListener.php @@ -21,7 +21,7 @@ use Psr\Container\ContainerInterface; use Sentry\Tracing\SpanStatus; -class RedisCommandExecutedListener implements ListenerInterface +class TracingRedisListener implements ListenerInterface { use SpanStarter; From 54b5bb6541fbd7ae01244bdde1614128ca629306 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:21:32 +0800 Subject: [PATCH 3/8] refactor: improve parameter handling and connection name usage in TracingRedisListener --- .../Tracing/Listener/TracingRedisListener.php | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/sentry/src/Tracing/Listener/TracingRedisListener.php b/src/sentry/src/Tracing/Listener/TracingRedisListener.php index 5440054ea..7e6e9d541 100644 --- a/src/sentry/src/Tracing/Listener/TracingRedisListener.php +++ b/src/sentry/src/Tracing/Listener/TracingRedisListener.php @@ -49,20 +49,17 @@ public function process(object $event): void return; } - $arguments = $event->parameters; - - $poolName = $event->connectionName; - $pool = $this->container->get(PoolFactory::class)->getPool($poolName); - $config = $this->config->get('redis.' . $poolName, []); + $pool = $this->container->get(PoolFactory::class)->getPool($event->connectionName); + $config = $this->config->get('redis.' . $event->connectionName, []); $data = [ 'coroutine.id' => Coroutine::id(), 'db.system' => 'redis', - 'db.redis.connection' => $poolName, + 'db.redis.connection' => $event->connectionName, 'db.redis.database_index' => $config['db'] ?? 0, - 'db.redis.parameters' => $arguments['arguments'], - // 'db.statement' => strtoupper($arguments['name']) . implode(' ', $arguments['arguments']), - 'db.redis.pool.name' => $poolName, + 'db.redis.parameters' => $event->parameters, + // 'db.statement' => strtoupper($event->command) . implode(' ', $event->parameters), + 'db.redis.pool.name' => $event->connectionName, 'db.redis.pool.max' => $pool->getOption()->getMaxConnections(), 'db.redis.pool.max_idle_time' => $pool->getOption()->getMaxIdleTime(), 'db.redis.pool.idle' => $pool->getConnectionsInChannel(), @@ -70,13 +67,11 @@ public function process(object $event): void ]; // rule: operation db.table - // $op = sprintf('%s %s', $arguments['name'], $arguments['arguments']['key'] ?? ''); - // $description = sprintf('%s::%s()', $proceedingJoinPoint->className, $arguments['name']); - $key = $arguments['arguments'][0] ?? ''; + $key = $event->parameters[0] ?? ''; $op = 'db.redis'; $description = sprintf( '%s %s', - strtoupper($arguments['name'] ?? ''), + strtoupper($event->command), is_array($key) ? implode(',', $key) : $key ); $span = $this->startSpan($op, $description); @@ -86,7 +81,7 @@ public function process(object $event): void } if ($this->switcher->isTracingExtraTagEnable('redis.result')) { - $data['redis.result'] = $event->result; + $data['db.redis.result'] = $event->result; } if ($exception = $event->throwable) { @@ -100,8 +95,6 @@ public function process(object $event): void if ($this->switcher->isTracingExtraTagEnable('exception.stack_trace')) { $data['exception.stack_trace'] = (string) $exception; } - - throw $exception; } $span->setData($data); From 156e9de1ed1873807be3bfc85c6b66827a491291 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:26:29 +0800 Subject: [PATCH 4/8] feat: add SetRedisEventEnableListener to enable Redis event tracking on application boot --- src/sentry/src/ConfigProvider.php | 1 + .../Listener/RedisCommandExecutedListener.php | 15 +------ .../Listener/SetRedisEventEnableListener.php | 42 +++++++++++++++++++ .../Tracing/Listener/TracingRedisListener.php | 14 ++----- 4 files changed, 48 insertions(+), 24 deletions(-) create mode 100644 src/sentry/src/Listener/SetRedisEventEnableListener.php diff --git a/src/sentry/src/ConfigProvider.php b/src/sentry/src/ConfigProvider.php index 087c9b79e..6c13c400c 100644 --- a/src/sentry/src/ConfigProvider.php +++ b/src/sentry/src/ConfigProvider.php @@ -58,6 +58,7 @@ public function __invoke(): array Listener\KafkaExceptionListener::class, Listener\RedisCommandExecutedListener::class, Listener\RequestExceptionListener::class, + Listener\SetRedisEventEnableListener::class, Listener\SetRequestLifecycleListener::class, Crons\Listener\CronEventListener::class, Tracing\Listener\TracingAmqpListener::class, diff --git a/src/sentry/src/Listener/RedisCommandExecutedListener.php b/src/sentry/src/Listener/RedisCommandExecutedListener.php index f9b764646..d7c650d6b 100644 --- a/src/sentry/src/Listener/RedisCommandExecutedListener.php +++ b/src/sentry/src/Listener/RedisCommandExecutedListener.php @@ -13,18 +13,14 @@ use FriendsOfHyperf\Sentry\Integration; use FriendsOfHyperf\Sentry\Switcher; -use Hyperf\Contract\ConfigInterface; use Hyperf\Event\Contract\ListenerInterface; use Hyperf\Redis\Event\CommandExecuted; use Sentry\Breadcrumb; class RedisCommandExecutedListener implements ListenerInterface { - public function __construct( - protected ConfigInterface $config, - protected Switcher $switcher - ) { - $this->setRedisEventEnable(); + public function __construct(private Switcher $switcher) + { } public function listen(): array @@ -58,11 +54,4 @@ public function process(object $event): void ] )); } - - private function setRedisEventEnable() - { - foreach ((array) $this->config->get('redis', []) as $connection => $_) { - $this->config->set('redis.' . $connection . '.event.enable', true); - } - } } diff --git a/src/sentry/src/Listener/SetRedisEventEnableListener.php b/src/sentry/src/Listener/SetRedisEventEnableListener.php new file mode 100644 index 000000000..6a7a4e44b --- /dev/null +++ b/src/sentry/src/Listener/SetRedisEventEnableListener.php @@ -0,0 +1,42 @@ +config->has('redis')) { + return; + } + + foreach ($this->config->get('redis', []) as $pool => $_) { + $this->config->set("redis.{$pool}.event.enable", true); + } + } +} diff --git a/src/sentry/src/Tracing/Listener/TracingRedisListener.php b/src/sentry/src/Tracing/Listener/TracingRedisListener.php index 7e6e9d541..3c63a1644 100644 --- a/src/sentry/src/Tracing/Listener/TracingRedisListener.php +++ b/src/sentry/src/Tracing/Listener/TracingRedisListener.php @@ -26,11 +26,10 @@ class TracingRedisListener implements ListenerInterface use SpanStarter; public function __construct( - protected ContainerInterface $container, - protected ConfigInterface $config, - protected Switcher $switcher + private ContainerInterface $container, + private ConfigInterface $config, + private Switcher $switcher ) { - $this->setRedisEventEnable(); } public function listen(): array @@ -100,11 +99,4 @@ public function process(object $event): void $span->setData($data); $span->finish(); } - - private function setRedisEventEnable() - { - foreach ((array) $this->config->get('redis', []) as $connection => $_) { - $this->config->set('redis.' . $connection . '.event.enable', true); - } - } } From c9535ce87b57e4530c5a940831c9d8b1c8477268 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:28:41 +0800 Subject: [PATCH 5/8] refactor: simplify constructor syntax in SetRedisEventEnableListener --- src/sentry/src/Listener/SetRedisEventEnableListener.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/sentry/src/Listener/SetRedisEventEnableListener.php b/src/sentry/src/Listener/SetRedisEventEnableListener.php index 6a7a4e44b..14608ab72 100644 --- a/src/sentry/src/Listener/SetRedisEventEnableListener.php +++ b/src/sentry/src/Listener/SetRedisEventEnableListener.php @@ -17,9 +17,8 @@ class SetRedisEventEnableListener implements ListenerInterface { - public function __construct( - protected ConfigInterface $config - ) { + public function __construct(protected ConfigInterface $config) + { } public function listen(): array From a55ab371f4a1b010d41972dff93138f4d8bc64f6 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:31:28 +0800 Subject: [PATCH 6/8] fix: correct formatting of Redis command parameters in TracingRedisListener --- src/sentry/src/Tracing/Listener/TracingRedisListener.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/src/Tracing/Listener/TracingRedisListener.php b/src/sentry/src/Tracing/Listener/TracingRedisListener.php index 3c63a1644..50bd1eba8 100644 --- a/src/sentry/src/Tracing/Listener/TracingRedisListener.php +++ b/src/sentry/src/Tracing/Listener/TracingRedisListener.php @@ -57,7 +57,7 @@ public function process(object $event): void 'db.redis.connection' => $event->connectionName, 'db.redis.database_index' => $config['db'] ?? 0, 'db.redis.parameters' => $event->parameters, - // 'db.statement' => strtoupper($event->command) . implode(' ', $event->parameters), + 'db.statement' => strtoupper($event->command) . implode(' ', $event->parameters), 'db.redis.pool.name' => $event->connectionName, 'db.redis.pool.max' => $pool->getOption()->getMaxConnections(), 'db.redis.pool.max_idle_time' => $pool->getOption()->getMaxIdleTime(), @@ -71,7 +71,7 @@ public function process(object $event): void $description = sprintf( '%s %s', strtoupper($event->command), - is_array($key) ? implode(',', $key) : $key + implode(' ', $event->parameters) ); $span = $this->startSpan($op, $description); From fd6b02eeb42acca400b4ecf5734ff08a72b9c66f Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:31:51 +0800 Subject: [PATCH 7/8] refactor: remove unused key variable in TracingRedisListener --- src/sentry/src/Tracing/Listener/TracingRedisListener.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sentry/src/Tracing/Listener/TracingRedisListener.php b/src/sentry/src/Tracing/Listener/TracingRedisListener.php index 50bd1eba8..384a95c1a 100644 --- a/src/sentry/src/Tracing/Listener/TracingRedisListener.php +++ b/src/sentry/src/Tracing/Listener/TracingRedisListener.php @@ -66,7 +66,6 @@ public function process(object $event): void ]; // rule: operation db.table - $key = $event->parameters[0] ?? ''; $op = 'db.redis'; $description = sprintf( '%s %s', From 0c783a91e5facaec8e51e70c6db6607739792f26 Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:35:31 +0800 Subject: [PATCH 8/8] refactor: add duration field to Redis command event data for improved tracking --- src/sentry/src/Listener/RedisCommandExecutedListener.php | 4 ++-- src/sentry/src/Tracing/Listener/TracingRedisListener.php | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sentry/src/Listener/RedisCommandExecutedListener.php b/src/sentry/src/Listener/RedisCommandExecutedListener.php index d7c650d6b..218feec12 100644 --- a/src/sentry/src/Listener/RedisCommandExecutedListener.php +++ b/src/sentry/src/Listener/RedisCommandExecutedListener.php @@ -48,9 +48,9 @@ public function process(object $event): void 'redis', $event->command, [ - 'result' => $event->result, 'arguments' => $event->parameters, - 'timeMs' => $event->time * 1000, + 'result' => $event->result, + 'duration' => $event->time * 1000, ] )); } diff --git a/src/sentry/src/Tracing/Listener/TracingRedisListener.php b/src/sentry/src/Tracing/Listener/TracingRedisListener.php index 384a95c1a..55ae52f6c 100644 --- a/src/sentry/src/Tracing/Listener/TracingRedisListener.php +++ b/src/sentry/src/Tracing/Listener/TracingRedisListener.php @@ -53,6 +53,7 @@ public function process(object $event): void $data = [ 'coroutine.id' => Coroutine::id(), + 'duration' => $event->time * 1000, 'db.system' => 'redis', 'db.redis.connection' => $event->connectionName, 'db.redis.database_index' => $config['db'] ?? 0,