refactor(sentry): extract setup logic to dedicated listener #961
Conversation
- Move Sentry initialization logic from EventHandleListener to new SetupSentryListener - Remove BootApplication event handling and setupRequestLifecycle/setupRedisEventEnable methods from EventHandleListener - Add logs_channel_level configuration option to sentry.php config file - Register logs_channel_level in ClientBuilderFactory's user options - Simplify EventHandleListener by focusing only on runtime event handling This refactoring improves separation of concerns by moving one-time setup logic to a dedicated listener, making the code more maintainable and easier to understand.
Walkthrough将启动期 Sentry 配置从 EventHandleListener 中拆出为新监听器 SetupSentryListener；新增配置项 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App as Hyperf 应用
participant Boot as BootApplication 事件
participant Setup as SetupSentryListener
participant Cfg as ConfigInterface
participant LogFactory as LoggerFactory
participant ServerMgr as ServerManager
participant RedisCfg as Redis 配置
App->>Boot: 触发启动事件
Boot->>Setup: process(event)
Setup->>Cfg: 读取特性开关与 logs_channel_level
Setup->>ServerMgr: setupRequestLifecycle()
note over ServerMgr,Setup: 若 HTTP/RPC 匹配则启用 enable_request_lifecycle
Setup->>RedisCfg: setupRedisEventEnable()
note over RedisCfg,Setup: 若存在 Redis 配置则为各池启用 event.enable
Setup->>LogFactory: registerLoggerChannel()
note over LogFactory,Setup: 若无 sentry_logs 则注册 LogsHandler（使用 logs_channel_level）
sequenceDiagram
autonumber
participant Old as 旧流程: EventHandleListener
participant New as 新流程: SetupSentryListener
participant Boot as BootApplication
Boot-->>Old: 过去由 EventHandleListener 处理启动期设置（请求生命周期/Redis/日志通道）
note over Old: 启动期逻辑混合在事件处理器内
Boot-->>New: 现在由 SetupSentryListener 接管这些职责
note over New: 专职处理启动期配置与日志通道注册
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)src/*/src/**/*.php📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.php📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧬 Code graph analysis (1)src/sentry/src/Listener/SetupSentryListener.php (2)
🪛 PHPMD (2.15.0)src/sentry/src/Listener/SetupSentryListener.php75-75: Avoid unused parameters such as '$event'. (undefined) (UnusedFormalParameter) 143-143: Avoid unused local variables such as '$_'. (undefined) (UnusedLocalVariable) ⏰ 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). (12)
🔇 Additional comments (7)
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.31)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: 0
🧹 Nitpick comments (1)
src/sentry/src/Listener/SetupSentryListener.php (1)
25-60: 建议重命名常量以提高可读性。常量名
MISSING_KEYS可能会引起混淆——这些键并非"缺失"的配置项,而是需要设置默认值的 Sentry 功能标志。考虑重命名为DEFAULT_FEATURE_FLAGS或DEFAULT_CONFIG_KEYS以更清晰地表达其用途。可选的重构方案:
- protected const MISSING_KEYS = [ + protected const DEFAULT_FEATURE_FLAGS = [ // Enable 'sentry.enable.amqp',并在
setupRequestLifecycle()方法中相应更新:- foreach (self::MISSING_KEYS as $key) { + foreach (self::DEFAULT_FEATURE_FLAGS as $key) { if (! $this->config->has($key)) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Listener/SetupSentryListener.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/*/src/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All component PHP code must use the namespace pattern FriendsOfHyperf{ComponentName}
Files:
src/sentry/src/Listener/SetupSentryListener.php
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Code style must follow PSR-12 and be formatted by PHP-CS-Fixer per .php-cs-fixer.php
Run PHPStan and keep the codebase passing per phpstan.neon.dist
Files:
src/sentry/src/Listener/SetupSentryListener.php
🧬 Code graph analysis (1)
src/sentry/src/Listener/SetupSentryListener.php (2)
src/sentry/src/Feature.php (2)
Feature(18-71)isTracingEnabled(34-41)src/sentry/src/Monolog/LogsHandler.php (1)
LogsHandler(20-68)
🪛 PHPMD (2.15.0)
src/sentry/src/Listener/SetupSentryListener.php
75-75: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
137-137: Avoid unused local variables such as '$_'. (undefined)
(UnusedLocalVariable)
⏰ 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). (12)
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
🔇 Additional comments (6)
src/sentry/src/Listener/SetupSentryListener.php (6)
68-73: 实现正确!事件监听器的实现符合 Hyperf 框架规范,正确监听了
BootApplication事件。
75-80: 实现正确!方法按逻辑顺序调用了三个设置方法。
$event参数虽未使用,但这是ListenerInterface接口的要求,静态分析工具的提示可以忽略。
82-98: 实现正确!方法正确地注册了专用的日志通道配置:
- 防御性检查避免覆盖已存在的配置
- 正确使用了新引入的
logs_channel_level配置项- 分组名称
sentry_logs与之前的评审建议一致
114-128: 服务器配置修改逻辑正确!代码正确处理了服务器配置的修改:
- 使用引用 (
&$server) 实现就地修改,避免额外拷贝- 通过空合并运算符防御性处理缺失的回调配置
- 使用
is_a()的第三个参数true安全地检查类名,支持字符串和对象- 仅为 HTTP 和 RPC 服务器启用请求生命周期
131-140: 实现正确!方法正确地为所有 Redis 连接池启用事件追踪:
- 防御性检查 Redis 配置是否存在
- 使用
$_作为未使用迭代值的占位符是 PHP 的惯用法- 静态分析工具关于"未使用变量"的提示可以忽略
102-106: 原始评论基于代码误读，需要撤回。代码在第 102-106 行的逻辑是条件性的，而非无条件设置所有功能为真：
if (! $this->config->has($key)) { // 仅当配置键不存在时 $this->config->set($key, true); }配置文件
src/sentry/publish/sentry.php已经为所有这些键定义了默认值，并支持环境变量覆盖：
'amqp' => env('SENTRY_ENABLE_AMQP', true)'request' => env('SENTRY_ENABLE_REQUEST', true)'enable_tracing' => env('SENTRY_ENABLE_TRACING', true)- （以及其他相关的 breadcrumbs 和 tracing 配置）
因此该 Listener 只是为完全不存在的配置提供默认值，尊重用户通过环境变量或配置文件的显式配置。用户可以通过设置环境变量（如
SENTRY_ENABLE_AMQP=false）或在配置中覆盖来禁用特定功能。原始评论中的安全性/成本担忧不成立。
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/sentry/src/Listener/SetupSentryListener.php (1)
103-135: 请求生命周期设置逻辑可优化可读性整体逻辑正确：先设置默认值，再根据功能开关为 HTTP/RPC 服务器启用 request lifecycle。
第 122 行的回调查找逻辑较为复杂，可考虑提取为辅助方法以提升可读性：
private function getServerHandler(array $server): ?string { $callbacks = $server['callbacks'] ?? []; return $callbacks[Event::ON_REQUEST][0] ?? $callbacks[Event::ON_RECEIVE][0] ?? null; }然后在第 122 行使用：
- $callbacks = $server['callbacks'] ?? []; - $handler = $callbacks[Event::ON_REQUEST][0] ?? $callbacks[Event::ON_RECEIVE][0] ?? null; + $handler = $this->getServerHandler($server);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Listener/SetupSentryListener.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/*/src/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All component PHP code must use the namespace pattern FriendsOfHyperf{ComponentName}
Files:
src/sentry/src/Listener/SetupSentryListener.php
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Code style must follow PSR-12 and be formatted by PHP-CS-Fixer per .php-cs-fixer.php
Run PHPStan and keep the codebase passing per phpstan.neon.dist
Files:
src/sentry/src/Listener/SetupSentryListener.php
🧬 Code graph analysis (1)
src/sentry/src/Listener/SetupSentryListener.php (3)
src/sentry/src/Feature.php (2)
Feature(18-71)isTracingEnabled(34-41)src/sentry/src/Monolog/LogsHandler.php (1)
LogsHandler(20-68)src/sentry/src/Function.php (1)
feature(24-27)
🪛 PHPMD (2.15.0)
src/sentry/src/Listener/SetupSentryListener.php
75-75: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
143-143: Avoid unused local variables such as '$_'. (undefined)
(UnusedLocalVariable)
⏰ 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). (6)
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
🔇 Additional comments (5)
src/sentry/src/Listener/SetupSentryListener.php (5)
25-60: 完善的功能特性默认值定义MISSING_KEYS 常量全面覆盖了 Sentry 的各项功能开关（enable、breadcrumbs、tracing），确保在应用启动时为未配置的项设置合理的默认值（true）。这种集中管理的方式清晰且易于维护。
62-66: 依赖注入简洁明了构造函数使用 PHP 8 的构造器属性提升语法，直接注入 ConfigInterface 和 Feature 依赖，代码简洁且符合最佳实践。
68-73: 正确实现监听器接口listen() 方法返回 BootApplication 事件，符合 ListenerInterface 规范，确保在应用启动时触发 Sentry 的初始化逻辑。
82-101: 日志通道注册逻辑正确且安全registerLoggerChannel() 方法：
- 守卫条件恰当：仅在 enable_logs 为 true 且通道未存在时执行
- 配置结构符合 Hyperf 日志格式规范
- 正确使用新增的 logs_channel_level 配置项（第 95 行）
- LogsHandler 的 constructor 参数设置合理
137-146: Redis 事件启用逻辑简洁正确setupRedisEventEnable() 方法遍历所有 Redis 连接池并启用事件监听，逻辑清晰。第 143 行使用
$_忽略值是 PHP 中的常见惯用法，PHPMD 的警告可以忽略。
…ad of 'logger.sentry_logs'
* refactor(sentry): extract setup logic to dedicated listener - Move Sentry initialization logic from EventHandleListener to new SetupSentryListener - Remove BootApplication event handling and setupRequestLifecycle/setupRedisEventEnable methods from EventHandleListener - Add logs_channel_level configuration option to sentry.php config file - Register logs_channel_level in ClientBuilderFactory's user options - Simplify EventHandleListener by focusing only on runtime event handling This refactoring improves separation of concerns by moving one-time setup logic to a dedicated listener, making the code more maintainable and easier to understand. * 更新 SetupSentryListener.php * fix(sentry): update log channel registration condition to respect config setting * refactor(sentry): improve readability of request feature check condition * fix(sentry): update logger configuration to use 'logger.sentry' instead of 'logger.sentry_logs' --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Summary
EventHandleListenerto newSetupSentryListenerlogs_channel_levelconfiguration option with support for environment variableSENTRY_LOGS_CHANNEL_LEVELEventHandleListenerby removing one-time setup codeChanges Made
New File
src/sentry/src/Listener/SetupSentryListener.php: New dedicated listener for Sentry setup that handles:Modified Files
src/sentry/publish/sentry.php: Addedlogs_channel_levelconfiguration option with default value fromSENTRY_LOGS_CHANNEL_LEVELenvironment variablesrc/sentry/src/Factory/ClientBuilderFactory.php: Addedlogs_channel_levelto the list of user options passed to Sentry SDKsrc/sentry/src/Listener/EventHandleListener.php:BootApplicationevent from listened events$missingKeysproperty (moved to SetupSentryListener)handleBootApplication(),setupRequestLifecycle(), andsetupRedisEventEnable()methodsBenefits
EventHandleListeneris now cleaner and focused only on handling runtime eventslogs_channel_leveloption provides more granular control over Sentry log levelsTest Plan
Summary by CodeRabbit
新功能
重构