Skip to content

Conversation

@huangdijia
Copy link
Contributor

@huangdijia huangdijia commented Nov 27, 2025

Summary by CodeRabbit

  • 新功能

    • 可配置的性能指标开关(默认开启)与指标采集间隔。
    • 新增多种运行时指标采集能力:队列、数据库/Redis 连接池、请求、协程/定时器、服务器统计与计数器注解支持。
    • 提供轻量服务器统计结构与指标发布辅助工具,便于统一采集与导出度量数据。
  • 优化改进

    • 扩展刷新流程,flush 时一并上报追踪/度量数据,提升可观测性与稳定性。
    • 新增运行时常量与特性接口,便于在不同执行场景下控制指标行为。

✏️ Tip: You can customize this high-level summary in your review settings.

Add support for new Sentry singleton classes:
- LogLevel::getInstance
- Metrics::getInstance
- MetricsUnit::getInstance
- Unit::getInstance

This ensures proper aspect-based handling for these Sentry components.
@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Warning

Rate limit exceeded

@huangdijia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 26 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 87c9180 and 9e62103.

📒 Files selected for processing (11)
  • src/sentry/src/ConfigProvider.php (2 hunks)
  • src/sentry/src/Metrics/Annotation/Counter.php (1 hunks)
  • src/sentry/src/Metrics/Annotation/Histogram.php (1 hunks)
  • src/sentry/src/Metrics/Aspect/CounterAspect.php (1 hunks)
  • src/sentry/src/Metrics/Aspect/HistogramAspect.php (1 hunks)
  • src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (1 hunks)
  • src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (1 hunks)
  • src/sentry/src/Metrics/Listener/OnWorkerStart.php (1 hunks)
  • src/sentry/src/Metrics/Listener/PoolWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Listener/QueueWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Listener/RequestWatcher.php (1 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit 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

引入 Sentry 度量子系统:新增配置与 Feature 判定、常量与单例扩展、在 Integration flush 时 flush TraceMetrics;添加度量事件、若干监听器、度量工具类与轻量服务器统计结构,用于定时采集并上报运行时指标并在进程退出时清理定时器。

Changes

内聚体 / 文件(s) 变更摘要
单例提供者扩展
\src/sentry/src/Aspect/SingletonAspect.php``
SingletonAspect::$classes 列表中加入 Sentry\Logs\LogLevel::class . '::getInstance'Sentry\Metrics\TraceMetrics::class . '::getInstance'Sentry\Unit::class . '::getInstance'
配置与 Feature
\src/sentry/publish/sentry.php`, `src/sentry/src/Feature.php`, `src/sentry/src/Constants.php``
新增配置项 enable_metricsmetrics_interval;在 Feature 中新增 isMetricsEnabled(bool $default = true)getMetricsInterval(int $default = 10)(最小返回值 5);新增常量 Constants::RUN_IN_COMMAND = 'sentry.run_in_command'
Integration 扩展
\src/sentry/src/Integration.php``
Integration::flushEvents 中(存在 client 时)新增调用 TraceMetrics::getInstance()->flush()
度量事件与数据结构
\src/sentry/src/Metrics/Event/MetricFactoryReady.php`, `src/sentry/src/Metrics/CoroutineServerStats.php``
新增事件类 MetricFactoryReady(含 public workerId),新增轻量统计类 CoroutineServerStats(实现 Arrayable,动态属性与 toArray,含若干 server 指标初值)。
度量工具
\src/sentry/src/Metrics/Traits/MetricSetter.php`, `src/sentry/src/Metrics/Annotation/Counter.php``
新增 MetricSetter trait,提供 trySet 辅助方法;新增 Counter 注解类用于计数埋点(可用于类/方法)。
度量 Aspect
\src/sentry/src/Metrics/Aspect/CounterAspect.php``
新增 CounterAspect,在启用 metrics 时读取注解或生成指标名并调用 TraceMetrics::getInstance()->count(...),随后继续执行原 join point。
Pool 与队列监听器(基类 + 实现)
\src/sentry/src/Metrics/Listener/PoolWatcher.php`, `src/sentry/src/Metrics/Listener/DBPoolWatcher.php`, `src/sentry/src/Metrics/Listener/RedisPoolWatcher.php`, `src/sentry/src/Metrics/Listener/QueueWatcher.php``
新增抽象 PoolWatcher(提供 watch 定时上报连接池指标)及其实现 DBPoolWatcherRedisPoolWatcher;新增 QueueWatcher 定时采集 async queue 指标并上报。
生命周期与采集监听器
\src/sentry/src/Metrics/Listener/OnWorkerStart.php`, `src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php`, `src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php`, `src/sentry/src/Metrics/Listener/OnBeforeHandle.php`, `src/sentry/src/Metrics/Listener/RequestWatcher.php``
新增监听器:在 Worker/Coroutine Server 启动时触发 MetricFactoryReady、建立周期性采集(定时器收集 GC、rusage、内存、协程/定时器/服务器统计等),在进程退出/WORKER_EXIT 时清理定时器;OnBeforeHandle 设置/销毁 RUN_IN_COMMAND 上下文;RequestWatcher 更新 CoroutineServerStats。
工厂与选项调整 / 注册
\src/sentry/src/Factory/ClientBuilderFactory.php`, `src/sentry/src/ConfigProvider.php`, `composer.json``
SPECIFIC_OPTIONS 中新增 'metrics_interval';在 ConfigProvider::__invoke__ 中注册并新增多项 metrics 监听器(DBPoolWatcher、OnBeforeHandle、OnCoroutineServerStart、OnMetricFactoryReady、OnWorkerStart、QueueWatcher、RedisPoolWatcher、RequestWatcher 等)。

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Worker as Worker/Process
    participant EventBus as 事件分发器
    participant Feature as Feature (配置)
    participant Container as DI 容器
    participant MetricEvent as MetricFactoryReady
    participant Timer as 周期定时器
    participant TraceMetrics as Sentry\Metrics\TraceMetrics
    participant Coordinator as CoordinatorManager

    Worker->>EventBus: BeforeWorkerStart / MainCoroutineServerStart
    EventBus->>Feature: isMetricsEnabled()
    alt metrics enabled
        EventBus->>Container: 实例化/注册 监听器(OnWorkerStart/OnMetricFactoryReady/PoolWatcher/QueueWatcher/RequestWatcher)
        OnWorkerStart->>EventBus: dispatch MetricFactoryReady(workerId)
        MetricEvent->>OnMetricFactoryReady: process -> 启动 Timer
        Timer->>Container: 收集 stats (gc,rusage,Coroutine/Timer stats, pools, queues, server stats)
        Timer->>TraceMetrics: TraceMetrics::getInstance()->gauge(...)
        TraceMetrics->>TraceMetrics: 缓存/发送 指标
        Worker->>Coordinator: 等待 WORKER_EXIT
        Coordinator->>Timer: 清理定时器
        Worker->>EventBus: 关闭 -> Integration::flushEvents
        Integration->>TraceMetrics: TraceMetrics::getInstance()->flush()
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 分钟

重点复核项:

  • 各监听器中定时器/协程的生命周期管理与 WORKER_EXIT 清理逻辑(OnWorkerStart、OnCoroutineServerStart、PoolWatcher.watch 等)。
  • OnMetricFactoryReady 中针对不同运行环境(Swoole / 非 Swoole / RUN_IN_COMMAND)对 serverStatsFactory 的选择分支与异常容错。
  • TraceMetrics/Unit 单例的获取、使用与 flush 时机(Integration::flushEvents)在未初始化或无 client 时的健壮性。
  • metrics_interval 配置的读取、最小值约束(Feature::getMetricsInterval)与在 ClientBuilderFactory 中的识别。
  • 高频采集对性能与默认间隔的影响(定时器默认间隔、trySet 调用频率)。

Possibly related PRs

Suggested reviewers

  • xuanyanwow
  • zds-s

Poem

🐰✨ 新指标轻轻生,
定时滴答数声声。
Trace 与 Unit 排队来,
单例守候不曾眠。
小兔跳跃庆此更。

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: Support Trace Metrics' accurately reflects the main changes - adding comprehensive trace metrics support through multiple new listener classes, configuration options, and metrics infrastructure components.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@huangdijia huangdijia marked this pull request as draft November 27, 2025 23:21
Copy link

@coderabbitai coderabbitai bot left a 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/publish/sentry.php (1)

43-45: 新增 enable_metrics 配置项与 Sentry 官方选项保持一致,可以考虑显式布尔化作为小优化

这里新增:

// @see: https://docs.sentry.io/platforms/php/guides/laravel/configuration/options/#enable_metrics
'enable_metrics' => env('SENTRY_ENABLE_METRICS', true),

优点是:

  • key 命名与 Sentry 官方 enable_metrics 一致,方便对照文档。
  • 默认值为 true,与上方 enable_logs、以及官方示例的习惯保持一致。

如果想进一步降低配置被误解析为字符串的风险,可以考虑(仅为可选优化)改成:

'enable_metrics' => (bool) env('SENTRY_ENABLE_METRICS', true),

以确保后续传给 Sentry SDK 的始终是布尔值。

请你再确认:

  • 当前把整个配置数组传入 SDK/集成层时,enable_metrics 是否确实被按布尔值消费;
  • 以及是否与 Sentry 官方文档中该选项的预期行为保持一致。
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 83dfd96 and 14c9808.

📒 Files selected for processing (3)
  • src/sentry/publish/sentry.php (1 hunks)
  • src/sentry/src/Aspect/SingletonAspect.php (1 hunks)
  • src/sentry/src/Integration.php (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/sentry/src/Aspect/SingletonAspect.php
⏰ 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.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.2 with Swoole 6.1.3
  • 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.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.1.3
  • 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 5.1.8
  • GitHub Check: Test on PHP 8.3 with Swoole 6.1.3
  • 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 6.1.3
  • GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
🔇 Additional comments (1)
src/sentry/src/Integration.php (1)

18-18: flushEvents() 中增加 TraceMetrics flush 与现有模式一致,但需要确认 SDK 版本与配置联动是否符合预期

这里引入 Sentry\Metrics\TraceMetrics 并在已有 Logs::getInstance()->flush() 之后调用 TraceMetrics::getInstance()->flush(),与现有日志单例的处理方式保持一致,而且仍然受 $client !== null 保护,从集成层设计上看是合理的。

建议你再确认两点:

  1. 当前锁定的 sentry/sentry 版本中确实提供 Sentry\Metrics\TraceMetrics::getInstance()->flush(),避免在旧版本 SDK 场景下出现致命错误。
  2. 当配置中关闭 metrics(如通过新加入的 enable_metrics 或相关开关)时,TraceMetrics::flush() 是否为安全的 no‑op,或者是否需要在这里额外根据配置做一次条件判断。

整体实现没问题,仅需确认以上兼容性与配置联动是否符合预期。

Also applies to: 111-116

@huangdijia huangdijia changed the title feat: add additional Sentry singleton classes to aspect configuration feat: Support Trace Metrics Nov 28, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (11)
src/sentry/src/Metrics/Listener/OnBeforeHandle.php (1)

20-40: 实现正确,类命名可考虑优化。

代码逻辑清晰,使用 match 表达式处理不同事件类型。但类名 OnBeforeHandle 仅反映了 BeforeHandle 事件,实际上也处理了 AfterExecute 事件。可考虑更具描述性的命名,如 CommandLifecycleListenerCommandContextListener

src/sentry/src/Metrics/CoroutineServerStats.php (1)

42-50: 建议为魔术方法添加类型声明。

__get__set 方法缺少类型声明,这可能影响静态分析工具的效果。

-    public function __get($name)
+    public function __get(string $name): mixed
     {
         return $this->stats[$name] ?? 0;
     }

-    public function __set($name, $value)
+    public function __set(string $name, mixed $value): void
     {
         $this->stats[$name] = $value;
     }
src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (1)

98-99: 资源使用指标的单位不一致

gc_status()getrusage() 返回的数据包含不同类型的值(计数、时间、字节等),但这里统一使用 Unit::second() 作为单位,这可能导致指标语义不清晰。

考虑根据具体指标类型使用不同的单位,或者移除单位参数让 Sentry 自动处理。

src/sentry/src/Metrics/Listener/RequestWatcher.php (1)

35-42: 缺少 isMetricsEnabled() 检查

与其他监听器(如 OnCoroutineServerStartQueueWatcher)不同,此监听器没有检查 Feature::isMetricsEnabled()。如果指标功能被禁用,仍然会执行计数操作,虽然这些统计数据可能不会被使用。

建议添加特性检查以保持一致性并避免不必要的开销:

+    public function __construct(
+        protected CoroutineServerStats $stats,
+        protected Feature $feature,
+    ) {
+    }
+
     public function process(object $event): void
     {
+        if (! $this->feature->isMetricsEnabled()) {
+            return;
+        }
+
         match (true) {
src/sentry/src/Metrics/Listener/QueueWatcher.php (3)

56-60: 定时器回调中重复获取容器服务

每秒钟的定时器回调都会从容器中获取 ConfigInterfaceDriverFactory,这是不必要的开销。应该在 process() 方法中提前获取并缓存这些依赖。

     public function process(object $event): void
     {
         if (! $this->feature->isMetricsEnabled()) {
             return;
         }

+        $config = $this->container->get(ConfigInterface::class);
+        $driverFactory = $this->container->get(DriverFactory::class);
+        $queues = $config->get('async_queue', []);
+
-        $timerId = $this->timer->tick(1, function () {
-            $config = $this->container->get(ConfigInterface::class);
-            foreach ($config->get('async_queue', []) as $name => $options) {
-                $queue = $this->container->get(DriverFactory::class)->get($name);
+        $timerId = $this->timer->tick(1, function () use ($driverFactory, $queues) {
+            foreach (array_keys($queues) as $name) {
+                $queue = $driverFactory->get($name);

62-85: 队列计数指标使用了错误的单位

queue_waitingqueue_delayedqueue_failedqueue_timeout 是计数指标,使用 Unit::second() 作为单位在语义上不正确。

建议移除单位参数或使用更合适的单位:

                 TraceMetrics::getInstance()->gauge(
                     'queue_waiting',
                     (float) $info['waiting'],
-                    ['queue' => $name],
-                    \Sentry\Unit::second()
+                    ['queue' => $name]
                 );

58-58: 未使用的 $options 变量

静态分析工具正确地识别了 $options 变量未被使用。可以使用 array_keys() 来简化循环。

-            foreach ($config->get('async_queue', []) as $name => $options) {
+            foreach (array_keys($config->get('async_queue', [])) as $name) {
src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (2)

83-88: 冗余的类型检查

$this->container->get(SwooleServer::class) 返回的对象在容器正确配置的情况下必然是 SwooleServer 实例,第85行的 instanceof 检查是冗余的。

可以简化为:

         if (! Context::get(\FriendsOfHyperf\Sentry\Constants::RUN_IN_COMMAND, false)) {
-            if ($this->container->has(SwooleServer::class) && $server = $this->container->get(SwooleServer::class)) {
-                if ($server instanceof SwooleServer) {
-                    $serverStatsFactory = fn (): array => $server->stats();
-                }
+            if ($this->container->has(SwooleServer::class)) {
+                $server = $this->container->get(SwooleServer::class);
+                $serverStatsFactory = fn (): array => $server->stats();
             }

但如果这是防御性编程以应对容器配置错误,也可以保留。


114-125: 内存指标使用了不正确的单位

OnCoroutineServerStart 相同的问题,metric_process_memory_usagemetric_process_memory_peak_usage 使用 Unit::second() 作为单位,但这些是字节值。

             TraceMetrics::getInstance()->gauge(
                 'metric_process_memory_usage',
                 (float) memory_get_usage(),
                 ['worker' => '0'],
-                Unit::second()
+                Unit::byte()
             );
             TraceMetrics::getInstance()->gauge(
                 'metric_process_memory_peak_usage',
                 (float) memory_get_peak_usage(),
                 ['worker' => '0'],
-                Unit::second()
+                Unit::byte()
             );
src/sentry/src/Metrics/Listener/PoolWatcher.php (2)

64-90: 连接池指标使用了不正确的单位

_connections_in_use_connections_in_waiting_max_connections 是连接数量计数,使用 Unit::second() 在语义上不正确。

建议移除单位参数,让这些指标成为无单位的计数值:

             TraceMetrics::getInstance()->gauge(
                 $this->getPrefix() . '_connections_in_use',
                 (float) $pool->getCurrentConnections(),
                 [
                     'pool' => $poolName,
                     'worker' => (string) $workerId,
-                ],
-                Unit::second()
+                ]
             );

这个问题在整个 PR 中多处出现,建议统一修复。


53-53: 缺少返回类型声明

watch() 方法缺少返回类型声明,建议添加 : void 以保持与其他方法的一致性。

-    public function watch(Pool $pool, string $poolName, int $workerId)
+    public function watch(Pool $pool, string $poolName, int $workerId): void
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14c9808 and 255a74f.

📒 Files selected for processing (15)
  • src/sentry/src/ConfigProvider.php (1 hunks)
  • src/sentry/src/Constants.php (1 hunks)
  • src/sentry/src/Feature.php (1 hunks)
  • src/sentry/src/Metrics/CoroutineServerStats.php (1 hunks)
  • src/sentry/src/Metrics/Event/MetricFactoryReady.php (1 hunks)
  • src/sentry/src/Metrics/Listener/DBPoolWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Listener/OnBeforeHandle.php (1 hunks)
  • src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (1 hunks)
  • src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (1 hunks)
  • src/sentry/src/Metrics/Listener/OnWorkerStart.php (1 hunks)
  • src/sentry/src/Metrics/Listener/PoolWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Listener/QueueWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Listener/RequestWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Traits/MetricSetter.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
src/sentry/src/Metrics/Listener/DBPoolWatcher.php (2)
src/sentry/src/Metrics/Listener/PoolWatcher.php (4)
  • PoolWatcher (27-97)
  • getPrefix (49-49)
  • process (51-51)
  • watch (53-96)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (2)
  • getPrefix (19-22)
  • process (24-36)
src/sentry/src/Metrics/Listener/OnBeforeHandle.php (1)
src/sentry/src/Constants.php (1)
  • Constants (14-43)
src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (5)
src/sentry/src/Feature.php (3)
  • Feature (18-80)
  • __construct (20-22)
  • isMetricsEnabled (34-37)
src/sentry/src/Metrics/CoroutineServerStats.php (2)
  • CoroutineServerStats (35-56)
  • toArray (52-55)
src/sentry/src/Metrics/Event/MetricFactoryReady.php (2)
  • MetricFactoryReady (14-19)
  • __construct (16-18)
src/sentry/src/Constants.php (1)
  • Constants (14-43)
src/sentry/src/Metrics/Traits/MetricSetter.php (1)
  • trySet (19-32)
src/sentry/src/Metrics/Event/MetricFactoryReady.php (2)
src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (1)
  • __construct (36-41)
src/sentry/src/Metrics/Listener/OnWorkerStart.php (1)
  • __construct (35-40)
src/sentry/src/Metrics/Listener/RequestWatcher.php (1)
src/sentry/src/Metrics/CoroutineServerStats.php (1)
  • CoroutineServerStats (35-56)
src/sentry/src/ConfigProvider.php (9)
src/sentry/src/Metrics/Listener/DBPoolWatcher.php (1)
  • DBPoolWatcher (17-38)
src/sentry/src/Metrics/Listener/OnBeforeHandle.php (1)
  • OnBeforeHandle (20-41)
src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (1)
  • OnCoroutineServerStart (28-120)
src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (1)
  • OnMetricFactoryReady (30-133)
src/sentry/src/Metrics/Listener/OnWorkerStart.php (1)
  • OnWorkerStart (29-125)
src/sentry/src/Metrics/Listener/PoolWatcher.php (1)
  • PoolWatcher (27-97)
src/sentry/src/Metrics/Listener/QueueWatcher.php (1)
  • QueueWatcher (26-93)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (1)
  • RedisPoolWatcher (17-37)
src/sentry/src/Metrics/Listener/RequestWatcher.php (1)
  • RequestWatcher (19-57)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (2)
src/sentry/src/Metrics/Listener/PoolWatcher.php (4)
  • PoolWatcher (27-97)
  • getPrefix (49-49)
  • process (51-51)
  • watch (53-96)
src/sentry/src/Metrics/Listener/DBPoolWatcher.php (2)
  • getPrefix (19-22)
  • process (24-37)
src/sentry/src/Metrics/Listener/QueueWatcher.php (3)
src/sentry/src/Feature.php (3)
  • Feature (18-80)
  • __construct (20-22)
  • isMetricsEnabled (34-37)
src/sentry/src/Metrics/Event/MetricFactoryReady.php (2)
  • MetricFactoryReady (14-19)
  • __construct (16-18)
src/facade/src/AsyncQueue.php (1)
  • AsyncQueue (27-50)
src/sentry/src/Metrics/Listener/PoolWatcher.php (2)
src/sentry/src/Metrics/Listener/DBPoolWatcher.php (2)
  • getPrefix (19-22)
  • process (24-37)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (2)
  • getPrefix (19-22)
  • process (24-36)
src/sentry/src/Metrics/Listener/OnWorkerStart.php (4)
src/sentry/src/Feature.php (3)
  • Feature (18-80)
  • __construct (20-22)
  • isMetricsEnabled (34-37)
src/sentry/src/Metrics/Event/MetricFactoryReady.php (2)
  • MetricFactoryReady (14-19)
  • __construct (16-18)
src/sentry/src/Constants.php (1)
  • Constants (14-43)
src/sentry/src/Metrics/Traits/MetricSetter.php (1)
  • trySet (19-32)
🪛 PHPMD (2.15.0)
src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php

50-50: Avoid unused parameters such as '$event'. (undefined)

(UnusedFormalParameter)

src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php

53-53: Avoid unused parameters such as '$event'. (undefined)

(UnusedFormalParameter)

src/sentry/src/Metrics/Listener/QueueWatcher.php

50-50: Avoid unused parameters such as '$event'. (undefined)

(UnusedFormalParameter)


58-58: Avoid unused local variables such as '$options'. (undefined)

(UnusedLocalVariable)

⏰ 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.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 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 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.0.2
🔇 Additional comments (15)
src/sentry/src/Feature.php (1)

34-37: LGTM!实现遵循现有模式。

新方法 isMetricsEnabled 与类中其他方法保持一致的实现风格。

需要注意的是,配置键命名略有不同:其他方法使用 sentry.enable.{key} 模式(如 isEnabled 方法),而此处使用 sentry.enable_metrics。如果这是有意为之以区分顶级功能开关,则可以接受;否则建议统一为 sentry.enable.metrics

src/sentry/src/Constants.php (1)

41-42: LGTM!常量定义符合规范。

新增的 RUN_IN_COMMAND 常量遵循现有的命名约定和值格式,用于在命令上下文中标识运行环境,与 OnBeforeHandleOnMetricFactoryReady 监听器配合使用。

src/sentry/src/Metrics/Event/MetricFactoryReady.php (1)

14-19: LGTM!简洁的事件类实现。

使用 PHP 8.0+ 的构造函数属性提升语法,代码简洁明了。作为事件载荷类,设计符合单一职责原则。

src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (1)

14-36: Implementation verified and correct.

The Hyperf\Redis\Pool\PoolFactory import is confirmed across multiple files in the codebase:

  • src/sentry/src/Tracing/Aspect/RedisAspect.php uses the identical factory class
  • src/sentry/src/Listener/EventHandleListener.php and Tracing variant both use the same factory (aliased as RedisPoolFactory)
  • The usage pattern $this->container->get(PoolFactory::class)->getPool($poolName) is consistent throughout

The implementation correctly mirrors DBPoolWatcher with appropriate substitutions ('redis' config key for Redis pools) and follows established patterns in the codebase.

src/sentry/src/Metrics/Traits/MetricSetter.php (1)

17-33: 实现简洁有效。

Trait 提供了统一的度量发布模式:$metrics 作为白名单过滤,$stats 提供实际值。这种设计使得调用方可以灵活控制哪些指标需要被发布。

src/sentry/src/Metrics/Listener/DBPoolWatcher.php (1)

17-37: 实现与 RedisPoolWatcher 保持一致,代码结构正确。

正确使用了 Hyperf\DbConnection\Pool\PoolFactorydatabases 配置键,与 RedisPoolWatcher 的模式保持对称。

src/sentry/src/Metrics/Listener/OnWorkerStart.php (1)

120-123: 定时器清理逻辑正确。

使用协程等待 WORKER_EXIT 信号后清理定时器,确保资源正确释放。

src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (3)

59-63: $running 标志提供了良好的重入保护

使用 $running 标志防止重复初始化定时器是正确的做法,确保了每个进程只会启动一次指标收集。


115-118: 定时器清理逻辑正确

通过协程等待 WORKER_EXIT 信号后清理定时器,确保了资源的正确释放。


101-112: I'll help you verify this review comment about the unit usage for memory metrics in the Sentry PHP SDK. Let me investigate whether Unit::byte() exists and if the suggested fix is appropriate.
<function_calls>

Search for Unit class definition and available unit constants in the Sentry SDK

#!/bin/bash

Find files related to Unit class in Sentry SDK

fd -t f "Unit\.php" --type f


</function_calls>

<function_calls>

Search for Unit class usage and definition patterns

#!/bin/bash

Search for Unit class definition and method calls

rg -n "class Unit" --type php


</function_calls>

<function_calls>

Check what other metric units are used in the codebase

#!/bin/bash

Search for Unit:: method calls to see what units are being used

rg -n "Unit::" --type php -A 0 -B 0 | head -50


</function_calls>

<function_calls>

Check the OnCoroutineServerStart.php file to understand the context better

#!/bin/bash

Read the OnCoroutineServerStart.php file to see imports and full context

cat -n "src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php" | head -120


</function_calls>

src/sentry/src/Metrics/Listener/RequestWatcher.php (1)

37-41: match 表达式结构清晰

使用 match 表达式根据事件类型分派处理逻辑,代码简洁易读。

src/sentry/src/Metrics/Listener/QueueWatcher.php (1)

88-91: 定时器清理逻辑正确实现

与其他监听器保持一致,正确地在协程中等待 WORKER_EXIT 后清理定时器。

src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (2)

107-113: 系统负载归一化计算正确

使用 sys_getloadavg()[0] / System::getCpuCoresNum() 对系统负载进行归一化是正确的做法,便于在不同配置的服务器之间进行比较。


103-105: Swoole Timer 检测逻辑良好

使用 class_exists('Swoole\Timer') 进行运行时检测,确保在非 Swoole 环境下不会出错。

src/sentry/src/Metrics/Listener/PoolWatcher.php (1)

27-51: 抽象类设计合理

PoolWatcher 作为抽象基类,通过 getPrefix() 抽象方法让子类(DBPoolWatcherRedisPoolWatcher)定义自己的指标前缀,同时复用 watch() 方法中的通用逻辑。这是良好的模板方法模式应用。

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds comprehensive trace metrics support to the friendsofhyperf/sentry component, enabling automatic collection and reporting of runtime performance metrics to Sentry. The feature is configurable via sentry.enable_metrics (default: enabled) and integrates seamlessly with the existing Hyperf event system.

Key Changes

  • Metrics Infrastructure: New event-driven architecture with MetricFactoryReady event, CoroutineServerStats for tracking server statistics, and MetricSetter trait for unified metric publishing
  • Runtime Collectors: Eight new listener classes collect metrics for database/Redis connection pools, async queues, request handling, worker processes, coroutine servers, and system resources
  • Integration Updates: Extended flush mechanism to include metrics alongside existing trace data, added singleton aspect support for Sentry metrics classes

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
src/sentry/publish/sentry.php Adds enable_metrics configuration option with environment variable support
src/sentry/src/Feature.php Implements isMetricsEnabled() method to check metrics feature flag
src/sentry/src/Constants.php Adds RUN_IN_COMMAND constant for command context detection
src/sentry/src/Integration.php Extends flushEvents() to flush trace metrics alongside logs
src/sentry/src/ConfigProvider.php Registers all new metric listener classes in the DI container
src/sentry/src/Aspect/SingletonAspect.php Adds AOP support for TraceMetrics, LogLevel, and Unit singletons
src/sentry/src/Metrics/Event/MetricFactoryReady.php Event dispatched when metric collection should start, carries worker ID
src/sentry/src/Metrics/CoroutineServerStats.php Lightweight stats container mimicking Swoole server stats for coroutine mode
src/sentry/src/Metrics/Traits/MetricSetter.php Reusable trait for batch-setting gauge metrics from arrays
src/sentry/src/Metrics/Listener/OnBeforeHandle.php Tracks command execution context to disable server metrics in CLI mode
src/sentry/src/Metrics/Listener/OnWorkerStart.php Collects worker-specific metrics (requests, memory, GC, resource usage)
src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php Collects metrics for coroutine server mode (memory, GC, resource usage)
src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php Collects global metrics (system load, coroutines, timers, connections)
src/sentry/src/Metrics/Listener/PoolWatcher.php Abstract base for monitoring connection pool metrics (in-use, waiting, max)
src/sentry/src/Metrics/Listener/DBPoolWatcher.php Monitors database connection pool metrics with 'mysql' prefix
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php Monitors Redis connection pool metrics with 'redis' prefix
src/sentry/src/Metrics/Listener/QueueWatcher.php Monitors async queue metrics (waiting, delayed, failed, timeout counts)
src/sentry/src/Metrics/Listener/RequestWatcher.php Tracks request lifecycle events to update connection and request counters

Copy link

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (4)
src/sentry/src/Metrics/Listener/OnWorkerStart.php (1)

91-93: 需要添加 Server::class 存在性检查。

直接调用 $this->container->get(Server::class) 而不检查容器中是否存在该服务可能导致异常。虽然 OnWorkerStart 监听 BeforeWorkerStart 事件(通常在 worker 模式下触发),但为了健壮性,应遵循 OnMetricFactoryReady 中使用的防御性模式。

         $timerId = $this->timer->tick($this->feature->getMetricsInterval(), function () use ($metrics, $event) {
+            if (! $this->container->has(Server::class)) {
+                return;
+            }
             $server = $this->container->get(Server::class);
             $serverStats = $server->stats();
src/sentry/src/Metrics/Listener/PoolWatcher.php (2)

48-48: getPrefix() 增加 DocBlock 说明前缀语义

这里作为抽象基类的约定,建议补充简短 DocBlock,说明前缀的含义和格式(例如 'redis''db',以及会组合成 {prefix}_connections_in_use 这类 metric 名),方便后续实现类保持一致。


55-95: watch() 补充返回类型声明 : void

watch() 只负责注册定时任务和清理,不会返回值,建议显式声明返回类型为 void,与接口约定和其它 Listener 保持一致,也提升静态分析友好度:

public function watch(Pool $pool, string $poolName, int $workerId): void

其余逻辑(特性开关、定时采集、在 WORKER_EXIT 时清理定时器)整体看起来合理。

src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (1)

34-41: $running 防重入标志在协程环境下仍存在理论竞态风险

当前通过私有布尔成员 $running 防止 process() 重复执行,一般情况下事件只会触发一次,问题不大。但在协程环境下,如果未来有并发触发的可能,多协程几乎同时看到 $running === false 仍可能各自继续执行一次。若希望更强保证,可以考虑使用更强的同步原语(如基于 Channel / 原子标志的封装),或者如果确定事件生命周期内只会调用一次,也可以直接移除该标志简化代码。

🧹 Nitpick comments (4)
src/sentry/publish/sentry.php (1)

43-45: 文档链接可能需要验证。

enable_metrics 的注释引用了 Sentry Laravel 配置选项文档,但 enable_metrics 可能是此包自定义的配置项,而非 Sentry SDK 原生支持的选项。如果链接指向的文档中不存在此选项,建议更新注释或移除链接以避免混淆。

此外,metrics_interval 缺少类似的文档注释,建议添加说明此配置项的用途(例如:指标收集的时间间隔,单位为秒)。

-    // @see: https://docs.sentry.io/platforms/php/guides/laravel/configuration/options/#enable_metrics
+    // Enable trace metrics collection (custom option, not part of Sentry SDK)
     'enable_metrics' => env('SENTRY_ENABLE_METRICS', true),
+    // Metrics collection interval in seconds
     'metrics_interval' => (int) env('SENTRY_METRICS_INTERVAL', 10),
src/sentry/src/Metrics/Listener/QueueWatcher.php (2)

50-54: 未使用的 $event 参数可用于提取 workerId

静态分析工具标记 $event 参数未使用。虽然当前实现可以工作,但为了与其他监听器保持一致性(如 OnMetricFactoryReadyOnWorkerStart),可以考虑使用 $event->workerId 作为 gauge 标签。

     public function process(object $event): void
     {
         if (! $this->feature->isMetricsEnabled()) {
             return;
         }

-        $timerId = $this->timer->tick($this->feature->getMetricsInterval(), function () {
+        $workerId = $event->workerId;
+        $timerId = $this->timer->tick($this->feature->getMetricsInterval(), function () use ($workerId) {
             $config = $this->container->get(ConfigInterface::class);
             $queues = array_keys($config->get('async_queue', []));

             foreach ($queues as $name) {
                 $queue = $this->container->get(DriverFactory::class)->get($name);
                 $info = $queue->info();

                 TraceMetrics::getInstance()->gauge(
                     'queue_waiting',
                     (float) $info['waiting'],
-                    ['queue' => $name]
+                    ['queue' => $name, 'worker' => (string) $workerId]
                 );
                 // ... 其他 gauge 调用同理

60-84: 建议添加防御性检查以处理队列信息获取失败的情况。

如果 $queue->info() 返回的数组缺少预期的键(如 waitingdelayed 等),或者 DriverFactory::get() 抛出异常,当前代码可能会导致错误。

             foreach ($queues as $name) {
-                $queue = $this->container->get(DriverFactory::class)->get($name);
-                $info = $queue->info();
+                try {
+                    $queue = $this->container->get(DriverFactory::class)->get($name);
+                    $info = $queue->info();
+                } catch (\Throwable) {
+                    continue;
+                }

                 TraceMetrics::getInstance()->gauge(
                     'queue_waiting',
-                    (float) $info['waiting'],
+                    (float) ($info['waiting'] ?? 0),
                     ['queue' => $name]
                 );
                 TraceMetrics::getInstance()->gauge(
                     'queue_delayed',
-                    (float) $info['delayed'],
+                    (float) ($info['delayed'] ?? 0),
                     ['queue' => $name]
                 );
                 TraceMetrics::getInstance()->gauge(
                     'queue_failed',
-                    (float) $info['failed'],
+                    (float) ($info['failed'] ?? 0),
                     ['queue' => $name]
                 );
                 TraceMetrics::getInstance()->gauge(
                     'queue_timeout',
-                    (float) $info['timeout'],
+                    (float) ($info['timeout'] ?? 0),
                     ['queue' => $name]
                 );
             }
src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (1)

53-67: process()$event 未使用,可考虑显式抑制 PHPMD 提示

根据静态分析提示,$event 参数当前未被使用,但方法签名受 ListenerInterface 约束无法移除。为避免 PHPMD 的 UnusedFormalParameter 报警,可以考虑:

  • 在方法 DocBlock 上增加抑制注解,例如:

    /**
     * @param object|MainCoroutineServerStart $event
     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
     */
    public function process(object $event): void
  • 或在方法开头显式 unset($event); 标明该参数故意不使用。

两种方式都能让工具安静下来,按你们项目约定选一种即可。

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94cd380 and 082920d.

📒 Files selected for processing (10)
  • src/sentry/publish/sentry.php (1 hunks)
  • src/sentry/src/Factory/ClientBuilderFactory.php (1 hunks)
  • src/sentry/src/Feature.php (1 hunks)
  • src/sentry/src/Metrics/Listener/DBPoolWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (1 hunks)
  • src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (1 hunks)
  • src/sentry/src/Metrics/Listener/OnWorkerStart.php (1 hunks)
  • src/sentry/src/Metrics/Listener/PoolWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Listener/QueueWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (2)
src/sentry/src/Metrics/Listener/PoolWatcher.php (4)
  • PoolWatcher (26-96)
  • getPrefix (48-48)
  • process (53-53)
  • watch (55-95)
src/sentry/src/Metrics/Listener/DBPoolWatcher.php (2)
  • getPrefix (21-24)
  • process (29-42)
src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (4)
src/sentry/src/Feature.php (4)
  • Feature (18-85)
  • __construct (20-22)
  • isMetricsEnabled (34-37)
  • getMetricsInterval (39-42)
src/sentry/src/Metrics/Event/MetricFactoryReady.php (2)
  • MetricFactoryReady (14-19)
  • __construct (16-18)
src/sentry/src/Constants.php (1)
  • Constants (14-43)
src/sentry/src/Metrics/Traits/MetricSetter.php (1)
  • trySet (19-32)
src/sentry/src/Metrics/Listener/DBPoolWatcher.php (2)
src/sentry/src/Metrics/Listener/PoolWatcher.php (4)
  • PoolWatcher (26-96)
  • getPrefix (48-48)
  • process (53-53)
  • watch (55-95)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (2)
  • getPrefix (21-24)
  • process (29-41)
src/sentry/src/Metrics/Listener/QueueWatcher.php (3)
src/sentry/src/Feature.php (4)
  • Feature (18-85)
  • __construct (20-22)
  • isMetricsEnabled (34-37)
  • getMetricsInterval (39-42)
src/sentry/src/Metrics/Event/MetricFactoryReady.php (2)
  • MetricFactoryReady (14-19)
  • __construct (16-18)
src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (3)
  • __construct (36-41)
  • listen (43-48)
  • process (53-135)
src/sentry/src/Metrics/Listener/OnWorkerStart.php (4)
src/sentry/src/Feature.php (4)
  • Feature (18-85)
  • __construct (20-22)
  • isMetricsEnabled (34-37)
  • getMetricsInterval (39-42)
src/sentry/src/Metrics/Event/MetricFactoryReady.php (2)
  • MetricFactoryReady (14-19)
  • __construct (16-18)
src/sentry/src/Constants.php (1)
  • Constants (14-43)
src/sentry/src/Metrics/Traits/MetricSetter.php (1)
  • trySet (19-32)
src/sentry/src/Metrics/Listener/PoolWatcher.php (2)
src/sentry/src/Metrics/Listener/DBPoolWatcher.php (1)
  • getPrefix (21-24)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (1)
  • getPrefix (21-24)
src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (5)
src/sentry/src/Feature.php (4)
  • Feature (18-85)
  • __construct (20-22)
  • isMetricsEnabled (34-37)
  • getMetricsInterval (39-42)
src/sentry/src/Metrics/Event/MetricFactoryReady.php (2)
  • MetricFactoryReady (14-19)
  • __construct (16-18)
src/sentry/src/Constants.php (1)
  • Constants (14-43)
src/sentry/src/Metrics/Listener/OnWorkerStart.php (3)
  • __construct (35-40)
  • listen (42-47)
  • process (52-125)
src/sentry/src/Metrics/Traits/MetricSetter.php (1)
  • trySet (19-32)
🪛 PHPMD (2.15.0)
src/sentry/src/Metrics/Listener/QueueWatcher.php

50-50: Avoid unused parameters such as '$event'. (undefined)

(UnusedFormalParameter)

src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php

53-53: Avoid unused parameters such as '$event'. (undefined)

(UnusedFormalParameter)

⏰ 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). (7)
  • GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
  • GitHub Check: Test on PHP 8.2 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 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: Test on PHP 8.3 with Swoole 5.1.8
🔇 Additional comments (9)
src/sentry/src/Feature.php (1)

34-42: 实现正确,遵循既有模式。

新增的两个方法与类中其他配置读取方法(如 isEnabledisBreadcrumbEnabled)保持一致的模式。类型转换和默认值设置合理。

src/sentry/src/Metrics/Listener/DBPoolWatcher.php (1)

19-43: 实现正确,遵循 PoolWatcher 模式。

DBPoolWatcher 正确扩展了 PoolWatcher 抽象类,使用通用的 'db' 前缀(而非硬编码的数据库类型),与 RedisPoolWatcher 的实现模式保持一致。

src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (1)

19-42: 实现正确,与 DBPoolWatcher 保持一致。

RedisPoolWatcher 正确实现了 PoolWatcher 抽象类,使用 Hyperf\Redis\Pool\PoolFactory 获取 Redis 连接池,与 DBPoolWatcher 的实现模式一致。

src/sentry/src/Metrics/Listener/OnWorkerStart.php (1)

97-118: 指标收集逻辑正确,单位使用合理。

内存指标正确使用 Unit::byte(),计数指标正确省略了单位参数。这与之前审查意见中指出的问题已得到修复。

src/sentry/src/Metrics/Listener/OnMetricFactoryReady.php (2)

85-97: 服务器统计工厂的防御性检查模式良好。

正确使用 container->has() 检查 SwooleServer::class 是否存在,并提供 CoroutineServerStats 作为回退方案。这种模式确保了在不同运行环境下的兼容性。


99-129: 指标收集实现正确。

  • trySet 调用用于收集计数类指标,正确地未指定单位
  • sys_load 是无量纲比率,正确地未指定单位
  • 内存指标使用 Unit::byte(),符合语义
src/sentry/src/Factory/ClientBuilderFactory.php (1)

34-34: The original review comment's concern about enable_metrics is unfounded. enable_metrics is a framework-level feature flag used by the component to control its own metrics functionality (as seen in Feature::isMetricsEnabled()), not a Sentry SDK configuration option. It should not be added to SPECIFIC_OPTIONS.

The pattern is consistent: enable_logs is similarly excluded from SPECIFIC_OPTIONS because it's also a framework feature flag, not a Sentry SDK option. Only Sentry SDK configuration keys (like metrics_interval, logs_channel_level, etc.) are in SPECIFIC_OPTIONS to prevent them from being passed to the SDK during initialization.

The addition of metrics_interval to SPECIFIC_OPTIONS (line 34) is correct because it's a Sentry SDK option, while enable_metrics correctly remains outside this list since it's only used internally by the component's Feature class.

src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (2)

28-48: 整体结构与现有 Listener 保持一致,构造与监听事件设计合理

构造函数里初始化 Timer,并通过依赖注入拿到 ContainerInterfaceFeaturelisten() 只关注 MainCoroutineServerStart 事件,这与类职责(在协程服务器启动时初始化度量采集)是对齐的,实现看起来没问题。


68-118: GC / rusage / 内存度量指标采集逻辑清晰,单位选择正确

这里通过 $metrics 白名单配合 MetricSetter::trySet()gc_status()getrusage() 中挑选需要的字段,并且对 memory_usage / memory_peak_usage 使用 Unit::byte(),与指标语义一致;定时器周期由 Feature::getMetricsInterval() 控制,并在 WORKER_EXIT 时清理定时任务,整体实现合理且易于维护。

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (1)

59-63: $running 标志存在潜在的竞态条件。

在协程环境中,多个协程可能在任何一个将其设置为 true 之前同时检查 $this->running,导致竞态条件。建议使用原子操作或互斥锁机制确保只执行一次。

src/sentry/src/Metrics/Listener/PoolWatcher.php (1)

60-60: 添加缺失的返回类型声明。

方法 watch() 缺少返回类型声明。由于该方法不返回任何值,应声明为 void

应用以下差异:

-    public function watch(Pool $pool, string $poolName, int $workerId)
+    public function watch(Pool $pool, string $poolName, int $workerId): void
🧹 Nitpick comments (2)
src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (2)

51-53: 考虑抑制或使用未使用的 $event 参数。

静态分析工具标记 $event 参数未被使用。虽然这在事件监听器中很常见,但可以通过添加 @SuppressWarnings(PHPMD.UnusedFormalParameter) 注解来明确表示这是有意为之。

     /**
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      * @param object|MainCoroutineServerStart $event
      */
     public function process(object $event): void

97-113: 建议为定时器回调添加异常处理和平台兼容性检查。

getrusage() 在 Windows 平台返回 false。虽然 trySet 可能会处理这种情况,但建议添加显式检查。此外,如果回调中的任何操作抛出异常,可能会影响后续的指标收集。

         $timerId = $this->timer->tick($this->feature->getMetricsInterval(), function () use ($metrics) {
-            $this->trySet('gc_', $metrics, gc_status());
-            $this->trySet('', $metrics, getrusage());
+            try {
+                $this->trySet('gc_', $metrics, gc_status());
+                
+                $rusage = getrusage();
+                if ($rusage !== false) {
+                    $this->trySet('', $metrics, $rusage);
+                }
 
-            TraceMetrics::getInstance()->gauge(
-                'memory_usage',
-                (float) memory_get_usage(),
-                [],
-                Unit::byte()
-            );
-            TraceMetrics::getInstance()->gauge(
-                'memory_peak_usage',
-                (float) memory_get_peak_usage(),
-                [],
-                Unit::byte()
-            );
+                TraceMetrics::getInstance()->gauge(
+                    'memory_usage',
+                    (float) memory_get_usage(),
+                    [],
+                    Unit::byte()
+                );
+                TraceMetrics::getInstance()->gauge(
+                    'memory_peak_usage',
+                    (float) memory_get_peak_usage(),
+                    [],
+                    Unit::byte()
+                );
+            } catch (\Throwable $e) {
+                // Log or handle metric collection errors gracefully
+            }
         });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 082920d and 60f7e90.

📒 Files selected for processing (2)
  • src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (1 hunks)
  • src/sentry/src/Metrics/Listener/PoolWatcher.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (2)
src/sentry/src/Feature.php (4)
  • Feature (18-85)
  • __construct (20-22)
  • isMetricsEnabled (34-37)
  • getMetricsInterval (39-42)
src/sentry/src/Metrics/Event/MetricFactoryReady.php (2)
  • MetricFactoryReady (14-19)
  • __construct (16-18)
src/sentry/src/Metrics/Listener/PoolWatcher.php (2)
src/sentry/src/Metrics/Listener/DBPoolWatcher.php (2)
  • getPrefix (21-24)
  • process (29-42)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (2)
  • getPrefix (21-24)
  • process (29-41)
🪛 PHPMD (2.15.0)
src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php

53-53: Avoid unused parameters such as '$event'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (3)
src/sentry/src/Metrics/Listener/OnCoroutineServerStart.php (2)

101-112: 内存指标单位使用正确。

内存使用量指标现已正确使用 Unit::byte(),与之前审查中指出的问题相比已得到修复。


115-118: 清理逻辑实现良好。

使用协程在 WORKER_EXIT 事件时清理定时器的方式是正确的,能够防止资源泄漏。

src/sentry/src/Metrics/Listener/PoolWatcher.php (1)

60-101: 实现逻辑正确!

watch() 方法的实现是合理的:

  • 正确使用 feature flag 控制指标采集
  • 定时器回调中按规范发出三个 gauge 指标(使用中连接数、等待连接数、最大连接数)
  • 标签(pool、worker)在所有指标中保持一致
  • 通过协程在 worker 退出时正确清理定时器,避免资源泄漏

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/sentry/src/Feature.php (1)

34-48: 实现正确,代码风格一致。

两个新方法 isMetricsEnabled()getMetricsInterval() 遵循了类中其他方法的模式。getMetricsInterval() 中强制最小值为 5 秒的逻辑合理,可防止过于频繁的指标采集。

关于测试覆盖率的问题已在之前的评审中提出。

🧹 Nitpick comments (2)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (1)

34-41: 可选:将 $workerId 计算移到循环外以简化代码

$workerId$poolName 无关,目前在 foreach 内每次都重复计算一次,虽然开销极小,但挪到循环外可读性更好一些:

-        foreach ($poolNames as $poolName) {
-            $workerId = (int) ($event->workerId ?? 0);
+        $workerId = (int) ($event->workerId ?? 0);
+
+        foreach ($poolNames as $poolName) {
             $pool = $this
                 ->container
                 ->get(PoolFactory::class)
                 ->getPool($poolName);
             $this->watch($pool, $poolName, $workerId);
         }

属于纯粹的代码风格优化,可按团队习惯决定是否采纳。

src/sentry/src/Metrics/Aspect/CounterAspect.php (1)

57-65: 命名空间分隔符未被正确处理。

fromCamelCase() 方法的正则表达式不匹配反斜杠 \,当 $proceedingJoinPoint->className 包含完整命名空间时(如 App\Service\UserService::getUser),反斜杠会被丢弃,可能导致意外的指标名称。

考虑以下方案之一:

  1. 仅使用短类名:
-$source = $this->fromCamelCase($proceedingJoinPoint->className . '::' . $proceedingJoinPoint->methodName);
+$shortClassName = substr($proceedingJoinPoint->className, strrpos($proceedingJoinPoint->className, '\\') + 1);
+$source = $this->fromCamelCase($shortClassName . '::' . $proceedingJoinPoint->methodName);
  1. 或在 fromCamelCase() 中先处理命名空间分隔符:
 private function fromCamelCase(string $input): string
 {
+    $input = str_replace('\\', '.', $input);
     preg_match_all('!([A-Z][A-Z0-9]*(?=$|[A-Z][a-z0-9])|[A-Za-z][a-z0-9]+)!', $input, $matches);
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60f7e90 and 87c9180.

📒 Files selected for processing (5)
  • src/sentry/src/Feature.php (1 hunks)
  • src/sentry/src/Metrics/Annotation/Counter.php (1 hunks)
  • src/sentry/src/Metrics/Aspect/CounterAspect.php (1 hunks)
  • src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (1 hunks)
  • src/sentry/src/Metrics/Listener/RequestWatcher.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/sentry/src/Metrics/Listener/RequestWatcher.php
🧰 Additional context used
🧬 Code graph analysis (2)
src/sentry/src/Metrics/Annotation/Counter.php (1)
src/sentry/src/Metrics/Aspect/CounterAspect.php (1)
  • __construct (28-30)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (2)
src/sentry/src/Metrics/Listener/PoolWatcher.php (4)
  • PoolWatcher (26-102)
  • getPrefix (53-53)
  • process (58-58)
  • watch (60-101)
src/sentry/src/Metrics/Listener/DBPoolWatcher.php (2)
  • getPrefix (21-24)
  • process (29-42)
⏰ 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.1 with Swoole 6.1.3
  • GitHub Check: Test on PHP 8.3 with Swoole 6.1.3
  • GitHub Check: Test on PHP 8.3 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 5.1.8
  • 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.3 with Swoole 6.0.2
  • GitHub Check: Test on PHP 8.2 with Swoole 6.1.3
  • 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.2 with Swoole 6.1.3
  • GitHub Check: Test on PHP 8.1 with Swoole 6.1.3
  • GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
  • GitHub Check: Test on PHP 8.3 with Swoole 6.1.3
  • GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
  • GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
  • GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
🔇 Additional comments (2)
src/sentry/src/Metrics/Listener/RedisPoolWatcher.php (1)

19-42: RedisPoolWatcher 实现与既有 PoolWatcher 模式保持一致,逻辑清晰

整体实现与 DBPoolWatcher 的模式完全对齐:按配置枚举 redis 连接池,通过 PoolFactory 拿到 Pool 实例并委托给基类 watch(),前缀也正确返回 redis,符合指标命名约定,目前看不到明显的功能或性能问题。

src/sentry/src/Metrics/Annotation/Counter.php (1)

17-23: 实现简洁正确。

注解类设计合理,支持 CLASSMETHOD 两个目标,与 CounterAspect 的使用方式一致。可选的 $name 参数允许自定义指标名称或自动生成。

@huangdijia huangdijia marked this pull request as ready for review November 28, 2025 05:36
@huangdijia huangdijia merged commit 9cdbcdf into main Nov 28, 2025
22 checks passed
@huangdijia huangdijia deleted the sentry-aspect-singleton-updates branch November 28, 2025 05:36
huangdijia added a commit that referenced this pull request Nov 28, 2025
* feat: add additional Sentry singleton classes to aspect configuration

Add support for new Sentry singleton classes:
- LogLevel::getInstance
- Metrics::getInstance
- MetricsUnit::getInstance
- Unit::getInstance

This ensures proper aspect-based handling for these Sentry components.

* fix: reorder Sentry singleton classes in SingletonAspect

* fix: update SingletonAspect to use TraceMetrics singleton instance

* feat: add enable_metrics configuration option to Sentry settings

* feat: add metrics tracking and related listeners for enhanced performance monitoring

* feat: add new metrics listeners for enhanced monitoring capabilities

* fix: remove PoolWatcher listener from ConfigProvider

* feat: update event parameter types in metrics listeners for improved clarity

* fix: optimize queue retrieval in QueueWatcher listener

* feat: add metrics interval configuration and update timer intervals in listeners

* feat: add metrics interval configuration to Sentry settings

* feat: remove unnecessary comment and improve PoolWatcher with metric name prefix method

* fix: add return type declaration to watch method in PoolWatcher class

* fix: add missing newline for better code readability in PoolWatcher class

* feat: enforce minimum metrics interval of 5 in getMetricsInterval method

* refactor: simplify process method in RequestWatcher class by removing match expression

* fix: add missing newline for improved readability in RedisPoolWatcher class

* feat: add Counter and CounterAspect classes for metrics annotation and processing

* feat: add CounterAspect to ConfigProvider for metrics tracking

* feat: add HistogramAspect and Histogram annotation for enhanced metrics tracking

* feat: add Unit::second() to HistogramAspect for improved metrics tracking

* feat: enhance RequestWatcher to track HTTP request metrics with TraceMetrics

* feat: add defer calls to flush TraceMetrics in various aspects and listeners

---------

Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants