php-http / logger-plugin

PSR-3 Logger plugin for HTTPlug
http://docs.php-http.org/en/latest/plugins/logger.html
MIT License
282 stars 7 forks source link

Request and response in context always empty. #16

Closed bu4ak closed 1 month ago

bu4ak commented 3 years ago

PHP version: 8.0.3

Description Request and response in context always empty.

How to reproduce Enable plugin and do eny request.

Additional context

{"request":{"GuzzleHttp\\Psr7\\Request":[]},"response":{"GuzzleHttp\\Psr7\\Response":[]},"milliseconds":1185}

FullHttpMessageFormatter can add a request and a response to a log message, but they are still empty in context.

krewetka commented 3 years ago

Yes, it would be handy to have not empty items in context as well.

Right now all is in message body which make it not nice for kibana and other type of log display.

It would be good to have an option to put it to context without writing our own logger.

ro0NL commented 2 years ago

See https://github.com/php-http/logger-plugin/pull/19#discussion_r801740748, my proposal is to remove request/response from the context. It defeats a custom log message in the first place.

dbu commented 2 years ago

the current situation seems to make no sense. completely removing them seems the consistent approach. i was considering if we should have a configuration option, but the only purpose of this plugin is logging. so if anything, we should provide a second plugin in this repository that would use the FullRequestFormatter for the request and response in the context and maybe SimpleFormatter for the log message itself. not sure for a good name, could call it ContextLoggerPlugin.

@krewetka or @bu4ak would you be motivated to do a pull request to add such a logger plugin?

@ro0NL are you motivated to remove request/response from the context, as they don't do anything except add confusion as it currently is?

krewetka commented 2 years ago

Hi!

To be honest it is a while since I last used this library in my previous work project.

I am also not sure I am even following fully what the plan is :)

dbu commented 2 years ago

@krewetka i was referring to "It would be good to have an option to put it to context without writing our own logger." - my suggestion is to create a separate plugin class in this repository that only logs a short message as "message" but then uses a different message formatter (ideally the Full formatter) to put request and response into the context. that way we would have your use case covered as well as the one from @ro0NL

no obligation of course, just tought because you mentioned it you might be motivated to do that ;-)

ro0NL commented 2 years ago

see #20

so if anything, we should provide a second plugin in this repository that would use the FullRequestFormatter for the request and response in the context and maybe SimpleFormatter for the log message itself. not sure for a good name, could call it ContextLoggerPlugin.

IMHO the purpose of a formatter is to control what request/response details should leak into logs. Receiving the full request/response in context by other means defeats that purpose.

That said, a follow up could be to introduce a new extension point for providing extra log context: Formatter::getExtraContext(MessageInterface $message): array

ostrolucky commented 2 years ago

How about taking advantage of how FullHttpMessageFormatter is doing it and simply break the message by newline? Put everything in $message[0] to message, rest to context. It's quick & dirty, but very easy and would be completely enough for now IMHO.

ro0NL commented 2 months ago

I'm pretty sure this issue can be closed due to #22

Actually the empty object in context is caused by monolog's JSON encoder (json_encode), due to using private properties.

You can already extend it using custom monolog processor.

Or extend the log message using formatRequest($request) here.

dbu commented 1 month ago

yep, fixed in #22