feat: Add RedisCommandExecutedListener #833
Conversation
Walkthrough该更改主要涉及多个文件。首先，在 Changes
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.0.3)/bin/bash: line 1: /vendor/bin/phpstan: No such file or directory Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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
🧹 Nitpick comments (1)
src/telescope/src/ConfigProvider.php (1)
49-49: Redis 命令执行监听器的集成符合最佳实践新增的 RedisCommandExecutedListener 与现有的监听器体系完美融合，扩展了 Telescope 的监控能力。
建议确保在项目文档中添加关于此新功能的使用说明。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/telescope/src/Aspect/RedisAspect.php(2 hunks)src/telescope/src/ConfigProvider.php(1 hunks)src/telescope/src/Listener/RedisCommandExecutedListener.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.6
🔇 Additional comments (2)
src/telescope/src/Aspect/RedisAspect.php (1)
53-56: 请确认条件逻辑变更的意图在第53-56行，条件判断增加了
class_exists('Hyperf\Redis\Event\CommandExecuted')，导致当该类存在时，process方法将直接返回，不再执行后续逻辑。请确认这是否与您的预期一致，以确保在适当的情况下记录 Redis 命令。src/telescope/src/ConfigProvider.php (1)
44-51: 监听器配置更改看起来合理！监听器的排序和新增都符合预期：
- CronEventListener 的位置调整不会影响功能
- RedisCommandExecutedListener 的添加完善了 Redis 命令执行的监控能力
| try { | ||
| $unpacked = $packer->unpack((string) $parameter); | ||
| $parameter = match (true) { | ||
| is_null($unpacked) => 'null', | ||
| is_array($unpacked) => json_encode($unpacked), | ||
| default => $unpacked, | ||
| }; | ||
| } catch (Throwable $e) { | ||
| } | ||
| } |
There was a problem hiding this comment.
在 catch 块中处理异常以避免潜在问题
在第92-101行的 catch 块中，捕获的异常被忽略，没有任何处理。这可能会导致潜在的问题被隐藏。建议在 catch 块中记录异常信息或采取适当的处理措施，以提高代码的可维护性和调试能力。
应用以下修改来处理异常：
+use Psr\Log\LoggerInterface;
class RedisCommandExecutedListener implements ListenerInterface
{
public function __construct(
private ContainerInterface $container,
private ConfigInterface $config,
private TelescopeConfig $telescopeConfig,
+ private LoggerInterface $logger,
) {
$this->setRedisEventEnable();
}
// ...
} catch (Throwable $e) {
+ $this->logger->error('解包参数时发生异常', ['exception' => $e]);
}📝 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.
| try { | |
| $unpacked = $packer->unpack((string) $parameter); | |
| $parameter = match (true) { | |
| is_null($unpacked) => 'null', | |
| is_array($unpacked) => json_encode($unpacked), | |
| default => $unpacked, | |
| }; | |
| } catch (Throwable $e) { | |
| } | |
| } | |
| <?php | |
| // ... Other imports | |
| use Psr\Log\LoggerInterface; | |
| class RedisCommandExecutedListener implements ListenerInterface | |
| { | |
| public function __construct( | |
| private ContainerInterface $container, | |
| private ConfigInterface $config, | |
| private TelescopeConfig $telescopeConfig, | |
| private LoggerInterface $logger, | |
| ) { | |
| $this->setRedisEventEnable(); | |
| } | |
| // ... Other methods and code | |
| public function someMethod() // Assuming the try-catch block is inside a method | |
| { | |
| // ... Some preceding code | |
| try { | |
| $unpacked = $packer->unpack((string) $parameter); | |
| $parameter = match (true) { | |
| is_null($unpacked) => 'null', | |
| is_array($unpacked) => json_encode($unpacked), | |
| default => $unpacked, | |
| }; | |
| } catch (Throwable $e) { | |
| $this->logger->error('解包参数时发生异常', ['exception' => $e]); | |
| } | |
| // ... Some following code | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/telescope/src/Listener/DbQueryListener.php (1)
49-50: 移除冗余的类型检查由于在第42行已经进行了类型检查，这里的
if ($event instanceof QueryExecuted)检查是冗余的。建议移除冗余检查，简化代码：
- if ($event instanceof QueryExecuted) { - $sql = $event->sql; + $sql = $event->sql;
♻️ Duplicate comments (1)
src/telescope/src/Listener/RedisCommandExecutedListener.php (1)
92-101:⚠️ Potential issue需要在 catch 块中处理异常
在 catch 块中忽略异常可能会隐藏潜在问题。建议添加日志记录或适当的异常处理逻辑。
🧹 Nitpick comments (3)
src/telescope/src/Listener/RedisCommandExecutedListener.php (3)
36-41: 建议添加返回值类型提示为了提高代码的类型安全性，建议为
listen方法添加返回值类型提示。- public function listen(): array + public function listen(): array
43-67: 优化参数类型提示以提高代码可读性虽然
@param注释中指明了参数类型，但建议直接在参数声明中使用更具体的类型。这样可以提供更好的 IDE 支持和类型安全性。- public function process(object $event): void + public function process(CommandExecuted $event): void
111-116: 建议优化方法名称以更好地表达其功能当前方法名称
setRedisEventEnable不够清晰。建议修改为更具描述性的名称，如enableRedisEventsForAllConnections。- private function setRedisEventEnable() + private function enableRedisEventsForAllConnections(): void
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/telescope/src/Listener/CommandListener.php(1 hunks)src/telescope/src/Listener/CronEventListener.php(1 hunks)src/telescope/src/Listener/DbQueryListener.php(1 hunks)src/telescope/src/Listener/ExceptionHandlerListener.php(5 hunks)src/telescope/src/Listener/RedisCommandExecutedListener.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.6
🔇 Additional comments (4)
src/telescope/src/Listener/CronEventListener.php (1)
41-44: 类型检查逻辑优化条件判断的重构提高了代码的类型安全性，通过显式检查事件类型来确保只处理预期的事件类型。
src/telescope/src/Listener/CommandListener.php (1)
39-46: 类型安全性增强通过添加实例类型检查和更新参数类型注解，确保了事件处理的类型安全性，同时保持了接口的灵活性。
src/telescope/src/Listener/ExceptionHandlerListener.php (1)
55-57: 代码现代化和类型安全性改进
- 使用箭头函数提高了代码的可读性
- 添加了 Throwable 类型提示增强了类型安全性
- Collection 的使用更加简洁和优雅
Also applies to: 81-84
src/telescope/src/Listener/RedisCommandExecutedListener.php (1)
26-34: 类的声明和构造函数实现得当！构造函数的依赖注入和初始化逻辑清晰，类型提示完整，符合单一职责原则。
* feat: Add RedisCommandExecutedListener * fix: 更新事件处理逻辑以支持多种事件类型 * fix: 修复 CronEventListener 中的默认输出逻辑 * fix: 优化 RedisCommandExecutedListener 中的事件启用逻辑 --------- Co-authored-by: Deeka Wong <[email protected]>
Summary by CodeRabbit