Feat/sentry coroutine context propagation improvements #919
Conversation
…nd context propagation
|
Warning Rate limit exceeded@huangdijia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
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将大量 Aspect/Listener 的手动 span 生命周期迁移为基于 Scope + SpanContext 的 trace 抽象；新增 SpanStarter::trace 与 startTransaction，协程追踪改为事务子 span 并加入上下文复制守卫；新增 Carrier::fromRequest；升级 sentry/sentry 到 ^4.16.0。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller as 调用者
participant CoroutineAspect as Tracing\\Aspect\\CoroutineAspect
participant ParentHub as 父 Hub/Transaction
participant Engine as Hyperf\\Engine\\Coroutine
participant ChildHub as 子 Hub/Span
Caller->>CoroutineAspect: process(callable)
CoroutineAspect->>ParentHub: 读取需复制的 keys（受保护列表）
CoroutineAspect->>Engine: create(包装 callable，携带 parent coroutine.id + context)
Engine->>ChildHub: 绑定新 Hub，复制上下文（仅当源存在且目标未设置）
ChildHub->>CoroutineAspect: 在 trace 回调内创建 child span (op=coroutine.create)
ChildHub->>CoroutineAspect: 执行包装 callable（trace 回调）
alt 正常完成
CoroutineAspect->>ChildHub: finish child span，恢复父活动 span
CoroutineAspect-->>Caller: 返回结果
else 抛出异常
CoroutineAspect->>ChildHub: 写入异常信息并 finish，恢复父活动 span
CoroutineAspect--x Caller: 异常上抛
end
sequenceDiagram
autonumber
participant Caller as 使用 trace 的调用方
participant SpanStarter as SpanStarter::trace
participant Scope as Sentry\\State\\Scope
participant Parent as 父 Transaction/Span
participant Child as 子 Span
Caller->>SpanStarter: trace(fn, SpanContext)
SpanStarter->>Scope: 推入新 Scope
Scope->>Parent: 读取当前父
alt 父为 Transaction 且未被采样
SpanStarter-->>Caller: 短路返回（不创建 span）
else
SpanStarter->>Child: start child span(SpanContext)
Caller->>SpanStarter: 执行 fn()
alt 抛出异常
SpanStarter->>Child: 标注错误信息/数据
SpanStarter->>Child: finish()
SpanStarter--x Caller: 重新抛出异常
else 正常
SpanStarter->>Child: finish()
SpanStarter-->>Caller: 返回结果
end
end
SpanStarter->>Scope: 弹出/恢复 Scope
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–70 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (2)
71-121: 协程内生命周期与上下文传播实现整体靠谱；建议两点补强
- defer 顺序“先结束 inner span，再切回 transaction 并 finish”合理，能保证层级收尾正确。
- 传播
$keys时“存在且不覆盖当前”策略稳妥。改进建议：
- 与 SpanStarter::trace 复用：当前错误标注与可选堆栈写入逻辑与
SpanStarter::trace()高度相似。若将trace()扩展为支持asParent（或在回调前后setSpan），此处可直接复用，减少重复代码与分歧风险。- 异常 message 上报同样建议加开关（与
SpanStarter::trace()一致），降低隐私风险。参考改造（思路示例，具体以你们 API 为准）：
// 伪代码示例：将 asParent 能力加入 trace() 后复用 $spanContext = SpanContext::make() ->setOp('coroutine.execute.inner') ->setDescription($callingOnFunction) ->setOrigin('auto.coroutine'); $this->trace(function (?Span $span) use ($callable) { $callable(); return null; }, $spanContext/*, asParent: true*/);
26-33: 为同名切面显式设置 priority，保证 AOP 执行顺序仓库中发现拦截 Hyperf\Coroutine\Coroutine::create 的切面且未声明 $priority，执行顺序由扫描/注册决定，可能导致异常捕获与 Tracing 的不稳定行为。受影响文件：
- src/sentry/src/Tracing/Aspect/CoroutineAspect.php (Tracing 切面)
- src/sentry/src/Aspect/CoroutineAspect.php (Sentry 异常/上下文 切面)
- src/telescope/src/Aspect/CoroutineAspect.php (Telescope 切面)
动作要求：
- 为至少 Sentry 的异常/上下文 切面和 Tracing 切面显式添加互异的 $priority 属性并在类注释中说明预期顺序：异常捕获/上下文 在外层，Tracing 在内层（按 Hyperf 的 priority 语义设定具体数值）。
- 如果 telescope 切面会影响同一 joinpoint，亦同步声明 priority 或确认其不参与该顺序。
- 在 PR 描述或类注释中记录所选优先级与理由，避免未来回归。
src/sentry/src/Aspect/CoroutineAspect.php (1)
47-54: 必须固定切面执行顺序（通过 priority）脚本确认同时拦截 Hyperf\Coroutine\Coroutine::create 的切面：src/telescope/src/Aspect/CoroutineAspect.php、src/sentry/src/Aspect/CoroutineAspect.php（captureException + flush）、src/sentry/src/Tracing/Aspect/CoroutineAspect.php（仅标注 Span）。请显式通过 $priority 或配置固定外/内层关系——Tracing 切面应在外层，Sentry 的 capture 切面在内层，确保 captureException 在活跃 Span 内执行，避免重复 capture 或漏标注。
🧹 Nitpick comments (4)
src/sentry/src/Tracing/SpanStarter.php (1)
30-31: 避免命名歧义：移除未使用的use function Sentry\trace;同时本 trait 定义了
trace()方法，再引入同名函数易混淆，且当前未使用该函数导入。-use function Sentry\trace;src/sentry/src/Tracing/Aspect/CoroutineAspect.php (2)
34-37: 上下文传播键收敛为 ServerRequestInterface：方向正确，建议集中管理以避免两个切面分歧两个切面各自维护
$keys，后续容易漂移。建议提取到单一来源（常量/配置/小型服务），便于演进与测试复用。
57-68: 小优化：重复设置 origin，可去除冗余
startSpan(..., origin: 'auto.coroutine')已设置 origin，随后再次?->setOrigin('auto.coroutine')冗余。- $parent = $this->startSpan( + $parent = $this->startSpan( op: 'coroutine.create', description: $callingOnFunction, origin: 'auto.coroutine', - )?->setOrigin('auto.coroutine'); + );src/sentry/src/Aspect/CoroutineAspect.php (1)
27-29: 上下文传播键收敛：与 Tracing 切面保持一致性并集中管理此处仅保留
ServerRequestInterface，与 Tracing 版切面一致是好事。建议将$keys提取为公共常量或配置，避免两个切面后续分歧。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/sentry/src/Aspect/CoroutineAspect.php(1 hunks)src/sentry/src/Tracing/Aspect/CoroutineAspect.php(5 hunks)src/sentry/src/Tracing/SpanStarter.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/sentry/src/Tracing/SpanStarter.php (2)
src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnabled(85-88)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (3)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(72-96)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnabled(85-88)
⏰ 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.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.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 5.1.7
🔇 Additional comments (2)
src/sentry/src/Aspect/CoroutineAspect.php (1)
41-44: LGTM：避免覆盖当前上下文键的保护判断正确
isset($from[$key]) && ! isset($current[$key])能避免误覆盖现有上下文。src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
19-19: 依赖兼容性核查：确认 Hyperf\Engine\Coroutine::id 与 getContextFor 在目标版本可用composer.json 中包含 "hyperf/engine": "^2.0"；仓库中已使用 Hyperf\Engine\Coroutine（位于：src/telescope/src/Aspect/CoroutineAspect.php:17、src/sentry/src/Tracing/Aspect/CoroutineAspect.php:19、src/sentry/src/Aspect/CoroutineAspect.php:17）。
确认该 ^2.0（或目标部署的 engine 实现）确实提供 Coroutine::id 和 getContextFor（并在 Swoole/Swow 运行时可用）；若不支持，改回 Hyperf\Coroutine\Coroutine 或调整 composer 约束以避免兼容性问题。
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
435-438: 建议：避免固定 100ms 硬等待，改为可配置或基于 flush硬编码 100ms 会影响所有被采样的命令执行尾延迟。建议：
- 优先调用 Sentry flush；仅在无法使用 flush 时再 sleep；
- 将等待时间或是否等待做成配置，如 sentry.command_exit_flush_ms（默认 0 或 100）。
示例（示意）：
- // Make sure all spans are flushed before the command exits - Coroutine::sleep(0.1); + // Make sure all spans are flushed before the command exits + $delayMs = (int) ($this->config->get('sentry.command_exit_flush_ms', 0)); + if (\function_exists('\\Sentry\\flush')) { + \Sentry\flush(); + } elseif ($delayMs > 0) { + Coroutine::sleep($delayMs / 1000); + }请确认该逻辑在以下环境表现一致：CLI 常驻进程、短命令、未启用协程的场景。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
src/sentry/src/Tracing/Listener/EventHandleListener.php
[failure] 437-437:
Function msleep not found.
🪛 GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
src/sentry/src/Tracing/Listener/EventHandleListener.php
[failure] 437-437:
Function msleep not found.
🪛 GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
src/sentry/src/Tracing/Listener/EventHandleListener.php
[failure] 437-437:
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). (9)
- GitHub Check: Test on PHP 8.2 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 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 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
…oved aspect handling
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
125-127:coroutine.createspan 结束过早，未覆盖实际创建调用在调用
process()前结束$parent，导致该 span 的持续时间不包含真实创建成本，统计会失真。应将finish()放到process()之后（try/finally）。- $parent->finish(); - - return $proceedingJoinPoint->process(); + try { + return $proceedingJoinPoint->process(); + } finally { + $parent->finish(); + }
🧹 Nitpick comments (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
108-119: 异常状态也同步标记到 transaction（可选）目前仅对 inner span 标记
internalError与异常信息。为便于聚合与筛选，建议同时给$transaction设置相同状态/标签。} catch (Throwable $exception) { - $span?->setStatus(SpanStatus::internalError()) + $span?->setStatus(SpanStatus::internalError()) ->setTags([ 'error' => 'true', 'exception.class' => $exception::class, 'exception.message' => $exception->getMessage(), 'exception.code' => (string) $exception->getCode(), ]); + // 可选：transaction 也标记错误，便于顶层汇总 + $transaction->setStatus(SpanStatus::internalError()) + ->setTags([ + 'error' => 'true', + 'exception.class' => $exception::class, + ]); if ($this->switcher->isTracingExtraTagEnabled('exception.stack_trace')) { $span?->setData([ 'exception.stack_trace' => (string) $exception, ]); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/sentry/src/Aspect/CoroutineAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/CoroutineAspect.php(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/sentry/src/Aspect/CoroutineAspect.php
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (4)
src/sentry/src/Aspect/CoroutineAspect.php (1)
__construct(32-35)src/sentry/src/Switcher.php (3)
__construct(21-23)Switcher(19-137)isTracingExtraTagEnabled(85-88)src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(78-102)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)
⏰ 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 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.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 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
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
🔇 Additional comments (6)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (6)
41-41: 确认优先级升到 PHP_INT_MAX 的动机与影响从
PHP_INT_MAX - 1调到PHP_INT_MAX可能改变与其他切面的执行顺序（例如 HTTP、DB、RPC 相关切面）。请确认该变更是有意而为，并不会引入排序相关的副作用。
19-19: 切换到Hyperf\Engine\Coroutine别名是合理的与 Hyperf 3.x 的实现保持一致。配合上一条对 AOP 目标的补充即可。
59-59: 变量快照 OK在闭包中使用局部
$keys以避免属性后续被修改，做法稳妥。
98-103: 收尾顺序合理先结束 inner span，再切回并结束 transaction，层级与时间线正确。
30-32: 同时拦截 Engine 与 Coroutine 门面，避免漏采样git grep 未检索到
Hyperf\Engine\Coroutine::create的直接调用，但为覆盖所有调用入口建议同时拦截两个门面。
文件：src/sentry/src/Tracing/Aspect/CoroutineAspect.php（附近）public array $classes = [ - 'Hyperf\Coroutine\Coroutine::create', + // 拦截两个门面，覆盖所有调用入口 + 'Hyperf\Coroutine\Coroutine::create', + 'Hyperf\Engine\Coroutine::create', ];
91-97: 确保 inner span 以 transaction 为父级，避免误挂startCoroutineTransaction 在 src/sentry/src/Tracing/SpanStarter.php 中只是委托到 continueTrace()，需确认 continueTrace 是否会把刚创建的 $transaction 设为当前 Hub 的 Span；若不会，创建 transaction 后显式调用 SentrySdk::getCurrentHub()->setSpan($transaction)（幂等）。
)->setData(['coroutine.id' => Co::id()]); + // 确保后续子 span 的父级为该 transaction（若方法内部已设置，此操作幂等） + SentrySdk::getCurrentHub()->setSpan($transaction); $span = $this->startSpan( op: 'coroutine.execute.inner', description: $callingOnFunction, origin: 'auto.coroutine', asParent: true )?->setData(['coroutine.id' => Co::id()]);可用脚本检查 continueTrace 的实现是否会 setSpan($transaction)：
#!/bin/bash rg -nP "function\\s+continueTrace\\b|continueTrace\\(" -n -C5 src
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/sentry/src/Aspect/CoroutineAspect.php (1)
47-50: 用 offsetExists/array_key_exists 保留 null 并兼容 ArrayAccess
isset()会把值为 null 的键视为不存在；协程上下文实现了 ArrayAccess，建议改为显式存在性判断。- foreach ($keys as $key) { - if (isset($from[$key]) && ! isset($current[$key])) { - $current[$key] = $from[$key]; - } - } + foreach ($keys as $key) { + $fromHas = $from instanceof \ArrayAccess + ? $from->offsetExists($key) + : (is_array($from) && array_key_exists($key, (array) $from)); + $curHas = $current instanceof \ArrayAccess + ? $current->offsetExists($key) + : (is_array($current) && array_key_exists($key, (array) $current)); + if ($fromHas && ! $curHas) { + $current[$key] = $from[$key]; + } + }src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
77-81: 同样改为 offsetExists/array_key_exists，避免丢失 null 与提升兼容性与另外一个 Aspect 一致，避免
isset()误判。- foreach ($keys as $key) { - if (isset($from[$key]) && ! isset($current[$key])) { - $current[$key] = $from[$key]; - } - } + foreach ($keys as $key) { + $fromHas = $from instanceof \ArrayAccess + ? $from->offsetExists($key) + : (is_array($from) && array_key_exists($key, (array) $from)); + $curHas = $current instanceof \ArrayAccess + ? $current->offsetExists($key) + : (is_array($current) && array_key_exists($key, (array) $current)); + if ($fromHas && ! $curHas) { + $current[$key] = $from[$key]; + } + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/sentry/src/Aspect/CoroutineAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/CoroutineAspect.php(5 hunks)src/sentry/src/Tracing/SpanStarter.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/sentry/src/Tracing/SpanStarter.php (2)
src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnabled(85-88)
src/sentry/src/Aspect/CoroutineAspect.php (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
__construct(39-42)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (2)
src/sentry/src/Switcher.php (3)
__construct(21-23)Switcher(19-137)isTracingExtraTagEnabled(85-88)src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(78-102)
⏰ 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.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.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.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 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
🔇 Additional comments (7)
src/sentry/src/Aspect/CoroutineAspect.php (1)
32-35: 优先级设置合理将优先级设为
PHP_INT_MAX - 1用于与 Tracing 侧配合排序，认可。src/sentry/src/Tracing/Aspect/CoroutineAspect.php (3)
34-37: 恢复 Hub 与 Request 的协程上下文传播，赞同包含
SentrySdk::class与ServerRequestInterface::class的复制键集合已恢复，避免 trace 断裂。
109-119: 异常 message 作为 tag：建议走开关并截断或降级到 data与先前建议一致，避免无条件上报敏感信息。
- ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.message' => $exception->getMessage(), - 'exception.code' => (string) $exception->getCode(), - ]); + ->setTags([ + 'error' => 'true', + 'exception.class' => $exception::class, + 'exception.code' => (string) $exception->getCode(), + ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $span?->setTags([ + 'exception.message' => substr($exception->getMessage(), 0, 200), + ]); + }
108-114: 修复 nullsafe 链式调用（与 SpanStarter 同类问题）第二行应继续使用 ?->，否则 $span 为 null 时会在调用 setTags 时触发致命错误。
位置：src/sentry/src/Tracing/Aspect/CoroutineAspect.php 行 108–114修复示例：
$span?->setStatus(SpanStatus::internalError()) ?->setTags([ 'error' => 'true', 'exception.class' => $exception::class, 'exception.message' => $exception->getMessage(), 'exception.code' => (string) $exception->getCode(), ]);全仓库扫描（更稳健）：
rg -nUP --type=php '\?\->\s*setStatus\s*\([^)]*\)\s*->\s*setTags\s*\(' -C1src/sentry/src/Tracing/SpanStarter.php (3)
55-66: 异常 message 作为 tag 存在隐私与体量风险，建议加开关并截断与先前讨论一致，应避免无条件写入
exception.message到 tag；可加开关并做长度截断，或降级到 data。- ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.message' => $exception->getMessage(), - 'exception.code' => (string) $exception->getCode(), - ]); + ->setTags([ + 'error' => 'true', + 'exception.class' => $exception::class, + 'exception.code' => (string) $exception->getCode(), + ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $span?->setTags([ + 'exception.message' => substr($exception->getMessage(), 0, 200), + ]); + }
41-76: finally 需恢复原 Hub 以防回调内替换 Hub 导致错配仅恢复 Span 不足以防止回调内更换了当前 Hub 的情况；建议在进入前缓存 Hub，并在 finally 中恢复 Hub 与 Span。
- return SentrySdk::getCurrentHub()->withScope(function (Scope $scope) use ($context, $trace) { + $hub = SentrySdk::getCurrentHub(); + return $hub->withScope(function (Scope $scope) use ($context, $trace, $hub) { $parentSpan = $scope->getSpan(); @@ - } finally { - if (isset($span)) { - $span->finish(); - - $scope->setSpan($parentSpan); - } - } + } finally { + if (isset($span)) { + $span->finish(); + } + // 恢复进入 trace() 前的 Hub/Span + SentrySdk::setCurrentHub($hub); + $hub->setSpan($parentSpan); + } });
88-90: 采样短路逻辑 LGTM — 需手动验证 startSpan 返回 null 的兼容性已用 rg 列出所有 startSpan 调用点，但在 sandbox 自动化校验时遇到 /dev/fd/63 错误，未完成对调用处空值处理的自动判定。在本地或 CI 运行下列命令并检查输出，关注直接链式 startSpan(...)-> 或赋值后对 $var-> 直接访问且未使用 ?->/显式判空的情况：
# 列出所有调用点 rg -n --type=php -C2 '\->startSpan\s*\(' # 直接链式调用（潜在不安全） rg -n --type=php 'startSpan\s*\([^)]*\)\s*->' -S # 赋值形式（找到 $var = ...->startSpan(...)，然后用实际变量名检查后续用法） rg -n --type=php '\$[A-Za-z_][A-Za-z0-9_]*\s*=\s*[^;]*->startSpan' -S # 查看某个调用的上下文（替换 FILE 和 LINE） sed -n '$((LINE-6)),$((LINE+30))p' FILE # 在函数范围内搜索变量是否以 -> 访问（替换 VAR/FILE） rg -n --no-heading '\$VAR\s*->' FILE若发现直接链式或赋值后在同函数内以 -> 使用且未见 ?->/判空，需改为使用 ?-> 或先判空；确认无不安全用法则可接受该变更。
… method for better context management
…ove error handling
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/sentry/src/Tracing/Aspect/FilesystemAspect.php (1)
39-56: 建议：移除本地 catch，交由 SpanStarter::trace 统一打标与收尾此处的异常打标与 SpanStarter::trace() 内逻辑重复，且重复维护易出错。建议直接让异常冒泡，统一由 trace() 处理。
- try { - return $proceedingJoinPoint->process(); - } catch (Throwable $exception) { - ... // 本地异常打标 - throw $exception; - } + return $proceedingJoinPoint->process();src/sentry/src/Tracing/SpanStarter.php (1)
31-31: 清理未使用导入
use function Sentry\trace;未在本文件使用，可移除。-use function Sentry\trace;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/sentry/src/Tracing/Aspect/FilesystemAspect.php(2 hunks)src/sentry/src/Tracing/Listener/EventHandleListener.php(2 hunks)src/sentry/src/Tracing/SpanStarter.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/sentry/src/Tracing/SpanStarter.php (2)
src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnabled(85-88)
src/sentry/src/Tracing/Aspect/FilesystemAspect.php (3)
src/sentry/src/Tracing/SpanStarter.php (1)
trace(41-77)src/sentry/src/Aspect/FilesystemAspect.php (1)
process(48-65)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnabled(85-88)
src/sentry/src/Tracing/Listener/EventHandleListener.php (2)
src/sentry/src/Tracing/SpanStarter.php (1)
trace(41-77)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnabled(85-88)
⏰ 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.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.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 6.0.2
- 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
🔇 Additional comments (5)
src/sentry/src/Tracing/SpanStarter.php (3)
43-77: 恢复原 Hub/Span，避免回调内替换 Hub 导致错配（重复提醒）回调内若调用 continueTrace() 等会更换当前 Hub，仅设置旧 scope 的 span 不足以复位。请缓存并恢复原 Hub 与 Span。
- return SentrySdk::getCurrentHub()->withScope(function (Scope $scope) use ($context, $trace) { + $hub = SentrySdk::getCurrentHub(); + return $hub->withScope(function (Scope $scope) use ($context, $trace, $hub) { $parentSpan = $scope->getSpan(); @@ - } finally { - if (isset($span)) { - $span->finish(); - - $scope->setSpan($parentSpan); - } - } + } finally { + if (isset($span)) { + $span->finish(); + } + // 恢复进入 trace() 前的 Hub/Span + SentrySdk::setCurrentHub($hub); + $hub->setSpan($parentSpan); + } });
89-91: 采样短路 LGTM未采样 Transaction 下不创建子 Span 的守卫合理，能降开销并与采样策略一致。
55-66: 合规：不要无条件上报 exception.message 到 tagexception.message 可能含用户/敏感信息，建议受开关控制或降级到 data。
- $span->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.message' => $exception->getMessage(), - 'exception.code' => (string) $exception->getCode(), - ]); + $span->setStatus(SpanStatus::internalError()) + ->setTags([ + 'error' => 'true', + 'exception.class' => $exception::class, + 'exception.code' => (string) $exception->getCode(), + ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $span->setTags(['exception.message' => $exception->getMessage()]); + }src/sentry/src/Tracing/Listener/EventHandleListener.php (2)
45-46: LGTM：新增导入以支持 trace/Scope无问题。
449-488: 总体 LGTM：Redis 事件改为基于 trace/SpanContext 的封装改造方向正确，数据填充与可观测性字段齐全。
…and tracing context
…ve tracing and error handling
…cing and context management
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php (1)
79-94: 数据收集逻辑正确，但建议添加边界检查。数据收集逻辑合理，包含了消息系统的关键追踪信息。不过
$bodySize的计算可能在某些边界情况下失败。考虑为 packer 操作添加异常处理：
- $bodySize = (fn ($job) => strlen($this->packer->pack($job)))->call($driver, $job); + $bodySize = (function ($job) { + try { + return strlen($this->packer->pack($job)); + } catch (\Throwable $e) { + return 0; + } + })->call($driver, $job);src/sentry/src/Tracing/Aspect/CacheAspect.php (2)
53-61: op 派生映射清晰；可小幅增强可读性/覆盖面当前映射已满足本文件 methods，若后续支持 has/increment/decrement 等方法，可在此集中扩展，或抽为常量表以复用到其他 Aspect。
76-110: 去掉重复异常处理，交由 SpanStarter::trace 统一落标签/状态本闭包内的 try/catch 与 SpanStarter::trace 的 catch 基本重复（同样设置 internalError、异常标签与可选堆栈），可直接移除以避免二次标记和重复维护。
建议补丁：
- try { - return tap($proceedingJoinPoint->process(), function ($result) use ($span, $method) { + return tap($proceedingJoinPoint->process(), function ($result) use ($span, $method) { $data = match ($method) { 'get' => [ 'cache.hit' => ! is_null($result), 'cache.item_size' => strlen((string) json_encode($result)), ], 'fetch' => [ 'cache.hit' => ($result[0] ?? false) !== false, 'cache.item_size' => strlen((string) json_encode($result[1] ?? '')), ], 'getMultiple' => [ 'cache.hit' => ! empty($result), 'cache.item_size' => strlen((string) json_encode(array_values((array) $result))), ], default => [], }; $span?->setData($data); - }); - } catch (Throwable $exception) { - $span?->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.message' => $exception->getMessage(), - 'exception.code' => (string) $exception->getCode(), - ]); - if ($this->switcher->isTracingExtraTagEnabled('exception.stack_trace')) { - $span?->setData([ - 'exception.stack_trace' => (string) $exception, - ]); - } - throw $exception; - } + });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/CacheAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/GrpcAspect.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/sentry/src/Tracing/Aspect/CacheAspect.php (2)
src/sentry/src/Tracing/SpanStarter.php (1)
trace(41-77)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnabled(85-88)
src/sentry/src/Tracing/Aspect/GrpcAspect.php (2)
src/sentry/src/Tracing/SpanStarter.php (1)
trace(41-77)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnabled(85-88)
src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php (3)
src/sentry/src/Util/Carrier.php (4)
get(88-91)Carrier(21-111)fromSpan(52-59)with(61-66)src/sentry/src/Tracing/SpanStarter.php (1)
trace(41-77)src/sentry/src/Constants.php (1)
Constants(14-21)
⏰ 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.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.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 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
🔇 Additional comments (13)
src/sentry/src/Tracing/Aspect/GrpcAspect.php (4)
21-22: 新增导入符合重构需求新增的
Scope和SpanContext导入支持了从手动 span 管理迁移到基于作用域的追踪流程，符合 PR 的整体架构改进目标。
69-95: 追踪流程重构改进了代码质量将原有的手动 span 生命周期管理替换为
trace()方法封装，显著改进了以下方面：
- 简化了 span 生命周期管理 - 不再需要手动创建、完成和清理 span
- 统一了错误处理 - 异常处理逻辑现在集中在
trace()方法中- 提高了代码可维护性 - 减少了重复的 try/catch/finally 样板代码
- 改进了上下文传播 - 通过
Scope参数自动处理 span 上下文实现细节正确：
- 正确使用
SpanContext::make()构建追踪上下文- 适当设置了 gRPC 相关的操作类型、描述和来源标识
- 保留了原有的数据负载传递
90-95: SpanContext 配置完整且恰当SpanContext 的配置涵盖了所有必要的追踪信息：
setOp('grpc.client')- 正确标识了 gRPC 客户端操作setDescription($method)- 使用方法名作为描述，便于追踪分析setOrigin('auto.grpc')- 标识为自动生成的 gRPC 追踪setData($data)- 包含了完整的上下文数据（方法、选项、协程ID）
72-88: 验证 SpanStarter::trace 与 GrpcAspect 的异常处理一致性（需本地核实）
沙箱脚本未能定位 SpanStarter::trace 的实现；GrpcAspect 的 catch（src/sentry/src/Tracing/Aspect/GrpcAspect.php，第 72–88 行）对 span 调用了 setStatus/setTags，并在开启时写入 exception.stack_trace 后重新抛出异常。 在本地确认 SpanStarter::trace 是否也对异常执行相同的 span 修改；若存在重复，应合并或移除重复设置以避免重复上报或语义冲突。
在仓库根目录运行：
rg -nP '\bclass\s+SpanStarter\b' -S || rg -nP '\bfunction\s+trace\s*(' -Ssrc/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php (5)
22-23: 新增导入看起来合适。新增的
Scope和SpanContext导入符合从手动 span 生命周期管理迁移到基于 Scope 的追踪流程的目标。
96-115: 追踪流程重构完成良好。从手动 span 管理成功迁移到基于
trace方法的 Scope 追踪流程。SpanContext 配置完整，包含了所有必要的追踪元数据。Carrier 的创建和上下文存储逻辑也正确实现。
137-150: 序列化处理逻辑正确。正确处理了不同数组结构的 carrier 注入，包括列表数组和关联数组的情况。逻辑覆盖完整。
152-168: 反序列化处理逻辑正确。正确从序列化数据中提取 carrier 信息并恢复到上下文中，与序列化逻辑保持一致。
96-115: 确认：TRACE_CARRIER 在协程上下文中的传播正常。验证结果：Hyperf\Context 为协程隔离上下文（支持按协程 id 读写）；发布端在 trace 回调通过 Context::set 注入 Carrier，__serialize 拦截器将 carrier.toJson() 写入消息，消费端 __unserialize 拦截器从消息恢复并 Context::set，EventHandleListener 在 async_queue 处理时通过 Context::get(..., Coroutine::parentId()) 正确从父协程读取该 carrier。
位置：src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php（handlePush / handleSerialize / handleUnserialize），src/sentry/src/Tracing/Listener/EventHandleListener.php（Context::get(..., Coroutine::parentId())）。
src/sentry/src/Tracing/Aspect/CacheAspect.php (4)
18-21: 采纳 Scope + SpanContext 的引入方向正确统一走 trace(Scope, SpanContext) 流程更安全可维护，imports 使用合理、无冗余。
72-75: 从 Scope 取子 span 的方式正确SpanStarter::trace 在 sampled=TRUE 时会设置子 span 到 Scope；此处 $scope->getSpan() 拿到的即当前子 span，符合预期。
79-87: 确认 fetch 返回结构 — 现有实现正确已核实 Hyperf 的 Cache::fetch 返回顺序为 [bool $success, mixed $value]，因此使用 ($result[0] ?? false) 判命中、$result[1] 计算大小的写法正确，无需修改。
62-70: 无法定位文件 — 需要确认路径/分支以完成验证
- 仓库搜索返回 NOT_FOUND，未找到 src/sentry/src/Tracing/Aspect/CacheAspect.php。
- 原评论结论成立：$arguments['keys'] / $arguments['values'] 可能为 iterable/Traversable，直接传入 implode 会触发 TypeError。建议统一将 iterable 转为数组并将键强制为 string（例如：$toArray = static fn($it)=> is_array($it)? $it : (is_iterable($it)? iterator_to_array($it, false) : []); 然后对各分支使用 (string) 或 array_map('strval', ...)）。
- 提供正确文件路径或把该文件包含到 PR 后我将继续验证。
…nd error handling
…ng and transaction management
… and error handling
…eamline data setting
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/sentry/src/Tracing/Listener/EventHandleListener.php (3)
204-213: SQL 查询 span：建议补充 duration.ms 便于查询与聚合当前仅设置起止时间戳，附带毫秒时长可提升可观测性。
可在 setData 处合并时长（DB 的 $event->time 为毫秒）：
- ->setData($data) + ->setData($data + ['duration.ms' => $event->time])
348-354: 建议同时下发 sentry-trace 与 baggage 响应头当前仅返回 trace id。若要便于前端/下游继续链路，可考虑同时写入 sentry-trace、baggage。
366-369: 同时记录原始 HTTP 状态码，便于检索在设置 SpanStatus 的同时，把数值码落到 span data 中更易查询过滤。
$span->setStatus( SpanStatus::createFromHttpStatusCode($event->response->getStatusCode()) ); + $span->setData(['http.status_code' => $event->response->getStatusCode()]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (3)
src/sentry/src/Tracing/SpanStarter.php (1)
trace(45-86)src/sentry/src/Switcher.php (2)
isTracingSpanEnabled(68-75)isTracingExtraTagEnabled(85-88)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)
⏰ 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.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 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
- 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 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
🔇 Additional comments (10)
src/sentry/src/Tracing/Listener/EventHandleListener.php (10)
45-46: Scope/SpanContext 引入正确，便于与 trace API 对齐这些 use 补全合理，与后续闭包签名和 SpanContext 流程一致。
321-337: 请求阶段子 span + defer 清理：设计合理将 transaction 与处理子 span 分离，并在 defer 中恢复/结束，流程正确。
366-369: HTTP 状态映射到 SpanStatus：✅ 替代了已移除的 setHttpStatus实现符合 sentry-php v4 的用法。
542-551: Crontab 异常 message 需走开关与其它位置保持一致，避免默认记录敏感信息。
- ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => $exception->getMessage()]); + }
222-235: 事务 span 生命周期错误：BEGIN 立即结束，COMMIT/ROLLBACK 错对象BEGIN 使用 trace(fn()=>null) 会立刻 finish 子 span；后续 COMMIT/ROLLBACK 通过 getSpan() 得到的是父级，导致标记/结束错对象。
按连接名缓存事务 span，并在 COMMIT/ROLLBACK 正确取出与结束：
@@ - $this->trace( - fn () => null, - SpanContext::make() - ->setOp('db.transaction') - ->setDescription('BEGIN') - ->setOrigin('auto.db') - ->setStartTimestamp(microtime(true)) - ->setData([ - 'coroutine.id' => Coroutine::id(), - 'db.system' => $event->connection->getDriverName(), - 'db.name' => $event->connection->getDatabaseName(), - 'db.pool.name' => $event->connectionName, - ]) - ); + $parent = SentrySdk::getCurrentHub()->getSpan(); + if (! $parent || ! $parent->getSampled()) { + return; + } + $span = $parent->startChild( + SpanContext::make() + ->setOp('db.transaction') + ->setDescription('BEGIN') + ->setOrigin('auto.db') + ->setStartTimestamp(microtime(true)) + ->setData([ + 'coroutine.id' => Coroutine::id(), + 'db.system' => $event->connection->getDriverName(), + 'db.name' => $event->connection->getDatabaseName(), + 'db.pool.name' => $event->connectionName, + ]) + ); + Context::set(__CLASS__ . ':db.tx:' . $event->connectionName, $span); @@ - if (! $span = SentrySdk::getCurrentHub()->getSpan()) { + $key = __CLASS__ . ':db.tx:' . $event->connectionName; + if (! $span = Context::get($key)) { return; } $span->setStatus(SpanStatus::ok()) ->setDescription('COMMIT') ->finish(); + Context::set($key, null); @@ - if (! $span = SentrySdk::getCurrentHub()->getSpan()) { + $key = __CLASS__ . ':db.tx:' . $event->connectionName; + if (! $span = Context::get($key)) { return; } $span->setStatus(SpanStatus::internalError()) ->setDescription('ROLLBACK') ->finish(); + Context::set($key, null);Also applies to: 243-249, 257-263
371-385: 对异常 message 走开关，避免潜在隐私泄露异常 message 可能包含 PII/敏感信息，建议与 stack_trace 一样受开关控制。
- if ($exception = $event->getThrowable()) { - $span->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]) - ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + if ($exception = $event->getThrowable()) { + $span->setStatus(SpanStatus::internalError()) + ->setTags([ + 'error' => 'true', + 'exception.class' => $exception::class, + 'exception.code' => (string) $exception->getCode(), + ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $span->setData(['exception.message' => $exception->getMessage()]); + } if ($this->switcher->isTracingExtraTagEnabled('exception.stack_trace')) { $span->setData([ 'exception.stack_trace' => (string) $exception, ]); } }
433-436: 命令异常 message 也应受开关控制同上，避免无意记录敏感信息。
- ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => $exception->getMessage()]); + }
628-631: AMQP 异常 message 需走开关保持一致的隐私控制。
- ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => $exception->getMessage()]); + }
702-705: Kafka 异常 message 需走开关同上。
- ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => $exception->getMessage()]); + }
763-766: AsyncQueue 异常 message 需走开关同上。
- ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => $exception->getMessage()]); + }
…ter extensibility
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/sentry/src/Tracing/Listener/EventHandleListener.php (4)
348-369: 请求处理完成的 span 状态更新将
setHttpStatus()改为SpanStatus::createFromHttpStatusCode()，这符合 Sentry PHP SDK v4+ 的 API 变化。注释掉的旧代码应该被移除。建议移除被注释的代码：
- // $span->setHttpStatus($event->response->getStatusCode());
372-386: 异常处理的 span 数据设置异常信息直接添加到 span 数据中，包括可选的堆栈跟踪。这里应该考虑对敏感信息加开关控制。
建议为
exception.message添加开关控制，防止敏感信息泄露：- ]) - ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $span->setData(['exception.message' => $exception->getMessage()]); + }
433-436: 命令异常处理缺少开关控制与其他地方类似，命令执行中的异常消息也应该添加开关控制。
- ]) - ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => $exception->getMessage()]); + }
548-550: 各事件处理中的异常消息开关控制在 crontab、AMQP、Kafka 和 AsyncQueue 事件处理中，异常消息都直接添加到数据中，应该统一添加开关控制。
为保持一致性，建议为所有异常消息添加开关控制：
- ]) - ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => $exception->getMessage()]); + }Also applies to: 628-630, 702-704, 763-765
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php(23 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (3)
src/sentry/src/Tracing/SpanStarter.php (1)
trace(45-86)src/sentry/src/Switcher.php (2)
isTracingSpanEnabled(68-75)isTracingExtraTagEnabled(85-88)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)
🪛 PHPMD (2.15.0)
src/sentry/src/Tracing/Listener/EventHandleListener.php
238-238: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
252-252: Avoid unused parameters such as '$event'. (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). (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 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 (7)
src/sentry/src/Tracing/Listener/EventHandleListener.php (7)
45-46: 新增导入语句以支持 trace 基础架构新增的
Scope和SpanContext导入支持从手动 span 生命周期迁移到基于 trace 的抽象，这是此次重构的核心组件。
166-166: 方法可见性从 private 改为 protected将事件处理方法的可见性从 private 改为 protected 允许继承和覆写，这是合理的扩展设计。
Also applies to: 216-216, 238-238, 252-252, 266-266, 346-346, 389-389, 409-409, 453-453, 509-509, 526-526, 562-562, 593-593, 642-642, 673-673, 716-716, 737-737
204-213: 数据库查询追踪迁移到 trace 模式从手动 span 生命周期管理迁移到
trace()模式，这简化了代码并确保正确的资源清理。fn () => null表明这是副作用追踪而非返回值追踪。
222-235: 修复：数据库事务 BEGIN span 生命周期错误使用
trace(fn()=>null, ...)会在 finally 块中立即 finish 子 span，而后续的 COMMIT/ROLLBACK 通过getSpan()只会获取到父级 span，导致标记/结束错误的对象。这个问题在之前的评审中已经标记过。需要将 BEGIN 改为手动
startChild并缓存到 Context 中，COMMIT/ROLLBACK 从 Context 取出相应的 span 进行操作。
321-332: 请求接收处理的 span 创建和管理新的实现通过
SpanContext创建子 span 并设置到 hub 中，这比之前的手动管理更清晰。span 数据包含了协程 ID 等有用信息。
336-336: 简化 span 结束逻辑移除了冗余的状态检查，直接调用
finish()更简洁。defer 确保资源正确清理。
463-506: Redis 命令追踪的敏感信息和时间单位问题这个问题在之前的评审中已经详细分析过：
db.redis.parameters可能包含敏感信息，需要添加开关控制duration计算错误：$event->time * 1000将毫秒乘以1000变成微秒，应该直接使用$event->timeexception.message也需要开关控制
…se startTransaction and continueTrace for better trace handling
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
639-643: 需要为异常消息添加开关保护在各个事件处理器中，
exception.message仍然直接写入数据，存在敏感信息泄露风险。建议在所有异常处理中都加入开关控制。对所有异常处理应用类似的保护：
搜索结果确认了 `SpanStatus::createFromHttpStatusCode()` 方法在 Sentry PHP SDK v4+ 中是正确的 API。从搜索结果中可以看到在 v4.16.0 版本的文档中明确使用了 `SpanStatus::createFromHttpStatusCode($response->getStatusCode())` 这个方法来设置 span 状态。->setTags([ 'error' => 'true', 'exception.class' => $exception::class, 'exception.code' => (string) $exception->getCode(), ]) - ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ; + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => $exception->getMessage()]); + }Also applies to: 712-716, 773-776
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php(2 hunks)src/sentry/src/Tracing/Listener/EventHandleListener.php(23 hunks)src/sentry/src/Tracing/SpanStarter.php(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/sentry/src/Tracing/SpanStarter.php (4)
src/sentry/src/Switcher.php (2)
Switcher(19-137)isTracingExtraTagEnabled(85-88)src/sentry/src/Util/Carrier.php (6)
Carrier(21-111)has(83-86)get(88-91)fromJson(38-50)getSentryTrace(68-71)getBaggage(73-76)src/sentry/class_map/SentrySdk.php (3)
SentrySdk(24-65)setCurrentHub(61-64)getCurrentHub(51-54)src/sentry/src/Constants.php (1)
Constants(14-21)
src/sentry/src/Tracing/Listener/EventHandleListener.php (3)
src/sentry/src/Tracing/SpanStarter.php (4)
continueTrace(139-168)trace(46-87)startTransaction(170-181)parseSentryTraceAndBaggage(186-209)src/sentry/src/Switcher.php (3)
isTracingSpanEnabled(68-75)isTracingExtraTagEnabled(85-88)isTracingEnabled(51-58)src/sentry/src/Util/Carrier.php (2)
getSentryTrace(68-71)getBaggage(73-76)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (3)
src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/SpanStarter.php (2)
continueTrace(139-168)startTransaction(170-181)src/sentry/src/Switcher.php (3)
__construct(21-23)Switcher(19-137)isTracingExtraTagEnabled(85-88)
🪛 PHPMD (2.15.0)
src/sentry/src/Tracing/Listener/EventHandleListener.php
240-240: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
254-254: Avoid unused parameters such as '$event'. (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). (6)
- 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
- 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 (22)
src/sentry/src/Tracing/SpanStarter.php (5)
48-50: 内联检测 Switcher 并判断追踪开关代码直接检测
$this->switcher属性，但根据过往评审，应该避免使用实例状态的静态缓存。当前这种直接检测方式是合理的，避免了之前评审中指出的跨实例污染问题。
102-104: LGTM：采样短路逻辑正确正确地在未采样的事务下返回 null，避免创建不必要的子 Span，符合采样策略。
123-123: 重构：用新的 trace API 替代 startRequestTransaction已正确地将
startRequestTransaction重构为使用新的parseSentryTraceAndBaggage方法。这种分离关注点的方式更加清晰。
170-181: 新增的 startTransaction 方法实现合理方法正确地创建新的 Hub scope，启动事务并设置为当前 span。实现与现有的
continueTrace方法保持一致性。
186-209: 追踪头解析逻辑完整且支持多种场景解析逻辑支持 sentry-trace、traceparent 头以及 RPC 上下文中的追踪信息，覆盖了多种分布式追踪场景。
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (7)
36-39: 正确包含 SentrySdk 以支持 Hub 传播之前评审指出注释掉
\Sentry\SentrySdk::class会破坏 Hub 的协程上下文传播，现在已正确包含，确保子协程能继承父协程的追踪上下文。
71-78: 协程追踪重构为事务+子 Span 模式将协程追踪重构为基于事务的子 Span 方式，这与新的追踪架构保持一致。正确设置了
op、description和origin等元数据。
89-93: 上下文传播仍使用 isset 检查，可能丢失 null 值如过往评审所指出，
isset()会将null值视为不存在，可能导致本应传播的null值被忽略。虽然在大多数情况下这可能不是问题，但为了保持一致性，建议使用更精确的检查。foreach ($keys as $key) { - if (isset($from[$key]) && ! isset($current[$key])) { + $fromHas = $from instanceof \ArrayAccess ? $from->offsetExists($key) : array_key_exists($key, $from ?? []); + $curHas = $current instanceof \ArrayAccess ? $current->offsetExists($key) : array_key_exists($key, $current ?? []); + if ($fromHas && ! $curHas) { $current[$key] = $from[$key]; } }
95-102: 协程内创建新事务的逻辑正确在协程内部创建新的事务来追踪协程执行，使用
continueTrace继承父 span 的上下文，设置适当的元数据。这种设计保证了完整的分布式追踪链路。
104-107: 使用 defer 确保事务清理正确使用 defer 确保协程事务在协程执行完成后被清理，避免资源泄露。
112-125: 异常处理和元数据设置完整异常处理逻辑完整，正确设置了错误状态和相关标签，并通过开关控制堆栈追踪的记录。
131-136: 正确的资源清理和状态恢复在 finally 块中正确清理父 span 并恢复原始事务状态，确保追踪状态的一致性。
src/sentry/src/Tracing/Listener/EventHandleListener.php (10)
206-215: 数据库查询追踪重构为 trace 方法将数据库查询追踪从手动 span 管理重构为使用
trace方法。逻辑正确，元数据完整，包括了协程信息、数据库信息和连接池状态。
303-310: 请求事务创建使用新的 API正确地从注释掉的
startRequestTransaction迁移到新的startTransactionAPI，使用parseSentryTraceAndBaggage解析追踪头。
332-343: 请求接收 span 创建逻辑正确为请求接收创建子 span，包含协程信息，并正确设置为当前活跃 span。这种细粒度的追踪有助于性能分析。
377-380: HTTP 状态设置使用正确的 API正确地从注释掉的
setHttpStatus迁移到SpanStatus::createFromHttpStatusCode，符合 Sentry SDK 的最新 API。
411-418: 命令追踪使用新的事务 API命令处理正确地使用新的
startTransaction方法创建事务，元数据设置完整。
529-536: 定时任务追踪使用新 API定时任务正确地使用新的
startTransactionAPI，设置了合适的事务源类型TransactionSource::task()。
595-602: 消息队列追踪统一使用新架构AMQP、Kafka 和异步队列的追踪都正确地重构为使用新的
startTransactionAPI，并通过continueTrace继承分布式追踪上下文。处理 carrier 信息的逻辑也很完整。Also applies to: 674-681, 737-744
377-380: HTTP 状态映射使用正确的 API根据 Sentry PHP SDK 文档，
SpanStatus::createFromHttpStatusCode($response->getStatusCode())是 v4+ 版本中设置 HTTP 响应状态的正确方式。代码正确地从已弃用的setHttpStatus迁移到新的 API。
224-237: 事务生命周期管理存在问题BEGIN 使用
trace(fn()=>null, ...)会立即完成子 span，而 COMMIT/ROLLBACK 通过getSpan()获取的是父级 span，导致操作错误的对象。需要手动管理事务 span 的生命周期：
- $this->trace( - fn () => null, - SpanContext::make() - ->setOp('db.transaction') - ->setDescription('BEGIN') - ->setOrigin('auto.db') - ->setStartTimestamp(microtime(true)) - ->setData([ - 'coroutine.id' => Coroutine::id(), - 'db.system' => $event->connection->getDriverName(), - 'db.name' => $event->connection->getDatabaseName(), - 'db.pool.name' => $event->connectionName, - ]) - ); + $parent = SentrySdk::getCurrentHub()->getSpan(); + if (! $parent || ! $parent->getSampled()) { + return; + } + $span = $parent->startChild( + SpanContext::make() + ->setOp('db.transaction') + ->setDescription('BEGIN') + ->setOrigin('auto.db') + ->setStartTimestamp(microtime(true)) + ->setData([ + 'coroutine.id' => Coroutine::id(), + 'db.system' => $event->connection->getDriverName(), + 'db.name' => $event->connection->getDatabaseName(), + 'db.pool.name' => $event->connectionName, + ]) + ); + Context::set(__CLASS__ . ':db.tx:' . $event->connectionName, $span);对应地更新 COMMIT/ROLLBACK 处理逻辑以从 Context 获取正确的事务 span。
475-518: Redis 命令追踪重构完整但存在敏感信息风险Redis 追踪重构为使用
trace方法，逻辑完整。但存在两个问题：
db.redis.parameters可能包含敏感信息，应该加开关控制duration计算错误：$event->time已经是毫秒，乘以 1000 会变成微秒应用此修复：
if ($this->switcher->isTracingExtraTagEnabled('redis.result')) { $span->setData(['db.redis.result' => $event->result]); } + // 参数可能包含敏感信息，加开关控制 if ($exception = $event->throwable) { $span->setStatus(SpanStatus::internalError()) ->setTags([ 'error' => 'true', 'exception.class' => $exception::class, 'exception.code' => (string) $exception->getCode(), ]) - ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ; + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $span->setData(['exception.message' => $exception->getMessage()]); + } if ($this->switcher->isTracingExtraTagEnabled('exception.stack_trace')) { $span->setData(['exception.stack_trace' => (string) $exception]); } } }, SpanContext::make() ->setOp('db.redis') ->setDescription($redisStatement) ->setData([ 'coroutine.id' => Coroutine::id(), 'db.system' => 'redis', 'db.statement' => $redisStatement, 'db.redis.connection' => $event->connectionName, 'db.redis.database_index' => $config['db'] ?? 0, - 'db.redis.parameters' => $event->parameters, 'db.redis.pool.name' => $event->connectionName, 'db.redis.pool.max' => $pool->getOption()->getMaxConnections(), 'db.redis.pool.max_idle_time' => $pool->getOption()->getMaxIdleTime(), 'db.redis.pool.idle' => $pool->getConnectionsInChannel(), 'db.redis.pool.using' => $pool->getCurrentConnections(), - 'duration' => $event->time * 1000, + 'duration' => $event->time, // 已经是毫秒 ]) ->setStartTimestamp(microtime(true) - $event->time / 1000) + + // 仅在开关启用时记录参数 + if ($this->switcher->isTracingExtraTagEnabled('redis.parameters')) { + $span?->setData(['db.redis.parameters' => $event->parameters]); + }
…ility for improved trace handling and refactor transaction methods
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/sentry/src/Tracing/Listener/EventHandleListener.php (2)
240-252: COMMIT 操作应作用于 BEGIN 的子 Span（从 Context 取回后再 finish）- if (! $span = SentrySdk::getCurrentHub()->getSpan()) { + $key = __CLASS__ . ':db.tx:' . $event->connectionName; + if (! $span = Context::get($key)) { return; } $span->setStatus(SpanStatus::ok()) ->setDescription('COMMIT') ->finish(); + Context::set($key, null);
254-266: ROLLBACK 操作同样取回 BEGIN 子 Span 并结束- if (! $span = SentrySdk::getCurrentHub()->getSpan()) { + $key = __CLASS__ . ':db.tx:' . $event->connectionName; + if (! $span = Context::get($key)) { return; } $span->setStatus(SpanStatus::internalError()) ->setDescription('ROLLBACK') ->finish(); + Context::set($key, null);
🧹 Nitpick comments (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
240-240: 移除未使用参数或改名为 $_event 以消除静态分析告警静态分析提示 $event 未使用。可改名为 $_event 或添加注释抑制。
Also applies to: 254-254
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/sentry/src/Tracing/Listener/EventHandleListener.php(23 hunks)src/sentry/src/Tracing/SpanStarter.php(3 hunks)src/sentry/src/Util/Carrier.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/sentry/src/Util/Carrier.php (2)
src/helpers/src/Functions.php (1)
request(423-436)src/http-client/src/Request.php (1)
hasHeader(76-91)
src/sentry/src/Tracing/SpanStarter.php (3)
src/sentry/src/Switcher.php (2)
Switcher(19-137)isTracingExtraTagEnabled(85-88)src/sentry/src/Util/Carrier.php (4)
Carrier(25-143)fromRequest(65-91)getSentryTrace(100-103)getBaggage(105-108)src/sentry/class_map/SentrySdk.php (3)
SentrySdk(24-65)setCurrentHub(61-64)getCurrentHub(51-54)
🪛 PHPMD (2.15.0)
src/sentry/src/Tracing/Listener/EventHandleListener.php
240-240: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
254-254: Avoid unused parameters such as '$event'. (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). (6)
- 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 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
🔇 Additional comments (13)
src/sentry/src/Tracing/SpanStarter.php (3)
112-115: 采样短路逻辑正确当父级为未采样 Transaction 时直接返回 null，能避免无意义的子 Span 创建和开销。
128-136: 从请求继续链路时建议支持 traceparent 回退startRequestTransaction 仅使用 sentry-trace 与 baggage。若只存在 traceparent，将导致无法继续上游链路（或在当前实现下之前误传字符串）。在 Carrier 修正后，建议在此处增加 traceparent 回退逻辑（前提是 SDK 支持）。
我可按最终确认的 SDK 能力提交相应兼容补丁。
[ suggest_recommended_refactor ]
56-97: 异常信息含敏感内容；请对 exception.message 加开关并限制长度当前无条件写入 exception.message 到 span data，存在隐私与体量风险。建议与 stack_trace 一致走开关（如 'exception.message'），并可截断长度。
- if ($span !== null) { - $span->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => 'true', - 'exception.class' => $exception::class, - 'exception.code' => (string) $exception->getCode(), - ]) - ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + if ($span !== null) { + $span->setStatus(SpanStatus::internalError()) + ->setTags([ + 'error' => 'true', + 'exception.class' => $exception::class, + 'exception.code' => (string) $exception->getCode(), + ]); + if ($isTracingExtraTagEnabled /* or new switch key e.g. 'exception.message' */) { + $msg = (string) $exception->getMessage(); + $msg = mb_substr($msg, 0, 1024); // cap length + $span->setData(['exception.message' => $msg]); + }src/sentry/src/Tracing/Listener/EventHandleListener.php (9)
366-370: ✅ 用 SpanStatus 替代 setHttpStatus 的做法正确createFromHttpStatusCode 的映射方式更通用且与 SDK 兼容。
428-437: 命令异常的 exception.message 同样建议走开关- ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ; + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => mb_substr((string) $exception->getMessage(), 0, 1024)]); + }
545-553: Crontab：exception.message 建议走开关- ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ; + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => mb_substr((string) $exception->getMessage(), 0, 1024)]); + }
623-632: AMQP：exception.message 建议走开关- ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ; + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => mb_substr((string) $exception->getMessage(), 0, 1024)]); + }
696-705: Kafka：exception.message 建议走开关- ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ; + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => mb_substr((string) $exception->getMessage(), 0, 1024)]); + }
756-765: AsyncQueue：exception.message 建议走开关- ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ; + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $transaction->setData(['exception.message' => mb_substr((string) $exception->getMessage(), 0, 1024)]); + }
371-386: 对 exception.message 加保护开关（隐私/体量）与其它位置一致，建议对异常 message 走开关，或至少截断长度。
- ]) - ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ]); + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $msg = mb_substr((string) $exception->getMessage(), 0, 1024); + $span->setData(['exception.message' => $msg]); + }
464-507: Redis：duration 单位错误（ms 被乘以 1000），参数与异常 message 建议加开关
- Hyperf Redis 事件 time 为毫秒，此处再乘以 1000 会变成微秒。
- db.redis.parameters 及 exception.message 可能含敏感信息，建议按开关采集。
- if ($this->switcher->isTracingExtraTagEnabled('redis.result')) { + if ($this->switcher->isTracingExtraTagEnabled('redis.result')) { $span->setData(['db.redis.result' => $event->result]); } @@ - if ($exception = $event->throwable) { + if ($exception = $event->throwable) { $span->setStatus(SpanStatus::internalError()) ->setTags([ 'error' => 'true', 'exception.class' => $exception::class, 'exception.code' => (string) $exception->getCode(), ]) - ->setData([ - 'exception.message' => $exception->getMessage(), - ]); + ; + if ($this->switcher->isTracingExtraTagEnabled('exception.message')) { + $span->setData(['exception.message' => mb_substr((string) $exception->getMessage(), 0, 1024)]); + } @@ - ->setData([ + ->setData([ 'coroutine.id' => Coroutine::id(), 'db.system' => 'redis', 'db.statement' => $redisStatement, 'db.redis.connection' => $event->connectionName, 'db.redis.database_index' => $config['db'] ?? 0, - 'db.redis.parameters' => $event->parameters, + // 仅在开启时记录参数 + ...($this->switcher->isTracingExtraTagEnabled('redis.parameters') ? ['db.redis.parameters' => $event->parameters] : []), 'db.redis.pool.name' => $event->connectionName, 'db.redis.pool.max' => $pool->getOption()->getMaxConnections(), 'db.redis.pool.max_idle_time' => $pool->getOption()->getMaxIdleTime(), 'db.redis.pool.idle' => $pool->getConnectionsInChannel(), 'db.redis.pool.using' => $pool->getCurrentConnections(), - 'duration' => $event->time * 1000, + 'duration.ms' => $event->time, ]) ->setStartTimestamp(microtime(true) - $event->time / 1000)
224-237: 事务 BEGIN 子 Span 被立即 finish，后续 COMMIT/ROLLBACK 操作到父级 Span（逻辑错误）使用 trace(fn()=>null, ...) 会在回调结束立刻结束子 Span；随后在 COMMIT/ROLLBACK 通过 getSpan() 拿到的是父级 Span，导致状态和 finish 误作用在父级。
建议：BEGIN 手动 startChild 并缓存到 Context；COMMIT/ROLLBACK 从 Context 取出对应子 Span 再 finish。
- $this->trace( - fn () => null, - SpanContext::make() - ->setOp('db.transaction') - ->setDescription('BEGIN') - ->setOrigin('auto.db') - ->setStartTimestamp(microtime(true)) - ->setData([ - 'coroutine.id' => Coroutine::id(), - 'db.system' => $event->connection->getDriverName(), - 'db.name' => $event->connection->getDatabaseName(), - 'db.pool.name' => $event->connectionName, - ]) - ); + $parent = SentrySdk::getCurrentHub()->getSpan(); + if (! $parent || ($parent instanceof \Sentry\Tracing\Transaction && ! $parent->getSampled())) { + return; + } + $span = $parent->startChild( + SpanContext::make() + ->setOp('db.transaction') + ->setDescription('BEGIN') + ->setOrigin('auto.db') + ->setStartTimestamp(microtime(true)) + ->setData([ + 'coroutine.id' => Coroutine::id(), + 'db.system' => $event->connection->getDriverName(), + 'db.name' => $event->connection->getDatabaseName(), + 'db.pool.name' => $event->connectionName, + ]) + ); + Context::set(__CLASS__ . ':db.tx:' . $event->connectionName, $span);并在 handleDbTransactionCommitted / RolledBack 中取回并 finish（见下两处建议）。
src/sentry/src/Util/Carrier.php (1)
65-91: 不要把 traceparent 当作 sentry-trace 使用；分别保存并在下游显式处理W3C 的 traceparent 与 sentry-trace 格式不同；把 traceparent 直接赋给 $sentryTrace 会导致 continueTrace 解析异常或丢链路，建议按下面 diff 分别保留 sentry-trace / traceparent / baggage 并在 RPC 覆盖时一并覆盖。
- // Get sentry-trace and baggage - $sentryTrace = match (true) { - $request->hasHeader('sentry-trace') => $request->getHeaderLine('sentry-trace'), - $request->hasHeader('traceparent') => $request->getHeaderLine('traceparent'), - default => '', - }; - $baggage = $request->getHeaderLine('baggage'); + // Read headers + $sentryTrace = $request->getHeaderLine('sentry-trace'); + $traceparent = $request->getHeaderLine('traceparent'); + $baggage = $request->getHeaderLine('baggage'); @@ - if ($payload) { - $carrier = Carrier::fromJson($payload); - [$sentryTrace, $baggage] = [$carrier->getSentryTrace(), $carrier->getBaggage()]; - } + if ($payload) { + $carrier = Carrier::fromJson($payload); + $sentryTrace = $carrier->getSentryTrace() ?: $sentryTrace; + $traceparent = $carrier->getTraceparent() ?: $traceparent; + $baggage = $carrier->getBaggage() ?: $baggage; + } @@ - return new static([ - 'sentry-trace' => $sentryTrace, - 'baggage' => $baggage, - ]); + return new static([ + 'sentry-trace' => $sentryTrace, + 'traceparent' => $traceparent, + 'baggage' => $baggage, + ]);根据 sentry-php（4.x）文档，SDK 支持发出 W3C traceparent（propagateTraceparent），但 continueTrace 文档只明确提到提取 sentry-trace 与 baggage，未明确把 traceparent 当作 sentry-trace 解析——按上面修正确保不误传格式。请确认项目使用的 sentry-php 版本是否需要从 traceparent 回退处理；需要我补一处 SpanStarter 的 fallback 实现吗？
Simplifies the condition for setting the 'sentry-trace-id' header by removing the check for the span's sampled status. The header is now set whenever a span and trace ID are present.
…agement in process method
…ce ID handling logic
Simplifies the logic for setting the 'sentry-trace-id' by always casting the trace ID to string and removing redundant conditional checks. This improves code clarity and ensures consistent trace ID handling for both RPC and HTTP responses.
…ct, CacheAspect, FilesystemAspect, GrpcAspect, GuzzleHttpClientAspect, KafkaProducerAspect): add coroutine ID to tracing data
huangdijia
deleted the
feat/sentry-coroutine-context-propagation-improvements
branch
* feat(tracing): enhance span management with improved error handling and context propagation * refactor(span): update trace method parameter type hint for improved type safety * refactor(span): improve trace method to enhance scope handling and error management * fix(listener): ensure all spans are flushed before command exits * refactor(listener): remove unnecessary sleep to ensure spans are flushed before command exits * refactor(aspect): set priority to PHP_INT_MAX in constructor for improved aspect handling * refactor(coroutine-aspect): remove commented SentrySdk class reference for clarity * fix(coroutine-aspect): add missing SentrySdk import for exception handling * fix(aspect): uncomment SentrySdk class reference in keys array for proper integration * fix(span-starter): reorder parameters in withScope closure for consistency * fix(span-starter): set current span in scope for child span tracking * fix(event-handle-listener): refactor Redis span handling to use trace method for better context management * fix(grpc-aspect): refactor tracing logic to use trace method and improve error handling * fix(cache-aspect): refactor process method to enhance error handling and tracing context * fix(async-queue-job-message-aspect): refactor process method to improve tracing and error handling * fix(amqp-producer-aspect): refactor message publishing to enhance tracing and context management * fix(coordinator-aspect): refactor process method to enhance tracing and error handling * fix(coroutine-aspect): refactor span handling to improve error tracking and transaction management * fix(db-aspect): refactor process method to enhance tracing and error handling * fix(db-aspect): remove redundant variable assignment for operation * fix(db-aspect): remove unused SentrySdk check to streamline process method * fix(elasticsearch-aspect): refactor process method to improve tracing and error handling * fix(aspect): refactor tracing logic to improve error handling and streamline data setting * fix(redis-aspect): refactor process method to enhance tracing and error handling * fix(aspect): refactor tracing data structure for Kafka, Redis, and RPC aspects to improve clarity and consistency * fix(aspect): update span context reference in GuzzleHttpClientAspect and refactor RpcAspect for consistency * fix(aspect): refactor TraceAnnotationAspect to improve span handling and streamline tracing logic * fix(aspect): refactor EventHandleListener to enhance span handling and streamline tracing logic * fix(aspect): ensure span is finished correctly in EventHandleListener * fix(aspect): refactor GuzzleHttpClientAspect to improve span handling and trace context injection * fix(aspect): refactor CoroutineAspect to improve span handling and ensure proper transaction management fix(span): mark startSpan method as deprecated and recommend using trace() instead * Add comments to clarify coroutine tracing logic Added explanatory comments in CoroutineAspect to clarify the logic for tracing coroutine creation and context transfer. This improves code readability and maintainability. Co-Authored-By: Deeka Wong <8337659+huangdijia@users.noreply.github.com> * fix(dependencies): update sentry/sentry to version ^4.16.0 * fix(aspect): add null check for span in AmqpProducerAspect to prevent potential errors * fix(aspect): cast Elasticsearch result to string before setting as span data * fix(aspect): cast arguments to string in ElasticsearchAspect for proper logging * fix(aspect): add error handling and cleanup for RPC tracing in RpcAspect * fix(aspect): simplify tracing span check in GuzzleHttpClientAspect * fix(aspect): add exception message to span tags in GuzzleHttpClientAspect * fix(aspect): refactor HTTP status handling in GuzzleHttpClientAspect and EventHandleListener * fix(aspect): inject RPC context data and tracing carrier into span in RpcAspect * fix(aspect): ensure span is valid before setting RPC result data in RpcAspect * fix(aspect): move exception message to span data in CoroutineAspect, GuzzleHttpClientAspect, EventHandleListener, and SpanStarter * fix(aspect): rename transaction variable to coTransaction for clarity in CoroutineAspect * fix(span): add detailed description for trace method in SpanStarter trait * fix(span): enhance error handling and tracing in trace method of SpanStarter trait * fix(span): set start timestamp in trace method of SpanStarter trait if not already set * fix(span): correct condition for enabling tracing extra tag in trace method of SpanStarter trait * fix(span): simplify tracing extra tag condition in trace method of SpanStarter trait * fix(span): ensure span is valid before setting data and status in EventHandleListener * fix(EventHandleListener): change private methods to protected for better extensibility * fix(CoroutineAspect, EventHandleListener, SpanStarter): refactor to use startTransaction and continueTrace for better trace handling * feat(Carrier, EventHandleListener, SpanStarter): implement Carrier utility for improved trace handling and refactor transaction methods * fix(SpanStarter): remove duplicate startTransaction method implementation * Remove sampled check from trace ID header logic Simplifies the condition for setting the 'sentry-trace-id' header by removing the check for the span's sampled status. The header is now set whenever a span and trace ID are present. * fix(EventHandleListener): simplify span validation logic and remove commented code * fix(EventHandleListener): add carrier initialization for improved trace handling * fix(EventHandleListener): streamline span creation logic in process method * fix(EventHandleListener): refactor error handling and transaction management in process method * fix(EventHandleListener): add return type annotation for parseRoute method * fix(EventHandleListener): optimize span data setting and simplify trace ID handling logic * Refactor trace ID handling in EventHandleListener Simplifies the logic for setting the 'sentry-trace-id' by always casting the trace ID to string and removing redundant conditional checks. This improves code clarity and ensures consistent trace ID handling for both RPC and HTTP responses. * fix(EventHandleListener): simplify trace ID handling in process method * fix(EventHandleListener, AmqpProducerAspect, AsyncQueueJobMessageAspect, CacheAspect, FilesystemAspect, GrpcAspect, GuzzleHttpClientAspect, KafkaProducerAspect): add coroutine ID to tracing data --------- Co-Authored-By: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Summary by CodeRabbit
新功能
修复
重构
维护