Skip to content

Conversation

@huangdijia
Copy link
Contributor

@huangdijia huangdijia commented Sep 18, 2025

Summary

  • Add custom FriendsOfHyperf\Sentry\Monolog\LogsHandler implementation
  • Update documentation to reference the new custom handler
  • Enhance Monolog integration with proper configuration parameters

Changes

  • New: src/sentry/src/Monolog/LogsHandler.php - Custom Monolog handler for Sentry integration
  • Updated: src/sentry/README.md - Configuration example now uses custom handler with group and bubble parameters
  • Updated: src/sentry/src/SentryHandler.php - Deprecation notice updated to point to new custom handler

Configuration

The new handler supports additional configuration options:

  • group: Sentry log group classification
  • bubble: Whether to bubble the log record up the handler stack

Test plan

  • Verify the new Monolog handler integrates properly with Sentry
  • Test log grouping functionality
  • Ensure backward compatibility with existing configurations
  • Validate that deprecation warnings guide users to the correct new handler

Summary by CodeRabbit

  • 新功能
    • 新增日志处理器,将应用日志转发至监控平台,支持分组聚合与可配置的冒泡传播;兼容现有日志级别映射与记录结构。
  • 文档
    • 更新注册示例以使用新的日志处理器。
    • 增加配置示例,包含 group 与 bubble 选项,并保留 level 配置。
    • 调整弃用说明以指向新的处理器。

- Add FriendsOfHyperf\Sentry\Monolog\LogsHandler as custom implementation
- Update README.md to reference new handler class with proper configuration
- Update SentryHandler deprecation notice to point to new custom handler
- Enhance Monolog integration with group and bubble parameters
@coderabbitai
Copy link

coderabbitai bot commented Sep 18, 2025

Walkthrough

新增 FriendsOfHyperf\Sentry\Monolog\LogsHandler 类以桥接 Monolog 到 Sentry Logs 聚合器,支持分组与冒泡控制;README 中示例切换到该类并增加 group、bubble 配置;SentryHandler 的弃用注释目标更新为新类,未改动逻辑。

Changes

Cohort / File(s) Summary
Docs 示例更新
src/sentry/README.md
README 中注册 Logger Handler 示例由 \Sentry\Monolog\LogsHandler::class 改为 \FriendsOfHyperf\Sentry\Monolog\LogsHandler::class;构造配置新增 'group' => 'sentry''bubble' => true
新 Monolog→Sentry 桥接处理器
src/sentry/src/Monolog/LogsHandler.php
新增 FriendsOfHyperf\Sentry\Monolog\LogsHandler(继承 Sentry 原 LogsHandler)。构造签名:__construct(string $group = 'default', ?LogLevel $logLevel = null, bool $bubble = true);实现 handle($record): bool,映射级别、合并 context/extra/channel,写入 Sentry Logs aggregator;当 context 中含 Throwable 的 exception 时跳过;依据 bubble 控制传播。
弃用说明调整
src/sentry/src/SentryHandler.php
Docblock 的弃用指向由 Sentry\Monolog\LogsHandler 改为 FriendsOfHyperf\Sentry\Monolog\LogsHandler,无行为改动。

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Application
  participant ML as Monolog Logger
  participant FH as FriendsOfHyperf LogsHandler
  participant SA as Sentry Logs Aggregator
  participant S as Sentry

  Dev->>ML: log(level, message, context, extra)
  ML->>FH: handle(record)
  alt record.context.exception is Throwable
    note right of FH: 跳过转发
    FH-->>ML: return false
  else 可处理
    FH->>SA: aggregator.add(level, message, {}, meta{context, extra, logger, group})
    SA->>S: store/ingest log
    FH-->>ML: return (bubble == false)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

小兔敲键轻如风,
新桥一架日志通。
分组成行云里走,
冒泡与否耳边风。
异常掠过不作声,
Sentry 篇章再添工。

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.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 PR 标题 "feat(sentry): add custom Monolog LogsHandler implementation" 简洁且聚焦,明确指出新增自定义 Monolog LogsHandler 的功能,与变更集中新增类、README 更新和废弃说明的主要目的直接对应,能够让审阅者快速理解主要变更并与提交内容匹配,因此满足标题检查标准。
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/sentry-custom-monolog-handler

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.17)

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.

❤️ Share

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

