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

Change to log4j2 logging library implementation for logging messages (components choose log4j 1.2 api or log4j 2 api to generate messages) #6397

Merged
merged 14 commits into from
Aug 31, 2022

Conversation

jodygarnett
Copy link
Contributor

@jodygarnett jodygarnett commented Jun 15, 2022

This has for the most part gone quite smoothly, the log4j 1.2 bridge is available to continue supporting the existing log4j 1.2 xml configurations which we have tested. Also tested the harvester logging continues to work as expected (where it programmatically adds an file appender).

Phase 1:

  • introduce log4j2-core
  • bridge slf4j to log4j2-core
  • bridge java util logging to log4j2-core
  • bridge apache commons logging to log4j2-core (done by spring-framework)
  • Update LoggingAPI to list log4j 1.2 and log4j2 configuration files for selection
  • Update LogUitl to support log4j2 configuration files
  • double check status ability to view and download log file determined from file appender
  • double check harvester code that programatically adds an appender

Phase 2:

  • Replace built-in log4j 1.2 configuration files (optional)
  • transition geonetwork codebase to log4j2-api (optioonal - a big search and replace)

Reference:

@jodygarnett
Copy link
Contributor Author

jodygarnett commented Jun 15, 2022

This work is going well, you can try running it right now using log4j-core backend.

One odd thing was core requiring use of bridge to slf4j 1.7 api for running apacheds; while the workers require a newer slf4j18 bridge to support slf4j1.8 api for camel.

@jodygarnett
Copy link
Contributor Author

@juanluisrp what do you think about doing this work in several steps? The use of log4j-core as a back end is now complete for example. If we merge this you could sort out your camel dependencies.

@jodygarnett
Copy link
Contributor Author

You are welcome to merge 6397 as is; the part that does not work is the harvesters.

Log4j does not want us to dynamically create appenders as programmers; instead it strictly wants everything defined by the configuration (so the sys admin as control).

So the approach to use is to:

  1. make a thread context map with the name of the harvester
ThreadContext.put("harvester", name);
  1. in the configuration generate the log file name dynamically using the name of the harvester from the thread locale
<appenders>
    <File name="harvester" fileName="$${ctx:harvester}">
        <PatternLayout pattern="%-4r %d{${datestamp}} [%t] %-5level %logger{36} - %msg%n"/>
    </File>
</appenders>
  1. If that does not work we should try routing (which has a more complete example):
<Routing name="Routing">
  <Routes pattern="$${ctx:harvester}">

    <Route key="$${ctx: harvester}">
      <File name="harvester-default" fileName="logs/harvester.log">
        <PatternLayout>
	  <pattern>%d{ISO8601} [%t] %p %c{3} - %m%n</pattern>
        </PatternLayout>
      </File>
    </Route>

    <!-- value dynamically determines the name of the log file. -->
    <Route>
      <File name="harvester-${ctx: harvester}" fileName="logs/harvester-${ctx:harvester}.log">
	<PatternLayout>
	  <pattern>%d{ISO8601} [%t] %p %c{3} - %m%n</pattern>
	</PatternLayout>
      </File>
    </Route>
  </Routes>
</Routing>

Reference:

Copy link
Member

@josegar74 josegar74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have done minimal testing (harvesters, metadata creation, thesaurus, searches, pdf export) including the changes from jodygarnett#2 also and looks no, no issues found.

The application starts up now properly in Tomcat.

common/src/main/java/org/fao/geonet/utils/Log.java Outdated Show resolved Hide resolved
/**
* This is the name of the RollingFileAppender in your log4j2.xml configuration file.
*
* LogConofig uses this name to lookup RollingFileAppender to check configuration in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small typo in LogConofig

@jodygarnett jodygarnett changed the title Change to log4j2 logging library Change to log4j2 logging library implementation for logging messages (components choose log4j 1.2 api or log4j 2 api to generate messages) Aug 27, 2022
@jodygarnett
Copy link
Contributor Author

@fxprunayre there is nothing to be done here, I will wait for it to be merged

@josegar74 josegar74 merged commit c9bde35 into geonetwork:main Aug 31, 2022
josegar74 added a commit to GeoCat/core-geonetwork that referenced this pull request Aug 31, 2022
….2.17.

The log from the libraries using log2j 1.2 should work fine, using the log4j 1.x bridge.

Related to geonetwork#6397
@jodygarnett jodygarnett mentioned this pull request Aug 31, 2022
11 tasks
josegar74 added a commit that referenced this pull request Sep 1, 2022
….2.17. (#6522)

The log from the libraries using log2j 1.2 should work fine, using the log4j 1.x bridge.

Related to #6397
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants