search

sendgrid / python-http-client

Twilio SendGrid's Python HTTP Client for calling APIs
https://sendgrid.com
MIT License
126 stars 101 forks source link

Feature Request: Implement Request Logging Options #141

Closed LaikaN57 closed 4 years ago

LaikaN57 commented 4 years ago

Issue Summary

We should implement request logging. SendGrid support will frequent want this information when helping customers debug their application. This information should include what API they are calling, what response code, and what error message we get back.

Possible Solutions

  1. Natively create a logging.Logger object.
  2. Switch to using urllib3 instead of urllib as it has a logging.Logger which can be configured.
  3. Add an option to be able to pass a custom urllib.requests.OpenerDirector or similar.
  4. A workaround I found was setting http.client.HTTPConnection.debuglevel = 1 but this did not seem to work for me in an AWS lambda environment. (I still need to investigate if there is an issue with my code on this though.)

I welcome feedback and would like to know if the community has any objections to adding dependencies (option 1) or changing dependencies (option 2) as I think these are the simplest methods.

Technical details:

childish-sambino commented 4 years ago

What about just migrating to requests which uses urllib3 underneath?

LaikaN57 commented 4 years ago

@childish-sambino Yeah really anything that just allows us to set logging.getLogger(<3rd-party-module>).setLevel(logging.DEBUG) or similar will do for my particular use-case (and I suspect the majority of use-cases). I do not have a preference on which 3rd party we go with. I will leave that to maintainers to provide direction on the prefered dependency.

childish-sambino commented 4 years ago

requests is preferred given that we also use this in the twilio-python library.

This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

LaikaN57 commented 4 years ago

@childish-sambino I am no longer convinced this was a good idea. It adds dependencies on multiple 3rd party libraries and it does not actually solve my header logging issues (see https://github.com/sendgrid/python-http-client/pull/142#issuecomment-665343151 for details).

Also feel free to work off of https://github.com/sendgrid/python-http-client/pull/142 if you still want to pursue swithing over to requests. The PR just needs some name changes of opener to session, documentation added, and more testing added (although the coverage only fell because we reduced total line count).