Until the delayed handler is activated, drop anything below INFO level #16265
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
quarkus-bot
bot
added
the
area/devtools
|
So the idea is to not accumulate anything below INFO? |
|
Yeah. We can't decide if the message will be logged as it's too early for that so either we accumulate everything or we filter with static rules. I think accumulating everything doesn't work very well so, while imperfect, I think it's the best compromise we can have. |
gsmet
force-pushed
the
log-drop-below-info
branch
from
00aa269 to
3a8a9a6
Compare
|
I don't like it, but I don't like the current situation either, so it's worth giving a shot |
geoand
approved these changes
Apr 6, 2021
Welcome to the club :) |
|
@gsmet I'll try to test this afternoon with your branch. I couldn't reproduce it with a small project. |
|
@gsmet I tested and Hazelcast logs are back after your fix |
This is definitely not a perfect solution but keeping all the log messages in memory sounds worse anyway.
gsmet
force-pushed
the
log-drop-below-info
branch
from
3a8a9a6 to
7a89e04
Compare
|
@gsmet I did some more debugging to find the root cause of this accumulated log messages problem. I added a the following line just before your change to see what happens.
I see lots of Hibernate related logs are processed by |
|
Yeah, this thing receives everything logged. And it cannot discard things dynamically as it's too early, we don't have the config yet. That being said... where did you put your System.out? Was it in the not activated part (where I put my code) or at the top of the method? Because Hibernate should be started AFTER our loggers are initialized so you should be in the activated case with the Hibernate ORM messages. |
|
|
Hum. There is a problem then. |
Test Failures⚙️ jvm-linux-jdk8📦 integration-tests/kafka# Tests: 10
+ Success: 8
- Failures: 0
- Errors: 2
! Skipped: 0❌
|
|
So, in fact, it's totally normal. The logging infrastructure is initialized at runtime init so part of the init is done before it is fully initialized thus explaining what you see. This makes this patch a bit counter productive as it might hide some debug you would want. But I think I'll push it to 1.13 and that we can try to find a better solution for the next micro. |
|
What about this approach instead: #16303 Basically only drop if we hit the limit, and drop lowest priority messages first, so you loose TRACE before DEBUG etc. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is definitely not a perfect solution but keeping all the log
messages in memory sounds worse anyway.
/cc @geoand
This could fix #16239 if I'm not mistaken.
@mincel any chance you could test this change? You can follow the instructions here https://github.com/quarkusio/quarkus/blob/main/CONTRIBUTING.md#checking-an-issue-is-fixed-in-main except you need to get my branch.
If you can come up with a reproducer, I can have a look myself.