refactor(sentry): replace CarrierPacker with new Carrier utility class #897
Conversation
- Replace CarrierPacker dependency with new Carrier utility class in AmqpProducerAspect and TracingAmqpListener - Add new Carrier utility class with fluent API for span carrier operations - Deprecate CarrierPacker class (marked for removal in v3.2) - Simplify constructor dependencies by removing CarrierPacker injection - Update carrier packing/unpacking to use new Carrier::fromSpan() and Carrier::fromJson() methods
|
Warning Rate limit exceeded@huangdijia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 30 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 (4)
Walkthrough将追踪载体从已弃用的 CarrierPacker 切换为新引入的 Carrier：生产端以 Carrier::fromSpan(...)->with(...) 构建并以 JSON 字符串写入 headers，消费端用 Carrier::fromJson(...) 恢复并通过访问器读取字段；新增 Carrier 工具类并为 CarrierPacker 添加弃用注释。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Prod as 生产端 Aspect
participant Carrier as Carrier 工具
participant Broker as 消息 Broker
participant Cons as 消费端 Listener
participant Sentry as Sentry SDK
Note over Prod,Carrier: 生产时构建并序列化载体（JSON）
Prod->>Sentry: 读取当前 Span
Prod->>Carrier: Carrier::fromSpan(span).with(meta)
Prod->>Broker: 发布消息（headers[TRACE_CARRIER]=carrier.toJson()）
Note over Broker,Cons: 消息到达并被消费
Cons->>Broker: 读取消息 headers
Cons->>Carrier: Carrier::fromJson(headers[TRACE_CARRIER])
Cons->>Sentry: startTransaction(carrier.getSentryTrace(), carrier.getBaggage())
Cons->>Cons: 处理消息并收集 meta（message_id/body_size/publish_time）
Cons->>Sentry: finishTransaction(使用 carrier.get(...) 填充字段)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/Listener/TracingAmqpListener.php (1)
69-77: Cast 为 string 避免 TypeError，并可选兼容旧的 sentry-trace/baggage
- 强制将
$applicationHeaders[Constants::TRACE_CARRIER]转为(string)后再调用Carrier::fromJson()，并将该字符串写入Context，防止非 string 类型导致的 TypeError。- 可选：若未检测到
TRACE_CARRIER，回退读取旧头sentry-trace/traceparent/baggage，通过Carrier::fromArray()统一构建后再写入Context，确保与现有 SpanStarter 等模块使用的一致性。
🧹 Nitpick comments (7)
src/sentry/src/Util/CarrierPacker.php (1)
17-19: 弃用声明可再完善：补充指向与移除计划说明建议在注释中增加
@see FriendsOfHyperf\Sentry\Util\Carrier，并在 README/UPGRADE 指南同步 v3.2 的移除计划与迁移步骤，避免使用方遗漏。src/sentry/src/Util/Carrier.php (3)
37-42: fromJson 对非法 JSON 静默吞掉数据，确认是否符合预期当前非法 JSON 会解码为 null 并转为空数组，后续 extract() 将返回空字段。若希望及早发现异常载体，可考虑新增 tryFromJson/validate，或埋点日志。若保持静默，请在调用方确保来源可信（本仓自产）。
44-51: 消除魔法字符串：为关键字段建立类常量并复用减少硬编码与打字错误风险，也便于 IDE 重构。
应用以下补丁（节选，多处替换同理）：
class Carrier implements JsonSerializable, Arrayable, Stringable { + public const KEY_SENTRY_TRACE = 'sentry-trace'; + public const KEY_BAGGAGE = 'baggage'; + public const KEY_TRACEPARENT = 'traceparent'; @@ - return new static([ - 'sentry-trace' => $span->toTraceparent(), - 'baggage' => $span->toBaggage(), - 'traceparent' => $span->toW3CTraceparent(), - ]); + return new static([ + self::KEY_SENTRY_TRACE => $span->toTraceparent(), + self::KEY_BAGGAGE => $span->toBaggage(), + self::KEY_TRACEPARENT => $span->toW3CTraceparent(), + ]);并在 withSpan()/extract() 等位置同步替换上述常量。
88-96: toArray 可直接返回底层数组，避免通过 jsonSerialize 间接调用虽无功能差异，但直接返回更直观。
应用以下补丁：
- public function toArray(): array - { - return $this->jsonSerialize(); - } + public function toArray(): array + { + return $this->data; + }src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php (2)
36-38: 构造函数移除 CarrierPacker 依赖可能影响容器装配如外部有显式注入/别名绑定
CarrierPacker到该 Aspect，会在升级后报错。请在升级指南中注明依赖变更，并检查 di 配置是否需要清理。
85-91: 基于 Carrier 构建载体的改造清晰可读字段齐全，加入业务侧辅助信息（publish_time/message_id 等）有助排障。这里使用 float 的 publish_time 可能有精度差异，若后续用于对账/排序，可考虑存为整数毫秒或字符串。
src/sentry/src/Tracing/Listener/TracingAmqpListener.php (1)
99-100: 仅提取所需字段，减少不必要的数据耦合当前拿到完整数组再手动挑字段。用 Carrier::only 显式限制键集合，表达更清晰，也避免无关键渗透到后续逻辑。
-$carrier = Carrier::fromJson((string) Context::get(Constants::TRACE_CARRIER)); -$payload = $carrier->toArray(); +$payload = Carrier::fromJson((string) Context::get(Constants::TRACE_CARRIER)) + ->only(['message_id', 'body_size', 'publish_time', 'destination_name']);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php(4 hunks)src/sentry/src/Tracing/Listener/TracingAmqpListener.php(4 hunks)src/sentry/src/Util/Carrier.php(1 hunks)src/sentry/src/Util/CarrierPacker.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php (2)
src/sentry/src/Util/Carrier.php (4)
Carrier(20-97)__construct(22-25)fromSpan(44-51)with(53-58)src/sentry/src/Constants.php (1)
Constants(14-21)
src/sentry/src/Util/Carrier.php (2)
src/macros/output/Hyperf/Collection/Arr.php (1)
Arr(17-132)src/macros/output/Hyperf/Stringable/Stringable.php (1)
Stringable(14-89)
src/sentry/src/Tracing/Listener/TracingAmqpListener.php (2)
src/sentry/src/Util/Carrier.php (5)
Carrier(20-97)fromJson(37-42)extract(74-81)get(69-72)toArray(93-96)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). (9)
- 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 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 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 (6)
src/sentry/src/Util/Carrier.php (3)
53-58: 不可变语义实现清晰，点赞clone + 合并的写法简洁安全，便于链式调用。
69-72: get 实现符合预期返回默认值的逻辑简单直观，无额外意见。
74-81: extract 字段顺序与兼容性 OK，但请与消费端保持一致顺序与旧 CarrierPacker::unpack 保持一致（sentry-trace/traceparent, baggage, traceparent）。请确认 TracingAmqpListener 的读取顺序与之严格一致，防止回归。
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php (1)
17-17: 替换为新 Carrier 导入正确命名空间与类引用无误。
src/sentry/src/Tracing/Listener/TracingAmqpListener.php (2)
17-17: 替换为 Carrier 的 use 导入 — LGTM与 PR 目标一致，移除对 CarrierPacker 的直接依赖，导入 Carrier 符合方向。
34-36: 无需变更：TracingAmqpListener 移除 CarrierPacker 不影响 DI 绑定
脚本扫描未发现对旧构造签名或 CarrierPacker 注入的硬编码绑定或工厂定义，ConfigProvider 仅通过类名引用 TracingAmqpListener，无需额外修改。
…nd update related methods
… method for AMQP and Kafka listeners
huangdijia
left a comment
huangdijia
left a comment
There was a problem hiding this comment.
代码审查结果
整体评价 ✅
这是一个设计良好的重构PR，Carrier类的API设计比原来的CarrierPacker更清晰直观，成功简化了构造函数依赖。
主要问题需要修复
1. 空安全处理问题 ⚠️ (高优先级)
位置: TracingAmqpListener.php:96-106
当前代码可能导致null调用错误:
$data = [
'messaging.message.id' => $carrier->get('message_id'),
'messaging.message.body.size' => $carrier->get('body_size'),建议修复:
$data = [
'messaging.message.id' => $carrier?->get('message_id'),
'messaging.message.body.size' => $carrier?->get('body_size'),相同问题也存在于 TracingAsyncQueueListener.php 和 TracingKafkaListener.php 中。
2. 类型转换安全 ⚠️ (中优先级)
位置: TracingKafkaListener.php:79-80
建议避免强制转换null值:
sentryTrace: $carrier?->getSentryTrace() ?? '',
baggage: $carrier?->getBaggage() ?? '',3. JSON解析错误处理 ⚠️ (中优先级)
位置: Carrier.php:42
建议添加错误处理:
try {
$data = json_decode($json, true, 512, JSON_THROW_ON_ERROR);
if (!is_array($data)) {
$data = [];
}
} catch (JsonException $e) {
$data = [];
}测试建议
- 验证所有追踪组件的载体数据正确打包/解包
- 测试错误场景下的空值处理
- 确认AMQP、Kafka、异步队列的追踪功能正常
修复空安全问题后建议合并 👍
fix(tracing): use null coalescing for optional carrier properties in TracingAmqpListener and TracingKafkaListener
There was a problem hiding this comment.
Actionable comments posted: 4
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/Listener/TracingAmqpListener.php (1)
81-87: 当未携带 TRACE_CARRIER 时，将 null 传给 string 参数导致 TypeErrorcontinueTrace 的签名为 string 参数；$carrier 为 null 时
$carrier?->get...()返回 null，会触发类型错误。- $this->continueTrace( - sentryTrace: $carrier?->getSentryTrace(), - baggage: $carrier?->getBaggage(), + $this->continueTrace( + sentryTrace: $carrier?->getSentryTrace() ?? '', + baggage: $carrier?->getBaggage() ?? '', name: $message::class, op: 'queue.process', description: $message::class, source: TransactionSource::custom() )->setStartTimestamp(microtime(true));
♻️ Duplicate comments (1)
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php (1)
94-94: 使用$carrier->toJson()写入 header，已规避先前依赖__toString()的容错风险，赞。
🧹 Nitpick comments (17)
src/sentry/src/Tracing/Aspect/RpcAspect.php (1)
101-101: 在 RpcContext 中同时写入 JSON 与原生 header 字段，提升向后兼容性（可选）。部分老组件可能直接读取
sentry-trace/baggage。建议在写入 JSON 的同时补写两个键，便于降级与跨语言互通。- $this->container->get(Rpc\Context::class)->set(Constants::TRACE_CARRIER, Carrier::fromSpan($span)->toJson()); + $rpcCtx = $this->container->get(Rpc\Context::class); + $carrier = Carrier::fromSpan($span); + $rpcCtx->set(Constants::TRACE_CARRIER, $carrier->toJson()); + $rpcCtx->set('sentry-trace', $carrier->getSentryTrace()); + $rpcCtx->set('baggage', $carrier->getBaggage());src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php (1)
85-91: 发布时间建议写入毫秒整数，避免浮点精度与跨语言差异（可选）。保持现有
publish_time不变的同时，新增publish_time_ms（int）。- $carrier = Carrier::fromSpan($span)->with([ - 'publish_time' => microtime(true), + $nowMs = (int) floor(microtime(true) * 1000); + $carrier = Carrier::fromSpan($span)->with([ + 'publish_time' => microtime(true), + 'publish_time_ms' => $nowMs, 'message_id' => $messageId, 'destination_name' => $destinationName, 'body_size' => $bodySize, ]);src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php (4)
29-29:@property PackerInterface $packer标注易引发误解（可选）。该属性实际来自绑定到
Driver的上下文，通过->call($driver, ...)访问。建议在此处补充注释说明“用于闭包绑定到 Driver 时的属性提示”，或将说明移至使用处，避免读者误判为本类成员。
101-107: 与 AMQP 一致，建议补充毫秒时间戳字段（可选）。- $carrier = Carrier::fromSpan($span)->with([ - 'publish_time' => microtime(true), + $nowMs = (int) floor(microtime(true) * 1000); + $carrier = Carrier::fromSpan($span)->with([ + 'publish_time' => microtime(true), + 'publish_time_ms' => $nowMs, 'message_id' => $messageId, 'destination_name' => $destinationName, 'body_size' => $bodySize, ]);
108-108: 在 Context 中存放对象 vs. 字符串的统一性（可选）。为降低类型分歧，建议也在 Context 中存 JSON 字符串（或新增
TRACE_CARRIER_JSON键），保持各链路语义一致。- Context::set(Constants::TRACE_CARRIER, $carrier); + Context::set(Constants::TRACE_CARRIER, $carrier->toJson());
165-167: 反序列化阶段建议增强健壮性（可选）。为兼容潜在的数组/对象载体，增加分支：
- /** @var string|null $carrier */ - if ($carrier) { - Context::set(Constants::TRACE_CARRIER, Carrier::fromJson($carrier)); - } + if ($carrier) { + if (is_string($carrier)) { + Context::set(Constants::TRACE_CARRIER, Carrier::fromJson($carrier)); + } elseif ($carrier instanceof Carrier) { + Context::set(Constants::TRACE_CARRIER, $carrier); + } elseif (is_array($carrier)) { + Context::set(Constants::TRACE_CARRIER, Carrier::fromArray($carrier)); + } + }src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php (2)
68-71: 消息 ID 唯一性与熵：优先使用随机字节
uniqid(..., true)基于时间戳，熵不足。建议改为random_bytes，并在不可用时回退。应用以下修改：
- $messageId = uniqid('kafka_', true); + try { + $messageId = bin2hex(random_bytes(12)); + } catch (\Throwable) { + $messageId = uniqid('kafka_', true); + }
96-123: 补充批量 Span 的汇总指标（可选）
sendBatchAsync仅为每条消息注入 Header，未为批量 Span 记录统计信息。建议为 Span 添加如批量大小与目标主题等字段，便于观测。示例：
$span = $this->startSpan( 'kafka.send_batch', sprintf('%s::%s', $proceedingJoinPoint->className, $proceedingJoinPoint->methodName) ); + $span?->setData([ + 'messaging.system' => 'kafka', + 'messaging.operation' => 'publish_batch', + 'messaging.batch.count' => count($messages), + 'messaging.destination.name' => $messages[0]->getTopic() ?? 'unknown', + ]);src/sentry/src/Tracing/Listener/TracingKafkaListener.php (2)
71-73: 在 Context 中存放 Carrier 对象：建议在消费完成后清理将
Carrier放入Context便于后续访问，但长生命周期进程中可能出现跨消息“泄漏”。建议在finishTransaction末尾清理该键。$transaction->finish(microtime(true)); + // 清理上下文中的载体，避免跨消息污染 + \Hyperf\Context\Context::set(\FriendsOfHyperf\Sentry\Constants::TRACE_CARRIER, null);如有协程/多消费者并发，请评估是否需要按协程维度清理（与设置位置一致）。
97-109: 数据映射细节：对时钟偏差与缺失字段更稳健
- 不同主机时钟可能不一致，
receive.latency可能为负值；建议下限为 0。- 当
destination_name缺失时可回退到实际topic。- 'messaging.message.receive.latency' => $carrier?->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, + 'messaging.message.receive.latency' => $carrier?->has('publish_time') + ? max(0, microtime(true) - (float) $carrier->get('publish_time')) + : null, - 'messaging.destination.name' => $carrier?->get('destination_name'), + 'messaging.destination.name' => $carrier?->get('destination_name') ?: $consumer->getTopic(),src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php (2)
95-100: 数据稳健性：处理时钟偏差与目的地回退与 Kafka 类似，建议：
receive.latency最小为 0；destination.name缺失时提供回退（例如从 Job/队列名推断，若不可得则保持现状）。- 'messaging.message.receive.latency' => $carrier?->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, + 'messaging.message.receive.latency' => $carrier?->has('publish_time') + ? max(0, microtime(true) - (float) $carrier->get('publish_time')) + : null, - 'messaging.destination.name' => $carrier?->get('destination_name'), + 'messaging.destination.name' => $carrier?->get('destination_name'),（如可从消息或队列上下文获取队列名，可在空值时回退填充。）
116-121: 建议在交易结束后清理 Context 载体防止长生命周期/复用协程导致的上下文污染。
$transaction->finish(microtime(true)); + \Hyperf\Context\Context::set(\FriendsOfHyperf\Sentry\Constants::TRACE_CARRIER, null);如需要按协程维度清理，请确认
Context::set是否支持指定协程 ID，或改为在设置位置采用便于对称清理的策略。src/sentry/src/Util/Carrier.php (4)
39-44: fromJson 对非法/非字符串输入的容错不足（会静默变成空数组）建议接受 Stringable 并开启 JSON_THROW_ON_ERROR 与 UTF-8 处理，失败时回退为空数据，避免悄然吞掉错误。
应用示例补丁：
- public static function fromJson(string $json): static + public static function fromJson(Stringable|string $json): static { - $data = (array) json_decode($json, true); + try { + $data = json_decode((string) $json, true, 512, JSON_THROW_ON_ERROR | JSON_INVALID_UTF8_SUBSTITUTE); + } catch (JsonException $e) { + $data = []; + } return new static($data); }
86-89: has 使用 isset 会把 null 视为“不存在”若需要区分“键存在但值为 null”，请改用 array_key_exists。
- return isset($this->data[$key]); + return array_key_exists($key, $this->data);
96-103: 为 extract 增加返回顺序/含义注释以降低误用风险当前返回 [sentry-trace 或 traceparent, baggage, traceparent]，建议加 PHPDoc 说明顺序与字段含义，便于调用方解构。
120-127: toJson 建议优先保留可序列化部分，避免整体丢弃使用 JSON_PARTIAL_OUTPUT_ON_ERROR 和 JSON_INVALID_UTF8_SUBSTITUTE 可最大化保留数据，减少回退到 "{}" 的概率。
- try { - return json_encode($this->toArray(), $options | JSON_THROW_ON_ERROR); - } catch (JsonException $e) { - return '{}'; - } + $json = json_encode($this->toArray(), $options | JSON_PARTIAL_OUTPUT_ON_ERROR | JSON_INVALID_UTF8_SUBSTITUTE); + return is_string($json) ? $json : '{}';src/sentry/src/Tracing/Listener/TracingAmqpListener.php (1)
75-77: AMQP header 类型不确定时 fromJson 可能触发 TypeErrorapplication_headers 的值可能并非严格 string（取决于 AMQP 类型映射）。建议在传入前显式转为字符串或做类型校验。
- $carrier = Carrier::fromJson($applicationHeaders[Constants::TRACE_CARRIER]); + $raw = (string) $applicationHeaders[Constants::TRACE_CARRIER]; + $carrier = Carrier::fromJson($raw); Context::set(Constants::TRACE_CARRIER, $carrier);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php(4 hunks)src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php(6 hunks)src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php(4 hunks)src/sentry/src/Tracing/Aspect/RpcAspect.php(3 hunks)src/sentry/src/Tracing/Listener/TracingAmqpListener.php(4 hunks)src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php(4 hunks)src/sentry/src/Tracing/Listener/TracingKafkaListener.php(4 hunks)src/sentry/src/Tracing/SpanStarter.php(2 hunks)src/sentry/src/Util/Carrier.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php (2)
src/sentry/src/Util/Carrier.php (5)
Carrier(22-128)__construct(24-27)fromSpan(46-53)with(55-60)toJson(120-127)src/sentry/src/Constants.php (1)
Constants(14-21)
src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php (2)
src/sentry/src/Util/Carrier.php (4)
Carrier(22-128)fromSpan(46-53)with(55-60)toJson(120-127)src/sentry/src/Constants.php (1)
Constants(14-21)
src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php (3)
src/sentry/src/Util/Carrier.php (5)
Carrier(22-128)get(91-94)getSentryTrace(71-74)getBaggage(76-79)has(86-89)src/sentry/src/Constants.php (1)
Constants(14-21)src/sentry/src/Tracing/SpanStarter.php (1)
continueTrace(80-109)
src/sentry/src/Util/Carrier.php (1)
src/macros/output/Hyperf/Collection/Arr.php (1)
Arr(17-132)
src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php (2)
src/sentry/src/Util/Carrier.php (5)
Carrier(22-128)fromSpan(46-53)with(55-60)toJson(120-127)fromJson(39-44)src/sentry/src/Constants.php (1)
Constants(14-21)
src/sentry/src/Tracing/Listener/TracingAmqpListener.php (3)
src/sentry/src/Util/Carrier.php (6)
Carrier(22-128)has(86-89)get(91-94)fromJson(39-44)getSentryTrace(71-74)getBaggage(76-79)src/sentry/src/Constants.php (1)
Constants(14-21)src/sentry/src/Tracing/SpanStarter.php (1)
continueTrace(80-109)
src/sentry/src/Tracing/Listener/TracingKafkaListener.php (3)
src/sentry/src/Util/Carrier.php (6)
Carrier(22-128)fromJson(39-44)getSentryTrace(71-74)getBaggage(76-79)get(91-94)has(86-89)src/sentry/src/Constants.php (1)
Constants(14-21)src/sentry/src/Tracing/SpanStarter.php (1)
continueTrace(80-109)
src/sentry/src/Tracing/SpanStarter.php (2)
src/sentry/src/Util/Carrier.php (6)
Carrier(22-128)has(86-89)get(91-94)fromJson(39-44)getSentryTrace(71-74)getBaggage(76-79)src/sentry/src/Constants.php (1)
Constants(14-21)
src/sentry/src/Tracing/Aspect/RpcAspect.php (2)
src/sentry/src/Util/Carrier.php (4)
Carrier(22-128)get(91-94)fromSpan(46-53)toJson(120-127)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). (12)
- 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.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 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.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 (15)
src/sentry/src/Tracing/SpanStarter.php (1)
15-15: 切换到 Carrier：一致性与可读性更好，赞。src/sentry/src/Tracing/Aspect/RpcAspect.php (1)
17-17: 切换到 Carrier：方向正确。src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php (1)
36-38: 构造函数精简：无状态依赖更易测试与维护。src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php (3)
17-17: 切换到 Carrier：一致性 👍
43-44: 构造函数精简：去除多余注入无副作用。
140-140: 序列化阶段用toJson()写入：实现对齐，赞。Also applies to: 143-143
src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php (4)
17-17: 切换到 Carrier 引用看起来正确替换
CarrierPacker为Carrier的 use 语句无问题，命名空间一致。
80-91: Header 载体改为 JSON 字符串：序列化与兼容性 LGTM
Carrier::fromSpan(...)->with(...)->toJson()写入RecordHeader值是合理的，消费端也按 JSON 解析。注意 Kafka Header 为二进制字节，当前为 UTF-8 字符串可直接使用。
40-41: 构造函数依赖精简确认：已移除 CarrierPacker 注入，未在代码库中发现其引用或显式绑定，无需额外调整。
110-119: 确认ProduceMessage是否提供 Header API 或 已初始化$headers属性
请手动核实ProduceMessage类是否包含addHeader()/setHeaders()方法，或在类中已声明并初始化了$headers属性；若无，建议先将其初始化为数组，或优先调用可用的 header API。src/sentry/src/Tracing/Listener/TracingKafkaListener.php (2)
79-86: 继续链路参数来源于 Carrier：实现正确
(string) $carrier?->getSentryTrace()与(string) $carrier?->getBaggage()的传递与空值回退符合预期。
33-35: 构造函数签名变更：请检查监听器注册与注入
未在代码中检索到 TracingKafkaListener 注册或 CarrierPacker 注入的残留，请手动检查事件监听器的 DI/注册配置，确保不再请求已弃用的 CarrierPacker 服务。src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php (2)
66-74: 从 Context 取 Carrier 并继续链路：实现正确按父协程取值并用
Carrier提供的访问器继续链路，符合改造方向。
33-36: 验证通过：无残留 CarrierPacker 注入 已确认代码库中无任何配置或工厂仍绑定旧构造签名。src/sentry/src/Util/Carrier.php (1)
29-33: __toString 实现稳健，可读性好将字符串化委托给 toJson，且 toJson 已做异常兜底，保证总是返回字符串。赞。
代码审查总结✅ 整体评价这是一个优秀的重构PR，将替换为新的工具类。代码结构清晰，API设计良好，向后兼容性处理得当。 🎯 主要优点
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/sentry/src/Tracing/Listener/TracingKafkaListener.php (2)
104-109: 计算延迟时显式转为 float，并为目标名称提供兜底
publish_time可能为字符串或非数值；建议转为 float。另与 AMQP 监听器保持一致，为messaging.destination.name增加主题名兜底，避免空值。- 'messaging.message.receive.latency' => $carrier?->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, + 'messaging.message.receive.latency' => $carrier?->has('publish_time') ? (microtime(true) - (float) $carrier->get('publish_time')) : null, ... - 'messaging.destination.name' => $carrier?->get('destination_name'), + 'messaging.destination.name' => $carrier?->get('destination_name') ?? $consumer->getTopic(),
97-134: 建议在消费完成后清理上下文中的 Carrier，避免长生命周期任务的上下文污染消费者常驻运行时，未命中新头部时可能复用旧 Carrier。完成后显式清空更稳妥。
$transaction->finish(microtime(true)); + + // 清理上下文中的旧 Carrier，避免后续消息误用 + Context::set(Constants::TRACE_CARRIER, null);src/sentry/src/Tracing/Listener/TracingAmqpListener.php (3)
75-77: 读取 AMQP 应用头时更稳健地获取原生数据类型
AMQPTable的值可能带类型包装；建议先取原生数组并校验为字符串再解析，提升兼容性与健壮性。- if ($applicationHeaders && isset($applicationHeaders[Constants::TRACE_CARRIER])) { - $carrier = Carrier::fromJson($applicationHeaders[Constants::TRACE_CARRIER]); - Context::set(Constants::TRACE_CARRIER, $carrier); - } + if ($applicationHeaders) { + $headers = $applicationHeaders instanceof AMQPTable ? $applicationHeaders->getNativeData() : (array) $applicationHeaders; + $payload = $headers[Constants::TRACE_CARRIER] ?? null; + if (is_string($payload) && $payload !== '') { + $carrier = Carrier::fromJson($payload); + Context::set(Constants::TRACE_CARRIER, $carrier); + } + }
107-112: 与 Kafka 版本对齐的兜底逻辑与类型安全
publish_time建议转 float。- 目的地名称兜底已存在，保持不变。
- 'messaging.message.receive.latency' => $carrier?->has('publish_time') ? (microtime(true) - $carrier->get('publish_time')) : null, + 'messaging.message.receive.latency' => $carrier?->has('publish_time') ? (microtime(true) - (float) $carrier->get('publish_time')) : null,
135-140: 完成后清理 Carrier，避免上下文“漏刻”防止下一条消息在未设置新头时误用旧 Carrier。
$transaction->finish(microtime(true)); + + // 清理上下文，避免遗留 Carrier 影响后续消息 + Context::set(Constants::TRACE_CARRIER, null);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/sentry/src/Tracing/Listener/TracingAmqpListener.php(4 hunks)src/sentry/src/Tracing/Listener/TracingKafkaListener.php(4 hunks)src/sentry/src/Util/Carrier.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/sentry/src/Util/Carrier.php
🧰 Additional context used
🧬 Code graph analysis (2)
src/sentry/src/Tracing/Listener/TracingAmqpListener.php (3)
src/sentry/src/Util/Carrier.php (6)
Carrier(22-135)has(93-96)get(98-101)fromJson(39-51)getSentryTrace(78-81)getBaggage(83-86)src/sentry/src/Constants.php (1)
Constants(14-21)src/sentry/src/Tracing/SpanStarter.php (1)
continueTrace(80-109)
src/sentry/src/Tracing/Listener/TracingKafkaListener.php (3)
src/sentry/src/Util/Carrier.php (6)
Carrier(22-135)fromJson(39-51)getSentryTrace(78-81)getBaggage(83-86)get(98-101)has(93-96)src/sentry/src/Constants.php (1)
Constants(14-21)src/sentry/src/Tracing/SpanStarter.php (1)
continueTrace(80-109)
⏰ 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.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 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
🔇 Additional comments (1)
src/sentry/src/Tracing/Listener/TracingKafkaListener.php (1)
79-81: 空值回退处理到位，避免 strict_types 下的类型错误对
$carrier?->getSentryTrace() ?? ''与$carrier?->getBaggage() ?? ''的回退逻辑正确，和 AMQP 版本保持一致更佳。
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
tests/Sentry/CarrierTest.php (4)
103-103: 清理仅含空白的空行与行尾空格上述行包含空白字符的空行或行尾空格，触发
no_whitespace_in_blank_line。请删除空白字符，保留纯空行即可。建议交由php-cs-fixer一键处理。Also applies to: 223-223, 232-232, 242-242, 250-250, 258-258, 267-267, 281-281, 294-294, 302-302, 312-312, 316-316, 326-326, 336-336, 349-349, 353-353, 355-355
51-64: 补充 fromJson 边界用例：'null' 与 '[]'当前覆盖了空字符串与无效 JSON。建议新增：
- json 为
'null'时：非数组，期望[]- json 为
'[]'时：数组，期望[]（保持与实现一致）示例补充：
test('handles non-array JSON gracefully', function () { $carrier = Carrier::fromJson('"string value"'); expect($carrier->toArray())->toBe([]); }); + + test('handles null JSON value gracefully', function () { + $carrier = Carrier::fromJson('null'); + expect($carrier->toArray())->toBe([]); + }); + + test('handles empty array JSON', function () { + $carrier = Carrier::fromJson('[]'); + expect($carrier->toArray())->toBe([]); + });
237-245: 补充 __toString 的异常回退用例实现中
__toString()委托给toJson()，编码异常应返回'{}'。建议添加资源型数据的用例，验证字符串化同样走安全回退：test('toJson handles encoding errors gracefully', function () { // Create data that cannot be JSON encoded $resource = fopen('php://memory', 'r'); $carrier = new Carrier(['resource' => $resource]); fclose($resource); $json = $carrier->toJson(); expect($json)->toBe('{}'); }); + + test('__toString handles encoding errors gracefully', function () { + $resource = fopen('php://memory', 'r'); + $carrier = new Carrier(['resource' => $resource]); + fclose($resource); + expect((string) $carrier)->toBe('{}'); + });
308-319: 测试名称表述更精确该用例实质验证
toArray()的返回副本不会影响内部状态，而非对象的“深拷贝”。建议重命名以避免歧义：- test('deep cloning behavior', function () { + test('toArray does not mutate internal state', function () {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/Sentry/CarrierTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Sentry/CarrierTest.php (1)
src/sentry/src/Util/Carrier.php (7)
Carrier(22-135)fromSpan(53-60)withSpan(69-76)getSentryTrace(78-81)getBaggage(83-86)getTraceparent(88-91)extract(103-110)
🪛 GitHub Actions: tests
tests/Sentry/CarrierTest.php
[error] 1-1: PHP CS Fixer detected formatting issues in tests/Sentry/CarrierTest.php (phpdoc_no_alias_tag, no_unused_imports, header_comment, blank_lines_before_namespace, no_whitespace_in_blank_line, single_blank_line_at_eof). Command failed with exit code 8. Run 'vendor/bin/php-cs-fixer fix tests/Sentry/CarrierTest.php --dry-run --diff --verbose' to apply fixes.
🔇 Additional comments (2)
tests/Sentry/CarrierTest.php (2)
220-227: 赞：序列化正反向覆盖充分
toJson()默认选项、字符串性、以及反序列化一致性断言完备，可读性和稳定性良好。
66-77: 赞：Span 集成覆盖关键字段
fromSpan()的三要素（sentry-trace、baggage、traceparent）均被断言存在，契合实现与迁移目标。请确认 dev 依赖的
sentry/sentry版本包含Span::toTraceparent() / toBaggage() / toW3CTraceparent()（本地可用composer show sentry/sentry自查）。
#897) * refactor(sentry): replace CarrierPacker with new Carrier utility class - Replace CarrierPacker dependency with new Carrier utility class in AmqpProducerAspect and TracingAmqpListener - Add new Carrier utility class with fluent API for span carrier operations - Deprecate CarrierPacker class (marked for removal in v3.2) - Simplify constructor dependencies by removing CarrierPacker injection - Update carrier packing/unpacking to use new Carrier::fromSpan() and Carrier::fromJson() methods * refactor(tracing): replace CarrierPacker with Carrier utility class and update related methods * fix(tracing): initialize carrier variable to null in startTransaction method for AMQP and Kafka listeners * fix(carrier): handle JSON decoding errors in fromJson method fix(tracing): use null coalescing for optional carrier properties in TracingAmqpListener and TracingKafkaListener * test(carrier): add comprehensive tests for Carrier class functionality and error handling * fix(tracing): add type hint for payload variable in startRequestTransaction method * refactor(carrier): remove unused methods and clean up imports in Carrier class * refactor(carrier): remove withSpan method and related tests for cleaner code * refactor(async-queue): simplify carrier assignment logic in handleUnserialize method * chore: remove unused file to clean up the repository --------- Co-Authored-By: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Summary
Changes Made
Carrier::fromSpan()methodCarrier::fromJson()methodTest plan
Summary by CodeRabbit
新功能
重构
杂务
测试