Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate impact of switching default logging system to log4j2 #22149

Open
philwebb opened this issue Jun 29, 2020 · 11 comments
Open

Investigate impact of switching default logging system to log4j2 #22149

philwebb opened this issue Jun 29, 2020 · 11 comments
Labels
status: blocked An issue that's blocked on an external project change type: enhancement A general enhancement
Milestone

Comments

@philwebb
Copy link
Member

philwebb commented Jun 29, 2020

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])

@philwebb philwebb added this to the 3.x milestone Jun 29, 2020
@philwebb philwebb added the type: enhancement A general enhancement label Jun 29, 2020
@rgoers
Copy link
Contributor

rgoers commented Jan 3, 2021

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

@wilkinsona
Copy link
Member

@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
Copy link
Contributor

rgoers commented Apr 6, 2021

@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
Copy link
Contributor

rgoers commented Apr 11, 2021

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
Copy link
Member

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
Copy link
Contributor

rgoers commented Apr 12, 2021

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.

@scottfrederick scottfrederick added the status: blocked An issue that's blocked on an external project change label Nov 1, 2021
@unlimitedsola

This comment has been minimized.

@laxika

This comment has been minimized.

@unlimitedsola

This comment has been minimized.

@rgoers

This comment has been minimized.

@unlimitedsola

This comment has been minimized.

@spring-projects spring-projects locked as too heated and limited conversation to collaborators Jan 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: blocked An issue that's blocked on an external project change type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants