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

Logging send/receive data at DEBUG level #1087

Closed
pohmelie opened this issue Aug 15, 2016 · 14 comments
Closed

Logging send/receive data at DEBUG level #1087

pohmelie opened this issue Aug 15, 2016 · 14 comments

Comments

@pohmelie
Copy link

This issue is result of aio-libs google group discussion. Since often developer have no server-side code/protocol/api, but trying to reverse client-side by comparing requests/responses, ability to «see what happend» is very important. For insecure connections there is wireshark, but most of services are secure nowadays.
Easiest way is to wrap transport, but I don't really know how this will affect performance.

@asvetlov
Copy link
Member

I want just state it: logging should be performed in debug mode only. Perhaps it requires debug param in ClientSession constructor, that's fine.

Do you want to save request/response headers only or the whole bodies?
The last may be tricky: logging the whole content of downloaded file is very expensive, but saving the first 100 bytes maybe is now enough.
Suggestions?

@pohmelie
Copy link
Author

Don't really know which case is better, but sometimes you need 101 bytes, and you'll be sad if some "bad library developer" will cut you at 100. I think there definetly should be some options/log levels restrictions for logging all bytes.

@asvetlov
Copy link
Member

@pohmelie the decision is up to you.
I don't want to make a patch personally but rely on you.
It can be debug_info parameter with default value 100 but maybe you'll suggest better name.
debug_info=100 means 100 bytes, debug_info=0 or debug_info=None disables logging.
I really want to have the single param, blowing up leads to explosion.

@pohmelie
Copy link
Author

@asvetlov
Is this what you mean?

if debug_info is not None:
    logging.debug(...)

Also, I read requests/urllib3 sources to see what is going on there. And they are just logging method, url, http version, response status code and response size. So, after that I think request/response content should not be logged, since you can log it on your own in user code.
Since this, should we still make condition for logging debug info? In case of excluding content there should be not so much things to log to(just lines before content?).
Regardless of choosen solution I need help to find place(s), where aiohttp actually converts "dictionaries" to bytes and sending data.

@asvetlov
Copy link
Member

Ok, if we need boolean parameter than just debug=False is good name.
The explicit param is required, it allows to speed up execution in production mode a little bit.

Request sending is here: https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/client_reqrep.py#L482
Response info is available here: https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/client_reqrep.py#L668

Maybe pushing logging into redirection loop https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/client.py#L182 is cleaner than modifying request/response classes.

Let's cope with regular http requests first and add websocket logging in another PR.

@pohmelie
Copy link
Author

Maybe better solution is to use protocol.send_headers and protocol.HttpResponseParser? Cause this have all headers/cookies in native view. And there is no need to keep «http dicts» in mind.

@asvetlov
Copy link
Member

Please no.
protocol is overridden by implementation details but I want to refactor it very deeply for sake of performance.
Also think about HTTP/2. Perhaps it will require different implementation.
Let's keep the proper abstraction level for request/response logging.

@txomon
Copy link

txomon commented Feb 8, 2017

Guys, would it be possible to have some kind of logging. I am trying to understand how cookies and sessions get modified in my application and the docs speak about Logging but setting up client logging is totally useless at the moment.

Anything is better than something. If you are worried about performance go with the module variable way, but please, something!

@pohmelie
Copy link
Author

pohmelie commented Feb 8, 2017

@txomon, unfortunately I still have no time for this, sorry. As I realized this should be done very accurate: be in proper level of abstraction and don't decrease performance too much.

@txomon
Copy link

txomon commented Feb 8, 2017 via email

@fafhrd91 fafhrd91 added this to the 1.4 milestone Feb 8, 2017
@fafhrd91 fafhrd91 modified the milestones: 2.1, 2.0 Feb 16, 2017
@fafhrd91 fafhrd91 modified the milestones: 2.2, 2.1 Apr 8, 2017
@mikenerone
Copy link
Contributor

Just wanted to toss my vote in here, too. Situations come up for me regularly where some raw request logging would be very helpful.

@asvetlov asvetlov modified the milestones: 2.4.0, 2.2 Sep 8, 2017
@cecton
Copy link
Contributor

cecton commented Sep 15, 2017

What about receiving file objects for debugging? Like debug_in and debug_out. If the dev provides a valid file object opened in binary mode, every packet receive will be sent raw to debug_in and every packet sent will be sent to the other one.

With that, the implementation of the actual logging is done by the dev.

@asvetlov asvetlov modified the milestones: 2.4, 3.0 Oct 17, 2017
@asvetlov
Copy link
Member

Superseded by #2313

@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants