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 the log level of validateObject to WARN #3749

Closed
sazzad16 opened this issue Feb 29, 2024 · 6 comments · Fixed by #3750
Closed

Reduce the log level of validateObject to WARN #3749

sazzad16 opened this issue Feb 29, 2024 · 6 comments · Fixed by #3750

Comments

@sazzad16
Copy link
Collaborator

sazzad16 commented Feb 29, 2024

There have been confusions, misunderstandings, half-understandings regarding ERROR logs in JedisFactory#validateObject and ConnectionFactor#validateObject.

Here is a list of relevant concerns:

I think, reducing the log level to WARN would resolve or at least reduce these concerns to some extent.

@sazzad16
Copy link
Collaborator Author

@yangbodong22011
Copy link
Collaborator

I think general business will configure the level of info. There seems to be no difference between error or warn (both will be printed). Maybe we should consider debug?

@kdrakon
Copy link

kdrakon commented Feb 29, 2024

I think general business will configure the level of info. There seems to be no difference between error or warn (both will be printed). Maybe we should consider debug?

Both would be printed, but having them at specific levels means setting up alerts based on the level (eg Datadog, Pagerduty, etc) is possible. For example, we only trigger alerts for on-call when the level is ERROR, meaning something catastrophic, possibly unrecoverable, has happened. Since most people don't log production applications at the DEBUG level, those logs would be unseen.

@sazzad16
Copy link
Collaborator Author

The thing is we wanted those to be seen so that users can take actions. Sometimes its just network, sometimes tweaking Jedis/Redis configs were enough to improve the health/performance of the application.

IMO, we now go to WARN. Depending on the feedback/reaction, we'll consider DEBUG.

@kdrakon
Copy link

kdrakon commented Mar 6, 2024

Is there any reason this was leapfrogged and not included in release https://github.com/redis/jedis/releases/tag/v5.1.2?

@sazzad16
Copy link
Collaborator Author

sazzad16 commented Mar 8, 2024

@kdrakon This has breaking implication. Some applications may be dependant on the logs, but they will miss out if their log level is set to ERROR. So this change is not part of a patch release and will be part of 5.2.0. If you really want, this change is available in 5.2.0-SNAPSHOT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants