logging-log4j2 icon indicating copy to clipboard operation
logging-log4j2 copied to clipboard

SetLevel accepts boolean to check if its false set a OFF

Open RafaelGB opened this issue 2 years ago • 7 comments

Since yaml modify the OFF value as false. Would be nice to allow the boolean input or String input with value false to activate the off level

RafaelGB avatar Oct 06 '23 11:10 RafaelGB

@RafaelGB,

Apparently the interpretation of the literal OFF depends on the version of YAML (cf. this SO question):

  • in version 1.1 OFF is the boolean value false,
  • in version 1.2 OFF is the string value "OFF".

Most YAML parsers use either snakeyaml (YAML version 1.1) or snakeyaml-engine (YAML version 1.2) to parse files.

In version 2.x our YAML configuration parser relies on Jackson 2.x, which relies on snakeyaml. So I propose to fix the LevelConverter to accept "FALSE" as an alias of "OFF".

On the other hand in 3.x we will no longer rely on Jackson for the configuration, so we could use snakeyaml-engine directly, which will solve the problem.

@jvz, what do you think?

ppkarwasz avatar Oct 06 '23 11:10 ppkarwasz

In our case (same team as @RafaelGB ), we are setting our Log4j2 config programmatically, meaning we rely on Spring to parse the YAML file and give us the value in a class attribute. We are currently using Spring Boot 2.7.7.

luisrm1012 avatar Oct 06 '23 11:10 luisrm1012

@luisrm1012,

Can you provide more details? If Spring Boot injects your configuration into a class, you can use a custom Spring converter to convert false to Level.OFF.

ppkarwasz avatar Oct 10 '23 07:10 ppkarwasz

If you mean the general Spring Boot properties logging.level.[logger-name]: off, then this is specific to however Spring decided to handle this particular YAML parser. If this is for a log4j2.yml config file, then I think what @ppkarwasz suggested is a good idea.

jvz avatar Oct 15 '23 20:10 jvz

We have our own application.yml in our spring boot app, where we define the logs.root property. From one of our classes, we read the logs.root property defined in the application.yml, and when the app reads the off value, it sets it as false.

What we meant in our initial comment, was to give support to the false value, since it is how spring boot reads the value from the YAML file. Meaning that when you do Level.valueOf(false) behaves the same way as Level.valueOf("OFF") does.

luisrm1012 avatar Oct 16 '23 09:10 luisrm1012

Got it; thanks for the context! I think we could make Level.valueOf(false) equivalent to Level.OFF.

jvz avatar Oct 16 '23 17:10 jvz

Got it; thanks for the context! I think we could make Level.valueOf(false) equivalent to Level.OFF.

We only have Level.valueOf(String) right now, I don't feel comfortable adding a Level.valueOf(Boolean) method for a format-specific problem. I would prefer to add a ConditionalConverter that converts a Boolean or String to Level to log4j-spring-boot.

ppkarwasz avatar Oct 16 '23 18:10 ppkarwasz

@luisrm1012, @RafaelGB, I have the impression that the request here is to address a YAML inconsistency in Log4j, whereas

  • users could have simply quoted the value to avoid any ambiguity
  • Spring Boot's Level parsing could have addressed this
  • etc.

Put another way, I can think of several ways to solve your problem, but fixing it in Log4j. We can also find several other examples where a 3rd party integration behaves unintuitively and we can again land a fix to Log4j to work around that. IMHO, Log4j is not the right place to fix this problem and I am not inclined to proceed with this feature.

vy avatar Mar 06 '24 08:03 vy