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.

Sign up for GitHub

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

Jump to bottom

create global log4j config #23631

Closed
spbolton opened this issue Dec 21, 2022 · 2 comments · Fixed by #29694 or #29746
Closed

create global log4j config #23631

spbolton opened this issue Dec 21, 2022 · 2 comments · Fixed by #29694 or #29746
Labels
Changelog: Breaking Change Merged QA : Approved QA : Passed Internal Release : 24.08.27 Custom templates and loggin fixes Team : Scout

Comments

@spbolton
Copy link
Contributor

spbolton commented Dec 21, 2022 •
edited
Loading

Currently Tomcat has its own logging and Log4j only initialises when the webapp starts up. This makes it hard to track issues especially in our k8s environments.

This task is to setup tomcat to use log4j2 for its logging and to use the same single context for the webapp allowing for a single log4j config file to handle the whole running of the application.

There is an existing proof of concept but it was causing some tests to fail. We need to merge in these changes into the core

#23964
spbolton pushed a commit that referenced this issue Dec 21, 2022
global log4j config (#23631)
9850cb3
spbolton pushed a commit that referenced this issue Jan 31, 2023
global log4j config (#23631)
f4dcc83
@spbolton spbolton mentioned this issue Jan 31, 2023
Draft
spbolton pushed a commit that referenced this issue Feb 3, 2023
global log4j config (#23631)
a3d7c5a
spbolton pushed a commit that referenced this issue Feb 3, 2023
global log4j config (#23631)
5d41202
victoralfaro-dotcms added a commit that referenced this issue Feb 7, 2023
@victoralfaro-dotcms
#23631: docker copy fix
aa5f866
@github-actions
Copy link

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale label Mar 22, 2023
@nollymar nollymar removed the stale label Jul 10, 2024
@spbolton spbolton mentioned this issue Aug 21, 2024
Merged
3 tasks
@spbolton spbolton linked a pull request Aug 21, 2024 that will close this issue
feat(logging): create global log4j config (#23631) #29694
Merged
3 tasks
github-merge-queue bot pushed a commit that referenced this issue Aug 22, 2024
@spbolton
feat(logging): create global log4j config (#23631) (#29694)
46b0a3d 
### Proposed Changes
Issue #23631 

* Add global log4j2 configuration
* default log4j2.configurationFile set in tomcat setenv.sh
* default
Log4jContextSelector=org.apache.logging.log4j.core.async.BasicAsyncLoggerContextSelector
   * This provides a single context and is not based on the classloader
* created lib folder for logging jars and added to tomcat classpath
* moved redis session manager jars into their own folder
* Normalized maven dependencies to help with debugging and versions
centrally specified in bom
* Removed log4j reloading functionality 

As well as using a common configuration for tomcat and webapp logs. This
also resolves an inconsistency when The logging class is referenced in
code before the ContextLifecycleListener is called. Previously the wrong
configuration contained in the resources classpath of the ant-tooling
jar was picked up initially.

Tests

* Test tomcat log messages are output to the same location
* Test any issues with logging from plugins
* Test ability to override log4j config and change log levels for core
tomcat classes, webapp classes and plugin classes. currently can map to
/srv/OVERRIDE/WEB-INF/log4j in container or update CATALINA_OPTS to
include -Dlog4j2.configurationFile={path to file}
* Test session manager functionality is not broken
* Will need a review of the logging behavior from cloud engineering when
this image is applied into our cloud environment including any log-level
changes they may want to have by default


### Further changes
There is follow up work to this PR that improves the way that the config
can be specified and overridden. This change maintains the existing
config file behavior but just applies it across the entire application.



### Checklist
- [ ] Tests
- [ ] Translations
- [ ] Security Implications Contemplated (add notes if applicable)

### Additional Info
This PR resolves #23631 (create global log4j config).


### Screenshots
Original             |  Updated
:-------------------------:|:-------------------------:
** original screenshot **  |  ** updated screenshot **
@nollymar nollymar reopened this Aug 23, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 24, 2024
@spbolton
fix: revert session manager jar location change from PR #29694 (#23631)…
1c414a1 
… (#29746)

### Proposed Changes
* Fix classloader issue with session manager introduced in PR #29694
* Use CATALINA_HOME in setenv scripts instead of TOMCAT_HOME. The first
is always available, the second is only set within the docker startup
scripts

Instructions for Log4j require the classes to be added to the system
classloader when tomcat loads by adding to the main java CLASSPATH.
These logging jars do not need to reference any of the tomcat jars
the session manager jars do need to reference the tomcat jars e.g.
catalina.jar in the main lib folder. This is what is called the
common.loader and is defined in catalina.properties file.
the session manager jars either need to be in a folder referenced in
this common.loader property or as before dropped into the existing lib
folder. There is no need to change the old behaviour at this time so we
can just change the install location back.
@github-actions github-actions bot mentioned this issue Aug 24, 2024
Merged
@fabrizzio-dotCMS
Copy link
Contributor

fabrizzio-dotCMS commented Aug 27, 2024 •
edited
Loading

From what I can tell... This is in perfect shape
I tested using a fresh copy of Tomcat after having rebuilt the source base as follows:
./mvnw clean install -pl :dotcms-core -DskipTests -DskipITs -Ddocker.skip

  • the logs from the .sh that launches Tomcat and the logs from the web app are logged into the same destination file Which was the main goal pursued by this change.
/Users/fabrizzioaraya/Coding/tomcat-9.0.85/bin/catalina.sh run
Console output is saving to: /Users/fabrizzioaraya/Coding/tomcat.log
Log4j configuration already set

  • I tested that the logs written from plugins end in the same intended destination as expected.

  • I also tested the overriding log4j using CATALINA_OPTS like this:

  • export CATALINA_OPTS="$CATALINA_OPTS -Dlog4j2.configurationFile=/path/to/the/configFile.xml"

  • I was able to create a separate output file with different logging level

  • finally, I tested using new entries I added to the source code to verify that we can use the different logging levels DEBUG, INFO, WARN, and ERROR

  • Session manager seems to be working properly

@fabrizzio-dotCMS fabrizzio-dotCMS removed their assignment Aug 27, 2024
@bryanboza bryanboza added QA : Approved Release : 24.08.27 Custom templates and loggin fixes labels Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Changelog: Breaking Change Merged QA : Approved QA : Passed Internal Release : 24.08.27 Custom templates and loggin fixes Team : Scout
Projects
dotCMS - Product Planning
Archived in project
Milestone
No milestone
4 participants
@spbolton @bryanboza @nollymar @fabrizzio-dotCMS