fix(sentry): bound transport memory usage - #1079
Conversation
WalkthroughSentry 传输改用有界队列和固定 worker，并新增超时、统计及关闭等待逻辑。Sentry 新增请求级 runtime context 生命周期监听器，并调整监听器优先级。测试覆盖两项行为。 ChangesSentry 传输
请求上下文生命周期
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SentryEvent
participant CoHttpTransport
participant runWorker
participant HTTPTransport
SentryEvent->>CoHttpTransport: send(event)
CoHttpTransport->>runWorker: 入队 event
runWorker->>HTTPTransport: 发送 event
HTTPTransport-->>runWorker: 返回结果或异常
CoHttpTransport->>CoHttpTransport: close(timeout) 等待 idle
sequenceDiagram
participant RequestReceived
participant RequestContextLifecycleListener
participant RuntimeContext
RequestReceived->>RequestContextLifecycleListener: process(event)
RequestContextLifecycleListener->>RuntimeContext: 创建请求 context
RequestContextLifecycleListener->>RuntimeContext: 注册协程清理回调
RuntimeContext-->>RequestContextLifecycleListener: 请求结束时结束 context
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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.2.7)PHPStan was skipped because the config uses disallowed 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef2a61a620
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $result = $chan?->push($event, $this->timeout) ? ResultStatus::success() : ResultStatus::skipped(); | ||
| // Swoole treats a zero timeout as an infinite wait, so check capacity first | ||
| // to provide fail-fast backpressure without suspending the application coroutine. | ||
| if ($this->timeout === 0.0 && $chan !== null && $chan->getLength() >= $chan->getCapacity()) { |
There was a problem hiding this comment.
When Swoole preemptive scheduling is enabled and concurrent producers contend for the final queue slot, multiple coroutines can pass this capacity check before any performs push(). After one fills the queue, another calls push(..., 0), which—as noted immediately above—waits indefinitely, so the documented fail-fast mode can instead suspend an application request. Use an atomic nonblocking enqueue operation or serialize the capacity check and push.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/sentry/src/Listener/RequestContextLifecycleListener.php`:
- Around line 4-10: 为
src/sentry/src/Listener/RequestContextLifecycleListener.php 第4-10行及
tests/Sentry/RequestContextLifecycleListenerTest.php 第4-10行的 PHP
文件头补充项目许可证声明，两个文件使用一致的许可证信息，并保留现有项目链接、文档和联系信息。
In `@src/sentry/src/Transport/CoHttpTransport.php`:
- Around line 162-177: Move getHttpTransport() initialization from before the
loop into the per-event try block in the worker flow around CoHttpTransport’s
event consumption, so initialization failures are caught and logged like send
failures. Ensure a single transport initialization error does not terminate the
worker; after handling that event, continue consuming subsequent events while
preserving the existing inFlight accounting and logger behavior.
In `@tests/Sentry/CoHttpTransportTest.php`:
- Around line 4-10: 在 CoHttpTransportTest.php
的文件头注释中补充项目适用的许可证标识，保留现有项目链接、文档和联系信息不变，并遵循仓库其他 PHP 文件的许可证声明格式。
- Around line 87-145: 将 CoHttpTransportTest 中的 PHPUnit 测试类和方法改为 Pest 测试闭包，移除
final class、继承 TestCase 及 public 方法结构；在测试文件中配置
uses(TestCase::class)->group('sentry')，并为两个测试闭包应用组件对应的 ->group('sentry')
修饰符，同时保留现有测试逻辑与断言。
In `@tests/Sentry/RequestContextLifecycleListenerTest.php`:
- Around line 36-38: 将 RequestContextLifecycleListenerTest 从 PHPUnit 测试类迁移为 Pest
测试，移除类继承及 PHPUnit 类级属性，使用 uses() 配置测试依赖，并为测试链式添加对应的 Sentry 组件 ->group() 修饰符。
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6aa3c232-d752-4ea6-b058-f71268b0749c
📒 Files selected for processing (12)
docs/en/components/sentry.mddocs/zh-cn/components/sentry.mddocs/zh-hk/components/sentry.mddocs/zh-tw/components/sentry.mdsrc/sentry/README.mdsrc/sentry/README_CN.mdsrc/sentry/publish/sentry.phpsrc/sentry/src/ConfigProvider.phpsrc/sentry/src/Listener/RequestContextLifecycleListener.phpsrc/sentry/src/Transport/CoHttpTransport.phptests/Sentry/CoHttpTransportTest.phptests/Sentry/RequestContextLifecycleListenerTest.php
| /** | ||
| * This file is part of friendsofhyperf/components. | ||
| * | ||
| * @link https://github.com/friendsofhyperf/components | ||
| * @document https://github.com/friendsofhyperf/components/blob/main/README.md | ||
| * @contact huangdijia@gmail.com | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
在新 PHP 文件头中添加许可证信息。
当前文件头只包含项目链接和联系信息。请添加项目许可证声明。
src/sentry/src/Listener/RequestContextLifecycleListener.php#L4-L10：添加项目许可证信息。tests/Sentry/RequestContextLifecycleListenerTest.php#L4-L10：添加相同的项目许可证信息。
As per coding guidelines，所有 PHP 文件头必须包含许可证信息。
📍 Affects 2 files
src/sentry/src/Listener/RequestContextLifecycleListener.php#L4-L10(this comment)tests/Sentry/RequestContextLifecycleListenerTest.php#L4-L10
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/sentry/src/Listener/RequestContextLifecycleListener.php` around lines 4 -
10, 为 src/sentry/src/Listener/RequestContextLifecycleListener.php 第4-10行及
tests/Sentry/RequestContextLifecycleListenerTest.php 第4-10行的 PHP
文件头补充项目许可证声明，两个文件使用一致的许可证信息，并保留现有项目链接、文档和联系信息。
Source: Coding guidelines
| $transport = $this->getHttpTransport(); | ||
| $logger = $this->clientBuilder?->getLogger(); | ||
|
|
||
| while (true) { | ||
| /** @var Event|false $event */ | ||
| $event = $chan->pop(); | ||
| if (! $event) { | ||
| break; | ||
| } | ||
|
|
||
| $this->closeChannel(); | ||
| }); | ||
| ++$this->inFlight; | ||
|
|
||
| try { | ||
| $transport->send($event); | ||
| } catch (Throwable $e) { | ||
| $logger?->error('Failed to send event to Sentry: ' . $e->getMessage(), ['exception' => $e]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
防止初始化异常永久耗尽 worker。
第 162 行位于 try 块外。若 getHttpTransport() 抛出异常，每个新 worker 都会退出，
但 $workerExited 保持 false。后续 send() 仍会接受事件，队列无法消费，close() 会一直
等待至超时。将 HTTP transport 初始化移入每个事件的 try 块，使 worker 在单次失败后继续消费。
建议修复
- $transport = $this->getHttpTransport();
- $logger = $this->clientBuilder?->getLogger();
-
while (true) {
@@
++$this->inFlight;
try {
- $transport->send($event);
+ $this->getHttpTransport()->send($event);
} catch (Throwable $e) {
- $logger?->error('Failed to send event to Sentry: ' . $e->getMessage(), ['exception' => $e]);
+ $this->clientBuilder?->getLogger()?->error(
+ 'Failed to send event to Sentry: ' . $e->getMessage(),
+ ['exception' => $e]
+ );
} finally {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $transport = $this->getHttpTransport(); | |
| $logger = $this->clientBuilder?->getLogger(); | |
| while (true) { | |
| /** @var Event|false $event */ | |
| $event = $chan->pop(); | |
| if (! $event) { | |
| break; | |
| } | |
| $this->closeChannel(); | |
| }); | |
| ++$this->inFlight; | |
| try { | |
| $transport->send($event); | |
| } catch (Throwable $e) { | |
| $logger?->error('Failed to send event to Sentry: ' . $e->getMessage(), ['exception' => $e]); | |
| while (true) { | |
| /** `@var` Event|false $event */ | |
| $event = $chan->pop(); | |
| if (! $event) { | |
| break; | |
| } | |
| +$this->inFlight; | |
| try { | |
| $this->getHttpTransport()->send($event); | |
| } catch (Throwable $e) { | |
| $this->clientBuilder?->getLogger()?->error( | |
| 'Failed to send event to Sentry: ' . $e->getMessage(), | |
| ['exception' => $e] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/sentry/src/Transport/CoHttpTransport.php` around lines 162 - 177, Move
getHttpTransport() initialization from before the loop into the per-event try
block in the worker flow around CoHttpTransport’s event consumption, so
initialization failures are caught and logged like send failures. Ensure a
single transport initialization error does not terminate the worker; after
handling that event, continue consuming subsequent events while preserving the
existing inFlight accounting and logger behavior.
| /** | ||
| * This file is part of friendsofhyperf/components. | ||
| * | ||
| * @link https://github.com/friendsofhyperf/components | ||
| * @document https://github.com/friendsofhyperf/components/blob/main/README.md | ||
| * @contact huangdijia@gmail.com | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
在文件头中加入许可证信息。
新 PHP 文件的文件头未声明许可证。加入项目适用的许可证标识。
As per coding guidelines: "File headers must include license information."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Sentry/CoHttpTransportTest.php` around lines 4 - 10, 在
CoHttpTransportTest.php 的文件头注释中补充项目适用的许可证标识，保留现有项目链接、文档和联系信息不变，并遵循仓库其他 PHP
文件的许可证声明格式。
Source: Coding guidelines
| /** | ||
| * @internal | ||
| */ | ||
| final class CoHttpTransportTest extends TestCase | ||
| { | ||
| public function testTransportBoundsQueuedEventsAndUsesFixedWorkers(): void | ||
| { | ||
| $config = new Config([ | ||
| 'sentry' => [ | ||
| 'transport_channel_size' => 2, | ||
| 'transport_concurrent_limit' => 1, | ||
| 'transport_timeout' => 0, | ||
| ], | ||
| ]); | ||
| $container = new SentryTransportContainer($config); | ||
|
|
||
| $slowTransport = new SlowTransport(); | ||
| $transport = new TestCoHttpTransport($container, $slowTransport); | ||
|
|
||
| try { | ||
| self::assertSame('SUCCESS', (string) $transport->send(Event::createEvent())->getStatus()); | ||
| \Swoole\Coroutine::sleep(0.005); | ||
|
|
||
| self::assertSame(1, $transport->getStats()['in_flight']); | ||
| self::assertSame('SUCCESS', (string) $transport->send(Event::createEvent())->getStatus()); | ||
| self::assertSame('SUCCESS', (string) $transport->send(Event::createEvent())->getStatus()); | ||
| self::assertSame('SKIPPED', (string) $transport->send(Event::createEvent())->getStatus()); | ||
| self::assertSame( | ||
| [ | ||
| 'accepted' => 3, | ||
| 'dropped' => 1, | ||
| 'queued' => 2, | ||
| 'in_flight' => 1, | ||
| 'workers' => 1, | ||
| ], | ||
| $transport->getStats() | ||
| ); | ||
|
|
||
| self::assertSame('SUCCESS', (string) $transport->close(1)->getStatus()); | ||
| self::assertSame(3, $slowTransport->completed); | ||
| self::assertSame(0, $transport->getStats()['queued']); | ||
| self::assertSame(0, $transport->getStats()['in_flight']); | ||
| } finally { | ||
| $transport->shutdown(); | ||
| } | ||
| } | ||
|
|
||
| public function testTransportDropsNewEventsAfterShutdown(): void | ||
| { | ||
| $config = new Config(['sentry' => []]); | ||
| $container = new SentryTransportContainer($config); | ||
|
|
||
| $transport = new TestCoHttpTransport($container, new SlowTransport()); | ||
| $transport->shutdown(); | ||
|
|
||
| self::assertSame('SKIPPED', (string) $transport->send(Event::createEvent())->getStatus()); | ||
| self::assertSame(1, $transport->getStats()['dropped']); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
将该测试迁移到 Pest。
第 90 行定义 PHPUnit 风格测试类。测试规范要求使用 Pest、uses() 和组件 ->group()。
将测试改为 Pest 闭包，并配置 uses(TestCase::class)->group('sentry')。
As per coding guidelines: "Test files should be located in tests/ directory mirroring component structure, using Pest testing framework" and "All test classes must use Pest's uses() helper ... and use ->group() modifier to group tests by component."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Sentry/CoHttpTransportTest.php` around lines 87 - 145, 将
CoHttpTransportTest 中的 PHPUnit 测试类和方法改为 Pest 测试闭包，移除 final class、继承 TestCase 及
public 方法结构；在测试文件中配置 uses(TestCase::class)->group('sentry')，并为两个测试闭包应用组件对应的
->group('sentry') 修饰符，同时保留现有测试逻辑与断言。
Source: Coding guidelines
| #[RunTestsInSeparateProcesses] | ||
| #[PreserveGlobalState(false)] | ||
| final class RequestContextLifecycleListenerTest extends TestCase |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
将此测试迁移到 Pest。
此文件定义了 PHPUnit 测试类，因此无法使用必需的 uses() helper 和 ->group() 修饰符。请将测试改为 Pest 测试，并按 Sentry 组件分组。
As per coding guidelines，测试必须使用 Pest 的 uses() helper，并使用 ->group() 按组件分组。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Sentry/RequestContextLifecycleListenerTest.php` around lines 36 - 38, 将
RequestContextLifecycleListenerTest 从 PHPUnit 测试类迁移为 Pest 测试，移除类继承及 PHPUnit
类级属性，使用 uses() 配置测试依赖，并为测试链式添加对应的 Sentry 组件 ->group() 修饰符。
Source: Coding guidelines
Summary
Why
The previous transport could retain tens of thousands of complete Sentry events while also creating a coroutine for every dequeued event. Slow or unavailable Sentry endpoints therefore allowed retained event payloads and pending send coroutines to grow until the application exhausted memory.
HTTP and RPC server callbacks are root Swoole coroutines and do not pass through the existing coroutine aspects. Without an explicit request lifecycle hook, they also fell back to the global Sentry runtime context, allowing request state to be shared and retained across concurrent requests.
Verification
composer test:lintcomposer analyse -- src/sentry tests/Sentry/CoHttpTransportTest.php tests/Sentry/RequestContextLifecycleListenerTest.phpnpm run docs:check(56 pages across 4 locales)git diff --checkA controlled slow-transport benchmark with 3,000 unique 16 KiB events retained 2,899 queued and 100 in-flight events in the previous implementation, adding about 64.4 MiB. With the new defaults it retained at most 512 queued and 32 in-flight events, dropped the excess telemetry, and added about 8 MiB (22 MiB process peak).
Upgrade note
Hyperf does not overwrite published configuration. Applications that still carry the old
65535,1000, and-1values should update them to512,32, and0to use the bounded defaults.Summary by CodeRabbit
新功能
文档