-
-
Notifications
You must be signed in to change notification settings - Fork 27
feat(sentry): add AsyncHttpTransport for non-blocking error reporting #907
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
- Implements asynchronous HTTP transport for Sentry events - Uses Hyperf coroutines and channels for non-blocking operation - Handles graceful shutdown with worker exit coordination - Provides memory-efficient event queuing with 65535 capacity - Maintains compatibility with existing Sentry transport interface
|
Caution Review failedThe pull request is closed. 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此变更将 Sentry 集成从基于 HttpClient 的注入切换为基于 TransportInterface 的注入:更新 DI 配置与 ClientBuilderFactory 的选项与逻辑,新增协程版 CoHttpTransport 实现;同时在配置中新增 transport_* 选项并移除 http_* 选项;标记 HttpClient 与其工厂为弃用。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application
participant C as Container
participant CB as ClientBuilderFactory
participant T as CoHttpTransport
participant Ch as Channel
participant W as Worker Loop (Coroutine)
participant HT as HTTP Transport
note over CB,T: 绑定改为基于 TransportInterface 的注入
App->>C: 获取 Sentry Client
C->>CB: 构建 ClientBuilder (options)
CB-->>C: 若容器有 TransportInterface,注入 options.transport = T
C-->>App: 返回已配置的 Client
App->>T: send(event)
T->>Ch: push(event)
alt 首次发送
T->>W: 启动后台消费循环
W->>HT: 延迟创建 HTTP Transport
end
W->>Ch: pop(event)
par 并发发送(可限流)
W->>HT: send(event)
HT-->>W: result
end
W-->>T: 持续消费直到关闭/退出
rect rgb(245,245,255)
note over W,Ch: Worker 退出监视
App-->>T: 进程退出/Coordinator 触发
W->>Ch: drain & close
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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.17)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.
Pull Request Overview
This PR introduces a new AsyncHttpTransport class for Sentry that provides non-blocking, asynchronous error reporting using Hyperf coroutines and channels. The implementation wraps the standard Sentry HTTP transport and processes events in a separate coroutine to avoid blocking the main application flow.
Key changes:
- Added
AsyncHttpTransportclass with asynchronous event processing using channels - Implemented graceful shutdown handling with worker exit coordination
- Used bounded channel (65535 capacity) to prevent memory overflow
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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 (3)
src/sentry/src/Transport/AsyncHttpTransport.php (3)
86-90: 异常不应终止消费循环;记录日志并继续单条发送失败就
break会丢弃后续所有事件。改为记录错误并继续消费。- } catch (Throwable $e) { - break; + } catch (Throwable $e) { + if ($logger = $this->clientBuilder->getLogger()) { + $logger->error( + 'Failed to send Sentry event in AsyncHttpTransport: ' . $e->getMessage(), + ['exception' => $e, 'event_id' => (string) $event->getId()] + ); + } + // 不中断循环,继续处理后续事件
54-58: send(): 返回 Result 应传 EventId;push 改为非阻塞并处理队列满/已退出情形
- 目前把
Event作为Result第二参数传入,类型不符(应为EventId)。Channel::push()在队列满时会阻塞调用方,违背“非阻塞传输”的目标。应使用超时 0 的非阻塞 push,并在丢弃时记录日志。- $chan = $this->chan; - $chan?->push($event); - - return new Result(ResultStatus::success(), $event); + $chan = $this->chan; + if ($chan === null || $this->workerExited) { + if ($logger = $this->clientBuilder->getLogger()) { + $logger->warning('AsyncHttpTransport not running, dropping event.', [ + 'event_id' => (string) $event->getId(), + ]); + } + } elseif ($chan->push($event, 0) === false) { // 非阻塞;满则立即丢弃 + if ($logger = $this->clientBuilder->getLogger()) { + $logger->warning('AsyncHttpTransport channel full, dropping event.', [ + 'event_id' => (string) $event->getId(), + 'capacity' => $this->channelSize, + ]); + } + } + + return new Result(ResultStatus::success(), $event->getId());
96-98: 修复 WORKER_EXIT 处理中的潜在死循环与未定义函数;去除对非空类型的 nullsafe 调用
while (! $this->chan?->isEmpty())在$this->chan === null时会变为while(true),导致死循环。msleep不存在,应使用协程 sleep。- 静态检查提示对非空类型使用了
?->close();改为显式判空后使用->close()。- $this->chan?->close(); - $this->chan = null; + if ($this->chan !== null) { + $this->chan->close(); + $this->chan = null; + }- while (! $this->chan?->isEmpty()) { - msleep(100); - } - - $this->chan?->close(); - $this->chan = null; + $deadline = microtime(true) + 5.0; // 最长等待 5s,避免无限等待 + while ($this->chan !== null && ! $this->chan->isEmpty()) { + if (microtime(true) >= $deadline) { + break; + } + sleep(0.1); + } + if ($this->chan !== null) { + $this->chan->close(); + $this->chan = null; + }此外,需要引入协程 sleep(变更发生在所选区间之外):
use function Hyperf\Coroutine\sleep;Also applies to: 104-110
🧹 Nitpick comments (2)
src/sentry/src/Transport/AsyncHttpTransport.php (2)
60-63: 实现 close($timeout) 的有限等待以冲刷队列,顺带消除未使用参数告警提供按毫秒超时的可选 flush 行为,兼容非阻塞场景。
- public function close(?int $timeout = null): Result - { - return new Result(ResultStatus::success()); - } + public function close(?int $timeout = null): Result + { + if ($timeout !== null) { + $deadline = microtime(true) + max(0, $timeout) / 1000; + while ($this->chan !== null && ! $this->chan->isEmpty()) { + if (microtime(true) >= $deadline) { + break; + } + sleep(0.05); + } + } + if ($this->chan !== null) { + $this->chan->close(); + $this->chan = null; + } + $this->workerExited = true; + return new Result(ResultStatus::success()); + }
77-99: 一致化协程创建方式(go 或 Coroutine::create)以降低认知成本当前同时使用
go()与Coroutine::create()。建议统一一种(例如都用go())。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Transport/AsyncHttpTransport.php(1 hunks)
🧰 Additional context used
🪛 PHPMD (2.15.0)
src/sentry/src/Transport/AsyncHttpTransport.php
60-60: Avoid unused parameters such as '$timeout'. (undefined)
(UnusedFormalParameter)
🪛 GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
src/sentry/src/Transport/AsyncHttpTransport.php
[failure] 108-108:
Using nullsafe method call on non-nullable type Hyperf\Engine\Channel. Use -> instead.
[failure] 105-105:
Function msleep not found.
🪛 GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
src/sentry/src/Transport/AsyncHttpTransport.php
[failure] 108-108:
Using nullsafe method call on non-nullable type Hyperf\Engine\Channel. Use -> instead.
[failure] 105-105:
Function msleep not found.
🪛 GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
src/sentry/src/Transport/AsyncHttpTransport.php
[failure] 108-108:
Using nullsafe method call on non-nullable type Hyperf\Engine\Channel. Use -> instead.
[failure] 105-105:
Function msleep not found.
⏰ 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). (6)
- 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.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
…ttpTransport instances
… returning result
…d refactor coroutine handling
… in ClientBuilderFactory and update AsyncHttpTransportFactory to use them
…rt initialization to makeTransport method and improve error handling in event sending
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: 3
♻️ Duplicate comments (4)
src/sentry/src/Transport/AsyncHttpTransport.php (4)
38-38: 属性类型错误:Coroutine::create 返回 int,而非 Coroutine 对象。会在赋值时触发 TypeError。改为存协程 ID。
应用此差异:
- protected ?Coroutine $workerWatcher = null; + protected ?int $workerWatcherId = null;并同步更新使用处(见下方行 124)。
124-135: 同步修正 watcher 赋值与轻量化退出等待。
- 与上文类型修改配套,将属性名与赋值一并调整。
- 避免无限等待,增加上限等待时间,防止关闭阶段卡死。
- $this->workerWatcher ??= Coroutine::create(function () { + $this->workerWatcherId ??= Coroutine::create(function () { if (CoordinatorManager::until(Constants::WORKER_EXIT)->yield()) { $this->workerExited = true; - while (! $this->chan?->isEmpty()) { - msleep(100); - } + $deadlineMs = 5000; // 最长等待 5s + $start = microtime(true); + while (! $this->chan?->isEmpty()) { + if ((microtime(true) - $start) * 1000 > $deadlineMs) { + break; + } + msleep(50); + } $this->closeChannel(); } });
62-66: push 可能阻塞主协程,违背“非阻塞”目标;满队列时应丢弃或降级处理。Channel 满时
push()会阻塞直至有空位。建议使用零超时并记录丢弃。- $chan = $this->chan; - $chan?->push($event); - - return new Result(ResultStatus::success(), $event); + $chan = $this->chan; + if ($chan !== null && $chan->push($event, 0) === false) { + $this->clientBuilder->getLogger()?->warning('Sentry async transport queue is full, dropped event.'); + } + return new Result(ResultStatus::success(), $event);
112-115: 单个事件发送异常不应终止消费循环。当前
break会导致后续事件全部丢弃;建议记录后继续处理。- } catch (Throwable $e) { - $this->clientBuilder->getLogger()?->error('Failed to send event to Sentry: ' . $e->getMessage(), ['exception' => $e]); - break; + } catch (Throwable $e) { + $this->clientBuilder->getLogger()?->error('Failed to send event to Sentry: ' . $e->getMessage(), ['exception' => $e]); + continue;
🧹 Nitpick comments (2)
src/sentry/src/Transport/AsyncHttpTransport.php (2)
42-56: 并发任务未在关停时等待完成,可能导致进程退出时仍有在途请求。如使用 Concurrent,建议在 close()/退出路径上尽力等待已派发的任务结束(若 Concurrent 提供 wait/length 能力则调用;否则至少延长队列排空后的短暂等待)。
如果你确认 Hyperf\Coroutine\Concurrent 支持等待接口,我可以补一版精确的 close()/drain 实现。
44-45: 默认并发 1000 和队列 65535 偏激进。在高 QPS/抖动网络下可能放大内存与外连压力;建议默认更保守并由配置覆盖。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/sentry/src/ConfigProvider.php(1 hunks)src/sentry/src/Factory/ClientBuilderFactory.php(3 hunks)src/sentry/src/Transport/AsyncHttpTransport.php(1 hunks)src/sentry/src/Transport/AsyncHttpTransportFactory.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/sentry/src/Transport/AsyncHttpTransportFactory.php (2)
src/sentry/src/Factory/ClientBuilderFactory.php (1)
__invoke(39-90)src/sentry/src/Transport/AsyncHttpTransport.php (1)
AsyncHttpTransport(30-136)
src/sentry/src/ConfigProvider.php (1)
src/sentry/src/Transport/AsyncHttpTransportFactory.php (1)
AsyncHttpTransportFactory(16-28)
🪛 PHPMD (2.15.0)
src/sentry/src/Transport/AsyncHttpTransport.php
68-68: Avoid unused parameters such as '$timeout'. (undefined)
(UnusedFormalParameter)
⏰ 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.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 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 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
🔇 Additional comments (3)
src/sentry/src/Factory/ClientBuilderFactory.php (2)
32-36: SPECIFIC_OPTIONS 键重命名为 transport_ 👍*与新异步传输配置对齐,保持 options 干净。
79-84: 注意:这里自动注入 TransportInterface 将与 AsyncHttpTransportFactory 取 ClientBuilder 的行为产生循环。请结合我在 AsyncHttpTransportFactory 的修复(改为就地创建 ClientBuilder),即可安全保留此段注入;否则这里会触发递归构造。
src/sentry/src/ConfigProvider.php (1)
50-52: 已验证:composer.json 中存在 PSR-18 客户端/适配器脚本输出包含 php-http/guzzle7-adapter 与 symfony/http-client,通常可满足 Sentry HttpTransport 的 PSR-18 依赖,移除 HttpClientInterface 绑定不会导致运行期发送失败。
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 (3)
src/sentry/src/Transport/AsyncHttpTransport.php (3)
36-36: 修正属性类型:Coroutine::create 返回 int,当前声明会触发 TypeErrorHyperf\Engine\Coroutine::create(...) 返回协程 ID(int),而非对象。
?Coroutine在赋值时会报类型错误;同时建议重命名以体现其含义(ID)。- protected ?Coroutine $workerWatcher = null; + protected ?int $workerWatcherId = null; @@ - $this->workerWatcher ??= Coroutine::create(function () { + $this->workerWatcherId ??= Coroutine::create(function () {Also applies to: 112-112
50-58: send() 存在两处问题:可能阻塞与错误的成功回执
- 可能阻塞:
Channel::push()默认阻塞,队列满时会卡住调用方,违背“非阻塞”目标。- 错误回执:未检查 push 结果;队列已满或已关闭时返回 false,但仍回传 success,导致静默丢失。
建议改为非阻塞 push,并根据结果回执;同时对关闭/退出态短路返回失败:
public function send(Event $event): Result { - $this->loop(); - - $chan = $this->chan; - $chan?->push($event); - - return new Result(ResultStatus::success(), $event); + // 已关闭/退出时直接失败 + if ($this->workerExited) { + return new Result(ResultStatus::failed(), $event); + } + + $this->loop(); + + $chan = $this->chan; + if ($chan === null) { + return new Result(ResultStatus::failed(), $event); + } + + // 非阻塞入队;队列满则丢弃并记录 + if ($chan->push($event, 0) === false) { + $this->clientBuilder->getLogger()?->warning('AsyncHttpTransport channel full, dropping Sentry event.', [ + 'event_id' => $event->getId() ?? null, + ]); + return new Result(ResultStatus::failed(), $event); + } + + return new Result(ResultStatus::success(), $event); }
60-63: close() 未 flush 且忽略 $timeout当前实现直接返回 success,可能丢失队列中的事件;同时静态检查提示未使用参数。
- public function close(?int $timeout = null): Result - { - return new Result(ResultStatus::success()); - } + public function close(?int $timeout = null): Result + { + // 标记退出,停止新入队 + $this->workerExited = true; + + // 以毫秒为单位的超时(与 msleep 对齐) + $deadline = $timeout !== null ? microtime(true) + ($timeout / 1000) : null; + + // 等待队列排空(尊重超时) + while ($this->chan && ! $this->chan->isEmpty()) { + if ($deadline !== null && microtime(true) >= $deadline) { + break; + } + msleep(50); + } + + // 等待在途任务(若可用);注意:Concurrent::wait() 无超时。 + if ($this->concurrent !== null && $deadline === null) { + try { + $this->concurrent->wait(); + } catch (Throwable $_) { + // 忽略等待失败,继续关闭 + } + } + + $this->closeChannel(); + return new Result(ResultStatus::success()); + }
🧹 Nitpick comments (3)
src/sentry/src/Transport/AsyncHttpTransport.php (3)
82-106: 异常处理应落在发送任务内部;避免因调度异常中断消费循环当前 try/catch 仅覆盖“调度”过程(create),无法捕获发送协程中的异常;并且 catch 后
break会提前跳出当前内层循环。建议:
- 在发送任务内部 try/catch 并记录日志;
- 调度失败仅记录并继续处理下一个事件,避免中断消费。
[ duplicate_comment ]- try { - $callable = static fn () => $transport->send($event); - if ($this->concurrent !== null) { - $this->concurrent->create($callable); - } else { - Coroutine::create($callable); - } - } catch (Throwable $e) { - $logger?->error('Failed to send event to Sentry: ' . $e->getMessage(), ['exception' => $e]); - $transport->close(); - - break; - } finally { + // 在任务内部捕获发送异常,避免未捕获异常导致协程报错 + $callable = function () use ($transport, $logger, $event): void { + try { + $transport->send($event); + } catch (Throwable $e) { + $logger?->error('Failed to send Sentry event in AsyncHttpTransport: ' . $e->getMessage(), ['exception' => $e]); + } + }; + try { + if ($this->concurrent !== null) { + $this->concurrent->create($callable); + } else { + Coroutine::create($callable); + } + } catch (Throwable $e) { + $logger?->error('Failed to schedule Sentry send task: ' . $e->getMessage(), ['exception' => $e]); + // 不中断,继续处理下一条 + continue; + } finally { // Prevent memory leak $event = null; }
75-110: 资源与生命周期:发送中的在途任务未在退出时等待完成即使队列已空,已调度的在途协程仍可能未完成,close() 不等待会导致尾部事件丢失。建议在 close() 中(或监听退出后)尽可能等待
Concurrent->wait();若需超时等待,请标注限制或记录告警。如需确认 Hyperf 版本中
Concurrent::wait()的可用性/行为,请告知版本;我可以提供校验脚本或查阅链接。
125-133: 小优化:复用 PayloadSerializer 避免重复构造在长生命周期下每次重建 HttpTransport 都会 new 一个 PayloadSerializer。可以将其提升为懒加载的成员以降低分配。
可选实现思路:
- 新增
protected ?PayloadSerializer $serializer = null;- 在
makeTransport()中复用$this->serializer ??= new PayloadSerializer($this->clientBuilder->getOptions());
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Transport/AsyncHttpTransport.php(1 hunks)
🧰 Additional context used
🪛 PHPMD (2.15.0)
src/sentry/src/Transport/AsyncHttpTransport.php
60-60: Avoid unused parameters such as '$timeout'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (1)
src/sentry/src/Transport/AsyncHttpTransport.php (1)
112-121: 合并退出时的排空逻辑,避免重复实现与忙等worker 退出回调里用
while (!isEmpty()) msleep(100)忙等。建议直接复用close($timeout)的 flush 逻辑,避免两套代码漂移;也便于统一超时策略(例如 5s)。
[ suggest_optional_refactor ]
[ duplicate_comment ]- $this->workerWatcher ??= Coroutine::create(function () { - if (CoordinatorManager::until(Constants::WORKER_EXIT)->yield()) { - $this->workerExited = true; - - while (! $this->chan?->isEmpty()) { - msleep(100); - } - - $this->closeChannel(); - } - }); + $this->workerWatcherId ??= Coroutine::create(function () { + if (CoordinatorManager::until(Constants::WORKER_EXIT)->yield()) { + // 统一走 close() 的 flush 逻辑,避免忙等与重复实现 + $this->close(5000); + } + });
…stantiation in AsyncHttpTransport and update constructor to use container for configuration
… retrieval in AsyncHttpTransport
…tpClient and HttpClientFactory as deprecated
Eliminated the import and commented-out code related to Sentry\HttpClient\HttpClientInterface in ClientBuilderFactory, as it is no longer used.
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: 0
♻️ Duplicate comments (3)
src/sentry/src/Transport/AsyncHttpTransport.php (3)
36-36: 类型错误:Coroutine::create 返回 int,属性声明为 ?Coroutine 将触发 TypeError将属性改为保存协程 ID(int),并同步更新赋值处。
-protected ?Coroutine $workerWatcher = null; +protected ?int $workerWatcherId = null; @@ - $this->workerWatcher ??= Coroutine::create(function () { + $this->workerWatcherId ??= Coroutine::create(function () {Also applies to: 120-120
58-66: push 可能阻塞调用方,违背“非阻塞”目标;同时未处理 push 失败应使用非阻塞 push(超时 0),失败时返回 skipped 并可记录日志。
public function send(Event $event): Result { $this->loop(); - $chan = $this->chan; - $chan?->push($event); + // 通道可能未初始化或已关闭/已退出 + if ($this->workerExited || $this->chan === null) { + return new Result(ResultStatus::skipped(), $event); + } + $chan = $this->chan; + $ok = $chan->push($event, 0); // 非阻塞 + if ($ok === false) { + // 可选:$this->clientBuilder?->getLogger()?->warning('Sentry channel full, event dropped'); + return new Result(ResultStatus::skipped(), $event); + } return new Result(ResultStatus::success(), $event); }
68-71: close 未 flush 队列且未使用 $timeout实现有限等待并排空后关闭,避免丢事件,同时消除静态检查告警。
- public function close(?int $timeout = null): Result - { - return new Result(ResultStatus::success()); - } + public function close(?int $timeout = null): Result + { + $this->workerExited = true; + $deadline = $timeout !== null ? (microtime(true) + $timeout) : null; // $timeout 视为秒 + while ($this->chan && ! $this->chan->isEmpty()) { + if ($deadline !== null && microtime(true) >= $deadline) { + break; + } + msleep(50); + } + $this->closeChannel(); + return new Result(ResultStatus::success()); + }
🧹 Nitpick comments (8)
src/sentry/src/HttpClient/HttpClientFactory.php (1)
29-31: 配置键仍为旧命名且含拼写错误(http_chanel_*)尽管本类已弃用,仍建议在 AsyncHttpTransport 中兼容读取这些旧键,保障平滑迁移。
src/sentry/src/Transport/AsyncHttpTransport.php (7)
47-56: 向后兼容配置键:同时兼容旧的 http_ 键(含拼写错误 chanel)*避免一次性破坏性升级;优先读取新键,缺失时回退旧键。
- $config = $this->container->get('config'); - $channelSize = (int) $config->get('sentry.transport_channel_size', 65535); + $config = $this->container->get('config'); + $channelSize = (int) ($config->get('sentry.transport_channel_size') + ?? $config->get('sentry.http_chanel_size') /* BC */ + ?? 65535); if ($channelSize > 0) { $this->channelSize = $channelSize; } - $concurrentLimit = (int) $config->get('sentry.transport_concurrent_limit', 1000); + $concurrentLimit = (int) ($config->get('sentry.transport_concurrent_limit') + ?? $config->get('sentry.http_concurrent_limit') /* BC */ + ?? 1000); if ($concurrentLimit > 0) { $this->concurrent = new Concurrent($concurrentLimit); }
94-99: 正常退出时未关闭底层 HttpTransport,可能遗漏尾部 flush在检测到通道关闭(pop 返回 false/null)时显式 close 底层传输。
- if (! $event) { - break 2; - } + if (! $event) { + $transport->close(); + break 2; + }
98-105: 异步任务内异常未捕获,可能在子协程中无声失败将发送逻辑包裹 try/catch 并记录日志,避免丢失异常上下文。
- $callable = static fn () => $transport->send($event); + $logger = $this->clientBuilder?->getLogger(); + $callable = static function () use ($transport, $event, $logger) { + try { + $transport->send($event); + } catch (Throwable $e) { + $logger?->error('Async send failed: ' . $e->getMessage(), ['exception' => $e]); + } + };
83-86: loop 初始化存在竞态,可能重复创建通道/worker并发调用 send()->loop() 时,两个协程可能几乎同时通过空检查。可用 Channel 作为一次性“锁”或使用原子开关。
- $this->chan = new Channel($this->channelSize); + if ($this->chan !== null) { + return; + } + $this->chan = new Channel($this->channelSize);(或引入一个 atomic flag,通过 swap 判定首次初始化)
120-129: 优雅退出等待为无限期轮询,建议设置上限或复用 close 的超时避免极端情况下卡死回收。
- if (CoordinatorManager::until(Constants::WORKER_EXIT)->yield()) { + if (CoordinatorManager::until(Constants::WORKER_EXIT)->yield()) { $this->workerExited = true; - - while (! $this->chan?->isEmpty()) { - msleep(100); - } - - $this->closeChannel(); + // 复用有限等待逻辑 + $deadline = microtime(true) + 5; // 5s 上限,可抽为配置 + while (! $this->chan?->isEmpty() && microtime(true) < $deadline) { + msleep(100); + } + $this->closeChannel(); }
133-143: 避免重复构造 HttpTransport,降低开销可缓存单例并在 close()/退出时统一关闭。
- protected function makeHttpTransport(): TransportInterface + protected function makeHttpTransport(): TransportInterface { - $this->clientBuilder ??= $this->container->get(ClientBuilder::class); - - return new \Sentry\Transport\HttpTransport( + $this->clientBuilder ??= $this->container->get(ClientBuilder::class); + static $transport = null; + return $transport ??= new \Sentry\Transport\HttpTransport( $this->clientBuilder->getOptions(), $this->clientBuilder->getHttpClient(), new PayloadSerializer($this->clientBuilder->getOptions()), $this->clientBuilder->getLogger() ); }
58-66: 语义建议:在已退出/通道为 null 时返回 skipped 而非 success成功状态会误导上层 SDK 认为已发送。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/sentry/src/ConfigProvider.php(1 hunks)src/sentry/src/HttpClient/HttpClient.php(1 hunks)src/sentry/src/HttpClient/HttpClientFactory.php(1 hunks)src/sentry/src/Transport/AsyncHttpTransport.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/sentry/src/HttpClient/HttpClient.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/sentry/src/ConfigProvider.php
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/Transport/AsyncHttpTransport.php (1)
src/sentry/src/HttpClient/HttpClient.php (3)
__construct(40-51)loop(65-124)close(126-134)
🪛 PHPMD (2.15.0)
src/sentry/src/Transport/AsyncHttpTransport.php
68-68: Avoid unused parameters such as '$timeout'. (undefined)
(UnusedFormalParameter)
⏰ 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.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.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 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 5.1.7
🔇 Additional comments (1)
src/sentry/src/HttpClient/HttpClientFactory.php (1)
18-20: 弃用标注妥当 — 在 CHANGELOG/升级指南中声明替代方案并确认无遗留引用
在变更日志或升级指南中明确写明“改用 TransportInterface / AsyncHttpTransport”;确认仓库中已移除对 HttpClientFactory 或旧的 HttpClientInterface 的所有引用。自动化搜索未成功(rg 未检索到文件),需人工验证仓库是否仍有引用。
…and update transport interface mapping
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/publish/sentry.php(1 hunks)src/sentry/src/Factory/ClientBuilderFactory.php(3 hunks)
⏰ 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). (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 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
🔇 Additional comments (3)
src/sentry/src/Factory/ClientBuilderFactory.php (2)
18-18: 引入 TransportInterface 与 DI 绑定一致,OK
31-33: 确认已移除旧 http_ 配置键(需在仓库内验证)*文件: src/sentry/src/Factory/ClientBuilderFactory.php 行: 31–33 — SPECIFIC_OPTIONS 新增 transport_* 键,方向正确;同时确认已移除旧 http_* 键 (http_channel_size / http_chanel_size / http_concurrent_limit)。沙盒自动化搜索失败(rg 报 "No files were searched"),无法完成验证。
在本地仓库运行以下命令并回复输出:
# 推荐:ripgrep(支持多种拼写) rg -n -S -C2 -e 'http_channel_size' -e 'http_chanel_size' -e 'http_concurrent_limit' || true # 备选:git grep(更不依赖外部工具的忽略规则) git grep -n -e 'http_channel_size' -e 'http_chanel_size' -e 'http_concurrent_limit' || true期望:无匹配。
src/sentry/publish/sentry.php (1)
131-133: 降低 Sentry 传输队列与并发默认值并对负值做约束(需手动/重新验证)65535 的 channel 与 1000 的并发在常见部署下会带来内存与连接/限流压力,建议降低默认值并强制下限约束。
位置:src/sentry/publish/sentry.php(约第131–133行)建议修改(含注释):
- 'transport_channel_size' => (int) env('SENTRY_TRANSPORT_CHANNEL_SIZE', 65535), - 'transport_concurrent_limit' => (int) env('SENTRY_TRANSPORT_CONCURRENT_LIMIT', 1000), + // 最大待发送事件队列;建议按负载与内存调优 + 'transport_channel_size' => max(0, (int) env('SENTRY_TRANSPORT_CHANNEL_SIZE', 8192)), + // 最大并发发送数;建议从 64~256 起步 + 'transport_concurrent_limit' => max(1, (int) env('SENTRY_TRANSPORT_CONCURRENT_LIMIT', 128)),验证(在仓库根目录运行,排除 vendor/node_modules;如无输出,贴回结果以便进一步核查):
#!/bin/bash rg -nC2 --hidden --glob '!vendor/**' --glob '!node_modules/**' -e 'transport_channel_size|transport_concurrent_limit|SENTRY_TRANSPORT_CHANNEL_SIZE|SENTRY_TRANSPORT_CONCURRENT_LIMIT|http_chanel_size|http_concurrent_limit' || git grep -n -e 'transport_channel_size\|transport_concurrent_limit\|SENTRY_TRANSPORT_CHANNEL_SIZE\|SENTRY_TRANSPORT_CONCURRENT_LIMIT\|http_chanel_size\|http_concurrent_limit'
…sed container dependency
…move unused container dependency" This reverts commit b1d1492.
…#907) * feat(sentry): add AsyncHttpTransport for non-blocking error reporting - Implements asynchronous HTTP transport for Sentry events - Uses Hyperf coroutines and channels for non-blocking operation - Handles graceful shutdown with worker exit coordination - Provides memory-efficient event queuing with 65535 capacity - Maintains compatibility with existing Sentry transport interface * fix(sentry): prevent memory leak by nullifying event after sending * feat(transport): make channel size configurable in AsyncHttpTransport constructor * feat(sentry): implement AsyncHttpTransportFactory for creating AsyncHttpTransport instances * fix(transport): refactor close method to avoid code duplication * fix(transport): improve close method to prevent unnecessary null assignment * fix(transport): refactor close method to call closeChannel instead of returning result * fix(transport): throw exception in loop method when worker has exited * fix(sentry): log error when sending event to Sentry fails * fix(transport): add concurrent limit options to AsyncHttpTransport and refactor coroutine handling * fix(sentry): uncomment http channel size and concurrent limit options in ClientBuilderFactory and update AsyncHttpTransportFactory to use them * fix(transport): add nullsafe operator to channel pop method in AsyncHttpTransport * refactor(transport): restructure AsyncHttpTransport by moving transport initialization to makeTransport method and improve error handling in event sending * refactor(transport): replace AsyncHttpTransportFactory with direct instantiation in AsyncHttpTransport and update constructor to use container for configuration * fix(transport): handle closed transport gracefully and improve logger retrieval in AsyncHttpTransport * refactor(transport): rename transport methods for clarity and mark HttpClient and HttpClientFactory as deprecated * feat(sentry): add transport channel size and concurrent limit configuration options * Remove unused HttpClientInterface references Eliminated the import and commented-out code related to Sentry\HttpClient\HttpClientInterface in ClientBuilderFactory, as it is no longer used. * refactor(transport): replace AsyncHttpTransport with CoHttpTransport and update transport interface mapping * refactor(factory): mark http_channel_size and http_concurrent_limit as deprecated * refactor(factory): add comments to clarify transport selection logic * refactor(transport): simplify transport initialization and remove unused container dependency * Revert "refactor(transport): simplify transport initialization and remove unused container dependency" This reverts commit b1d1492. --------- Co-Authored-By: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Summary
AsyncHttpTransportclass that provides asynchronous, non-blocking HTTP transport for Sentry eventsKey Features
TransportInterfaceTechnical Implementation
Channelfor thread-safe event queuingCoordinatorManagerfor proper shutdown coordinationHttpTransportfor actual HTTP operationsTest Plan
Summary by CodeRabbit