-
Notifications
You must be signed in to change notification settings - Fork 725
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
base: main
Are you sure you want to change the base?
feat: allow using UVICORN_LOG_CONFIG env var to be used to configure uvicorn logging. #882
Conversation
…uvicorn logging. for instance using a logging config file.
Hope someone can take a look at this! |
We are also interested in having an option to customize the Uvicorn logging. @LasseGravesenSaxo why did you choose a plain environment variable? For me, it would make a lot more sense to introduce a new CLI option. Like this: @click.option(
"-l",
"--log-config",
envvar="LOG_LEVEL",
help="Uvicorn logging configuration file. Supported formats: .ini, .json, .yaml.",
)
def chainlit_run(target, watch, headless, debug, ci, no_cache, host, port, log_config):
pass If the option is not given, the default value of Uvicorn will be used anyway, so no need to define a custom default here. |
@smueller18 LOG_LEVEL and --log-config should be different. I picked an environment variable because of this bit already using an environment variable for Uvicorn:
But I can easily modify it such that it uses an option too. |
@smueller18, I made a change now that makes it possible to set log config as a click option. |
@smueller18 I applied changes based on your comments. |
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 isuvicorn.config.LOGGING_CONFIG
(unsure how to handle this best really, either we need a reference touvicorn.config.LOGGING_CONFIG
or it should instead not set thelog_config
kwarg in theuvicorn.Config
if it's not set, which would then fallback to theuvicorn
default value; because if we set it to None then it won't use the default logging config). This config value is then passed touvicorn.Config
which is used to configure theuvicorn
server.The value of
UVICORN_LOG_CONFIG
could belogging.conf
, andlogging.conf
could look something like this:Questions
To ensure that logging isn't configured multiple times.