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

Use standard logging style #2547

Merged
merged 8 commits into from
Mar 20, 2023
Merged

Use standard logging style #2547

merged 8 commits into from
Mar 20, 2023

Conversation

tomchristie
Copy link
Member

@tomchristie tomchristie commented Jan 5, 2023

Closes #2042

This proposal drops some of our customised logging handling to replace it with a more standard setup.

In particular, we would...

  • Drop our custom TRACE level logging. Instead use INFO for logging requests, and DEBUG for logging the SSL config.
  • Drop our custom auto-logging setup, previously enabled via HTTPX_LOG_LEVEL.

This would then give us a more standard setup.

For example, logging at INFO level...

import logging
import httpx

logging.basicConfig(
    format="%(levelname)s [%(asctime)s] %(name)s - %(message)s",
    datefmt="%Y-%m-%d %H:%M:%S",
    level=logging.INFO
)

httpx.get('https://www.example.com')

Output:

INFO [2023-01-05 15:25:26] httpx - HTTP Request: GET https://www.example.com "HTTP/1.1 200 OK"

Or logging at DEBUG level...

import logging
import httpx

logging.basicConfig(
    format="%(levelname)s [%(asctime)s] %(name)s - %(message)s",
    datefmt="%Y-%m-%d %H:%M:%S",
    level=logging.DEBUG
)

httpx.get('https://www.example.com')

Output:

DEBUG [2023-01-05 15:26:12] httpx - load_ssl_context verify=True cert=None trust_env=True http2=False
DEBUG [2023-01-05 15:26:12] httpx - load_verify_locations cafile='/Users/tomchristie/GitHub/encode/httpx/venv/lib/python3.10/site-packages/certifi/cacert.pem'
INFO [2023-01-05 15:26:13] httpx - HTTP Request: GET https://www.example.com "HTTP/1.1 200 OK"

I'd suggest that alongside this we'd also add DEBUG level logging into httpcore, at the points where we currently have trace extension.

Once we've done that you'd be able to have debug logging for both httpcore and httpx...

import logging
import httpx

logging.basicConfig(
    format="%(levelname)s [%(asctime)s] %(name)s - %(message)s",
    datefmt="%Y-%m-%d %H:%M:%S",
    level=logging.DEBUG
)

httpx.get('https://www.example.com')

Output:

DEBUG [2023-01-05 15:34:30] httpx - load_ssl_context verify=True cert=None trust_env=True http2=False
DEBUG [2023-01-05 15:34:30] httpx - load_verify_locations cafile='/Users/tomchristie/GitHub/encode/httpx/venv/lib/python3.10/site-packages/certifi/cacert.pem'
DEBUG [2023-01-05 15:34:30] httpcore - connection.connect_tcp.started host='www.example.com' port=443 local_address=None timeout=5.0
DEBUG [2023-01-05 15:34:30] httpcore - connection.connect_tcp.complete return_value=<httpcore.backends.sync.SyncStream object at 0x112074370>
DEBUG [2023-01-05 15:34:30] httpcore - connection.start_tls.started ssl_context=<ssl.SSLContext object at 0x1120489c0> server_hostname='www.example.com' timeout=5.0
DEBUG [2023-01-05 15:34:30] httpcore - connection.start_tls.complete return_value=<httpcore.backends.sync.SyncStream object at 0x1120742e0>
DEBUG [2023-01-05 15:34:30] httpcore - http11.send_request_headers.started request=<Request [b'GET']>
DEBUG [2023-01-05 15:34:30] httpcore - http11.send_request_headers.complete return_value=None
DEBUG [2023-01-05 15:34:30] httpcore - http11.send_request_body.started request=<Request [b'GET']>
DEBUG [2023-01-05 15:34:30] httpcore - http11.send_request_body.complete return_value=None
DEBUG [2023-01-05 15:34:30] httpcore - http11.receive_response_headers.started request=<Request [b'GET']>
DEBUG [2023-01-05 15:34:30] httpcore - http11.receive_response_headers.complete return_value=(b'HTTP/1.1', 200, b'OK', [(b'Content-Encoding', b'gzip'), (b'Accept-Ranges', b'bytes'), (b'Age', b'445646'), (b'Cache-Control', b'max-age=604800'), (b'Content-Type', b'text/html; charset=UTF-8'), (b'Date', b'Thu, 05 Jan 2023 15:34:30 GMT'), (b'Etag', b'"3147526947"'), (b'Expires', b'Thu, 12 Jan 2023 15:34:30 GMT'), (b'Last-Modified', b'Thu, 17 Oct 2019 07:18:26 GMT'), (b'Server', b'ECS (nyb/1D2E)'), (b'Vary', b'Accept-Encoding'), (b'X-Cache', b'HIT'), (b'Content-Length', b'648')])
INFO [2023-01-05 15:34:30] httpx - HTTP Request: GET https://www.example.com "HTTP/1.1 200 OK"
DEBUG [2023-01-05 15:34:30] httpcore - http11.receive_response_body.started request=<Request [b'GET']>
DEBUG [2023-01-05 15:34:30] httpcore - http11.receive_response_body.complete return_value=None
DEBUG [2023-01-05 15:34:30] httpcore - http11.response_closed.started 
DEBUG [2023-01-05 15:34:30] httpcore - http11.response_closed.complete return_value=None

