feat: add socket tracing support for RPC calls #1017
Conversation
|
Warning Rate limit exceeded@huangdijia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 44 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)
概览该PR为Sentry添加了RPC端点追踪支持。通过引入新的Hyperf AOP切面来拦截RPC套接字操作，捕获端点主机和端口信息，并将其存储在Sentry追踪上下文中，同时添加了相应的配置和常量定义。 变更
序列图sequenceDiagram
participant Client
participant RpcEndpointAspect
participant SocketFactory
participant Context as Sentry Context
Client->>RpcEndpointAspect: 调用 SocketFactory::get()
activate RpcEndpointAspect
RpcEndpointAspect->>SocketFactory: process() 继续原始调用
activate SocketFactory
SocketFactory-->>RpcEndpointAspect: 返回 Socket 对象
deactivate SocketFactory
rect rgb(200, 220, 255)
Note over RpcEndpointAspect,Context: 追踪信息捕获
RpcEndpointAspect->>Context: 存储 TRACE_RPC_ENDPOINT_HOST
RpcEndpointAspect->>Context: 存储 TRACE_RPC_ENDPOINT_PORT
end
RpcEndpointAspect-->>Client: 返回 Socket
deactivate RpcEndpointAspect
代码审查工作量评估🎯 2 (Simple) | ⏱️ ~10 分钟
诗歌
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: 1
🧹 Nitpick comments (1)
src/sentry/src/Tracing/Aspect/RpcAspect.php (1)
128-132: 可考虑在存在 socketOptions 时才写入 server.address / server.port当前使用 nullsafe 操作符，即使
$socketOptions为null，也会向 span data 中写入'server.address' => null和'server.port' => null。如果希望只在明确解析到目标地址时才打点，可以按条件构建 data 数组，例如：$data = [ 'rpc.context' => $rpcCtx->getData(), ]; if ($socketOptions) { $data['server.address'] = $socketOptions->getHost(); $data['server.port'] = $socketOptions->getPort(); } $span->setData($data);这属于埋点细节的小优化，可根据实际上报习惯自行取舍。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/sentry/src/ConfigProvider.php(1 hunks)src/sentry/src/Tracing/Aspect/RpcAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/SocketFactoryAspect.php(1 hunks)src/sentry/src/Util/SocketOptionContainer.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/sentry/src/ConfigProvider.php (1)
src/sentry/src/Tracing/Aspect/SocketFactoryAspect.php (1)
SocketFactoryAspect(22-39)
src/sentry/src/Tracing/Aspect/RpcAspect.php (2)
src/sentry/src/Util/SocketOptionContainer.php (2)
SocketOptionContainer(18-36)get(31-35)src/sentry/src/Util/Carrier.php (4)
get(118-121)has(113-116)Carrier(25-141)fromSpan(55-61)
src/sentry/src/Tracing/Aspect/SocketFactoryAspect.php (1)
src/sentry/src/Util/SocketOptionContainer.php (2)
SocketOptionContainer(18-36)set(25-29)
⏰ 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). (16)
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
🔇 Additional comments (4)
src/sentry/src/ConfigProvider.php (1)
41-43: 在 ConfigProvider 中注册 SocketFactoryAspect 的方式看起来合理将
Tracing\Aspect\SocketFactoryAspect::class加入aspects列表且紧跟在RpcAspect之后，符合本次 PR 的目标，也不会影响已有配置逻辑。src/sentry/src/Tracing/Aspect/RpcAspect.php (1)
14-35: 为 RPC trace 引入 SocketOption 相关依赖的整体方向是合理的在 RpcAspect 中引入
SocketOptionContainer以及SocketOptionInterface/SocketInterface，并保留原有 span 构建逻辑不变，仅附加网络维度数据，改动范围可控且契合“增强追踪而不改变业务语义”的目标。src/sentry/src/Util/SocketOptionContainer.php (1)
18-35: 基于 WeakMap 的 SocketOptionContainer 实现简洁且符合使用场景通过静态
WeakMap<SocketInterface, SocketOptionInterface>关联 socket 与其选项，并在set/get中 lazy 初始化，既避免了显式清理，又保持了 API 简单明了；当前实现没有明显逻辑问题。如果项目后续需要兼容更低版本 PHP，请额外确认运行环境对
WeakMap的支持情况。src/sentry/src/Tracing/Aspect/SocketFactoryAspect.php (1)
22-37: SocketFactoryAspect 拦截 socket 创建并记录选项的实现是非侵入且安全的
process()中复用原有SocketFactory::make结果，仅在$options为SocketOptionInterface且返回值为SocketInterface时写入SocketOptionContainer，既保证了类型安全，也不改变工厂方法原有行为，对后续在 RpcAspect 中按 socket 取回选项的设计是个不错的支撑点。
huangdijia
force-pushed
the
feature/sentry-rpc-socket-tracing
branch
from
9a623fc to
e217702
Compare
… method" This reverts commit e217702.
…ve RpcSocketFactoryAspect
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/sentry/src/Tracing/Aspect/RpcSocketFactoryAspect.php (1)
23-28: 考虑设置 aspect 优先级Aspect 属性未指定优先级。如果有其他 aspect 也拦截 socket 创建，执行顺序可能不确定。建议考虑是否需要设置明确的优先级。
另外，空括号可以省略：
#[Aspect]即可。-#[Aspect()] +#[Aspect(priority: 100)] class RpcSocketFactoryAspect extends AbstractAspect
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
composer.json(1 hunks)src/sentry/src/Constants.php(1 hunks)src/sentry/src/Tracing/Aspect/RpcSocketFactoryAspect.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Aspect/RpcSocketFactoryAspect.php (1)
src/sentry/src/Constants.php (1)
Constants(14-31)
⏰ 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). (18)
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
🔇 Additional comments (2)
src/sentry/src/Constants.php (1)
18-20: 常量定义正确新增的常量遵循现有命名约定，值与常量名称保持一致，将用于存储 RPC 端点的主机和端口信息。
src/sentry/src/Tracing/Aspect/RpcSocketFactoryAspect.php (1)
32-35: 实现与 PR 描述不一致PR 描述提到使用 WeakMap 和
SocketOptionContainer工具类来存储 socket 选项，但实际实现直接使用Context存储数据。这种不一致可能会造成理解上的困惑。如果
Context是有意的设计选择，建议更新 PR 描述以反映实际实现。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/sentry/src/Tracing/Aspect/RpcEndpointAspect.php (1)
28-28: 建议添加返回类型声明。为
process()方法添加返回类型声明可以提高代码的类型安全性和可读性。应用此差异添加返回类型：
- public function process(ProceedingJoinPoint $proceedingJoinPoint) + public function process(ProceedingJoinPoint $proceedingJoinPoint): mixed
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/sentry/src/ConfigProvider.php(1 hunks)src/sentry/src/Tracing/Aspect/RpcEndpointAspect.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/ConfigProvider.php (1)
src/sentry/src/Tracing/Aspect/RpcEndpointAspect.php (1)
RpcEndpointAspect(22-35)
⏰ 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). (16)
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
🔇 Additional comments (5)
src/sentry/src/ConfigProvider.php (1)
42-42: 切面注册正确。新切面的注册位置合理，与 RpcAspect 相邻，符合模块组织原则。
src/sentry/src/Tracing/Aspect/RpcEndpointAspect.php (4)
12-23: 导入和类声明正确。所有必要的依赖已正确导入，类结构遵循现有切面实现模式。
24-26: 切面目标配置正确。正确指定了要拦截的方法，符合 Hyperf AOP 的配置约定。
28-34: PR 描述与实现不一致。PR 目标中提到使用 WeakMap 和 SocketOptionContainer 来存储 socket 选项，但实际实现使用的是
Context::set()。虽然使用协程上下文存储是合理的做法（数据会在协程结束时自动清理），但这与 PR 描述不符，可能会让未来的维护者感到困惑。建议更新 PR 描述以反映实际的实现方式，或者如果确实需要 WeakMap 方案，请调整实现。
30-33: 原始审查注释不准确 - 代码类型安全性已验证。代码实际上是安全的：
SocketFactory::get()方法有明确的返回类型声明：public function get(): SocketSocket类继承自Client类（在vendor/multiplex/socket/src/Client.php中）Client类定义了：
public function getName(): string（第 184 行）public function getPort(): int（第 189 行）tap()回调中的类型提示Socket $socket与$proceedingJoinPoint->process()的返回类型相符代码已通过类型声明进行了充分保护，不存在运行时类型错误的风险。
Likely an incorrect or invalid review comment.
* fix: allow null socket parameter in SocketOptionContainer get method * Revert "fix: allow null socket parameter in SocketOptionContainer get method" This reverts commit e217702. * feat: add RpcSocketFactoryAspect for tracing RPC socket endpoints * feat: add RpcEndpointAspect for tracing RPC endpoint details and remove RpcSocketFactoryAspect * feat: enhance RPC tracing by adding server address and port to span data * feat: add reference link for RPC span semantic conventions in RpcAspect * feat: update RPC tracing constants to use server address and port * feat: add requirement for hyperf/rpc-multiplex to enable RPC tracing * feat: improve type hinting for socket in RpcEndpointAspect process method --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Summary
This PR enhances the Sentry tracing capabilities for RPC calls by adding socket-level tracing support that captures server address and port information.
Changes
Benefits
Technical Details
The implementation works by:
This enhancement will help developers better understand their RPC call patterns and server connectivity in distributed systems.
Summary by CodeRabbit
发行说明
新增功能
依赖更新