Investigate impact of switching default logging system to log4j2

[philwebb]

We've had requests before to switch the default logging system from logback to log4j2. In order to prepare for Spring Boot 3.x, we should reassess what the impact of such a switch would be for our users.

(somewhat triggered by the request to support JSON logging [#5479])

[rgoers]

@philwebb If there is anything the Apache Logging team can do to assist you in this please let us know.

[wilkinsona]

@rgoers Thank you.

One area that's just come up is providing log4j2-based support for the <springProfile> and the <springProperty> elements that we currently support with Logback.

<springProperty> is rather like a lookup and is quite similar to the existing Spring Boot lookup. <springProperty> doesn't have a dependency on Spring Cloud whereas it appears the Log4j2's Spring Boot lookup does?

<springProfile> provides some basic conditional configuration support, allowing configuration to be applied based on the Spring profiles that are active. From what I've seen, there isn't something similar in Log4j2 at the moment.

Spring Boot 3 is by no means imminent (it won't be in 2021) so there's no urgency here at the moment, but it would be interesting to explore what plugging <springProperty> and <springProfile> support into Log4j2 might look like.

[rgoers]

@wilkinsona The Spring Lookup was in log4j-spring-cloud-config when it was first released. However, it was broken out into log4j-spring-boot in 2.14.0. As you might imagine, that module only requires Spring Boot.

As for <springProperty>, I've never had a need for it so it never occurred to me to implement it. We simply configure the different config file locations for each profile in our Spring Cloud Config server in bootstrap.yml. That said I don't see any reason why we couldn't support "conditional" plugins. They would be a bit different than any other plugins as their action would take place while processing configuration file and would not appear in the resulting configuration object.

I have created LOG4J2-3064 to Log4j 2 to add support for conditionals. In general, we want to stay away from introducing any kind of programming logic in the config file but having a component that simply decides whether its children should be included or not seems reasonable.

[rgoers]

I have added support for <springProfile>. It should be in Log4j 2.15.0. I don't see a reason to add support for a SpringProperty plugin since

<springProperty scope="context" name="fluentHost" source="myapp.fluentd.host"
	defaultValue="localhost"/>
<appender name="FLUENT" class="ch.qos.logback.more.appenders.DataFluentAppender">
  <remoteHost>${fluentHost}</remoteHost>
  ...
</appender>`

can be accomplished in Log4j2 with

<Property name="fluentHost">${spring:myapp.fluentd.host:-localhost}</Property>
<Appenders>
    <Fluent remoteHost="${fluentHost}">
    ...
    </Fluent>`
</Appenders>

or just

<Appenders>
    <Fluent remoteHost="${spring:myapp.fluentd.host:-localhost}">
    ...
    </Fluent>`
</Appenders

[wilkinsona]

Thanks, @rgoers.

I'm a little bit wary of the circular dependency between the two projects. It looks like we could implement something that's equivalent to <springProperty> in Spring Boot itself via a StrLookup plugin that's similar to SpringLookup. With the new Arbiter support that's been added in LOG4J2-3064, it looks like we could implement an equivalent of <springProfile> as well without Log4j2 having a dependency on Spring Boot.

If we were to take this approach, would you be open to deprecating the support in Log4j2 so that we don't confuse users with two very similar ways of achieving the same thing?

[rgoers]

In general I am open to the idea but I would want it to completely replace log4j2-spring-boot. That would mean:

1. The SpringLookup would need to continue to be exposed in addition to <springProperty>, if you implement that for compatibility. Log4j users using SpringLookup (including me) would not want to lose that.
2. The enhancements in Log4j2CloudConfigLoggingSystem would either need to be included in Log4j2LoggingSystem or I would have to move that back to log4j2-spring-cloud-config-client. The enhancements include: a) storing the Environment in Log4j's LoggerContext (I didn't really understand why Spring didn't do that instead of storing a string in the external environment) which is used by SpringLookup and SpringProfile, b) adds Log4j's system property to the list of standard config locations c) adds support for the location string to be a list of locations or a single url with override parameters and constructs a CompositeConfiguration if there are multiple.
3. SpringPropertySource would need to be migrated. That allows any of Log4j's system properties to be retrieved from the Spring Environment.

I would also want to be able to check something in log4j2-spring-boot to determine if the Spring Boot equivalents are present so I can a) log a warning and b) delegate to Spring Boot since these components will have duplicate plugin names.

To add some color to this, all of the applications we are developing are moving to use Spring Cloud Config to host the logging configurations. To do this we configure our bootstrap.yml with

    logging:
       config: "https://spring-configuration-server.mycorp.io/MyService/default/${logging.label}/log4j2-dev.xml?override=https://spring-configuration-server.mycorp.io/MyService/default/${logging.label}/log4j2-MyService-dev.xml"
        auth:
           username: ${spring.cloud.config.username}
           password: ${spring.cloud.config.password}

The way our config servers are set up this allows all the applications to share a log4j2-dev.xml file in the root directory and then each application can have specific overrides to add additional loggers, filters, or whatever.

I should also note that log4j2 supports publishing properties in log4j2.system.properties as system properties. You will see an example of that in log4j2-spring-boot where Log4j overrides the Spring LoggingSystem. Since Log4j is usually initialized first this will normally mean the properties are published before anything that requires them tries to get them.

[unlimitedsola]

I am slightly afraid of the switch due to the JNDI RCE vulnerability that happened to log4j2 recently.

I don't think there is a legitimate reason for a logging library to make network requests based on the contents of what is being logged, not to mention it is enabled by default.

And now I'm glad that Spring Boot didn't choose log4j2 as the default logging system in the first place.

[laxika]

@unlimitedsola As far as I see, your thinking has some significant survivorship bias built into it. I'm not sure if you could review a library's usefulness based on one bug (even if that bug is 10/10 on the Richter scale).

[unlimitedsola]

@unlimitedsola As far as I see, your thinking has some significant survivorship bias built into it. I'm not sure if you could review a library's usefulness based on one bug (even if that bug is 10/10 on the Richter scale).

Don't get me wrong. Usefulness is not the only factor we should consider when making such decisions. Security concerns ARE legitimate concerns and, as I said, adding a "feature" that makes network requests based on the user's input won't make a good impression on anyone.

The issue here is not about the "one bug", it's about the trust instead.