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

feat: allow using UVICORN_LOG_CONFIG env var to be used to configure uvicorn logging. #882

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LasseGravesenSaxo
Copy link

@LasseGravesenSaxo LasseGravesenSaxo commented Apr 4, 2024

Motivation

The reason for this PR is that I'm having issues configuring all the loggers, and this causes problems with some log messages being in a different format than expected; We want all our logging to be json such that it can be ingested into our logging aggregator as a single blob, but with the default logging config that cannot be overridden currently we instead sometimes get a bunch of separate log messages that can't be ingested as a single blob and are missing important fields.

Implementation

We let UVICORN_LOG_CONFIG be loaded from the environment, its default value is uvicorn.config.LOGGING_CONFIG (unsure how to handle this best really, either we need a reference to uvicorn.config.LOGGING_CONFIG or it should instead not set the log_config kwarg in the uvicorn.Config if it's not set, which would then fallback to the uvicorn default value; because if we set it to None then it won't use the default logging config). This config value is then passed to uvicorn.Config which is used to configure the uvicorn server.

The value of UVICORN_LOG_CONFIG could be logging.conf, and logging.conf could look something like this:

[loggers]
keys = uvicorn

[logger_uvicorn]
level = DEBUG
handlers = custom
propagate = 0
qualname = uvicorn

[handlers]
keys = custom

[handler_custom]
class = StreamHandler
level = DEBUG
formatter = json
args = (sys.stdout,)

[formatters]
keys = json

[formatter_json]
class = ...

Questions

  1. It might be that this PR should also modify this file: https://github.com/Chainlit/chainlit/blob/main/backend/chainlit/logger.py

To ensure that logging isn't configured multiple times.

  1. It might be that there are other uvicorn configs that should passthru like this. I don't know if we can start uvicorn in such a way that it respects all environment variables without just duplicating what they've done here: https://github.com/encode/uvicorn/blob/master/uvicorn/main.py

…uvicorn logging.

for instance using a logging config file.
@LasseGravesenSaxo
Copy link
Author

Hope someone can take a look at this!

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.

None yet

1 participant