Skip to content

Conversation

@huangdijia
Copy link
Contributor

@huangdijia huangdijia commented Sep 28, 2025

Summary

This PR optimizes Sentry hub management and improves event listener execution order for better performance and reliability.

Key Changes

  • Simplified SentrySdk::setCurrentHub(): Removed unnecessary tap usage for cleaner, more direct hub assignment
  • Added explicit listener priorities: Assigned specific priority values (PHP_INT_MAX variants) to ensure deterministic execution order
  • Optimized EventHandleListener: Removed redundant hub context management and simplified setup methods
  • Improved Tracer initialization: Use existing hub (getCurrentHub()) instead of creating new instances for better resource utilization
  • Enhanced transaction isolation: Clear breadcrumbs on new transaction scopes to prevent cross-request data pollution

Benefits

  • Reduced memory overhead by reusing existing hub instances
  • Improved predictable listener execution order
  • Better transaction isolation preventing data leakage between requests
  • Cleaner, more maintainable code structure

Test Plan

  • Verify Sentry error tracking continues to work correctly
  • Check that transaction tracing functions properly
  • Ensure listener execution order is deterministic
  • Validate no cross-request breadcrumb pollution occurs
  • Run existing test suite to ensure no regressions

Summary by CodeRabbit

  • 新功能

    • 在开始新事务时自动清空面包屑,避免历史数据干扰。
  • Bug 修复

    • 明确事件监听器优先级,修正可能的处理顺序异常。
    • 调整“当前” Hub 的赋值与返回行为,提升捕获与跟踪一致性。
  • 重构

    • 精简 Sentry 初始化与上下文处理,移除冗余初始化路径与旧有上下文依赖,提升可维护性。
  • 配置

    • 更新命令忽略列表,新增对特定测试命令的忽略规则。

- Simplify SentrySdk::setCurrentHub() by removing unnecessary tap usage
- Add explicit priorities to event listeners for deterministic execution order
- Optimize EventHandleListener by removing redundant hub context management
- Improve Tracer initialization to use existing hub instead of creating new ones
- Clear breadcrumbs on new transaction scope to prevent cross-request pollution
@coderabbitai
Copy link

coderabbitai bot commented Sep 28, 2025

Walkthrough

调整 Sentry SDK 与监听器相关实现:SentrySdk::setCurrentHub 去除 tap 包装直接返回 Context::set 结果;EventHandleListener 移除 HUB 常量与 setupSentrySdk() 调用;为监听器注册添加显式优先级;Tracer 在新 scope 上清理面包屑并在末尾弹出 scope;发布配置新增 sentry:test 忽略命令。

Changes

Cohort / File(s) Summary
SDK 当前 Hub 管理
src/sentry/class_map/SentrySdk.php
setCurrentHub 不再使用 tap,改为直接调用并返回 Context::set(__CLASS__, $hub)init() 路径仍通过 tap 初始化。
监听器优先级配置
src/sentry/src/ConfigProvider.php
将监听器注册从无序列表改为带显式优先级的映射:Listener\EventHandleListenerCrons\Listener\EventHandleListenerPHP_INT_MAX - 1Tracing\Listener\EventHandleListenerPHP_INT_MAX;移除一处注释掉的依赖映射。
事件监听器实现调整
src/sentry/src/Listener/EventHandleListener.php
移除公有常量 HUBsetupSentrySdk() 方法及其所有调用点,去掉对 Hyperf\Context\Context 的依赖。
Tracing 启动流程
src/sentry/src/Tracing/Tracer.php
SentrySdk::getCurrentHub() 替换 SentrySdk::init() 获取 hub;push scope 后调用 configureScope 清理面包屑,并在方法结束处 defer 弹出 scope。
发布/忽略命令配置
src/sentry/publish/sentry.php
ignore_commands 列表中新增 'sentry:test'

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor App
  participant Listener as EventHandleListener
  participant Sdk as SentrySdk
  participant Context as Hyperf\Context

  Note over Listener: 监听器按优先级被触发
  App->>Listener: onEvent(...)
  activate Listener
  Note right of Listener: 不再调用 setupSentrySdk()
  Listener->>Sdk: SentrySdk::getCurrentHub() 或直接使用 Hub 接口
  Sdk->>Context: get/set Current Hub (setCurrentHub 直接返回 Context::set)
  Context-->>Listener: Hub 实例或 null
  Listener-->>App: 处理完成
  deactivate Listener
Loading
sequenceDiagram
  autonumber
  actor App
  participant Tracer
  participant Sdk as SentrySdk
  participant Hub as CurrentHub
  participant Scope

  App->>Tracer: startTransaction(...)
  activate Tracer
  Tracer->>Sdk: getCurrentHub()
  Sdk-->>Tracer: Hub
  Tracer->>Hub: pushScope()
  Hub->>Scope: new Scope
  Tracer->>Scope: configureScope(clear breadcrumbs)
  Tracer->>Hub: startTransaction(context)
  Note right of Tracer: defer popScope() at method end
  Hub-->>Tracer: Transaction
  Tracer-->>App: return Transaction
  deactivate Tracer
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • xuanyanwow
  • zds-s

Poem

我在代码巷里轻轻跳,🥕
Hub 改了手法不再绕;
监听有序排队好,事务面包屑一扫光;
兔子鼓掌庆新稿,代码园里笑声饶。

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed 标题简洁明了地描述了 Sentry 的 hub 管理优化和监听器优先级调整,与该 PR 的核心改动相符。
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/sentry-hub-management-optimization

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 755d68f and a4c272f.

