Sponge
Sponge copied to clipboard
Replace printStackTrace() with a proper logger
I originally had an issue where my plugin failed to construct but I had no traces in the log files so I wanted to replace the printStackTrace() with a proper logger. I took the opportunity to do this in several places.
There are still other printStackTrace()
indeed.
Most of them are in the test plugins / unit tests, I thought it wasn't really important.
Some others are in asm class visitors so wasn't sure if it would cause classloading issues or not.
I have now replaced all printStackTrace
except in test sourcesets, and except in SpongeWorldManager
because it will be fixed by PR #3702.
@Yeregorix @zml2008
I am aware at the amount of effort of such a change but zml you are 100% right, the "global logger" is not helpful, not to know where whatever is coming from. I think it is time to likely axe it so SpongeCommon.logger()
goes away and each class that logs creates it's own logger. Thoughts? Beyond being a massive change, I see no issue. This doesn't change giving each plugin a logger. If a plugin wants to do class loggers or not that is up to them. This is just a stylistic choice for us to keep doing this for them.
From a developer point of view, I prefer each class having its own logger.
From a server owner pov, it's easier to identify which mod is involded when you see [Sponge]
at the beginning of the logged lines.
-
[Sponge] Something went wrong.
(easy to identify that Sponge is involved) vs -
[SomeClass] Something went wrong.
(most people have no idea whatSomeClass
is or to which mod it belongs)
@Yeregorix I agree. However classes such as DataUpdaterDelegate fall right into this issue in this PR, no? I'd have to look at our typical log files but someone wouldn't know based on the logging lines.
Yes, I'm just opposing the pros and cons of each convention.
For the moment, for each class I used the common logger if I saw that it was already used in the class, otherwise I used a specific logger.