-
-
Notifications
You must be signed in to change notification settings - Fork 27
Refactor GuzzleHttpClientAspect #834
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough这个PR引入了新的GuzzleHttpClientAspect类,用于拦截Guzzle HTTP请求并记录详细数据,同时对原有的HttpClientAspect进行了修改:将其继承自GuzzleHttpClientAspect并标记弃用。ConfigProvider中的aspects数组也相应调整,移除旧的HttpClientAspect并添加新的相关Aspect,重新排列了各个方面的顺序。 Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant A as GuzzleHttpClientAspect
participant P as ProceedingJoinPoint
participant T as Telescope
C->>A: 触发HTTP请求
A->>A: 检查Telescope配置和上下文条件
alt 条件满足
A->>A: 修改on_stats选项,添加回调函数
else 条件不满足
A->>P: 直接调用原始方法
end
A->>P: 执行原始调用
P-->>A: 返回响应数据
A->>T: 调用recordClientRequest记录请求数据
T-->>A: 确认记录完成
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 PHPStan (2.0.3)/bin/bash: line 1: /vendor/bin/phpstan: No such file or directory 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/telescope/src/Aspect/GuzzleHttpClientAspect.php (3)
61-91: 通过 on_stats 回调记录关键请求信息
将请求方法、URL、头部、响应等信息全部纳入观察范围,这对调试和审计都非常有价值。请确保在生产环境中对敏感信息进行额外的隐藏或脱敏处理。
96-134: 处理响应负载的逻辑较完善,需注意潜在性能影响
在判断响应类型与执行大小限制等方面较为周到,但在处理超大响应体时,会有一定的性能开销。后续若对大文件下载有需求,建议设置特定的记录方式或分段写入以减少内存占用。
136-140: contentWithinLimits 使用魔法数字可以考虑提取到配置
目前 64 KB 的大小限制写死在方法中,如需灵活性,建议将其提取为可配置项或常量,以方便后续维护与调优。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/telescope/src/Aspect/GuzzleHttpClientAspect.php(1 hunks)src/telescope/src/Aspect/HttpClientAspect.php(1 hunks)src/telescope/src/ConfigProvider.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.6
🔇 Additional comments (6)
src/telescope/src/Aspect/HttpClientAspect.php (1)
15-17: 标记为弃用的做法有助于平滑过渡
在旧类中添加 @deprecated 注解,并继承新类是比较常见的做法,可以最大限度减小对现有使用者的影响。建议后续在发布重要版本变动时,移除该类,并在文档或发布说明中对迁移做详细说明,以帮助用户平稳升级。src/telescope/src/Aspect/GuzzleHttpClientAspect.php (4)
30-35: 类继承关系清晰,基于 AbstractAspect 合理
该类通过继承 AbstractAspect 并设置 $classes 属性来指定切入点,符合 AOP 设计要求,结构清晰,有利于后期维护和扩展。
36-38: 在构造函数中注入 TelescopeConfig 以便灵活控制
通过依赖注入获取配置示例,是一种简洁且可测试的设计。若后续有更多配置需求,可在此进行扩展。
40-59: 逻辑条件判断严谨,保留对原始流程的回退
在这里对 Telescope 的开启状态和上下文 ID 进行检测,并且当被显式指定忽略时可回退到原始流程,符合预期的不可侵入式设计。
142-154: 隐藏敏感参数功能符合隐私和安全需求
使用 hideParameters 对指定字段进行掩码,有助于保护用户数据隐私。可在文档中列出默认隐藏的字段,以便用户理解和自定义。src/telescope/src/ConfigProvider.php (1)
23-31: 统一更新 aspects 列表,移除旧的 HttpClientAspect 并新增 GuzzleHttpClientAspect
此变动与提交内容相对应,保持了配置文件的整洁与一致性。建议确认在其他地方没有残留对 HttpClientAspect 的调用,避免在升级后出现找不到类的问题。
* Refactor GuzzleHttpClientAspect * fix: 修复 GuzzleHttpClientAspect 中的响应处理逻辑 * fix: 优化 GuzzleHttpClientAspect 中的响应处理逻辑 * fix: 优化 GuzzleHttpClientAspect 中的请求记录逻辑 * fix: 修复 GuzzleHttpClientAspect 中的实例获取逻辑 * fix: 捕获 GuzzleHttpClientAspect 中的异常以防止请求中断 --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Summary by CodeRabbit
新功能
重构