Our documentation would also need updating, and could show users examples with both basicConfig and dictConfig, for example...

import logging.config
import httpx

LOGGING_CONFIG = {
    "version": 1,
    "handlers": {
        "default": {
            "class": "logging.StreamHandler",
            "formatter": "http",
            "stream": "ext://sys.stderr"
        }
    },
    "formatters": {
        "http": {
            "format": "%(levelname)s [%(asctime)s] %(name)s - %(message)s",
            "datefmt": "%Y-%m-%d %H:%M:%S",
        }
    },
    'loggers': {
        'httpx': {
            'handlers': ['default'],
            'level': 'DEBUG',
        },
        'httpcore': {
            'handlers': ['default'],
            'level': 'DEBUG',
        },
    }
}

logging.config.dictConfig(LOGGING_CONFIG)
httpx.get('https://www.example.com')

Input to this proposal is welcome. 🙏🏼

TODO:

  • Implement for httpx
  • Update documentation
  • Add DEBUG level logging to httpcore, at the points where we currently have the trace extension.

See also httpcore issue #648

@Kludex
Copy link
Member

Kludex commented Jan 5, 2023

It feels a bit invasive to use the INFO level for logging requests. Can we use DEBUG for both things?

@tomchristie
Copy link
Member Author

That'd be reasonable, yup.

If we did want to keep the request logging at DEBUG level, I'd wonder if we should just drop the TRACE level SSL config logging?

@zanieb
Copy link
Contributor

zanieb commented Jan 5, 2023

What about using httpx.requests for logging request related information and httpx.ssl for logging SSL related information? Then you'll be able to use INFO / DEBUG level logs according to the importance of the log in that domain and users can control levels at the httpx level or for the domain of interest.

@tomchristie
Copy link
Member Author

tomchristie commented Jan 6, 2023

Okay, I've got a PR up for httpcore now.

Let's see how this looks at the moment...

$ python -m venv venv
$ venv/bin/pip install --upgrade pip
$ venv/bin/pip install https://github.com/encode/httpcore/archive/add-debug-logging.zip
$ venv/bin/pip install https://github.com/encode/httpx/archive/review-logging.zip

Then...

example.py:

import logging
import httpx

logging.basicConfig(
    format="%(levelname)s [%(asctime)s] %(name)s - %(message)s",
    datefmt="%Y-%m-%d %H:%M:%S",
    level=logging.DEBUG
)

httpx.get('https://www.example.com')

Output...

$ venv/bin/python example.py
DEBUG [2023-01-06 11:13:37] httpx - load_ssl_context verify=True cert=None trust_env=True http2=False
DEBUG [2023-01-06 11:13:37] httpx - load_verify_locations cafile='/Users/tomchristie/Temp/venv/lib/python3.7/site-packages/certifi/cacert.pem'
DEBUG [2023-01-06 11:13:37] httpcore - connection.connect_tcp.started host='www.example.com' port=443 local_address=None timeout=5.0
DEBUG [2023-01-06 11:13:37] httpcore - connection.connect_tcp.complete return_value=<httpcore.backends.sync.SyncStream object at 0x10e336cc0>
DEBUG [2023-01-06 11:13:37] httpcore - connection.start_tls.started ssl_context=<ssl.SSLContext object at 0x10e300a20> server_hostname='www.example.com' timeout=5.0
DEBUG [2023-01-06 11:13:38] httpcore - connection.start_tls.complete return_value=<httpcore.backends.sync.SyncStream object at 0x10e336c88>
DEBUG [2023-01-06 11:13:38] httpcore - http11.send_request_headers.started request=<Request [b'GET']>
DEBUG [2023-01-06 11:13:38] httpcore - http11.send_request_headers.complete
DEBUG [2023-01-06 11:13:38] httpcore - http11.send_request_body.started request=<Request [b'GET']>
DEBUG [2023-01-06 11:13:38] httpcore - http11.send_request_body.complete
DEBUG [2023-01-06 11:13:38] httpcore - http11.receive_response_headers.started request=<Request [b'GET']>
DEBUG [2023-01-06 11:13:38] httpcore - http11.receive_response_headers.complete return_value=(b'HTTP/1.1', 200, b'OK', [(b'Content-Encoding', b'gzip'), (b'Age', b'155784'), (b'Cache-Control', b'max-age=604800'), (b'Content-Type', b'text/html; charset=UTF-8'), (b'Date', b'Fri, 06 Jan 2023 11:13:38 GMT'), (b'Etag', b'"3147526947+gzip"'), (b'Expires', b'Fri, 13 Jan 2023 11:13:38 GMT'), (b'Last-Modified', b'Thu, 17 Oct 2019 07:18:26 GMT'), (b'Server', b'ECS (nyb/1D04)'), (b'Vary', b'Accept-Encoding'), (b'X-Cache', b'HIT'), (b'Content-Length', b'648')])
INFO [2023-01-06 11:13:38] httpx - HTTP Request: GET https://www.example.com "HTTP/1.1 200 OK"
DEBUG [2023-01-06 11:13:38] httpcore - http11.receive_response_body.started request=<Request [b'GET']>
DEBUG [2023-01-06 11:13:38] httpcore - http11.receive_response_body.complete
DEBUG [2023-01-06 11:13:38] httpcore - http11.response_closed.started
DEBUG [2023-01-06 11:13:38] httpcore - http11.response_closed.complete

@florimondmanca
Copy link
Member

Logging request lines at the INFO level seems right to me. In comparison, SQLAlchemy logs SQL queries at the INFO level as well. https://docs.sqlalchemy.org/en/20/core/engines.html#configuring-logging

@tomchristie tomchristie marked this pull request as ready for review March 16, 2023 14:52

Trace output:

```console
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this TRACE documentation was already incorrect at this point since we lost httpcore logging for a while.

@tomchristie
Copy link
Member Author

There's more that we could do if we wanted to include extra logging here. Eg. client open/close.
But this at least gets us into using a more standard Python logging setup.
Needs to be called out as a behavioural change in the release notes.

@tomchristie tomchristie requested review from florimondmanca and a team March 16, 2023 15:02
response_line = f"{response.http_version} {status}"
logger.debug(
'HTTP Request: %s %s "%s"', request.method, request.url, response_line
logger.info(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the other python packages, I don't recall any other python client having an INFO log message level. 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i.e. can't it be debug as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could yes, though it's less useful to have logging if everything is at the same level.
I like the different "show me nothing", "show me the requests", "show me the debug" lighting levels.

(Similar: gunicorn, uvicorn using INFO for requests, and DEBUG for other stuffs)

@tomchristie
Copy link
Member Author

thank you for the reviews folks. 💛

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

Successfully merging this pull request may close these issues.

Review logging.
5 participants