-
-
Notifications
You must be signed in to change notification settings - Fork 27
feat(sentry): add producer identification to AMQP tracing #966
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
Add producer application name to AMQP message tracing data to enable better observability and message flow tracking across services. This enhancement allows identifying which service produced a message in distributed tracing scenarios. Changes: - Add 'producer' attribute to AMQP producer span data using app_name config - Capture producer information in consumer spans via message carrier - Use 'unknown' as fallback when producer information is unavailable
概览在 AMQP 生产者追踪中增加 'producer' 字段,通过应用配置名称标识消息来源。在 AMQP 消息处理事件监听器中添加对应的追踪数据提取逻辑,将生产者信息传递到跨度标签中以增强可观测性。 变更
评估代码审查工作量🎯 2 (简单) | ⏱️ ~8 分钟 可能相关的 PR
诗
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.1.31)At least one path must be specified to analyse. 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 |
Add producer application name to AMQP message tracing data to enable better observability and message flow tracking across services. This enhancement allows identifying which service produced a message in distributed tracing scenarios. Changes: - Add 'producer' attribute to AMQP producer span data using app_name config - Capture producer information in consumer spans via message carrier - Use 'unknown' as fallback when producer information is unavailable Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
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 (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
589-589: 消费者端生产者信息捕获正确!成功从消息载体中提取生产者信息并添加到追踪数据中,实现了从生产者到消费者的端到端可观测性。空值安全操作符和回退值的使用都很恰当。
可选建议:为保持一致性,可以考虑为 Kafka(第 628-677 行)和 AsyncQueue(第 692-728 行)消息处理也添加类似的生产者追踪功能,以便在整个消息系统中提供统一的可观测性。不过这可以作为后续改进,不影响当前 PR 的功能。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php(2 hunks)src/sentry/src/Tracing/Listener/EventHandleListener.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/*/src/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All component PHP code must use the namespace pattern FriendsOfHyperf{ComponentName}
Files:
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.php
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Code style must follow PSR-12 and be formatted by PHP-CS-Fixer per .php-cs-fixer.php
Run PHPStan and keep the codebase passing per phpstan.neon.dist
Files:
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.phpsrc/sentry/src/Tracing/Listener/EventHandleListener.php
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
src/sentry/src/Util/Carrier.php (1)
get(119-122)
⏰ 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.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 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.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 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 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
🔇 Additional comments (2)
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php (2)
28-28: 导入语句正确!正确导入了 Hyperf 的
config()辅助函数,符合框架规范。
90-90: 代码变更正确,已通过验证。通过代码库搜索确认:
代码模式已确立:同组件内 RpcAspect.php 也使用了相同的
config('app_name', ...)模式,说明这是框架内的约定做法。防御性设计完善:使用
config('app_name', 'unknown')的回退机制确保代码的健壮性。即使app_name未在配置中定义,也会优雅地使用默认值 'unknown'。无需额外配置:Sentry 组件本身无需在其 sentry.php 配置文件中定义
app_name,这是应用层的配置职责,组件只需在使用时提供合理的回退值。代码变更符合编码规范和框架约定,可以安全合并。
Summary
Changes
app_nameconfigTechnical Details
The producer name is extracted from the
app_nameconfiguration and included in the AMQP tracing span data. On the consumer side, this information is retrieved from the message carrier and added to the consumer span, allowing for end-to-end visibility of which service produced each message.Fallback to 'unknown' is used when producer information is unavailable.
Test Plan
Summary by CodeRabbit
发布说明