refactor(sentry): use Context API for transaction storage #1055
Conversation
Replace static property with Hyperf's Context API for storing transaction data, ensuring proper coroutine isolation in Swoole/Swow environments.
|
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将 Integration 中的事务（transaction）存储从私有静态属性迁移到 Hyperf Context，并新增公共常量 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Listener as OnBeforeHandle
participant SentryContext as SentryContext / Context
participant Integration as Integration
participant MetricListener as OnMetricFactoryReady
rect rgb(240,248,255)
Note right of Listener: 请求/任务开始
Listener->>SentryContext: setRunningInCommand(true) %% 标记命令模式
end
rect rgb(248,240,255)
Note right of Integration: 事务创建/获取
Integration->>SentryContext: get(TRANSACTION) -> maybe Transaction
alt transaction exists
Integration->>SentryContext: set(TRANSACTION, transaction)
end
end
rect rgb(240,255,240)
Note right of MetricListener: Metric 工厂就绪时
MetricListener->>SentryContext: isRunningInCommand()?
alt true
MetricListener->>MetricListener: 使用 server stats factory (command mode)
else false
MetricListener->>MetricListener: 使用默认 server stats factory
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 refactoring improves coroutine safety in the Sentry integration by replacing a static property with Hyperf's Context API for storing transaction data. In Swoole/Swow environments, static properties are shared across all coroutines, which can cause race conditions. The Context API provides proper request-scoped isolation.
Key changes:
- Replace static
$transactionproperty with Context API storage using a constant key - Add null safety check in event processing to avoid setting empty transactions
- Minor code style improvement for PHPStan ignore comment placement
| class Integration implements IntegrationInterface | ||
| { | ||
| private static ?string $transaction = null; | ||
| public const TRANSACTION = 'sentry.integration.transaction'; |
There was a problem hiding this comment.
The constant naming doesn't follow the established pattern in the codebase. Looking at the SentryContext class, all context key constants use a prefix like CTX_ (e.g., CTX_CRON_CHECKIN_ID, CTX_CARRIER). For consistency, this constant should be named CTX_TRANSACTION instead of TRANSACTION, and the value should follow the pattern 'sentry.ctx.transaction' instead of 'sentry.integration.transaction'.
| public static function getTransaction(): ?string | ||
| { | ||
| return self::$transaction; | ||
| return Context::get(self::TRANSACTION); | ||
| } | ||
|
|
||
| public static function setTransaction(?string $transaction): void | ||
| { | ||
| self::$transaction = $transaction; | ||
| Context::set(self::TRANSACTION, $transaction); | ||
| } |
There was a problem hiding this comment.
The refactored transaction storage methods lack test coverage. Since the Sentry component already has test files (CarrierTest, FeatureTest, SqlParseTest), consider adding tests to verify that getTransaction() and setTransaction() correctly store and retrieve transaction data from the Context API, and that the transaction data is properly isolated between different coroutines.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…text` with new setter and getter methods, deprecating the old static property.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/sentry/src/SentryContext.php (1)
164-172: 方法实现正确，考虑添加清理方法以保持一致性。这两个方法正确地使用了 Hyperf Context API，实现了命令运行状态的上下文管理。不过注意到类中其他类似的上下文管理（如
destroyRpcSpanContext()在第 159-162 行）提供了清理方法，建议考虑为CTX_RUNNING_IN_COMMAND也添加一个destroyRunningInCommand()方法以保持 API 的一致性。根据学习记录，对于请求范围的数据使用 Hyperf Context API 而非全局状态是正确的做法。
🔎 可选的清理方法实现
+ public static function destroyRunningInCommand(): void + { + Context::destroy(self::CTX_RUNNING_IN_COMMAND); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/sentry/src/Constants.phpsrc/sentry/src/Metrics/Listener/OnBeforeHandle.phpsrc/sentry/src/Metrics/Listener/OnMetricFactoryReady.phpsrc/sentry/src/SentryContext.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/SentryContext.phpsrc/sentry/src/Metrics/Listener/OnMetricFactoryReady.phpsrc/sentry/src/Metrics/Listener/OnBeforeHandle.phpsrc/sentry/src/Constants.php
src/**/src/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use namespace pattern
FriendsOfHyperf\{ComponentName}for all component classes
Files:
src/sentry/src/SentryContext.phpsrc/sentry/src/Metrics/Listener/OnMetricFactoryReady.phpsrc/sentry/src/Metrics/Listener/OnBeforeHandle.phpsrc/sentry/src/Constants.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/SentryContext.phpsrc/sentry/src/Metrics/Listener/OnMetricFactoryReady.phpsrc/sentry/src/Metrics/Listener/OnBeforeHandle.phpsrc/sentry/src/Constants.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/SentryContext.phpsrc/sentry/src/Metrics/Listener/OnMetricFactoryReady.phpsrc/sentry/src/Metrics/Listener/OnBeforeHandle.phpsrc/sentry/src/Constants.php
🧠 Learnings (2)
📓 Common learnings
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 : Use Hyperf's Context API for request-scoped data instead of global state
📚 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 : Use Hyperf's Context API for request-scoped data instead of global state
Applied to files:
src/sentry/src/SentryContext.php
🧬 Code graph analysis (2)
src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (1)
src/sentry/src/SentryContext.php (2)
SentryContext(18-173)isRunningInCommand(169-172)
src/sentry/src/Metrics/Listener/OnBeforeHandle.php (1)
src/sentry/src/SentryContext.php (2)
SentryContext(18-173)setRunningInCommand(164-167)
⏰ 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). (19)
- 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.3 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.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.2 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- 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.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.3
- GitHub Check: Seer Code Review
🔇 Additional comments (5)
src/sentry/src/SentryContext.php (1)
42-43: 常量定义正确！常量命名遵循类中的命名约定，使用清晰的语义化名称。
src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (2)
19-19: 导入语句正确！正确导入了
SentryContext以支持新的上下文 API 调用。
87-87: 成功迁移到 Context API！从静态属性
SentryConstants::$runningInCommand迁移到SentryContext::isRunningInCommand()是正确的，这改善了协程隔离性。逻辑语义保持不变。根据学习记录，这符合"对请求范围数据使用 Hyperf Context API 而非全局状态"的最佳实践。
src/sentry/src/Metrics/Listener/OnBeforeHandle.php (1)
61-61: 成功迁移到 Context API！从直接赋值静态属性
Constants::$runningInCommand = true改为调用SentryContext::setRunningInCommand()是正确的。这确保了命令执行状态在协程上下文中正确隔离，避免了不同协程间的状态污染。根据学习记录，这符合使用 Hyperf Context API 管理请求范围数据的最佳实践。
src/sentry/src/Constants.php (1)
69-72: 弃用标记正确，所有引用已成功迁移。代码库中未发现对
Constants::$runningInCommand的直接引用。已验证所有相关代码已成功迁移到SentryContextAPI（setRunningInCommand()和isRunningInCommand()方法）。弃用声明的版本信息清晰（v3.1 弃用，v3.2 移除），迁移工作已完成。
Summary
$transactionwith Hyperf'sContextAPI for storing transaction dataTRANSACTIONfor context keyDetails
This change enhances coroutine safety by using Hyperf's Context API instead of a static property. In a coroutine environment like Swoole/Swow, static properties are shared across all coroutines, which can lead to race conditions and incorrect transaction data being associated with the wrong request.
The Context API provides request-scoped storage that is properly isolated between coroutines.
Changes
Integration.php:private static ?string $transactionwithpublic const TRANSACTION = 'sentry.integration.transaction'getTransaction()to useContext::get(self::TRANSACTION)setTransaction()to useContext::set(self::TRANSACTION, $transaction)Test plan
Summary by CodeRabbit
发布说明
重构
改进
弃用
✏️ Tip: You can customize this high-level summary in your review settings.