-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
A full buffer should not log any errors #742
Comments
@frosiere I got your point. But, whether to move the log to DEBUG level or remove it depends on the use case. Also introducing a configuration to change the log level would be like whack-a-mole for other logs. How about handling the issue on logging configuration side like this? build.gradle implementation 'ch.qos.logback:logback-classic:1.4.11'
implementation 'org.slf4j:slf4j-api:2.0.9'
implementation 'org.codehaus.janino:janino:3.1.10' logback.xml <filter class="ch.qos.logback.core.filter.EvaluatorFilter">
<evaluator>
<expression>return message.startsWith("emit() failed due to buffer full");</expression>
</evaluator>
<OnMismatch>NEUTRAL</OnMismatch>
<OnMatch>DENY</OnMatch>
</filter> This isn't so robust, but very flexible so you can customize it if needed. |
Thanks for the reply. Logging an error can only lead to an infinite loop until we recover. This seems a weird behavior. I may use the proposal but we have multiple applications handled by multiple teams so, it would avoid it. |
How to handle I need to consider the balance between various use cases and the simplicity of the design and implementation of Fluency. So, unfortunately, I won't move forward on the option you suggested to change the log level, which some organizations have already depended on. |
Fluency currently logs an error if the buffer is full.
See Fluency.Emitter#emit
This can lead to a large number of errors (not to say an infinite loop) if Fluency can't recover quickly.
So, shouldn't we move this log to the debug level or simply abandon it?
We'd still be able to detect this type of problem because an exception is triggered after we've deleted the error log.
Any feedback on this subject is most welcome.
If we agree on the proposal, I can contribute with a PR.
The text was updated successfully, but these errors were encountered: