Skip to content
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

[💡 FEATURE REQUEST]: PSR-3 compatible logger RPC #1227

Closed
wolfy-j opened this issue Jul 25, 2022 · 10 comments
Closed

[💡 FEATURE REQUEST]: PSR-3 compatible logger RPC #1227

wolfy-j opened this issue Jul 25, 2022 · 10 comments
Assignees
Labels
C-feature-request Category: feature requested, but need to be discussed
Milestone

Comments

@wolfy-j
Copy link
Contributor

wolfy-j commented Jul 25, 2022

Plugin

RPC logger

I have an idea!

I have an idea, listen to me!!

It would be amazing if we can can publish to logger channels at RR side using PHP SDK via PSR-3 compatible API.

At the moment ALL the application logs can only be published to STDERR to be seen by RR, log context logs, all logs put using INFO level, all logs captured using server channel.

  • Add the ability to publish to named channel via RPC
  • Use the standard logging ability to render the log into the consolve (with colors and etc)

References: https://www.php-fig.org/psr/psr-3/

@wolfy-j wolfy-j added the C-feature-request Category: feature requested, but need to be discussed label Jul 25, 2022
@wolfy-j
Copy link
Contributor Author

wolfy-j commented Oct 11, 2022

Up.

@rustatian
Copy link
Member

Ok, guys, I see the demand ⚡ Will be in the v2.12.0.

@rustatian rustatian added this to the v2.12.0 milestone Oct 11, 2022
@rustatian
Copy link
Member

rustatian commented Oct 31, 2022

Hey, community ❤️

About this feature, I guess it would be better to print to the stderr completely untouched log from the PHP side. I mean, in the PSR-3 recommendation, we have a message (string) and an array (which I suppose is also a string), and we have two ways for the output:

  • using RR logger (like it works currently), but we have to map a Go's logger approach and PHP. In Go, we don't have an std interface; thus, not all PHP verbosity levels properly align with the Go's.
  • Do not depend on the RR's logger; combine a message and a context (from the PHP) and send the result to the stderr.

Personally, I'm leaning toward the second approach, like, do not touch PHP messages and context at all.

@benalf
Copy link
Sponsor

benalf commented Oct 31, 2022

The PHP context array can actually be anything and usually is an associative array (HashMap<String, Any>) with possibly deeply nested garbage. It even allows and reserves the exception context key in case the user also wants to provide an exception object with a stack trace.

What's the idea on how to combine the message and the context? Serializing in some way? Is it possible to get a few examples with the current options?

@rustatian
Copy link
Member

It'll be serialized into a string on the PHP side (options?). I guess, in the RPC we can accept only 1 arg - string and let PHP decide how to combine the context and message. Because, you know, we should send the data to the RR somehow.

@rustatian
Copy link
Member

Here is a very draft protobuf message: https://buf.build/roadrunner-server/api/file/main:proto/logger/psr3/v1/psr3.proto

And RR RPC part: https://github.com/roadrunner-server/psr-3-logger/blob/master/rpc.go
(you may see the 1st option implemented, where we use an RR logger mapped to the PSR-3 severity levels)

@benalf
Copy link
Sponsor

benalf commented Oct 31, 2022

OK, I see. Second option means PHPland is fully responsible of building the whole log entry string and therefore can produce funky levels like "critical" in whatever syntax they want. RR just accepts a new stderr entry as a single string and simply vomits it out, correct?

I'm personally totally down with such approach. Gives complete flexibility and doesn't enforce any serialization constraints. We could always have some generic PHP LoggerInterface implementation which would produce a jizzmal json similar to the current RR json output (in could also produce the extra "context" prop on the top level without needing to merge it with message) and users are free to swap it out to whatever they want.

This kinda means a simple RPC call for writing to stderr is needed and the rest falls on to the PHP implementation.

¯\(ツ)

@rustatian
Copy link
Member

Ok, I merged these two approaches. Here is the PHP client (the RR part will be in the next beta in a few days): link

So, debug, warning, info, and error are RR logger methods. The logger configuration is applied to these methods.

Next, the log method prints directly to the stderr. RR doesn't touch in any way this payload.

All methods receive a string.

@benalf
Copy link
Sponsor

benalf commented Nov 1, 2022

So in case somebody wants to wrap it into PSR3 compatible interface, it's up to them to map the available RR logger methods to PSR3 verbosity as they see fit?

Sounds like a great middleground, thanks. 👍

@rustatian
Copy link
Member

rustatian commented Nov 1, 2022

Yeah, in Go, generally, we have only error, warn, info, and debug + some like fatal, which sends os.Exit(1) to the process.

In this case, if you want to use the RR logger - feel free to use these methods. Otherwise - log method is your bridge to the RR's stderr.

EDIT: This feature is in beta (until v2.13). Discussion is in the process and feedbacks are welcome 😃

@rustatian rustatian mentioned this issue Nov 24, 2022
6 tasks
@rustatian rustatian modified the milestones: v2.12.0, v2.12.1 Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: feature requested, but need to be discussed
Projects
No open projects
Archived in project
Development

No branches or pull requests

3 participants