@huangdijia huangdijia merged commit 7e5b9d0 into main Sep 18, 2025
16 of 17 checks passed
@huangdijia huangdijia deleted the feat/sentry-custom-monolog-handler branch September 18, 2025 09:56
huangdijia added a commit that referenced this pull request Sep 18, 2025
- Add FriendsOfHyperf\Sentry\Monolog\LogsHandler as custom implementation
- Update README.md to reference new handler class with proper configuration
- Update SentryHandler deprecation notice to point to new custom handler
- Enhance Monolog integration with group and bubble parameters

Co-Authored-By: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
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

🧹 Nitpick comments (3)
src/sentry/src/SentryHandler.php (1)

30-31: 弃用说明已对齐新类,LGTM;建议加入运行期弃用提示

文档注释已指向新类,建议在构造函数触发 E_USER_DEPRECATED,便于使用方在运行期发现与迁移。

 public function __construct(
     $level = Logger::DEBUG,
     bool $bubble = true,
     protected bool $reportExceptions = true,
     protected bool $useFormattedMessage = false
 ) {
+    @trigger_error(
+        'FriendsOfHyperf\\Sentry\\SentryHandler 自 v3.1 起已弃用,将在 v3.2 移除,请改用 FriendsOfHyperf\\Sentry\\Monolog\\LogsHandler。',
+        E_USER_DEPRECATED
+    );
     parent::__construct($level, $bubble); /* @phpstan-ignore-line */
src/sentry/README.md (1)

33-38: 配置键名与构造函数参数名存在潜在不一致,需确认 DI 传参与示例一致

示例里使用了键名 level/bubble,但新构造函数参数为 (?LogLevel $logLevel, bool $bubble)。若容器按“键名映射参数名”注入,level 将无法匹配 $logLevel,建议:

  • 将构造函数参数名由 $logLevel 重命名为 $level;或
  • 在 README 明确说明此处为“按位置参数”注入,键名仅为可读性。

当前示例不会影响 group/bubble,但可能导致 level 未生效。

src/sentry/src/Monolog/LogsHandler.php (1)

41-43: 拼写修正:“seperately” → “separately”

仅注释拼写问题,避免在代码搜索时造成困扰。

📜 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 6963543 and 5234061.

📒 Files selected for processing (3)
  • src/sentry/README.md (1 hunks)
  • src/sentry/src/Monolog/LogsHandler.php (1 hunks)
  • src/sentry/src/SentryHandler.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/Monolog/LogsHandler.php (1)
src/sentry/src/SentryHandler.php (1)
  • __construct (56-72)
⏰ 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.2 with Swoole 5.1.7
  • 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.3 with Swoole 5.1.7
  • GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
  • 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.1 with Swoole 5.1.7
  • GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
  • GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
  • GitHub Check: Test on PHP 8.2 with Swoole 6.0.2

Comment on lines +24 to +30
public function __construct(
protected string $group = 'default',
?LogLevel $logLevel = null,
protected bool $bubble = true
) {
parent::__construct($logLevel, $bubble);
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

避免属性遮蔽父类 $bubble;参数名与 README 键对齐

  • 本类重复声明了 protected bool $bubble,会遮蔽父类(Monolog AbstractHandler)同名属性,易造成后续维护歧义。
  • 将参数名 $logLevel 改为 $level(类型不变),与 README 示例键名一致,降低注入踩坑概率。
-    public function __construct(
-        protected string $group = 'default',
-        ?LogLevel $logLevel = null,
-        protected bool $bubble = true
-    ) {
-        parent::__construct($logLevel, $bubble);
-    }
+    public function __construct(
+        protected string $group = 'default',
+        ?LogLevel $level = null,
+        bool $bubble = true
+    ) {
+        parent::__construct($level, $bubble);
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function __construct(
protected string $group = 'default',
?LogLevel $logLevel = null,
protected bool $bubble = true
) {
parent::__construct($logLevel, $bubble);
}
public function __construct(
protected string $group = 'default',
?LogLevel $level = null,
bool $bubble = true
) {
parent::__construct($level, $bubble);
}
🤖 Prompt for AI Agents
In src/sentry/src/Monolog/LogsHandler.php around lines 24-30, the constructor
currently shadows the parent AbstractHandler's $bubble property and uses
parameter name $logLevel that differs from README; change the constructor to
accept ?LogLevel $level instead of $logLevel and remove the protected
declaration from $bubble so it is a plain parameter (not a class property) to
avoid shadowing the parent property; call parent::__construct($level, $bubble)
unchanged and keep the protected string $group property as-is.

Comment on lines +35 to +58
public function handle($record): bool
{
if (! $this->isHandling($record)) {
return false;
}
// Do not collect logs for exceptions, they should be handled seperately by the `Handler` or `captureException`
if (isset($record['context']['exception']) && $record['context']['exception'] instanceof Throwable) {
return false;
}

Logs::getInstance()->aggregator()->add(
self::getSentryLogLevelFromMonologLevel($record['level']),
$record['message'],
[],
array_merge(
['log_context' => $record['context']],
['log_extra' => $record['extra']],
['logger' => $record['channel'] ?? ''],
['group' => $this->group]
)
);

return $this->bubble === false;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

兼容 Monolog v3:对 LogRecord 使用数组下标会导致致命错误

handle() 中大量使用 $record['...'],在 Monolog v3(LogRecord 对象)下会报错。建议规范化记录,兼容 v2 数组与 v3 LogRecord。

-    public function handle($record): bool
+    public function handle($record): bool
     {
         if (! $this->isHandling($record)) {
             return false;
         }
-        // Do not collect logs for exceptions, they should be handled seperately by the `Handler` or `captureException`
-        if (isset($record['context']['exception']) && $record['context']['exception'] instanceof Throwable) {
-            return false;
-        }
-
-        Logs::getInstance()->aggregator()->add(
-            self::getSentryLogLevelFromMonologLevel($record['level']),
-            $record['message'],
-            [],
-            array_merge(
-                ['log_context' => $record['context']],
-                ['log_extra' => $record['extra']],
-                ['logger' => $record['channel'] ?? ''],
-                ['group' => $this->group]
-            )
-        );
-
-        return $this->bubble === false;
+        // Do not collect logs for exceptions, they should be handled separately by the `Handler` or `captureException`
+        $isArray = is_array($record);
+        $level   = $isArray ? ($record['level'] ?? null)   : $record->level;
+        $message = $isArray ? ($record['message'] ?? '')   : $record->message;
+        $context = (array)($isArray ? ($record['context'] ?? []) : $record->context);
+        $extra   = (array)($isArray ? ($record['extra'] ?? [])   : $record->extra);
+        $channel = $isArray ? ($record['channel'] ?? '')         : ($record->channel ?? '');
+
+        $exception = $context['exception'] ?? null;
+        if ($exception instanceof Throwable) {
+            return false;
+        }
+
+        Logs::getInstance()->aggregator()->add(
+            self::getSentryLogLevelFromMonologLevel($level),
+            $message,
+            [],
+            array_merge(
+                ['log_context' => $context],
+                ['log_extra' => $extra],
+                ['logger' => $channel],
+                ['group' => $this->group]
+            )
+        );
+
+        // 返回 true 表示“阻止冒泡”,与 Monolog 语义一致
+        return $this->bubble === false;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function handle($record): bool
{
if (! $this->isHandling($record)) {
return false;
}
// Do not collect logs for exceptions, they should be handled seperately by the `Handler` or `captureException`
if (isset($record['context']['exception']) && $record['context']['exception'] instanceof Throwable) {
return false;
}
Logs::getInstance()->aggregator()->add(
self::getSentryLogLevelFromMonologLevel($record['level']),
$record['message'],
[],
array_merge(
['log_context' => $record['context']],
['log_extra' => $record['extra']],
['logger' => $record['channel'] ?? ''],
['group' => $this->group]
)
);
return $this->bubble === false;
}
public function handle($record): bool
{
if (! $this->isHandling($record)) {
return false;
}
// Do not collect logs for exceptions, they should be handled separately by the `Handler` or `captureException`
$isArray = is_array($record);
$level = $isArray ? ($record['level'] ?? null) : $record->level;
$message = $isArray ? ($record['message'] ?? '') : $record->message;
$context = (array)($isArray ? ($record['context'] ?? []) : $record->context);
$extra = (array)($isArray ? ($record['extra'] ?? []) : $record->extra);
$channel = $isArray ? ($record['channel'] ?? '') : ($record->channel ?? '');
$exception = $context['exception'] ?? null;
if ($exception instanceof Throwable) {
return false;
}
Logs::getInstance()->aggregator()->add(
self::getSentryLogLevelFromMonologLevel($level),
$message,
[],
array_merge(
['log_context' => $context],
['log_extra' => $extra],
['logger' => $channel],
['group' => $this->group]
)
);
// 返回 true 表示“阻止冒泡”,与 Monolog 语义一致
return $this->bubble === false;
}

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