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

Reduce log clutter #4477

Open
wants to merge 1 commit into
base: v2.x
Choose a base branch
from

Conversation

blaz-a
Copy link

@blaz-a blaz-a commented Mar 22, 2024

Reduce log clutter by setting console_logging_verbosity_level to "errors only" (by setting console_logging_verbosity_level= 1) or "errors & warnings" (= 2) in proxysql.cnf; default is "errors, warnings & info" (= 3)

…rs only (= 1) or errors & warnings (= 2) in proxysql.cnf; default is errors, warnings & info (= 3)
@mirostauder
Copy link
Collaborator

Automated message: PR pending admin approval for build testing

@renecannao
Copy link
Contributor

Hi @blaz-a .

Thank you for the PR.

In #4478 (comment) I commented why I am against filtering a specific log entry, but the argument can be generalized.

  • info/warning messages are extremely useful
  • removing info/warning messages could obscure important information and make it more difficult to diagnose issues, both in the present and in the future. It is important to maintain comprehensive logs for troubleshooting and auditing purposes

Please note that these are not debug messages: debug messages are not even present in Release builds.

We are aware that some part of ProxySQL codebase can generate excessive verbosity, and there are variables that control such verbosity like mysql-hostgroup_manager_verbose .
For everything else, the general rule is that if a log entry is generated, it is very likely it is extremely useful.

I think we can consider applying general filtering only if every call to proxy_info() , proxy_warning() and proxysql_error() increases an internal counter, and is optionally exported as Prometheus stats and on disk stats.
Yet, we need some way to make sure that these entries are not lost. For example, if general filtering is applied, internal counters are always updated on disk stats.
Actually, I think I am going to create an issue to later implement this.

@blaz-a
Copy link
Author

blaz-a commented Mar 27, 2024

Hello @renecannao !

I agree with your point on usefulness of log messages and should therefore be enabled/shown by default.

However there might be cases where they are not needed and knowledgeable user should be given an option to reduce output verbosity. Approaching this at specific log levels seems the most common way of achieving this. This has been requested before here, I merely implemented the change. My PR maintains status quo - nothing changes unless the user explicitly modifies the proxysql.cnf setting and knowingly reduces log verbosity.

On the subject of keeping track of errors: PR only suppresses text output - the rest of the code is kept intact so that Prometheus counters etc. get updated accordingly. This means the tell-tale signs that attention is required are still there, it's just that log output that potentially gets reduced.

@mirostauder
Copy link
Collaborator

Can one of the admins verify this patch?

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.

3 participants