Integrate RoadRunner logger #156
Conversation
WalkthroughDocumentation and configuration files were updated to reflect the integration of the RoadRunner Logger. The changelog and README were revised to describe the new feature and provide usage instructions, while the composer configuration added the required package dependency for the logger. Changes
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
README.md (2)
405-410: Missing fenced-block language identifierMarkdown-lint flags this snippet: the
.envfragment is inside a plain triple-back-tick block without a language tag.Add
dotenv(or at leastbash) to silence MD040 and enable proper highlighting:-``` +```dotenv RR_RPC=tcp://127.0.0.1:6001--- `419-423`: **Minor duplication** Given the corrected example above, the `Log::channel('rr')` snippet alone is sufficient; consider dropping one of the two to avoid redundancy. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 47c04daa7bb973167c451105d31f98962fbf6155 and bf42ca4793c41860207ea00ddd3d80ea435497d6. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `CHANGELOG.md` (1 hunks) * `README.md` (2 hunks) * `composer.json` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>README.md</summary> 407-407: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>composer.json (1)</summary> `46-48`: **✅ Hungthai1401/laravel-roadrunner-logger v1.0.0 checks out** - PHP constraint: requires ^8.1 → covers this lib’s ^8.2 - No security advisories found in FriendsOfPHP/security-advisories - No other runtime requirements beyond PHP → no new conflicts You can safely merge as-is. </details> <details> <summary>CHANGELOG.md (1)</summary> `8-11`: **Changelog entry looks good** The new bullet clearly states the added feature and follows the existing “Added” section style. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| ```php | ||
| logger('rr')->info('This is an info message'); | ||
| ``` |
There was a problem hiding this comment.
Incorrect usage example – logger('rr') logs a message, not selects a channel
In Laravel the logger() helper treats the first argument as the message.
To obtain a logger for a specific channel you need:
-logger('rr')->info('This is an info message');
+logger()->channel('rr')->info('This is an info message');or simply keep the Log::channel('rr') example below.
Please update to prevent confusion.
📝 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.
| ```php | |
| logger('rr')->info('This is an info message'); | |
| ``` |
🤖 Prompt for AI Agents
In README.md around lines 413 to 415, the example using logger('rr')->info('This
is an info message') is incorrect because the logger() helper treats the first
argument as the message, not as a channel selector. To fix this, replace the
example with the correct usage for logging to a specific channel, such as using
Log::channel('rr')->info('This is an info message'), or remove the incorrect
example and keep the existing Log::channel('rr') example to avoid confusion.
|
@roxblnfk What do you think about using for logger a third party pacakge? |
|
@butschster I think that the need for an RR-PSR logger arose with the addition of PSR logger support in Temporal. Providing the ability to log in RR in different ways (STDERR and RPC) is the right move, but our security policies do not allow us to easily use third-party libraries in common packages. I have considered moving the Monolog Handler from RR-Spiral-Bridge to a separate package, but haven't gotten around to it. |
|
@butschster @butschster I'm willing to join in. It will help me to learn more. |
|
I've created repository and issue for this roadrunner-php/psr-logger#3 |
|
@roxblnfk Sorry, I can't access it |
Forgot make it public 🤦 |
Description
Checklist
CHANGELOG.mdfileSummary by CodeRabbit
Documentation
Chores