Configurable logging for libraries #10614
Conversation
This used to be taken from endpoints configuration but it was undocumented and also not consistent, ie. pika did not utilize this for example. We also want to use env variables here (I think?).
| @@ -216,4 +212,4 @@ def _close(self) -> None: | |||
| @rasa.shared.utils.common.lazy_property | |||
| def rasa_environment(self) -> Optional[Text]: | |||
| """Get value of the `RASA_ENVIRONMENT` environment variable.""" | |||
| return os.environ.get("RASA_ENVIRONMENT") | |||
| return os.environ.get("RASA_ENVIRONMENT", "RASA_ENVIRONMENT_NOT_SET") | |||
There was a problem hiding this comment.
@melindaloubser1 I've added this default because otherwise kafka publish fails with
2021-12-31 16:26:18 ERROR rasa.core.brokers.kafka - Could not publish message to kafka url 'localhost'. Failed with error: encoding without a string argument
I think we also need to document this?
There was a problem hiding this comment.
noticed a similar change here
There was a problem hiding this comment.
Sorry I was not online so just seeing this now - is it resolved/documented already?
There was a problem hiding this comment.
@melindaloubser1 I've added the default RASA_ENVIRONMENT_NOT_SET but I haven't documented it as I'm not sure what the use case for this is. Seeing you added this you may be better suited to document it? We can also create a ticket for someone else. What do you think?
There was a problem hiding this comment.
Ohh wait a sec I remember this now! The default here should be production. This is about multiple deployment environments.
There was a problem hiding this comment.
@melindaloubser1 I think there's a typo in https://rasa.com/docs/rasa-x/enterprise/deployment-environments where environments are listed after By default, two environments are defined:. Worker should be development.
I've approved your PRs.
There was a problem hiding this comment.
The image shows development in addition to worker, but only worker is default, so no typo as far as I can see.
There was a problem hiding this comment.
I mean there are two environments, production and development but the list shows worker as an environment:
By default, two environments are defined:
production
worker
There was a problem hiding this comment.
It's referring to the right hand side box:
development is not a default environment. It must be added manually.
There was a problem hiding this comment.
@melindaloubser1 Hmm, I see. I think I'm just confused then :D I thought prod and dev are default environments whereas worker would be separate instance just for training. Thanks for the explanation.
|
Does this solution look good? I'll add tests and docs once we agree on a path to take. |
| @@ -216,4 +212,4 @@ def _close(self) -> None: | |||
| @rasa.shared.utils.common.lazy_property | |||
| def rasa_environment(self) -> Optional[Text]: | |||
| """Get value of the `RASA_ENVIRONMENT` environment variable.""" | |||
| return os.environ.get("RASA_ENVIRONMENT") | |||
| return os.environ.get("RASA_ENVIRONMENT", "RASA_ENVIRONMENT_NOT_SET") | |||
There was a problem hiding this comment.
noticed a similar change here
docs/docs/command-line-interface.mdx
Outdated
| 3. LOG_LEVEL_RABBITMQ: This is the specialized environment variable to configure log level only for AMQP libraries, at the moment it handles log levels from `aio_pika` and `aiormq`. | ||
| 4. LOG_LEVEL_KAFKA: This is the specialized environment variable to configure log level only for kafka. | ||
|
|
||
| General configuration (`LOG_LEVEL_LIBRARIES`) has less priority than library level specific configuration (`LOG_LEVEL_MATPLOTLIB`, `LOG_LEVEL_RABBITMQ` etc); and CLI parameter has the highest precedence. This means variables can be used together with a predictable result. As an example: |
There was a problem hiding this comment.
Does CLI parameter have the highest precedence really? In the example below the cli is set to DEBUG but does't override the WARNING level of matplotlib... or am I understanding the example incorrectly?
Or maybe it only overrides to a less verbose level?
There was a problem hiding this comment.
Good catch, CLI parameter sets the level at the root level (which also has the important handler - coloredlogs handler). This means CLI level will be used for every log which is not explicitly set (matplotlib, rabbitmq, or rasa and any other library) but it also means CLI level overrides all other log levels if it is a less verbose level, like you said.
I'll change the wording in the docs.
|
🚀 A preview of the docs have been deployed at the following URL: https://10614--rasahq-docs-rasa-v2.netlify.app/docs/rasa |
Proposed changes:
Fixes Make logging verbosity configurable per package for verbose packages. #10203
Add environment variables to configure logging at library level. This is enabled for matplotlib, rabbitmq (pika etc.) and kafka at the moment.
Status (please check what you already did):
black(please check Readme for instructions)