georchestra / mapstore2-georchestra

geOrchestra newest viewer
Other
6 stars 23 forks source link

Follow up update to log4j2 in mapstore2 #580

Closed taba90 closed 1 year ago

taba90 commented 1 year ago

Description

The master branch of MapStore2 has been updated in order to use log4j2. When georchestra will be aligned with the version having this updates some changes will be needed namely:

What kind of improvement you want to add? (check one with "x", remove the others)

Other useful information

tdipisa commented 1 year ago

Draft PR to be completed with the work for this issue to finalize the update to 2023.01.xx

offtherailz commented 1 year ago

The master branch of MapStore2 has been updated in order to use log4j2. When georchestra will be aligned with the version having this updates some changes will be needed namely:

  • the class referenced here will not be available along with the static method configure. Probably it should be enough to create a java class with a static method configure with same arguments of the one invoked in the applicationContext that reload the configuration. Something like:
public static void configure (String log4jPropertiesPath){
LoggerContext context = (org.apache.logging.log4j.core.LoggerContext) LogManager.getContext(false);
context.setConfigLocation(new URI(log4jPropertiesPath));
}

or

public static void configure (String log4jPropertiesPath){
Configurator.reconfigure(new URI(log4jPropertiesPath));
}

This is not necessary, I've found a more elegant and functional solution. The bean that used that configure method was neede to optionally load the file log4j.properties from datadir, when defined, or from the standard package.

In log4j2 we can implement this functionality by adding to the classpath the file log4j2.component.properties. This is a property configurer with a higher priority that can replace the env variable. Addint this file in the WEB-INF/classes by default and configuring it this way:

log4j.configurationFile=classpath:log4j2.properties,${sys:georchestra.datadir}mapstore/log4j2.properties

We can obtain the same result (given that anyway the log4j.properties files have to be replaced with log4j2.properties file, and use a new syntax)

log4j.configurationFile property configured in log4j2.component.properties can accept an array of file paths, that can exist or not, and that are applied in override to the previous ones.

So the logic of "loading from the classpath of from the datadir", can be completely replaced by this file logic (added in the Draft PR](https://github.com/georchestra/mapstore2-georchestra/pull/625) mentioned.

In particuar In the new Draft PR we have this new log4j2.properties

rootLogger.level = INFO
appenders= console, file

appender.console.type = Console
appender.console.name = LogToConsole
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = %p %d{yyyy-MM-dd HH:mm:ss.SSS} %c::%M:%L - %m%n
rootLogger.appenderRef.stdout.ref = LogToConsole
rootLogger.appenderRef.console.ref = LogToConsole

appender.file.type = File
appender.file.name = LogToFile
appender.file.fileName=${sys:catalina.base}/logs/mapstore.log
appender.file.layout.type=PatternLayout
appender.file.layout.pattern=%p   %d{yyyy-MM-dd HH:mm:ss.SSS}   %C{1}.%M() - %m %n
rootLogger.appenderRef.file.ref = LogToFile

logger.restsrv.name=it.geosolutions.geostore.services.rest
logger.restsrv.level=  INFO
logger.hibernate1.name=org.hibernate
logger.hibernate1.level=INFO
logger.trg1.name=com.trg
logger.trg1.level=INFO

If we put in datadirectory a file named log4j2.properties configured this way:

rootLogger.level = INFO
appenders= console, file

appender.console.type = Console
appender.console.name = LogToConsole
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = %p %d{yyyy-MM-dd HH:mm:ss.SSS} %c::%M:%L - %m%n
rootLogger.appenderRef.stdout.ref = LogToConsole
rootLogger.appenderRef.console.ref = LogToConsole

appender.file.type = File
appender.file.name = LogToFile
appender.file.fileName=${sys:catalina.base}/logs/mapstore_changed.log
appender.file.layout.type=PatternLayout
appender.file.layout.pattern=%p   %d{yyyy-MM-dd HH:mm:ss.SSS}   %C{1}.%M() - %m %n
rootLogger.appenderRef.file.ref = LogToFile

logger.restsrv.name=it.geosolutions.geostore.services.rest
logger.restsrv.level=  INFO
logger.hibernate1.name=org.hibernate
logger.hibernate1.level=INFO
logger.trg1.name=com.trg
logger.trg1.level=INFO

The application will log in mapstore_changed.log and not in mapstore.log anymore.

  • remove this dependency or introduce the log4j2 to log4j bridge in order to keep it usable (needs to be tested)

done

  • update log4j dependencies and getLogger method in the java code involving it.

There is not actually code that use Lof4j logic. This can be added to the guidelines for the upgrade of geOrchestra.

tdipisa commented 1 year ago

@offtherailz I'm sorry but, why the following new name?

The application will log in mapstore_changed.log and not in mapstore.log anymore.

offtherailz commented 1 year ago

Maybe I didn't explained this well. This mapstore_changed.log is only one example of successful test that explains that if I put one file in the datadir (named log4j2.properties) it will be used and override default settings in WEB-INF/classes/log4j2.properties

The default one will save logs in ${catalina.base}/logs/mapstore.log. This is for overriding the configuration, if needed. Originally this log4j configuration replacement was implemented by the class referenced here in this issue. Adding log4j2.component.properties to classPath, properly configured, I have more or less the same result.