refactor: centralize context management in Sentry integration #1035
Conversation
- Add new Context class to replace Hyperf Context usage - Move all Sentry-specific context constants to Context class - Update all tracing aspects to use centralized context management - Deprecate old Feature class methods in favor of Context class - Standardize server address/port handling across all components - Improve code organization and maintainability This refactor centralizes all context management for Sentry integration, making it easier to maintain and understand the context flow.
|
Warning Rate limit exceeded@huangdijia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 6 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 (15)
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新增 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
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/Feature.php (1)
100-114: 弃用方法文档有笔误，且可以顺手收口到新 Context API
isDisableCoroutineTracing()上的注释目前提示使用\FriendsOfHyperf\Sentry\Context::isDisableTracing()，但实际新 API 是isTracingDisabled()，这会误导迁移使用者，建议改正。- 方法实现本身没问题，但既然已经有集中封装的
Context::disableTracing()/isTracingDisabled()，这里可以考虑直接调用新 API，而不是重复操作底层Hyperf\Context\Context和常量键，后续维护会更简单（也更符合“集中管理上下文”的目标）。示例修正（仅供参考）：
- * @deprecated since v3.1, will be removed in v3.2, use `\FriendsOfHyperf\Sentry\Context::isDisableTracing()` instead. + * @deprecated since v3.1, will be removed in v3.2, use `\FriendsOfHyperf\Sentry\Context::isTracingDisabled()` instead.以及可选的实现收口：
- public static function disableCoroutineTracing(): void - { - Context::set(Constants::DISABLE_COROUTINE_TRACING, true); - } + public static function disableCoroutineTracing(): void + { + \FriendsOfHyperf\Sentry\Context::disableTracing(); + } @@ - public static function isDisableCoroutineTracing(): bool - { - return (bool) Context::get(Constants::DISABLE_COROUTINE_TRACING); - } + public static function isDisableCoroutineTracing(): bool + { + return \FriendsOfHyperf\Sentry\Context::isTracingDisabled(); + }
🧹 Nitpick comments (8)
src/sentry/src/Tracing/Aspect/RpcAspect.php (1)
15-15: RPC 服务器地址/端口读取改用 SentryContext 看起来是等价重构这里保持用
Context::has(Constants::TRACE_RPC_SERVER_ADDRESS)做存在性判断，同时通过SentryContext::getRpcServerAddress()/getRpcServerPort()取值，考虑到两边使用的是同一字符串 key，这个重构在行为上是等价的，也更符合“集中由 SentryContext 访问 tracing 上下文”的设计目标。如果后续希望进一步收口，也可以考虑改为直接根据
SentryContext::getRpcServerAddress()是否为null来判断，而不再依赖Constants，但这属于风格一致性的可选优化，不是必须修改点。Also applies to: 120-124
src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php (1)
14-14: 从 SentryContext 读取 spanData 的逻辑清晰且向后兼容通过
(array) SentryContext::getSpanData()强制转数组并仅在$data非空时调用$scope->getSpan()?->setData($data)，可以很好地避免空值写入，同时与前面在ElasticsearchRequestAspect中集中写入 spanData 的设计配套，整体逻辑清晰、行为也与旧实现一致。如果未来
TRACE_SPAN_DATA打算被更多场景复用，可以考虑在合适的时机（例如这里使用后）增加一次“清空/重置”的 API，以避免其他 span 误读到上一次请求残留的数据，不过这属于后续设计层面的可选优化。Also applies to: 74-76
src/sentry/src/Tracing/Aspect/DbAspect.php (1)
14-14: DB tracing 改用 SentryContext 读取主机/端口总体合理
在
getConnection分支中把连接配置写入SentryContext::setServerAddress/Port()，而 DB span 数据部分改为从SentryContext::getDbServerAddress()/getDbServerPort()读取并带默认值：'server.host' => SentryContext::getDbServerAddress() ?? 'localhost', 'server.port' => SentryContext::getDbServerPort() ?? 3306,这与
DbConnectionAspect中写入 DB 专用 key 的逻辑是匹配的，能稳定拿到真正的 DB 端点信息，同时保持原有的 fallback 行为。目前同一流程里既写了通用的
server.address/server.port（getConnection 分支），又读了 DB 专用的 host/port key（span 数据），语义上是合理的，但稍微有点分散。后续如果确认通用 server key 不再被其他地方使用，可以考虑统一只依赖 DB 专用 key，以降低心智负担——这属于结构上的可选优化。Also applies to: 68-76, 100-112
src/sentry/src/Crons/Listener/EventHandleListener.php (1)
86-93: Cron CheckIn ID 迁移到 SentryContext 逻辑正确，可考虑在结束后清理
- 在
handleCrontabTaskStarting中通过$hub->captureCheckIn(... inProgress)拿到$checkInId并用SentryContext::setCronCheckInId($checkInId)存入上下文，之后在handleCrontabTaskFinished/handleCrontabTaskFailed中用SentryContext::getCronCheckInId()读出再上报 OK/ERROR CheckIn，流程是自洽的。if (! $checkInId) { return; }也很好地兜底了“缺少起始 CheckIn”的情况，避免发出不完整事件。可以作为小优化考虑：在成功或失败上报后，将该值从上下文中显式清理（例如再
setCronCheckInId('')），以防在同一协程中后续逻辑意外读取到旧的 CheckIn ID，不过这不是功能性问题。Also applies to: 95-101, 113-119
src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php (1)
15-15: Carrier 改用 SentryContext 封装实现合理，可进一步统一 AsyncQueue 读取路径
handlePush中用Carrier::fromSpan(...)->with($extra)构建载体并通过SentryContext::setCarrier($carrier)写入上下文，随后handleSerialize在同一协程中用SentryContext::getCarrier()取出并序列化到消息体，逻辑与原先的直接 Context 调用一致且更集中。handleUnserialize从消息体中恢复 JSON，再SentryContext::setCarrier(Carrier::fromJson($carrier))回写到 Hyperf Context，保持了原有跨进程/跨协程的传播行为。目前 AsyncQueue 消费侧的
handleAsyncQueueJobProcessing仍通过Context::get(Constants::TRACE_CARRIER, null, Coroutine::parentId())直接从父协程读取 carrier，这在功能上是兼容的（底层 key 相同），但有两点可以作为后续优化考虑：
- 在
SentryContext中提供一个可以显式指定协程 ID 的getCarrier/getCarrierFromParent之类方法；- 然后把这里的
Context::get(...)与其它地方的 carrier 读写一起切到SentryContext，完全消除对 Hyperf\Context\Context 的散落依赖。这属于架构一致性的优化，可视情况在后续迭代中处理。
Also applies to: 96-112, 141-149, 156-169
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
492-505: Redis 端点信息改用 SentryContext，Carrier 读写可考虑后续统一
handleRedisCommandExecuted里将 Redis span 的
'server.address' => SentryContext::getRedisServerAddress() ?? 'localhost''server.port' => SentryContext::getRedisServerPort() ?? 6379
改为从SentryContext读取，同样达到了集中 Redis 端点上下文的目的，并通过默认值在“未写入时”保持可用且易于理解的标签。同一文件里 AMQP/Kafka/AsyncQueue 部分仍在用
Context::set(Constants::TRACE_CARRIER, $carrier)和Context::get(Constants::TRACE_CARRIER, ...)直接操作 Hyperf Context。功能上这与当前SentryContext::setCarrier/getCarrier是兼容的（底层 key 一致），但如果希望彻底统一 Sentry 的上下文访问方式，可以在后续迭代里：
- 将这些地方改成
SentryContext::setCarrier(...)/SentryContext::getCarrier(...)；- 若需要 parentId 之类高级用法，可在
SentryContext内部扩展 API，而不是在业务代码里直接操作 Context。这属于代码一致性/可维护性的提升，不影响当前功能。
src/sentry/src/Tracing/Aspect/RpcEndpointAspect.php (1)
14-15: RPC 端点地址/端口改用 SentryContext 封装，一致性上可再斟酌具体 key 选择
- 通过
use FriendsOfHyperf\Sentry\Context as SentryContext;将下面所有Context::set(...)替换为：
- RpcMultiplex Socket 分支：
SentryContext::setServerAddress($result->getName())/setServerPort($result->getPort())- JsonRpcHttpTransporter Node 分支：
SentryContext::setServerAddress($result->host)/setServerPort($result->port)- JsonRpcPoolTransporter RpcConnection 分支：
SentryContext::setRpcServerAddress($option->getHost())/setRpcServerPort($option->getPort())
这很好地把字符串 key 封装在一个地方，减少了散落依赖。唯一需要考虑的一点是：前两个分支写的是“通用 server” key，最后一个分支写的是 “RPC 专用” key。如果后续消费端（比如某个 Listener）只读取其中一种 key，可能会导致部分路径的标签不一致。可以：
- 检查一下现有消费端是读
getServerAddress/Port还是getRpcServerAddress/Port；- 若更偏向 RPC 语义，考虑前两个分支也改为
setRpcServerAddress/Port，或者在合适的地方将二者对齐。这更多是语义/一致性层面的优化，目前实现本身是正确可用的。
Also applies to: 35-63
src/sentry/src/Context.php (1)
17-171: 新的 Sentry Context 中心类设计清晰，便于后续维护，可适度加强类型友好性
- 将所有与 Sentry 相关的上下文 key（carrier、cron check-in、禁用标记、server/db/redis/rpc 端点、span data 等）集中到
FriendsOfHyperf\Sentry\Context中，并通过静态set*/get*方法统一访问，是本次重构的核心收益，能显著减少魔法字符串和到处直接操作 Hyperf\Context\Context 的情况。disableTracing/enableTracing/isTracingEnabled/isTracingDisabled通过DISABLE_COROUTINE_TRACING标志位控制协程级别的 tracing 开关，默认开启（未设置时为 enabled），语义直观且向后兼容。- 各类 getter（如
getDbServerAddress(): ?string、getCarrier(): ?Carrier、getSpanData(): ?array等）都直接返回Ctx::get(...)的结果并标为 nullable，呼应当前调用方的判空模式，没有明显逻辑漏洞。作为后续可选的小改进，可以考虑：
- 对部分场景较多的 getter（例如
getSpanData）提供一个合理的默认值（如返回[]而不是null），或者：- 在方法 PHPDoc 中注明约定返回的实际类型，以帮助 PHPStan 之类的静态分析工具更好地推断
Ctx::get的结果类型。当前实现已经足够稳定可用，上述建议更多是为了进一步提升类型友好性和可维护性。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/sentry/src/Context.php(1 hunks)src/sentry/src/Crons/Listener/EventHandleListener.php(3 hunks)src/sentry/src/Feature.php(1 hunks)src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php(4 hunks)src/sentry/src/Tracing/Aspect/CoroutineAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/DbAspect.php(3 hunks)src/sentry/src/Tracing/Aspect/DbConnectionAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/ElasticsearchRequestAspect.php(3 hunks)src/sentry/src/Tracing/Aspect/RedisConnectionAspect.php(3 hunks)src/sentry/src/Tracing/Aspect/RpcAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/RpcEndpointAspect.php(3 hunks)src/sentry/src/Tracing/Listener/EventHandleListener.php(3 hunks)types/Sentry/Sentry.php(0 hunks)
💤 Files with no reviewable changes (1)
- types/Sentry/Sentry.php
🧰 Additional context used
📓 Path-based instructions (2)
src/*/src/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All components use the namespace pattern:
FriendsOfHyperf\{ComponentName}
Files:
src/sentry/src/Tracing/Aspect/CoroutineAspect.phpsrc/sentry/src/Tracing/Aspect/ElasticsearchAspect.phpsrc/sentry/src/Crons/Listener/EventHandleListener.phpsrc/sentry/src/Tracing/Aspect/RedisConnectionAspect.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.phpsrc/sentry/src/Tracing/Aspect/ElasticsearchRequestAspect.phpsrc/sentry/src/Tracing/Aspect/DbAspect.phpsrc/sentry/src/Tracing/Aspect/RpcAspect.phpsrc/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.phpsrc/sentry/src/Tracing/Aspect/DbConnectionAspect.phpsrc/sentry/src/Feature.phpsrc/sentry/src/Context.phpsrc/sentry/src/Tracing/Aspect/RpcEndpointAspect.php
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow PSR-12 coding standards for all PHP code
Use PHP-CS-Fixer for code formatting as configured in.php-cs-fixer.php
Use PHPStan for static analysis as configured inphpstan.neon.dist
Files:
src/sentry/src/Tracing/Aspect/CoroutineAspect.phpsrc/sentry/src/Tracing/Aspect/ElasticsearchAspect.phpsrc/sentry/src/Crons/Listener/EventHandleListener.phpsrc/sentry/src/Tracing/Aspect/RedisConnectionAspect.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.phpsrc/sentry/src/Tracing/Aspect/ElasticsearchRequestAspect.phpsrc/sentry/src/Tracing/Aspect/DbAspect.phpsrc/sentry/src/Tracing/Aspect/RpcAspect.phpsrc/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.phpsrc/sentry/src/Tracing/Aspect/DbConnectionAspect.phpsrc/sentry/src/Feature.phpsrc/sentry/src/Context.phpsrc/sentry/src/Tracing/Aspect/RpcEndpointAspect.php
🧬 Code graph analysis (11)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
src/sentry/src/Context.php (2)
Context(17-172)isTracingDisabled(58-61)
src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php (1)
src/sentry/src/Context.php (2)
Context(17-172)getSpanData(168-171)
src/sentry/src/Crons/Listener/EventHandleListener.php (2)
src/sentry/src/Context.php (3)
Context(17-172)setCronCheckInId(63-66)getCronCheckInId(68-71)src/sentry/src/Listener/EventHandleListener.php (1)
handleCrontabTaskFinished(410-415)
src/sentry/src/Tracing/Aspect/RedisConnectionAspect.php (1)
src/sentry/src/Context.php (3)
Context(17-172)setRedisServerAddress(83-86)setRedisServerPort(93-96)
src/sentry/src/Tracing/Aspect/DbAspect.php (1)
src/sentry/src/Context.php (5)
Context(17-172)setServerAddress(103-106)setServerPort(113-116)getDbServerAddress(148-151)getDbServerPort(158-161)
src/sentry/src/Tracing/Aspect/RpcAspect.php (1)
src/sentry/src/Context.php (3)
Context(17-172)getRpcServerAddress(128-131)getRpcServerPort(138-141)
src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php (2)
src/sentry/src/Context.php (3)
Context(17-172)setCarrier(73-76)getCarrier(78-81)src/sentry/src/Util/Carrier.php (2)
Carrier(25-141)fromJson(41-53)
src/sentry/src/Tracing/Aspect/DbConnectionAspect.php (1)
src/sentry/src/Context.php (3)
Context(17-172)setDbServerAddress(143-146)setDbServerPort(153-156)
src/sentry/src/Feature.php (2)
src/sentry/src/Context.php (1)
Context(17-172)src/sentry/src/Constants.php (1)
Constants(14-43)
src/sentry/src/Context.php (1)
src/sentry/src/Util/Carrier.php (1)
Carrier(25-141)
src/sentry/src/Tracing/Aspect/RpcEndpointAspect.php (1)
src/sentry/src/Context.php (5)
Context(17-172)setServerAddress(103-106)setServerPort(113-116)setRpcServerAddress(123-126)setRpcServerPort(133-136)
⏰ 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). (19)
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.3
- 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.8
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.3
- 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.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.3
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Seer Code Review
🔇 Additional comments (6)
src/sentry/src/Tracing/Aspect/ElasticsearchRequestAspect.php (1)
14-14: Elasticsearch 请求元数据改写到 SentryContext 是合理的集中化两条分支（ES 8 的
sendRequest与 ES 7 的performRequest）现在都将server.address/server.port/http.request.method/url.full等信息写入SentryContext::setSpanData()，数据内容与之前保持一致，只是存储位置集中到了专用的 SentryContext，便于后续统一读取与维护，看起来没有行为回归风险。Also applies to: 39-49, 52-63
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
14-14: 协程 tracing 开关切换到 SentryContext 后语义保持不变这里将条件判断更新为：
if ( ! $this->feature->isTracingSpanEnabled('coroutine') || SentryContext::isTracingDisabled() ) { ... }结合 Feature 中对同一标记位的写入逻辑，这相当于直接使用新的集中式 Context API 来判断“是否全局禁用 tracing”，不会改变原有开关语义，同时也减少了 Feature 静态方法的耦合，改动是合理的。
Also applies to: 48-51
src/sentry/src/Tracing/Aspect/DbConnectionAspect.php (1)
14-14: DB 连接信息写入 SentryContext 的方式合理且类型更严格在
getPdoForSelect/getPdo首次调用时，通过 WeakMap 取出之前从 DSN 解析过的host/port，并使用：SentryContext::setDbServerAddress($server['host']); SentryContext::setDbServerPort((int) $server['port']);集中写入 DB 专用的 tracing 上下文 key，同时保证端口为
int类型，这与新 Context 设计保持一致，也不会改变原有的行为路径（Context::getOrSet 确保只执行一次）。整体实现看起来是安全且可维护的。Also applies to: 56-62
src/sentry/src/Tracing/Aspect/RedisConnectionAspect.php (1)
14-14: Redis 服务器信息迁移到 SentryContext 保持语义不变对于单节点/哨兵
Redis和RedisCluster，现在都通过：SentryContext::setRedisServerAddress(...); SentryContext::setRedisServerPort((int) ...);写入专用的 Redis tracing 上下文 key，行为与旧实现等价，同时对集群端口增加了
(int)转换，有利于保持类型一致性。整体改动集中且不会引入明显回归。Also applies to: 48-51, 53-61
src/sentry/src/Crons/Listener/EventHandleListener.php (1)
14-14: 统一使用 SentryContext 别名导入是合理的抽象收口这里将
FriendsOfHyperf\Sentry\Context作为SentryContext引入，与本 PR 的“集中管理上下文”目标一致，避免到处直接依赖 Hyperf\Context\Context 常量字符串，整体没问题。src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
16-16: DB 端点信息改用 SentryContext 封装，语义清晰且带合理默认值
- 新增的
use FriendsOfHyperf\Sentry\Context as SentryContext;结合SentryContext::getDbServerAddress()/getDbServerPort()，将数据库端点信息从散落的 Context key 访问收口到统一的 Context 类中，符合“集中管理上下文”的设计。- 在
handleDbQueryExecuted中将 span data 的
'server.address' => SentryContext::getDbServerAddress() ?? 'localhost''server.port' => SentryContext::getDbServerPort() ?? 3306
作为补充标签是合理的：当上游 Aspect 尚未写入 DB server 信息时，至少还能落到本地开发常见的默认值，避免字段缺失。整体来看，这部分改动没有引入新的逻辑风险，类型也与 Context API 匹配。
Also applies to: 190-199
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/sentry/src/Context.php (1)
73-81:getCarrier新增coroutineId参数已覆盖异步队列父协程场景，解决此前评审中提到的问题这里通过
setCarrier(Carrier $carrier)写入 Carrier 对象，并在getCarrier(?int $coroutineId = null)中使用命名参数coroutineId传递给Ctx::get，使调用方可以显式从指定协程（例如Coroutine::parentId()）读取 carrier，从而满足异步队列 job 继承父协程 trace 的需求。
这正好回应了此前针对EventHandleListener中父协程 carrier 获取不完整的评审意见，当前实现看起来是正确的集中封装。为保险起见，建议在仓库中搜索所有使用
TRACE_CARRIER或旧Context::get(..., Coroutine::parentId())的地方，确认都已经迁移为调用本方法，例如：#!/bin/bash rg -n 'TRACE_CARRIER' src -C3 rg -n 'getCarrier\(' src -C3以确保所有异步队列和 RPC 场景均通过该统一入口获取 carrier。
🧹 Nitpick comments (3)
src/sentry/src/Context.php (3)
43-62: 追踪开关语义自洽，默认开启，如需可考虑增加“重置”为未设置的能力
disableTracing()/enableTracing()与isTracingEnabled()/isTracingDisabled()之间的语义是一致的：未设置或显式为false视为“启用追踪”，true视为“禁用追踪”，符合直觉。
如果后续有场景需要“恢复到未写入 Context 的初始状态”，可以增加一个可选的resetTracingFlag()（调用Ctx::set(self::DISABLE_COROUTINE_TRACING, null)或Ctx::destroy(...)）来表达该意图，目前实现已可正常工作，这只是一个防御性的小优化建议。建议在现有测试或新增测试中覆盖
disableTracing()/enableTracing()与两个查询方法的组合调用，确认默认值和翻转逻辑符合业务预期。
63-72: Cron Check-In ID 读写简单直接，如需更强健可做轻量类型防御
setCronCheckInId(string $checkInId)与getCronCheckInId(): ?string的封装很直接，契约清晰。当前直接返回Ctx::get(self::CRON_CHECKIN_ID)，在正常只通过本类写入的前提下没有问题。
若希望在发生异常写入（比如其他地方误写成数组）的情况下也更安全，可以考虑在getCronCheckInId()中做一次is_string检查，不符合时返回null，以提升健壮性，此为可选优化。建议通过现有 cron 相关集成测试或手工验证一次 check-in ID 的写入/读取链路，确认与 Sentry 端上报格式完全一致。
83-171: 各类 Server/DB/Redis/RPC 与 span data 的上下文封装风格统一，利于静态分析和后续扩展Redis、普通 Server、RPC、DB 以及 span data 的一组
setXxx/getXxx方法都只是薄封装到Ctx::set/get，返回类型分别限定为?string/?int/?array，风格统一且便于 PHPStan 做类型推断，整体实现没有明显问题。
如果未来字段继续增多，可以考虑抽象出类似“按命名空间设置 server 信息”的内部私有帮助方法（如setServerInfo('redis', address, port)），以减少样板代码，不过以目前数量看完全可以接受。建议确保追踪相关的集成测试（DB、Redis、RPC、Elasticsearch 等）都已经更新为依赖此 Context 封装，并在日志或 Sentry UI 中确认地址/端口信息正确填充。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/sentry/src/Context.php(1 hunks)src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php(5 hunks)src/sentry/src/Tracing/Listener/EventHandleListener.php(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php
- src/sentry/src/Tracing/Listener/EventHandleListener.php
🧰 Additional context used
📓 Path-based instructions (2)
src/*/src/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All components use the namespace pattern:
FriendsOfHyperf\{ComponentName}
Files:
src/sentry/src/Context.php
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow PSR-12 coding standards for all PHP code
Use PHP-CS-Fixer for code formatting as configured in.php-cs-fixer.php
Use PHPStan for static analysis as configured inphpstan.neon.dist
Files:
src/sentry/src/Context.php
🧬 Code graph analysis (1)
src/sentry/src/Context.php (2)
src/sentry/src/Util/Carrier.php (1)
Carrier(25-141)src/sentry/src/Feature.php (1)
isTracingEnabled(68-71)
⏰ 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.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 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.0.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
🔇 Additional comments (1)
src/sentry/src/Context.php (1)
17-42: 集中定义 Sentry 上下文字段的设计合理、命名清晰这一段将所有与 Sentry 追踪相关的 Context key 集中在单一类常量中，前缀统一为
sentry.*，有利于全局检索、避免 key 冲突，也与组件命名空间保持一致，没有发现功能或风格问题。
There was a problem hiding this comment.
Pull request overview
This PR successfully introduces a centralized context management system for the Sentry integration by creating a new FriendsOfHyperf\Sentry\Context class. The refactoring replaces scattered usage of Hyperf\Context\Context with dedicated, type-safe methods for managing Sentry-specific context values across database, Redis, RPC, and Elasticsearch tracing.
Key Changes:
- New
Contextclass with specialized getter/setter methods for all Sentry context types (carrier, server addresses/ports, span data) - All tracing aspects updated to use
SentryContextalias instead of direct Hyperf Context calls - Deprecated old
Featureclass methods with clear migration path to new Context API - Added proper deprecation notices to Constants.php for old constant definitions
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/sentry/src/Context.php |
New centralized context management class with type-safe methods and constants |
src/sentry/src/Constants.php |
Added deprecation notices to context-related constants |
src/sentry/src/Feature.php |
Deprecated disableCoroutineTracing() and isDisableCoroutineTracing() methods with migration guidance |
src/sentry/src/Tracing/Aspect/DbAspect.php |
Updated to use generic setServerAddress/Port methods for DB tracing |
src/sentry/src/Tracing/Aspect/DbConnectionAspect.php |
Updated to use specific setDbServerAddress/Port methods |
src/sentry/src/Tracing/Aspect/RpcEndpointAspect.php |
Mixed usage of generic and RPC-specific context methods |
src/sentry/src/Tracing/Aspect/RpcAspect.php |
Updated to use getRpcServerAddress/Port with proper null coalescing |
src/sentry/src/Tracing/Aspect/RedisConnectionAspect.php |
Updated to use setRedisServerAddress/Port methods |
src/sentry/src/Tracing/Aspect/ElasticsearchRequestAspect.php |
Updated to use setElasticsearchSpanData method |
src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php |
Updated to use getElasticsearchSpanData with conditional setData call |
src/sentry/src/Tracing/Aspect/CoroutineAspect.php |
Updated to use isTracingDisabled() instead of deprecated method |
src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php |
Updated carrier management to use SentryContext methods |
src/sentry/src/Tracing/Aspect/GrpcAspect.php |
Added defensive type casting for server address |
src/sentry/src/Tracing/Listener/EventHandleListener.php |
Updated all context operations to use SentryContext methods |
src/sentry/src/Crons/Listener/EventHandleListener.php |
Updated cron check-in ID management to use SentryContext methods |
types/Sentry/Sentry.php |
Removed deprecated method references from type definitions |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* refactor: centralize context management in Sentry integration - Add new Context class to replace Hyperf Context usage - Move all Sentry-specific context constants to Context class - Update all tracing aspects to use centralized context management - Deprecate old Feature class methods in favor of Context class - Standardize server address/port handling across all components - Improve code organization and maintainability This refactor centralizes all context management for Sentry integration, making it easier to maintain and understand the context flow. * refactor: 更新获取Carrier的方法以支持协程ID * refactor: 使用call_user_func获取消息ID以提高兼容性 * refactor: 使用SentryContext替代Hyperf\Context以管理追踪载体 * refactor: 更新SentryContext以支持Elasticsearch和RPC追踪数据管理 * refactor: 优化RPC结果的服务器地址和端口数据设置逻辑 * refactor: 确保服务器地址在缺失时默认为'unknown' * refactor: 替换Context为SentryContext以统一追踪管理 * refactor: 更新SentryContext以使用数据库服务器地址和端口设置 * refactor: 移除不必要的类型转换以简化ElasticsearchSpanData获取逻辑 * refactor: 更新RpcEndpointAspect以使用SentryContext的RPC服务器地址和端口设置 * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor: 修正TRACE_CARRIER常量的格式以统一命名约定 * refactor: 统一使用Context类替代别名以提高代码可读性 * refactor: 调整TRACE_CARRIER常量的位置以提高代码结构清晰度 * refactor: 统一SentryContext常量命名约定以提高一致性 * refactor: 添加destroyRpcSpanContext方法以简化RPC上下文销毁逻辑 * refactor: 移除不必要的服务器地址和端口常量及相关方法以简化代码 * refactor: 修正CTX_CARRIER常量的命名以提高一致性 * refactor: 修正CTX_RPC_SPAN_CONTEXT常量的命名以提高一致性 * refactor: 移除isTracingEnabled方法以简化代码 * refactor: 强制转换isTracingDisabled方法的返回值为布尔类型 --------- Co-Authored-By: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Co-Authored-By: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
This PR introduces a centralized context management system for Sentry integration, replacing scattered Hyperf Context usage with a dedicated
FriendsOfHyperf\Sentry\Contextclass.Key Changes
Featureclass methods as deprecated with migration path to new Context methodsFiles Modified
src/sentry/src/Context.php(new) - Central context management classSentryContextinstead of directContextusageFeature.php- Added deprecation notices for old context methodsEventHandleListener.php- Updated cron and tracing event handlersBenefits
Test plan
Summary by CodeRabbit
发布说明
新功能
重构
弃用
✏️ Tip: You can customize this high-level summary in your review settings.