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

Improve logger middleware output #782

Closed
erichmachado opened this issue Mar 23, 2018 · 3 comments
Closed

Improve logger middleware output #782

erichmachado opened this issue Mar 23, 2018 · 3 comments

Comments

@erichmachado
Copy link
Contributor

erichmachado commented Mar 23, 2018

Basic Info

  • Faraday Version: 0.14.0
  • Ruby Version: 2.5.0

Issue description

I am currently using the Faraday response logger middleware in some projects, however the output is a bit confusing because it does not follow a similar structure for request/response log lines. Right now here is what you get in your logs if you make a request with this middleware enabled:

get https://api.example.com 
Status: 200

This might get even more confusing depending on you log formatter settings. I propose to follow the same structure for request/response logs, in a way that each line can be clearly inspected later. It would then look like this:

 request: GET https://api.example.com 
response: Status 200

Where the request and response strings are set as the progname of the INFO log calls accordingly, just like the debug lines currently work.

I've already made a branch with this implementation and added the required tests, if you like it I can open a PR.

@iMacTia
Copy link
Member

iMacTia commented Mar 23, 2018

Hi @erichmachado, thanks for raising this!
Yes please provide a link to your branch so I can have a better look.
Sounds solid in principal, I just need to consider if this is going to break existing behaviour and eventually plan it for v1.0

@erichmachado
Copy link
Contributor Author

Hi @iMacTia, thank you for your reply. Here is my branch:

https://github.com/erichmachado/faraday/tree/improve-response-logger-middleware-output

@iMacTia
Copy link
Member

iMacTia commented Mar 24, 2018

The branch looks good.
I also had a quick look at the file history and it looks like this inconsistency has been there since the beginning (6 years ago).
I don't think this should break any existing implementation as we're simply changing formatting for the logs.

Feel free to open the PR and I'll review 👍

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

No branches or pull requests

2 participants