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 errbot log level with webserver for consistent logging #1556

Merged
merged 3 commits into from Feb 23, 2022

Conversation

nzlosh
Copy link
Contributor

@nzlosh nzlosh commented Feb 9, 2022

This patch will take errbot's log level configuration and pass it to werkzeug for consistent logging.

fixes #1555

@nzlosh nzlosh force-pushed the log_level branch 2 times, most recently from 0074d02 to d09a5d1 Compare February 9, 2022 22:41
@sijis sijis changed the title Use errbot log level with webserver for consistent logging. fixes #1555 Use errbot log level with webserver for consistent logging Feb 14, 2022
@sijis
Copy link
Contributor

sijis commented Feb 14, 2022

@nzlosh Do you think this is still necessary after the workaround given in #1555 ?

@nzlosh
Copy link
Contributor Author

nzlosh commented Feb 14, 2022

Either we handle logging consistently in the core or we clearly document this workaround in the official documentation. I'm inclined to go with this patch since it's more obvious how logging is controlled.

I did wonder if people might want access logs (INFO level for werkzeug) but less verbose logging for errbot. If someone does create a request for such a feature, I think it'd make sense to split out werkzeug log level and log entries into a separate log file and create new variables WEBSERVER_LOG_FILE and WEBSERVER_LOG_LEVEL.

This would also encapsulate the webserver implementation details and avoid breaking configuration if we switch to a different webserver in the future.

@AshleyJackson
Copy link

In my implementation I use other modules that have their own logging (outside of werkzeug) that still send to console, even if the configuration is set correctly.

If there is an option for the Root configuration to take precedence over sub-modules, I believe that that would be beneficial for nearly everyone who relies on the log outputs for Errbot.

If logging is required, developers can create their own log output where necessary using self.log.*(msg) IMO.

@nzlosh
Copy link
Contributor Author

nzlosh commented Feb 15, 2022

Errbot should not be attempting to override logging in other peoples code as there is no reasonable to way to know if it is required for the use case. User code will need to handle logging based on their needs. They have access to self.bot_config.BOT_LOG_LEVEL if they want to set the log level for their loggers to be the same as errbot's configuration.

@sijis sijis merged commit 310531a into errbotio:master Feb 23, 2022
sijis added a commit that referenced this pull request Jun 11, 2022
* Use errbot log level with webserver for consistent logging.

* Formatting fixes for failing tests.

* fix: use correct PR number

Co-authored-by: Sijis Aviles <sijis.aviles+github@gmail.com>
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.

Incorrect log levels showing for werkzeug
3 participants