chainlit
chainlit copied to clipboard
feat: allow using UVICORN_LOG_CONFIG env var to be used to configure uvicorn logging.
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
- 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.
- 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
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.
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): passIf 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:
ws_per_message_deflate_env = os.environ.get(
"UVICORN_WS_PER_MESSAGE_DEFLATE", "true"
)
ws_per_message_deflate = ws_per_message_deflate_env.lower() in [
"true",
"1",
"yes",
] # Convert to boolean
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.
Interested in seeing this get attention from maintainers as it's causing an issue with logging for us.
Hey all, we're finally getting to give love to some of these long-pending contribs.
As in the meantime, we've landed the possibility to mount chainlit in FastAPI apps, so that we can run and configure our own uvicorn, is this still a relevant and necessary feature?
I'm asking as I'd imagine that anyone running chainlit in production will want to do so with their own uvicorn, where they have full control over it's config.
This way, we won't end up having to maintain wrappers for the plethora of uvicorn options without blocking devs accesss to lower-level stuff. The same goes for FastAPI.
In this perspective, the chainlit CLI is merely a wrapper to land quick PoC's and a full-fledged ASGI would be the production-grade wrapper for scale-out (similar to how e.g. Django does it).
Love to hear your feedback @LasseGravesenSaxo and @smueller18.
@dokterbob The option for mounting chainlit in a custom FastAPI app was a really nice feature and we migrated to. So for us, there is no need for this specific PR anymore.
@smueller18 Thanks for the head's up. I will close this one for now.
We are good also now using what smueller18 said.