📒 Files selected for processing (3)
  • src/sentry/publish/sentry.php (1 hunks)
  • src/sentry/src/ConfigProvider.php (1 hunks)
  • src/sentry/src/Tracing/Tracer.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/sentry/src/ConfigProvider.php
  • src/sentry/src/Tracing/Tracer.php
🧰 Additional context used
📓 Path-based instructions (2)
{src,tests}/**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Adhere to PSR-12 coding standards across PHP code

Files:

  • src/sentry/publish/sentry.php
src/*/**

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

New components must follow the standard component structure under src/{component-name}/ including .gitattributes, .github, LICENSE, README.md, composer.json, and a src/ subdirectory

Files:

  • src/sentry/publish/sentry.php
⏰ 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.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 6.0.2
  • 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 6.0.2
  • GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
  • 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 6.0.2
🔇 Additional comments (1)
src/sentry/publish/sentry.php (1)

86-93: 新增 sentry:test 忽略命令符合预期

sentry:test 纳入忽略列表与其他维护/运维型命令保持一致,有助于避免测试命令触发无意义的上报,改动合理。

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.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a615adb and ac94f80.

📒 Files selected for processing (4)
  • src/sentry/class_map/SentrySdk.php (1 hunks)
  • src/sentry/src/ConfigProvider.php (1 hunks)
  • src/sentry/src/Listener/EventHandleListener.php (7 hunks)
  • src/sentry/src/Tracing/Tracer.php (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/*/src/**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use the namespace pattern FriendsOfHyperf{ComponentName} in all component PHP source files

Files:

  • src/sentry/src/ConfigProvider.php
  • src/sentry/src/Listener/EventHandleListener.php
  • src/sentry/src/Tracing/Tracer.php
src/*/src/ConfigProvider.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Each component must include a ConfigProvider class (FriendsOfHyperf{ComponentName}\ConfigProvider) with __invoke() returning dependencies, commands, listeners, and annotations scan paths

Files:

  • src/sentry/src/ConfigProvider.php
{src,tests}/**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Adhere to PSR-12 coding standards across PHP code

Files:

  • src/sentry/src/ConfigProvider.php
  • src/sentry/src/Listener/EventHandleListener.php
  • src/sentry/src/Tracing/Tracer.php
  • src/sentry/class_map/SentrySdk.php
src/*/**

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

New components must follow the standard component structure under src/{component-name}/ including .gitattributes, .github, LICENSE, README.md, composer.json, and a src/ subdirectory

Files:

  • src/sentry/src/ConfigProvider.php
  • src/sentry/src/Listener/EventHandleListener.php
  • src/sentry/src/Tracing/Tracer.php
  • src/sentry/class_map/SentrySdk.php
🧠 Learnings (1)
📚 Learning: 2025-09-28T00:20:28.922Z
Learnt from: CR
PR: friendsofhyperf/components#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-28T00:20:28.922Z
Learning: Applies to src/*/src/ConfigProvider.php : Each component must include a ConfigProvider class (FriendsOfHyperf\{ComponentName}\ConfigProvider) with __invoke() returning dependencies, commands, listeners, and annotations scan paths

Applied to files:

  • src/sentry/src/ConfigProvider.php
🧬 Code graph analysis (2)
src/sentry/src/ConfigProvider.php (3)
src/sentry/src/Listener/EventHandleListener.php (1)
  • EventHandleListener (44-620)
src/sentry/src/Crons/Listener/EventHandleListener.php (1)
  • EventHandleListener (24-156)
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
  • EventHandleListener (61-810)
src/sentry/src/Tracing/Tracer.php (2)
src/sentry/class_map/SentrySdk.php (2)
  • SentrySdk (24-65)
  • getCurrentHub (51-54)
src/sentry/src/Integration.php (1)
  • configureScope (79-88)
⏰ 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.1 with Swoole 5.1.7
  • GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
  • GitHub Check: Test on PHP 8.3 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 6.0.2
  • GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
  • GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
  • 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.2 with Swoole 6.0.2
  • GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
🔇 Additional comments (1)
src/sentry/src/Listener/EventHandleListener.php (1)

356-357: 确认是否仍需保留 setupSentryCurrentHub 调用
当前在多个生命周期阶段(请求/命令/队列/定时/AMQP/Kafka)均调用此方法,但默认实现为空。未在 Tracer 或其他位置检索到 pushScope()clearBreadcrumbs() 的替代逻辑,请手动核实是否已通过其它途径创建新 Scope 并清理 breadcrumbs:

  • 若仅保留扩展点,需在方法及调用处添加注释说明目的;
  • 若已无必要,建议移除所有此类调用以降噪。

@huangdijia huangdijia merged commit e2ef34f into main Sep 28, 2025
16 checks passed
@huangdijia huangdijia deleted the refactor/sentry-hub-management-optimization branch September 28, 2025 08:37
huangdijia added a commit that referenced this pull request Sep 28, 2025
* refactor(sentry): optimize hub management and listener priorities

- Simplify SentrySdk::setCurrentHub() by removing unnecessary tap usage
- Add explicit priorities to event listeners for deterministic execution order
- Optimize EventHandleListener by removing redundant hub context management
- Improve Tracer initialization to use existing hub instead of creating new ones
- Clear breadcrumbs on new transaction scope to prevent cross-request pollution

* 删除未使用的 HttpClient 依赖项

* fix(tracer): use getCurrentHub instead of init for hub management

* refactor(listener): 移除未使用的 setupSentryCurrentHub 方法

* feat(sentry): 添加 sentry:test 命令到配置

* fix(tracer): 修复事务范围管理,确保在事务结束时正确弹出范围

* fix(ConfigProvider): 确保 Tracing\EventHandleListener 是第一个处理事件的监听器

* fix(tracer): 使用 getCurrentHub 替代 init 进行 Hub 管理

---------

Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants