Normalize loggers and upgrade log4j 1.2 to log4j 2.19 by JoaoJandre · Pull Request #7131 · apache/cloudstack · GitHub

Description

This PR normalizes ACS's loggers and upgrades log4j 1.2 to log4j 2.19. Back in 2015, PR #714 tried to normalize the loggers but, because that normalization caused some issues with the VMware module, it ended up being reverted. This PR intends to follow that PR's footsteps and normalize all of the loggers in ACS. The objective of reducing unnecessary logger declarations, and reduce the area of impact to upgrade ACS's logger framework from log4j 1.2 to log4 2.19.

The log4j upgrade was done similarly to the proposed here #2992 (comment), and is more detailed ahead.

The logger normalization includes:

Classes that could have their loggers inherited from their ancestors had their own loggers deleted;
Most loggers didn't have to be static, thus most of them were normalized so that they wouldn't be;
loggers are protected now;
Static logger's names are now 'LOGGER';
Non-static logger's names are now 'logger';
New class DbUpgradeAbstractImpl created so that all Upgraders extend it and inherit its logger;
The following classes now have loggers (so that all their children can inherit their loggers):
BaseElement, PipelineImpl, AbstractRect, AbstractSmartLifeCycle, ManagedContextRunnable, Command, RuleApplier, VmWork, ModelObjectBase, SnapshotStrategyBase, VifDriver.

To normalize all the loggers in ACS, the following steps were performed:

The DbUpgradeAbstractImpl class was created;
This script https://pastebin.pl/view/6ff71b26 was used to make all the Upgraders extend the DbUpgradeAbstractImpl class (I manually had to correct some of them afterward);
I ran this script https://pastebin.pl/view/89931790 for all ACS java classes; the, I checked if classes can inherit the logger from their ancestor*; if they can, then remove their current logger instantiation. If they cannot then leave the instantiation, but make it non-static. And in some cases, rename the logger references to logger;
I tried to build ACS many times and manually fixed the errors, most of them were because some loggers did indeed have to be static.

*The ancestor list I created came from a previous iteration of this process, where I tried to simply remove logger instantiation from any class that extended any other class, the list was created while manually fixing the errors.

For the log4j upgrade, the following steps were performed:

The relevant pom.xml files were updated;
This script https://pastebin.pl/view/0b6b77f9 was used to update all the logger imports;
I tried to build ACS many times and manually fixed the errors.

If you need to fix conflicts because of this PR, you can try using this script to help you:

https://pastebin.com/GzMP3PCA

About the log4j upgrade:

The DOMConfigurator does not exist in log4j2, its functionality was moved to the Configurator class;
Both NDC and MDC were merged into the ThreadContext class;

Disclaimers:

The new log4j2 configuration file format is not backwards compatible with the old one, users must update their configuration files to match with the new format. I used the official apache documentation to upgrade the default configurations, available here.
If you call EncryptorCLI manually, you must also pass the log4j2 jar path (which should be in the same directory as cloud-utils).

The are still some points to address/discuss in order to make this PR ready:

Refactor the following classes so that they can be used in log4j2:SnmpTrapAppender, AlertsSyslogAppender, TestAppender, SnmpEnhancedPatternLayout;
Convert CglibThrowableRenderer functionality to log4j2;
The log4j 1.2 dependency could not be completely removed because juniper contrail has log4j as a dependency.
Test logging for management, agent, usage, SSVMs and CPVMs

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

BLOCKER
Critical
Major
Minor
Trivial

Screenshots (if appropriate):

How Has This Been Tested?

I tested if both the console and the file appenders were logging correctly in the management server, agents, and system VMs.

All classes that could have their loggers inherited from their fathers had their own loggers deleted; Most loggers didn't have to be static, so most of them were normalized so that they wouldn't be; All loggers are protected now; Static logger's name are now 'LOGGER'; Non-static logger's name are now 'logger'; New class DbUpgradeAbstractImpl created so that all Upgraders extend it and inherit its logger

…grade-log4j

@DaanHoogland @GutoVeronezi could you please review?

…he last configuration files

@JoaoJandre , I didn´t yet review 2000 files, but on first sight it looks good. I hope you have scripts to regenerate the changes, because this PR will be a conflict magnet.

Codecov Report

Attention: 271 lines in your changes are missing coverage. Please review.

Comparison is base (2bfa9de) 30.94% compared to head (3cfc93a) 30.77%.

Files	Patch %	Lines
agent/src/main/java/com/cloud/agent/Agent.java	0.00%	93 Missing ⚠️
...nt/resource/consoleproxy/ConsoleProxyResource.java	0.00%	44 Missing ⚠️
...in/java/com/cloud/agent/api/storage/OVFHelper.java	10.71%	24 Missing and 1 partial ⚠️
...gent/src/main/java/com/cloud/agent/AgentShell.java	9.09%	20 Missing ⚠️
...rc/main/java/com/cloud/agent/mockvm/MockVmMgr.java	0.00%	11 Missing ⚠️
...va/com/cloud/agent/dao/impl/PropertiesStorage.java	11.11%	8 Missing ⚠️
...i/command/admin/vm/ImportUnmanagedInstanceCmd.java	0.00%	6 Missing ⚠️
...api/command/admin/systemvm/MigrateSystemVMCmd.java	0.00%	4 Missing ⚠️
...k/api/command/admin/systemvm/ScaleSystemVMCmd.java	0.00%	4 Missing ⚠️
...min/network/DeleteManagementNetworkIpRangeCmd.java	0.00%	3 Missing ⚠️
... and 37 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7131      +/-   ##
============================================
- Coverage     30.94%   30.77%   -0.18%     
+ Complexity    34219    33070    -1149     
============================================
  Files          5353     5352       -1     
  Lines        375914   374419    -1495     
  Branches      54634    54609      -25     
============================================
- Hits         116338   115221    -1117     
+ Misses       244301   243889     -412     
- Partials      15275    15309      +34

Flag	Coverage Δ
simulator-marvin-tests	24.64% <3.36%> (-0.10%)	⬇️
uitests	4.38% <ø> (ø)
unit-tests	16.41% <7.40%> (-0.21%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

SonarCloud Quality Gate failed.

28 Bugs 9 Vulnerabilities 6 Security Hotspots 11993 Code Smells

4.5% Coverage 5.1% Duplication

auch, I checked the security issues and marked them safe. @JoaoJandre can you go through the vulnerabilities and bugs, please. I think we must ignore the code smells for this PR. We can sample them but this is going to be a manyear of work to clear them all, even the more serious ones.

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

shall we keep count @JoaoJandre ? first conflict. (/me joking, let's not!)
I suggest we merge this as soon after 4.18 is out as we can.

shall we keep count @JoaoJandre ? first conflict. (/me joking, let's not!) I suggest we merge this as soon after 4.18 is out as we can.

I think it's best we don't keep :D. Sounds great, I'll try to be as fast as possible in completing this PR and addressing any reviews and conflicts.

SonarCloud Quality Gate failed.
28 Bugs 9 Vulnerabilities 6 Security Hotspots 11993 Code Smells
4.5% Coverage 5.1% Duplication

auch, I checked the security issues and marked them safe. @JoaoJandre can you go through the vulnerabilities and bugs, please. I think we must ignore the code smells for this PR. We can sample them but this is going to be a manyear of work to clear them all, even the more serious ones.

Sure, I'll go through them.

…grade-log4j

SonarCloud Quality Gate failed.
28 Bugs 9 Vulnerabilities 6 Security Hotspots 11993 Code Smells
4.5% Coverage 5.1% Duplication

auch, I checked the security issues and marked them safe. @JoaoJandre can you go through the vulnerabilities and bugs, please. I think we must ignore the code smells for this PR. We can sample them but this is going to be a manyear of work to clear them all, even the more serious ones.

@DaanHoogland, as this PR touched a lot of lines, any "bugs" or "vulnerabilities" that already existed are now being reported by Sonarcloud. I fixed one of them that I deemed relevant to this PR; however, in order to avoid raising the complexity of this PR, I think it would be better to create another PR to fix the other bugs and vulnerabilities. What do you think?

…variable that was mistakenly renamed on the normalization commit

SonarCloud Quality Gate failed.
28 Bugs 9 Vulnerabilities 6 Security Hotspots 11993 Code Smells
4.5% Coverage 5.1% Duplication

auch, I checked the security issues and marked them safe. @JoaoJandre can you go through the vulnerabilities and bugs, please. I think we must ignore the code smells for this PR. We can sample them but this is going to be a manyear of work to clear them all, even the more serious ones.

@DaanHoogland, as this PR touched a lot of lines, any "bugs" or "vulnerabilities" that already existed are now being reported by Sonarcloud. I fixed one of them that I deemed relevant to this PR; however, in order to avoid raising the complexity of this PR, I think it would be better to create another PR to fix the other bugs and vulnerabilities. What do you think?

Where it comes to code smells i would agree, but I would rather assess all bugs and vulnerabilities maybe even before merging this.

I updated the SnpmTrapAppender to work with log4j2 and have already tested if it gets created, and if its append method is called; however I did not test the SNMP functionality as it was not changed.

I removed the TestAppender, as it was only being used in a single test in a single class, and have rewritten the test.

About the AlertSyslogAppender, I have spent some time analyzing it, and it just does not seem to work at all. It will only append if _syslogAppenders is not empty nor null, but as far as I know this attribute cannot be configured via XML, and it is not being set anywhere else in the code, aside from a test class. @DaanHoogland, do you know how to configure and use this appender? If indeed it has never worked properly, could we remove it entirely?

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

…grade-log4j

[SF] Trillian test result (tid-9102)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41774 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7131-t9102-xenserver-71.zip
Smoke tests completed. 122 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

[SF] Trillian test result (tid-9104)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42873 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7131-t9104-kvm-centos7.zip
Smoke tests completed. 122 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

…grade-log4j

@blueorangutan package

@JoaoJandre a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

Seems that packaging got stuck on this one; running again:

@blueorangutan package

@GutoVeronezi a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

Seems that packaging got stuck on this one; running again:

might have been just the github api @GutoVeronezi (but rebuilding works;)

Seems that packaging got stuck on this one; running again:

might have been just the github api @GutoVeronezi (but rebuilding works;)

BO seems a bit under the weather today. At first, I thought it could be a problem with the PR; however, I built the packages normally, the tests ran successfully, and other PR's packaging tasks are not being reported as well (vide #8605 (comment)). Any clue if the problem is only with reporting the results?

Seems that packaging got stuck on this one; running again:

might have been just the github api @GutoVeronezi (but rebuilding works;)

BO seems a bit under the weather today. At first, I thought it could be a problem with the PR; however, I built the packages normally, the tests ran successfully, and other PR's packaging tasks are not being reported as well (vide #8605 (comment)). Any clue if the problem is only with reporting the results?

@GutoVeronezi
packages are ready 6 hours ago. it seems we have reached the github limits again.

@blueorangutan test matrix

@weizhouapache a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

[SF] Trillian test result (tid-9124)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42129 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7131-t9124-xenserver-71.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_cancel_host_maintenace_with_no_migration_jobs	Error	308.53	test_host_maintenance.py

@GutoVeronezi packages are ready 6 hours ago. it seems we have reached the github limits again.

I see, thanks @weizhouapache.

[SF] Trillian test result (tid-9126)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46966 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7131-t9126-kvm-centos7.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

[SF] Trillian test result (tid-9125)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 47402 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7131-t9125-vmware-67u3.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

Hello guys

I checked the test that failed in #7131 (comment), and it seems not related to the PR; therefore, as the last run passed successfully, I will merge this one. If necessary, further improvements or fixes will be addressed in new PRs. A note was already sent in the mailing list notifying the users and devs regarding the next steps; however, if you find someone struggling with the logs or anything that needs adjusting, just ping me or @JoaoJandre, so we can act as fast as possible.

JoaoJandre added 4 commits January 17, 2023 09:29

Upgrade log4j

a06bc15

Merge remote-tracking branch 'origin/main' into normalize-logs-and-up…

a0ea432

…grade-log4j

fix errors caused by the merge

29c4a78

boring-cyborg Bot added component:agent component:api labels Jan 24, 2023

weizhouapache modified the milestones: 4.19.0.0, 4.18.1.0 Jan 24, 2023

Refactor cglibThrowableRenderer functionality to log4j2 and upgrade t…

0973f7b

…he last configuration files

github-actions Bot added the status:has-conflicts label Jan 25, 2023

JoaoJandre added 2 commits January 25, 2023 14:06

Merge remote-tracking branch 'origin/main' into normalize-logs-and-up…

64cfab2

…grade-log4j

fix sonarcloud bug

ce25375

github-actions Bot removed the status:has-conflicts label Jan 25, 2023

Fix errors caused by merge, remove some unused loggers, and rename a …

21613ca

…variable that was mistakenly renamed on the normalization commit

Readd snmpTrapAppender, remove TestAppender

0a007b9

github-actions Bot added the status:has-conflicts label Jan 27, 2023

Merge remote-tracking branch 'origin/main' into normalize-logs-and-up…

3e512d2

…grade-log4j

JoaoJandre added 2 commits February 7, 2024 08:28

Merge remote-tracking branch 'origin/main' into normalize-logs-and-up…

82a5604

…grade-log4j

Regenerate changes

3cfc93a

JoaoJandre mentioned this pull request Apr 16, 2024

Go back to previous timestamp on logging #8925

Merged

2 of 13 tasks

JoaoJandre mentioned this pull request Jul 3, 2024

Fixup main build errors #9330

Merged

14 tasks

yadvr mentioned this pull request Jul 29, 2024

CloudStack mvn jetty:run failures on main branch with Darwin #9462

Closed

Luskan777 mentioned this pull request Nov 13, 2024

Error when using CLVM in KVM Hypervisor #9915

Closed

weizhouapache mentioned this pull request Mar 20, 2025

Cloudstack 4.20 UI becomes irresponsive. Restart the server to recover, it happens every 10 days. #10578

Open

gp-santos mentioned this pull request Mar 28, 2025

undo removal of accessLogger and deal with some warnings #10567

Merged

2 of 14 tasks

vishesh92 mentioned this pull request Nov 3, 2025

[4.20] More unit test errors than 4.19 #8770

Closed

Normalize loggers and upgrade log4j 1.2 to log4j 2.19 by JoaoJandre · Pull Request #7131 · apache/cloudstack · GitHub

Repository navigation

Normalize loggers and upgrade log4j 1.2 to log4j 2.19 - #7131

Normalize loggers and upgrade log4j 1.2 to log4j 2.19#7131GutoVeronezi merged 156 commits intoapache:mainapache/cloudstack:mainfrom scclouds:normalize-logs-and-upgrade-log4jscclouds/cloudstack:normalize-logs-and-upgrade-log4jCopy head branch name to clipboard

Conversation

JoaoJandre commented Jan 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

JoaoJandre commented Jan 24, 2023

Uh oh!

DaanHoogland commented Jan 24, 2023

Uh oh!

codecov Bot commented Jan 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland commented Jan 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jan 25, 2023

Uh oh!

github-actions Bot commented Jan 25, 2023

Uh oh!

DaanHoogland commented Jan 25, 2023

Uh oh!

JoaoJandre commented Jan 25, 2023

Uh oh!

JoaoJandre commented Jan 25, 2023

Uh oh!

DaanHoogland commented Jan 26, 2023

Uh oh!

JoaoJandre commented Jan 26, 2023

Uh oh!

github-actions Bot commented Jan 27, 2023

Uh oh!

blueorangutan commented Feb 7, 2024

Uh oh!

blueorangutan commented Feb 7, 2024

Uh oh!

github-actions Bot commented Feb 7, 2024

Uh oh!

JoaoJandre commented Feb 7, 2024

Uh oh!

blueorangutan commented Feb 7, 2024

Uh oh!

GutoVeronezi commented Feb 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blueorangutan commented Feb 7, 2024

Uh oh!

DaanHoogland commented Feb 7, 2024

Uh oh!

GutoVeronezi commented Feb 7, 2024

Uh oh!

weizhouapache commented Feb 7, 2024

Uh oh!

blueorangutan commented Feb 7, 2024

Uh oh!

blueorangutan commented Feb 8, 2024

Uh oh!

GutoVeronezi commented Feb 8, 2024

Uh oh!

blueorangutan commented Feb 8, 2024

Uh oh!

blueorangutan commented Feb 8, 2024

Uh oh!

GutoVeronezi commented Feb 8, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Normalize loggers and upgrade log4j 1.2 to log4j 2.19#7131
GutoVeronezi merged 156 commits into
apache:mainfrom
scclouds:normalize-logs-and-upgrade-log4j

JoaoJandre commented Jan 24, 2023 •
edited
Loading

codecov Bot commented Jan 24, 2023 •
edited
Loading

DaanHoogland commented Jan 25, 2023 •
edited
Loading

GutoVeronezi commented Feb 7, 2024 •
edited
Loading