fix(sentry): adjust event flush order in Integration #1052
Conversation
Move SentrySdk client flush to after Logs and TraceMetrics flush to ensure proper event ordering during shutdown sequence.
Walkthrough调整 Sentry 事件刷新顺序：将对 Sentry 客户端的 flush 调用从先行移动到在 Logs 与 TraceMetrics flush 之后；在单例切面文件中移除并注释了对 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (18)
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.33)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.
Pull request overview
This PR adjusts the event flush order in the Sentry Integration's flushEvents() method to ensure proper event ordering during shutdown sequences. The change moves the Sentry client flush to execute after the Logs and TraceMetrics flush operations.
Key changes:
- Reorders flush operations in
Integration::flushEvents()to callSentrySdk::getCurrentHub()->getClient()?->flush()last instead of first
![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/Aspect/SingletonAspect.php (1)
57-58: 建议：将// !!!注释替换为有意义的说明。当前的
// !!!注释不够清晰，无法说明为什么IntegrationRegistry需要与其他单例类（如HubAdapter、Logs、TraceMetrics）区别对待。其他单例使用Context::getOrSet()进行协程安全的缓存，而IntegrationRegistry直接透传到原始方法。建议添加解释性注释，说明为什么跳过单例缓存（例如与事件刷新顺序相关的原因）。
🔎 建议的改进
- // !!! - \Sentry\Integration\IntegrationRegistry::class => $proceedingJoinPoint->process(), + // IntegrationRegistry 不需要协程上下文缓存，直接使用原始单例以确保事件刷新顺序正确 + \Sentry\Integration\IntegrationRegistry::class => $proceedingJoinPoint->process(),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Aspect/SingletonAspect.php
🧰 Additional context used
📓 Path-based instructions (4)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: All PHP files must includedeclare(strict_types=1)at the top
Use PSR-12 coding standards for PHP code formatting
Use 4-space indentation with short array syntax in PHP code
Use.php-cs-fixer.phpconfiguration for PHP code formattingAll PHP files must include a strict_types declaration at the top
**/*.php: Follow project PHP coding standard enforced by php-cs-fixer with PSR-12 style, 4-space indentation, and short array syntax
Maintain type coverage by updating or adding tests when public APIs change; ensurecomposer test:typesstays green before pushing
Files:
src/sentry/src/Aspect/SingletonAspect.php
src/**/src/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use namespace pattern
FriendsOfHyperf\{ComponentName}for all component classes
Files:
src/sentry/src/Aspect/SingletonAspect.php
src/*/src/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
src/*/src/**/*.php: Namespace convention for components must follow FriendsOfHyperf{ComponentName}
All code must be coroutine-safe and avoid global state without proper context management, blocking I/O operations, and non-coroutine-safe third-party libraries without wrappers
Use Hyperf's Context API for request-scoped data instead of global state
Follows PSR-12 coding standards and use PHP-CS-Fixer for automatic formatting
Use PHPStan at maximum level for static analysis
Ensure component namespace doesn't conflict with existing Hyperf components or other packages in the ecosystem
Integrate deeply with Hyperf's Dependency Injection container for service registration
Leverage Hyperf's AOP (Aspect-Oriented Programming) for cross-cutting concerns via aspects defined in ConfigProvider
Use Hyperf's Event System to register listeners for framework events in components
All code must support coroutine-based concurrency using Swoole/Swow compatibility
Files:
src/sentry/src/Aspect/SingletonAspect.php
src/**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
Each component lives in its own subdirectory with PSR-4 namespaces matching
FriendsOfHyperf\*
Files:
src/sentry/src/Aspect/SingletonAspect.php
🧠 Learnings (1)
📚 Learning: 2025-12-12T05:33:43.947Z
Learnt from: CR
Repo: friendsofhyperf/components PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-12T05:33:43.947Z
Learning: Applies to src/*/src/**/*.php : Leverage Hyperf's AOP (Aspect-Oriented Programming) for cross-cutting concerns via aspects defined in ConfigProvider
Applied to files:
src/sentry/src/Aspect/SingletonAspect.php
🧬 Code graph analysis (1)
src/sentry/src/Aspect/SingletonAspect.php (3)
src/sentry/src/Aspect/CoroutineAspect.php (1)
process(40-49)src/sentry/src/Metrics/Listener/OnWorkerStart.php (1)
process(50-124)src/sentry/src/Aspect/RedisAspect.php (1)
process(37-62)
⏰ 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). (18)
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 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 6.1.3
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
🔇 Additional comments (1)
src/sentry/src/Aspect/SingletonAspect.php (1)
21-27: 确认：IntegrationRegistry是否仍需保留在$classes数组中？
IntegrationRegistry::class仍然在第 24 行的$classes数组中注册，但第 58 行的处理逻辑与default分支（第 69 行）完全相同，都是直接调用$proceedingJoinPoint->process()。如果
IntegrationRegistry不需要任何特殊的 AOP 处理，可以考虑将其从$classes数组中移除，这样代码会更简洁。如果保留是为了文档目的或未来可能的修改，建议添加注释说明。Also applies to: 47-70
… unexpected issues
* fix(sentry): adjust event flush order in Integration Move SentrySdk client flush to after Logs and TraceMetrics flush to ensure proper event ordering during shutdown sequence. * fix(sentry): adjust event flush order for IntegrationRegistry in SingletonAspect * fix(sentry): disable IntegrationRegistry instance creation to prevent unexpected issues --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Summary
Integration::flushEvents()SentrySdk::getCurrentHub()->getClient()?->flush()to execute afterLogsandTraceMetricsflushTest plan
Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.