Do not keep static references to log levels

Description

There could be others, but the JdbcBindingLogging holds static final references to whether trace or debug log levels are enabled. This should not be done. While it seems faster, you don’t gain much performance from this. It also does not allow levels to be dynamically changed at runtime.

See https://wildfly.zulipchat.com/#narrow/stream/196266-wildfly-user/topic/Adding.20logger.20category.20using.20jboss-cli/near/378714640 for some more details.

Activity

Show:

Sanne Grinovero July 26, 2023 at 8:44 PM

I’m indeed finding many cases in which the level is statically cached w/o it seemingly being justified. I’ll fix them.

James Perkins July 26, 2023 at 5:10 PM

I’m not sure what we could do to make levels immutable really. The JBoss Log Manager is an extension of JUL so we’re tied to that.

I can see arguments for using debug/trace levels at runtime. For example if they are seeing an issue they can’t explain, likely happening at random times, it could be useful to enable trace logging at runtime as restarting the JVM may cause the issue to go away.

Sanne Grinovero July 26, 2023 at 4:31 PM

I’m aware that all Loggers work like this, yet I wonder if we could do better.

Regarding WildFly.. I don’t expect users to reboot, but I also don't expect them to need the lowest level (trace), which we use exclusively for complex debugging cases - as mentioned, the alternative is that we strip them out.

But yes this is just my general answer about “the pattern”, we certainly do want users to be able to log binding parameters - that shouldn’t be done in that case.

James Perkins July 26, 2023 at 4:23 PM

I’m not sure how we could support an immutable level in the JBoss Log Manager. That’s how, to my knowledge, all log managers work. They allow levels to be changed at runtime. By keeping a static final reference, that makes it a requirement to restart the JVM if you want to see messages at that level logged.

This would be an issue for any app that allows for log level changes, not just WildFly. However, this definitely affects WildFly as changing a log level shouldn’t really require restarting the server.

Gavin King July 26, 2023 at 4:13 PM
Edited

The alternative to this pattern being to fully remove such statements from the source code.

I personally think that we could remove quite a lot of the debug/trace-level logging that Hibernate does, but the logging in JdbcBindingLogging is actually super-useful and important. These are the log messages we really can’t remove.

Fixed

Details

Assignee

Reporter

Sprint

Fix versions

Priority

Created July 26, 2023 at 3:18 PM
Updated August 31, 2023 at 5:28 PM
Resolved August 28, 2023 at 3:44 PM