refactor(sentry): consolidate all listeners into single EventListener #912
Conversation
- Merge 11 individual listener files into one unified EventListener.php - Maintain all existing functionality for exception handling and breadcrumb tracking - Improve code organization and reduce file complexity - Preserve configuration setup and event handling logic - Support all framework events: AMQP, AsyncQueue, Command, Crontab, Database, Kafka, Redis, Request
Walkthrough本次变更移除了原有多类分散的 Sentry 监听器（异常、请求、命令、队列、定时、AMQP、Kafka、DB、Redis、配置启用等），并新增一个统一的 EventListener，集中声明并处理上述事件的 setup/capture/flush 与面包屑记录逻辑。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor FW as Framework
participant EL as EventListener
participant SW as Switcher
participant Sdk as SentrySdk/Integration
FW->>EL: dispatch(Event)
EL->>SW: 检查对应功能开关
alt 开关启用
EL->>EL: setupSentrySdk(一次性)
opt DB/Redis 事件
EL->>Sdk: addBreadcrumb(查询/事务/Redis 命令)
end
alt 异常类事件(Fail*/Terminated)
EL->>Sdk: captureException(Throwable)
EL->>Sdk: flushEvents()
else 生命周期结束(After*/Terminated)
EL->>Sdk: flushEvents()
end
else 开关未启用
EL-->>FW: return
end
sequenceDiagram
autonumber
actor Q as Queue Worker
participant EL as EventListener
participant Sdk as SentrySdk/Integration
Q->>EL: BeforeHandle
EL->>EL: setupSentrySdk(若未设置)
Q->>EL: RetryHandle/FailedHandle
EL->>Sdk: captureException(t)
EL->>Sdk: flushEvents()
Q->>EL: AfterHandle
EL->>Sdk: flushEvents()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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.28)At least one path must be specified to analyse. Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 (9)
src/sentry/src/Listener/EventListener.php (9)
103-152: 使用类名做严格匹配可能错过子类事件
match ($event::class)对类名做精确匹配，若后续框架引入事件子类或兼容替代类，可能不会命中。建议评估是否需要基于instanceof的匹配，或至少在未来有子类化时留有扩展点。请确认 Hyperf 这些事件是否存在子类化或替换实现的可能；若有，考虑改为 if/elseif + instanceof。
154-169: 避免重复 flush；明确类型签名
captureException()内部 finally 调用$hub->getClient()?->flush()，而各处调用捕获后又额外调用了flushEvents()，存在重复 flush 的性能开销。建议统一在“边界”处 flush，捕获阶段不 flush；同时将参数类型声明为?Throwable更明确。[建议改动]
- protected function captureException($throwable): void + protected function captureException(?Throwable $throwable): void { - if (! $throwable instanceof Throwable) { + if (! $throwable instanceof Throwable) { return; } $hub = SentrySdk::getCurrentHub(); try { $hub->captureException($throwable); } catch (Throwable $e) { $this->container->get(StdoutLoggerInterface::class)->error((string) $e); - } finally { - $hub->getClient()?->flush(); - } + } }
171-183: 重复初始化日志级别建议降级，避免噪音重复 setup 时用 warning 容易造成日志噪音，建议降为 debug，并统一消息文案。
- $this->container->get(StdoutLoggerInterface::class)->warning('SentrySdk has been setup.'); + $this->container->get(StdoutLoggerInterface::class)->debug('SentrySdk already setup; skip.');
242-262: 回写 server 配置前的健壮性检查可再严谨些（可选）当前通过
??安全访问两级下标基本可行，但若回调项非数组/结构变化，仍可能引入边缘问题。可在读取前断言为数组。- $callbacks = $server['callbacks'] ?? []; - $handler = $callbacks[Event::ON_REQUEST][0] ?? $callbacks[Event::ON_RECEIVE][0] ?? null; + $callbacks = is_array($server['callbacks'] ?? null) ? $server['callbacks'] : []; + $onRequest = $callbacks[Event::ON_REQUEST] ?? null; + $onReceive = $callbacks[Event::ON_RECEIVE] ?? null; + $handler = is_array($onRequest) ? ($onRequest[0] ?? null) : (is_array($onReceive) ? ($onReceive[0] ?? null) : null);
270-272: 移除未使用变量，消除 PHPMD 提示
$_未使用。可直接遍历键名。- foreach ($this->config->get('redis', []) as $pool => $_) { + foreach (array_keys($this->config->get('redis', [])) as $pool) { $this->config->set("redis.{$pool}.event.enable", true); }
275-299: DB 执行时间单位需确认，避免与 Redis 不一致这里记录为
executionTimeMs = $event->time，而 Redis 将$event->time * 1000转为毫秒。请确认QueryExecuted::$time的单位（秒或毫秒），否则会出现单位不一致的问题。若确认为“秒”，建议改为毫秒以对齐：
- if ($event->time !== null) { - $data['executionTimeMs'] = $event->time; - } + if ($event->time !== null) { + $data['executionTimeMs'] = (int) round($event->time * 1000); + }
319-339: Redis 面包屑可能泄露敏感信息且负载较大（参数/结果）
parameters与result可能包含密码、令牌、或大体量数据。建议做脱敏与裁剪，降低泄露与体积风险。- Integration::addBreadcrumb(new Breadcrumb( + Integration::addBreadcrumb(new Breadcrumb( Breadcrumb::LEVEL_INFO, Breadcrumb::TYPE_DEFAULT, 'redis', $event->command, [ - 'arguments' => $event->parameters, - 'result' => $event->result, - 'duration' => $event->time * 1000, + 'arguments' => $this->sanitizeRedisArgs((array) $event->parameters), + 'result' => $this->shrinkForBreadcrumb($event->result), + 'duration' => (int) round($event->time * 1000), ] ));可在类内新增辅助方法（放于类末尾，供重用）：
protected function sanitizeRedisArgs(array $args): array { $sensitive = ['auth','password','passwd','token','secret','access_token','refresh_token']; $out = []; foreach ($args as $k => $v) { $key = is_string($k) ? strtolower($k) : $k; if (is_string($key) && in_array($key, $sensitive, true)) { $out[$k] = '***'; continue; } $out[$k] = is_string($v) && strlen($v) > 256 ? substr($v, 0, 256) . '…' : $v; } return $out; } protected function shrinkForBreadcrumb(mixed $value): mixed { if (is_string($value)) { return strlen($value) > 256 ? substr($value, 0, 256) . '…' : $value; } if (is_array($value)) { return count($value) > 50 ? array_slice($value, 0, 50, true) + ['__truncated__' => true] : $value; } if (is_object($value)) { return 'object(' . get_class($value) . ')'; } return $value; }
350-358: RequestTerminated 异常字段名兼容性
$event->exception在部分事件类型中可能不存在（RPC/HTTP 的命名可能不同）。建议兼容throwable字段并与上方captureException(?Throwable $)协同。- $this->captureException($event->exception); + $throwable = $event->exception ?? ($event->throwable ?? null); + $this->captureException($throwable);请确认 Hyperf 的 Http 与 RPC 的 Terminated 事件分别使用
exception还是throwable字段。
43-46: 抑制批量“未使用参数”告警（PHPMD）多处
handle*方法签名保留$event但未使用，便于将来扩展。可在类级别抑制 PHPMD 的 UnusedFormalParameter 提示。-class EventListener implements ListenerInterface +/** + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ +class EventListener implements ListenerInterface
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/sentry/src/Listener/AmqpExceptionListener.php(0 hunks)src/sentry/src/Listener/AsyncQueueExceptionListener.php(0 hunks)src/sentry/src/Listener/CaptureExceptionListener.php(0 hunks)src/sentry/src/Listener/CommandExceptionListener.php(0 hunks)src/sentry/src/Listener/CrontabExceptionListener.php(0 hunks)src/sentry/src/Listener/DbQueryListener.php(0 hunks)src/sentry/src/Listener/EventListener.php(1 hunks)src/sentry/src/Listener/KafkaExceptionListener.php(0 hunks)src/sentry/src/Listener/RedisCommandExecutedListener.php(0 hunks)src/sentry/src/Listener/RequestExceptionListener.php(0 hunks)src/sentry/src/Listener/SetRedisEventEnableListener.php(0 hunks)src/sentry/src/Listener/SetRequestLifecycleListener.php(0 hunks)
💤 Files with no reviewable changes (11)
- src/sentry/src/Listener/KafkaExceptionListener.php
- src/sentry/src/Listener/AsyncQueueExceptionListener.php
- src/sentry/src/Listener/RedisCommandExecutedListener.php
- src/sentry/src/Listener/DbQueryListener.php
- src/sentry/src/Listener/SetRequestLifecycleListener.php
- src/sentry/src/Listener/CaptureExceptionListener.php
- src/sentry/src/Listener/CommandExceptionListener.php
- src/sentry/src/Listener/AmqpExceptionListener.php
- src/sentry/src/Listener/CrontabExceptionListener.php
- src/sentry/src/Listener/RequestExceptionListener.php
- src/sentry/src/Listener/SetRedisEventEnableListener.php
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/Listener/EventListener.php (3)
src/sentry/src/Integration.php (1)
Integration(28-158)src/sentry/src/Switcher.php (3)
Switcher(19-85)isTracingEnable(35-42)isBreadcrumbEnable(30-33)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)
🪛 PHPMD (2.15.0)
src/sentry/src/Listener/EventListener.php
190-190: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
270-270: Avoid unused local variables such as '$_'. (undefined)
(UnusedLocalVariable)
341-341: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
360-360: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
378-378: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
387-387: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
396-396: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
425-425: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
434-434: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
453-453: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
462-462: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
481-481: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
490-490: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
🪛 GitHub Actions: tests
src/sentry/src/Listener/EventListener.php
[error] 505-505: PHP CS Fixer found issues and exited with code 8. Detected fixes suggested by phpdoc_no_alias_tag, unary_operator_spaces, not_operator_with_successor_space, header_comment, blank_lines_before_namespace, single_blank_line_at_eof. Found 1 of 844 files that can be fixed.
🔇 Additional comments (3)
src/sentry/src/Listener/EventListener.php (3)
43-53: 整合方向正确，单点监听更易维护将 11 个监听器整合为单类，职责划分清晰，可读性与维护性提升明显。继续保持这种集中式的事件路由模式。
1-14: 已修复 PHP CS Fixer 报错：空行与头注释
已在仓库运行 vendor/bin/php-cs-fixer fix 并自动修复了 src/sentry/src/Listener/EventListener.php 中的相关问题，无需进一步修改。
196-234: 确认：运行时填充 sentry. 默认为 true，与仓库发布配置一致*已验证：src/sentry/publish/sentry.php 中这些键默认为 true；src/sentry/src/Switcher.php 的 get(..., true) 也以 true 为默认；src/sentry/src/Factory/ClientBuilderFactory.php 对 enable_tracing 同样默认为 true。EventListener 在运行时补全为 true 与现有默认行为一致，不会意外开启此前预期关闭的功能。
Summary
EventListener.phpChanges
Deleted files: 11 individual listener classes
AmqpExceptionListener.phpAsyncQueueExceptionListener.phpCaptureExceptionListener.phpCommandExceptionListener.phpCrontabExceptionListener.phpDbQueryListener.phpKafkaExceptionListener.phpRedisCommandExecutedListener.phpRequestExceptionListener.phpSetRedisEventEnableListener.phpSetRequestLifecycleListener.phpCreated: Single unified
EventListener.phpwith all functionalityFeatures Preserved
Benefits
Test plan
Summary by CodeRabbit