-
-
Notifications
You must be signed in to change notification settings - Fork 27
feat(sentry): support nested spans for console commands #956
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This change enables console commands to create child spans when executed within an existing trace, rather than always creating a new transaction. Key improvements: - Check for existing span before creating transaction - Create child span if parent span exists - Properly restore parent span context after command execution - Unified span terminology (replaced transaction-specific references) This provides better trace continuity when commands are executed as part of a larger trace context, while maintaining backward compatibility for standalone command execution.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough将命令与协程追踪从以 Transaction 为中心切换为基于当前 Span 的管理:启动时创建根或 child Span、pushScope 并通过 CoContainer 保存父 span;结束时设置状态/标签、finish、flush 并恢复父 span,移除基于 defer 的事务完成逻辑。(≤50字) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller as 调用方(命令 / 协程创建)
participant Listener as EventHandleListener / CoroutineAspect
participant Hub as Sentry Hub / Scope
participant ParentSpan as 当前父 Span
participant Co as CoContainer
Caller->>Listener: start event / create coroutine
alt 无当前 span
Listener->>Hub: start root span, pushScope
Note right of Hub #D6EAF8: root span + 新 scope
else 有当前 span
Listener->>ParentSpan: start child span
Listener->>Co: set(调用对象, ParentSpan)
Listener->>Hub: pushScope 并将 child span 设为当前
Note right of Hub #F9E79F: child span + 新 scope
end
Note over Caller,Listener: 执行业务/协程主体(可能产生更多子 span)
Caller->>Listener: finish event / 协程结束
Listener->>Hub: 设置 exit code/status/tags(若采样)
Listener->>Hub: finish 当前 span
Listener->>Hub: flush events
Listener->>Hub: popScope(恢复先前 scope)
Listener->>Co: pull(调用对象) -> 恢复父 span 到 scope/hub
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 (beta)
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.30)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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php(2 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/Tracing/Listener/EventHandleListener.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/Tracing/Listener/EventHandleListener.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (4)
src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Tracer.php (1)
startTransaction(32-59)src/sentry/src/Function.php (1)
startTransaction(32-37)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)
⏰ 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). (8)
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- 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 6.0.2
- 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
…istener for improved tracing
…larity and consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php(4 hunks)src/sentry/src/Tracing/Listener/EventHandleListener.php(2 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/Tracing/Aspect/CoroutineAspect.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.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/Tracing/Aspect/CoroutineAspect.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.php
🧬 Code graph analysis (2)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (4)
src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Tracer.php (1)
startTransaction(32-59)src/sentry/src/Function.php (1)
startTransaction(32-37)src/sentry/src/Tracing/SpanStarter.php (2)
startTransaction(39-44)continueTrace(114-143)
src/sentry/src/Tracing/Listener/EventHandleListener.php (4)
src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Tracer.php (1)
startTransaction(32-59)src/sentry/src/Function.php (1)
startTransaction(32-37)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)
⏰ 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.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 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.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 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 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
…eference container
There was a problem hiding this 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
📒 Files selected for processing (2)
src/sentry/src/Tracing/Listener/EventHandleListener.php(3 hunks)src/sentry/src/Util/CoContainer.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/Tracing/Listener/EventHandleListener.phpsrc/sentry/src/Util/CoContainer.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/Tracing/Listener/EventHandleListener.phpsrc/sentry/src/Util/CoContainer.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (5)
src/sentry/src/Util/CoContainer.php (3)
CoContainer(21-50)set(39-44)get(29-32)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Tracer.php (1)
startTransaction(32-59)src/sentry/src/Function.php (1)
startTransaction(32-37)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)
⏰ 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.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.2 with Swoole 5.1.7
- 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.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
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
There was a problem hiding this 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
📒 Files selected for processing (2)
src/sentry/src/Tracing/Listener/EventHandleListener.php(3 hunks)src/sentry/src/Util/CoContainer.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/Tracing/Listener/EventHandleListener.phpsrc/sentry/src/Util/CoContainer.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/Tracing/Listener/EventHandleListener.phpsrc/sentry/src/Util/CoContainer.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (4)
src/sentry/src/Util/CoContainer.php (3)
CoContainer(21-70)set(30-35)pull(52-64)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Tracer.php (1)
startTransaction(32-59)src/sentry/src/Function.php (1)
startTransaction(32-37)
⏰ 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.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 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.2 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 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.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/sentry/src/Tracing/Listener/EventHandleListener.php(3 hunks)src/sentry/src/Util/CoContainer.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/Util/CoContainer.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.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/Util/CoContainer.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (5)
src/sentry/src/Util/CoContainer.php (3)
CoContainer(21-70)set(30-35)pull(52-64)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Tracer.php (1)
startTransaction(32-59)src/sentry/src/Function.php (1)
startTransaction(32-37)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)
⏰ 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). (11)
- 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.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 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.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 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/sentry/src/Util/CoContainer.php (1)
48-64: 修正pull的参数类型以避免运行时错误
WeakMap的键必须是对象,但当前签名pull(?object $key = null)允许调用方省略参数或传入null。一旦发生这种调用,isset($container[$key])将直接抛出TypeError: WeakMap key must be object,导致运行时崩溃。此外,可空签名也违背了 PHPDoc 中@param TKey $key的约束(TKey被限定为object)。应用以下修复移除可空签名和默认值,强制调用方传入有效的对象键:
/** * @param TKey $key * @return null|TValue */ - public static function pull(?object $key = null): ?object + public static function pull(object $key): ?object { $container = self::getContainer(); if (! isset($container[$key])) { return null; } $value = $container[$key]; unset($container[$key]); return $value; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/sentry/src/Tracing/Listener/EventHandleListener.php(3 hunks)src/sentry/src/Util/CoContainer.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/Util/CoContainer.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.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/Util/CoContainer.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (4)
src/sentry/src/Util/CoContainer.php (3)
CoContainer(21-75)set(30-35)pull(52-64)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Tracer.php (1)
startTransaction(32-59)src/sentry/src/Function.php (1)
startTransaction(32-37)
⏰ 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.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
- GitHub Check: Test on PHP 8.2 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 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.3 with Swoole 5.1.7
🔇 Additional comments (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
370-420: LGTM!命令启动处理逻辑清晰:无父 span 时创建根事务,有父 span 时推入新作用域并创建子 span。代码正确地在存在父 span 但未采样时提前返回(line 399-401),避免不必要的 span 创建。父 span 通过
CoContainer保存以便后续恢复(line 418)。
…etter span management
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php(3 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/Tracing/Aspect/CoroutineAspect.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/Tracing/Aspect/CoroutineAspect.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (4)
src/sentry/src/Tracing/Tracer.php (2)
trace(70-93)startTransaction(32-59)src/sentry/src/Function.php (2)
trace(48-53)startTransaction(32-37)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)
⏰ 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.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.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.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
…ved span handling
…ry scope management
…e for coroutine execution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the Sentry integration's console command tracing to support nested spans, enabling better trace continuity when commands are executed within an existing trace context. The changes move from a transaction-only model to a span-centric approach that creates proper parent-child relationships.
Key Changes:
- Nested span support: Console commands now detect existing spans and create child spans instead of always starting new transactions
- Context management: Added
CoContainerutility for managing parent span references across coroutine boundaries - Unified span terminology: Refactored code to consistently use
span/getSpan()instead of mixing transaction references
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/sentry/src/Util/CoContainer.php |
New utility class providing WeakMap-based context storage for managing parent spans in coroutine environments |
src/sentry/src/Tracing/Listener/EventHandleListener.php |
Enhanced command tracing to support nested spans with proper parent-child relationships and context restoration |
src/sentry/src/Tracing/Aspect/CoroutineAspect.php |
Refactored coroutine tracing to use span-centric approach with simplified execution flow |
…improved readability
…ck for streamlined processing
…xt to new coroutine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php(3 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/Tracing/Aspect/CoroutineAspect.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/Tracing/Aspect/CoroutineAspect.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (5)
src/sentry/src/Tracing/Tracer.php (2)
trace(70-93)startTransaction(32-59)src/sentry/src/Function.php (2)
trace(48-53)startTransaction(32-37)src/sentry/src/Tracing/SpanStarter.php (3)
trace(57-62)startTransaction(39-44)continueTrace(114-143)src/sentry/class_map/SentrySdk.php (3)
SentrySdk(24-65)getCurrentHub(51-54)init(42-45)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)
⏰ 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 6.0.2
- 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.1 with Swoole 5.1.7
- 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.1 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 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
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
63-95: 考虑改进变量命名和代码组织当前的嵌套事务创建逻辑在功能上是正确的,使用
continueTrace()来维护跨协程的分布式追踪链路。不过,可以考虑以下改进:
变量命名:第 75 行的
$transaction可以重命名为$coroutineTransaction以更清楚地表明它是子协程中的新事务,区别于可能存在的父 span 上下文。代码组织:第 63-95 行的 callable 包装逻辑较为复杂,包含了上下文传播、事务创建、defer 清理和内层 trace 调用。可以考虑将这部分逻辑提取到一个独立的辅助方法中,例如
wrapCallableForCoroutineTracing(),以提高可读性和可维护性。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php(2 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/Tracing/Aspect/CoroutineAspect.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/Tracing/Aspect/CoroutineAspect.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (4)
src/sentry/src/Tracing/Tracer.php (2)
startTransaction(32-59)trace(70-93)src/sentry/src/Function.php (2)
startTransaction(32-37)trace(48-53)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)init(42-45)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)
⏰ 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). (9)
- 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 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
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
🔇 Additional comments (2)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (2)
21-21: LGTM!导入的
Scope和trace函数在重构后的代码中正确使用。Also applies to: 25-25
88-94: LGTM!内层
trace()调用正确地将 callable 包装在不接受参数的闭包中(第 89 行),避免了之前版本中的ArgumentCountError问题。这为协程的实际执行创建了一个coroutine.execute操作的子 span。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
423-453: 修复清理与收尾:避免提前返回导致 Scope 泄漏,并在根场景正确结束事务
- 提前在 Line 425 返回会跳过 finally,导致已
pushScope()的子作用域不被pop,父级上下文不恢复,且可能遗留未完成的事务。- 根命令路径中创建的事务未在
AfterExecute被结束,导致事务悬挂与数据不完整。- 仅在拉取到父级时
popScope()会遗漏根场景中为子 span 推入的作用域,造成泄漏。修复要点:
- 不在采样判断处提前返回;将采样布尔值缓存为
$sampled,仅在采样为真时设置标签/状态并finish()与flush()。- 无论是否嵌套,均在采样为真时
popScope()(因为子 span 只会在采样时创建/设置到 scope)。- 当无父级 span 被拉取时(根场景),在子 span 完成后结束当前事务。
应用如下修改:
- $span = SentrySdk::getCurrentHub()->getSpan(); - - if (! $span?->getSampled()) { - return; - } - - $command = $event->getCommand(); + $span = SentrySdk::getCurrentHub()->getSpan(); + if (! $span instanceof Span) { + return; + } + $command = $event->getCommand(); + $sampled = $span->getSampled(); try { - /** @var int $exitCode */ - $exitCode = (fn () => $this->exitCode ?? SymfonyCommand::SUCCESS)->call($command); - - $span->setTags([ - 'command.exit_code' => (string) $exitCode, - ])->setStatus( - $event->getThrowable() || $exitCode !== SymfonyCommand::SUCCESS - ? SpanStatus::internalError() - : SpanStatus::ok() - ); + if ($sampled) { + /** @var int $exitCode */ + $exitCode = (fn () => $this->exitCode ?? SymfonyCommand::SUCCESS)->call($command); + + $span->setTags([ + 'command.exit_code' => (string) $exitCode, + ])->setStatus( + $event->getThrowable() || $exitCode !== SymfonyCommand::SUCCESS + ? SpanStatus::internalError() + : SpanStatus::ok() + ); + } } finally { - $span->finish(); - - Integration::flushEvents(); - - $parentSpan = CoContainer::pull($command); + if ($sampled) { + $span->finish(); + } + + $parentSpan = CoContainer::pull($command); - if ($parentSpan instanceof Span) { - SentrySdk::getCurrentHub()->setSpan($parentSpan); - SentrySdk::getCurrentHub()->popScope(); - } + if ($parentSpan instanceof Span) { + // 嵌套:恢复父级上下文 + SentrySdk::getCurrentHub()->setSpan($parentSpan); + } else { + // 根:结束事务 + if ($sampled) { + $transaction = SentrySdk::getCurrentHub()->getTransaction(); + if ($transaction) { + $transaction->finish(); + } + } + } + + if ($sampled) { + // 始终弹出为子 span 推入的作用域(仅在采样为真时创建) + SentrySdk::getCurrentHub()->popScope(); + Integration::flushEvents(); + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php(3 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/Tracing/Listener/EventHandleListener.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/Tracing/Listener/EventHandleListener.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (5)
src/sentry/src/Util/CoContainer.php (3)
CoContainer(21-75)set(30-35)pull(52-64)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Tracer.php (1)
startTransaction(32-59)src/sentry/src/Function.php (1)
startTransaction(32-37)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)
⏰ 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). (11)
- 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.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.2 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.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
…ycle in command execution
…xecute' for accurate command tracing
There was a problem hiding this 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
📒 Files selected for processing (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php(3 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/Tracing/Listener/EventHandleListener.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/Tracing/Listener/EventHandleListener.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (5)
src/sentry/src/Util/CoContainer.php (3)
CoContainer(21-75)set(30-35)pull(52-64)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Tracer.php (1)
startTransaction(32-59)src/sentry/src/Function.php (1)
startTransaction(32-37)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)
⏰ 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). (10)
- 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 6.0.2
- 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.2 with Swoole 6.0.2
- 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
…an instance and sampling before finishing
…ng in CoContainer
…pan is not sampled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/sentry/src/Tracing/Listener/EventHandleListener.php (2)
457-457: 移除对父级 span 的 finish() 调用第 457 行错误地终止了父级 span。父级 span 应在命令执行完毕后继续存在,以便后续操作使用。只有当前命令创建的子 span(第 450 行)应该被 finish。
应用以下修复:
$parentSpan = CoContainer::pull($command); if ($parentSpan instanceof Span) { - $parentSpan->finish(); SentrySdk::getCurrentHub()->setSpan($parentSpan); SentrySdk::getCurrentHub()->popScope(); }
381-420: 根命令与嵌套命令的处理逻辑存在严重错误当前实现存在两个关键问题:
根命令创建了多余的子 span 层级:
- 第 381-398 行:无父 span 时创建根事务并存储
- 第 406-419 行:无条件地从该事务创建子 span
- 问题:根命令不应创建额外的子 span,根事务本身就是应被追踪的 span
嵌套命令的父 span 未被存储:
- 当
getSpan()返回非空时(嵌套场景),第 381 行条件为 false- 第 398 行的
CoContainer::set()位于 if 块内,不会执行- 后果:
handleCommandFinished中第 454 行的pull将返回 null,无法恢复父 span,导致作用域泄漏且链路断裂正确的逻辑应该是:
- if (! $parentSpan = SentrySdk::getCurrentHub()->getSpan()) { - $parentSpan = startTransaction( + $parentSpan = SentrySdk::getCurrentHub()->getSpan(); + + if ($parentSpan === null) { + // 根命令:创建根事务并直接返回 + startTransaction( TransactionContext::make() ->setName($command->getName() ?: '<unnamed command>') ->setOp('console.command') ->setDescription($command->getDescription()) ->setOrigin('auto.console') ->setData([ 'command.arguments' => (fn () => $this->input->getArguments())->call($command), 'command.options' => (fn () => $this->input->getOptions())->call($command), ]) ->setTags([ 'command.name' => $command->getName(), ]) ->setSource(TransactionSource::task()) ); - - CoContainer::set($command, $parentSpan); + return; } + // 嵌套命令:检查采样并存储父 span if (! $parentSpan->getSampled()) { - CoContainer::del($command); return; } + CoContainer::set($command, $parentSpan); + $scope = SentrySdk::getCurrentHub()->pushScope(); $span = $parentSpan->startChild( SpanContext::make() ->setOp('console.command.execute') ->setDescription($command->getName() ?: $command->getDescription()) ->setData([ 'command.arguments' => (fn () => $this->input->getArguments())->call($command), 'command.options' => (fn () => $this->input->getOptions())->call($command), ]) ->setTags([ 'command.name' => $command->getName(), ]) ); $scope->setSpan($span);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php(3 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/Tracing/Listener/EventHandleListener.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/Tracing/Listener/EventHandleListener.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (5)
src/sentry/src/Util/CoContainer.php (4)
CoContainer(21-75)set(30-35)del(66-69)pull(52-64)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Tracer.php (1)
startTransaction(32-59)src/sentry/src/Function.php (1)
startTransaction(32-37)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)
⏰ 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 6.0.2
- 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.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 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
- GitHub Check: Test on PHP 8.1 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 5.1.7
* feat(sentry): support nested spans for console commands This change enables console commands to create child spans when executed within an existing trace, rather than always creating a new transaction. Key improvements: - Check for existing span before creating transaction - Create child span if parent span exists - Properly restore parent span context after command execution - Unified span terminology (replaced transaction-specific references) This provides better trace continuity when commands are executed as part of a larger trace context, while maintaining backward compatibility for standalone command execution. * fix(sentry): improve scope management for console command spans * fix(sentry): update coroutine tracing to use spans instead of transactions * fix(sentry): update span handling in CoroutineAspect and EventHandleListener for improved tracing * fix(sentry): refactor span handling in CoroutineAspect for improved clarity and consistency * fix(sentry): clarify purpose of commented span handling in EventHandleListener * feat(sentry): add CoContainer for managing Sentry context in a weak reference container * fix(sentry): refactor CoContainer method to improve span retrieval logic * fix(sentry): refactor CoContainer methods for improved clarity and organization * fix(sentry): add sampling check for parent span in EventHandleListener * fix(sentry): add del method to CoContainer for removing items from the container * fix(sentry): update getContainer method to use getOrSet for better context management * fix(sentry): refactor coroutine execution to use trace function for better span management * fix(sentry): adjust scope management in EventHandleListener for improved span handling * fix(sentry): ensure parent span is restored after coroutine processing * fix(sentry): streamline coroutine span creation by removing unnecessary scope management * fix(sentry): refactor callable in trace function to use static closure for coroutine execution * fix(CoContainer): update pull method to require a non-null object key * fix(EventHandleListener): streamline span tag and status setting for improved readability * fix(EventHandleListener): remove unnecessary parent span sampling check for streamlined processing * fix(CoroutineAspect): initialize Sentry SDK before transferring context to new coroutine * fix(CoroutineAspect): ensure Sentry SDK initialization in new coroutine context transfer * fix(CoroutineAspect): update operation name to 'coroutine.create' for accurate tracing * fix(CoroutineAspect): update operation names for better tracing accuracy * fix(EventHandleListener): streamline span handling by consolidating parent span checks * fix(EventHandleListener): add check for parent span sampling before proceeding * fix(EventHandleListener): ensure proper handling of parent span lifecycle in command execution * fix(EventHandleListener): update operation name to 'console.command.execute' for accurate command tracing * fix(EventHandleListener): ensure parent span is set in CoContainer only if sampled * fix(EventHandleListener): improve parent span handling by checking span instance and sampling before finishing * fix(EventHandleListener): ensure parent span is finished before setting in CoContainer * fix(EventHandleListener): remove command from CoContainer if parent span is not sampled --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Summary
This PR enhances the Sentry integration's console command tracing to support nested spans, enabling better trace continuity when commands are executed within an existing trace context.
Changes
span/getSpan()instead of mixingtransaction/getTransaction()referencesBenefits
Technical Details
The implementation checks for an active span before creating a transaction:
getCurrentHub()->getSpan()returns null → creates a new transaction (existing behavior)In coroutine contexts, the parent span is properly restored after the command span finishes to maintain correct span hierarchy.
Test Plan
Summary by CodeRabbit
新增/改进
Bug 修复