Refactor cloud-agent module logs for Log4j2 by KlausDornsbach · Pull Request #8714 · apache/cloudstack

KlausDornsbach · 2024-02-27T21:41:02Z

Description

With the new version of Log4j there is some space for improving log readability using the new features. This PR refactors the logs from the cloud-agent module.

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)
build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

How Has This Been Tested?

It was tested in a local lab:

I have done some operations on cloudstack to induce the generation of the logs
No problems or changes to the log output were found

boring-cyborg · 2024-02-27T21:41:06Z

Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:

In case of a new feature add useful documentation (raise doc PR at https://github.com/apache/cloudstack-documentation)
Be patient and persistent. It might take some time to get a review or get the final approval from the committers.
Pay attention to the quality of your code, ensure tests are passing and your PR doesn't have conflicts.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Issues, Mailing list and Slack.
Be sure to read the CloudStack Coding Conventions.
Apache CloudStack is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@cloudstack.apache.org (https://cloudstack.apache.org/mailing-lists.html)
Slack: https://apachecloudstack.slack.com/

codecov · 2024-02-27T21:50:47Z

Codecov Report

Attention: Patch coverage is 6.09756% with 77 lines in your changes are missing coverage. Please review.

Project coverage is 31.06%. Comparing base (e44c17e) to head (396034a).

Files	Patch %	Lines
agent/src/main/java/com/cloud/agent/Agent.java	0.00%	49 Missing ⚠️
...nt/resource/consoleproxy/ConsoleProxyResource.java	0.00%	17 Missing ⚠️
...gent/src/main/java/com/cloud/agent/AgentShell.java	16.66%	5 Missing ⚠️
...va/com/cloud/agent/dao/impl/PropertiesStorage.java	0.00%	3 Missing ⚠️
...rc/main/java/com/cloud/agent/mockvm/MockVmMgr.java	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8714      +/-   ##
============================================
+ Coverage     30.93%   31.06%   +0.12%     
- Complexity    33753    33909     +156     
============================================
  Files          5404     5404              
  Lines        380309   380289      -20     
  Branches      55507    55489      -18     
============================================
+ Hits         117658   118122     +464     
+ Misses       247041   246512     -529     
- Partials      15610    15655      +45

Flag	Coverage Δ
simulator-marvin-tests	`24.79% <0.00%> (+0.23%)`	⬆️
uitests	`4.34% <ø> (ø)`
unit-tests	`16.88% <6.09%> (+<0.01%)`	⬆️

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.

DaanHoogland

thanks for the effort @KlausDornsbach. Looks good to me, just one remark that actually is a bit out of scope but is touched upon in your change, so maybe we can add this easy win.

DaanHoogland · 2024-02-28T08:45:34Z

@blueorangutan package

blueorangutan · 2024-02-28T08:46:03Z

@DaanHoogland 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.

blueorangutan · 2024-02-28T09:38:36Z

Packaging result [SF]: ✖️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8808

JoaoJandre

Overall looks good, I've found one issue that should be fixed and some points that could be improved but are a little out of scope of this PR.

DaanHoogland

clgtm

DaanHoogland · 2024-03-04T09:03:56Z

@blueorangutan package

blueorangutan · 2024-03-04T09:06:03Z

@DaanHoogland 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.

blueorangutan · 2024-03-04T09:49:22Z

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 8843

JoaoJandre

CLGTM

JoaoJandre · 2024-03-20T12:50:43Z

@blueorangutan package

blueorangutan · 2024-03-20T12:52:03Z

@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.

blueorangutan · 2024-03-20T13:46:13Z

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 8982

DaanHoogland · 2024-03-25T10:06:55Z

@KlausDornsbach

13:59:26 [ERROR] /jenkins/workspace/acs-centos7-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.20.0.0-SNAPSHOT/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java:[420,12] error: cannot find symbol
13:59:26   symbol:   variable s_logger
13:59:26   location: class SnapshotSchedulerImpl

I can not find the reference to s_logger in SnapshotSchedulerImpl in main, so maybe rebase on main would solve the issue.

Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com>

revert a lambda expression because log4j2 does not support a non lambda + lambda as parameters

fixing little mistake

KlausDornsbach · 2024-03-25T19:22:09Z

Thanks for the heads up @DaanHoogland, hopefully issues are solved now.

JoaoJandre · 2024-03-26T13:00:11Z

@blueorangutan package

blueorangutan · 2024-03-26T13:02:03Z

@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.

blueorangutan · 2024-03-26T14:13:29Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9044

DaanHoogland · 2024-03-29T13:53:28Z

@blueorangutan test alma9 kvm-alma9

blueorangutan · 2024-03-29T13:54:03Z

@DaanHoogland a [SL] Trillian-Jenkins test job (alma9 mgmt + kvm-alma9) has been kicked to run smoke tests

blueorangutan · 2024-03-30T03:59:22Z

[SF] Trillian test result (tid-9623)
Environment: kvm-alma9 (x2), Advanced Networking with Mgmt server a9
Total time taken: 48862 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8714-t9623-kvm-alma9.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

JoaoJandre · 2024-04-01T11:53:53Z

Merging based on approvals and CI result

boring-cyborg · 2024-04-01T11:54:13Z

Awesome work, congrats on your first merged pull request!

* Refactor cloud-agent module logs for Log4j2 * better log4j2 logs for cloud-agent module * erasing commented lines * erasing unused import * fixing mistakes * more log refactor * fixing log refactor lambdas * adding suggestions to log refactor * Apply suggestions from code review Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com> * Update Agent.java revert a lambda expression because log4j2 does not support a non lambda + lambda as parameters * Update Agent.java fixing little mistake --------- Co-authored-by: klaus.freitas.scclouds <klaus.freitas@scclouds.com.br> Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com>

boring-cyborg Bot added the component:agent label Feb 27, 2024

GutoVeronezi requested a review from JoaoJandre February 27, 2024 21:42

DaanHoogland approved these changes Feb 28, 2024

View reviewed changes

Comment thread agent/src/main/java/com/cloud/agent/Agent.java Outdated

JoaoJandre reviewed Feb 28, 2024

View reviewed changes

Comment thread agent/src/main/java/com/cloud/agent/Agent.java Outdated

Comment thread agent/src/main/java/com/cloud/agent/Agent.java Outdated

Comment thread agent/src/main/java/com/cloud/agent/Agent.java Outdated

Comment thread agent/src/main/java/com/cloud/agent/Agent.java Outdated

DaanHoogland approved these changes Mar 4, 2024

View reviewed changes

JoaoJandre approved these changes Mar 4, 2024

View reviewed changes

klaus.freitas.scclouds and others added 11 commits March 25, 2024 12:47

Refactor cloud-agent module logs for Log4j2

5ccbe3f

better log4j2 logs for cloud-agent module

bec2d67

erasing commented lines

849b749

erasing unused import

8641c3d

fixing mistakes

311706e

more log refactor

c552c97

fixing log refactor lambdas

191245d

adding suggestions to log refactor

09cc53c

Apply suggestions from code review

7238f69

Co-authored-by: João Jandre <48719461+JoaoJandre@users.noreply.github.com>

Update Agent.java

dfc0f1c

revert a lambda expression because log4j2 does not support a non lambda + lambda as parameters

Update Agent.java

396034a

fixing little mistake

KlausDornsbach force-pushed the log4j2-refactor-cloud-agent-module branch from a7605e3 to 396034a Compare March 25, 2024 15:48

JoaoJandre merged commit a0b642c into apache:main Apr 1, 2024

DaanHoogland added this to the 4.20.0.0 milestone May 7, 2024

Conversation

KlausDornsbach commented Feb 27, 2024

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

How Has This Been Tested?

Uh oh!

boring-cyborg Bot commented Feb 27, 2024

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DaanHoogland commented Feb 28, 2024

Uh oh!

blueorangutan commented Feb 28, 2024

Uh oh!

blueorangutan commented Feb 28, 2024

Uh oh!

JoaoJandre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Mar 4, 2024

Uh oh!

blueorangutan commented Mar 4, 2024

Uh oh!

blueorangutan commented Mar 4, 2024

Uh oh!

JoaoJandre left a comment

Choose a reason for hiding this comment

Uh oh!

JoaoJandre commented Mar 20, 2024

Uh oh!

blueorangutan commented Mar 20, 2024

Uh oh!

blueorangutan commented Mar 20, 2024

Uh oh!

DaanHoogland commented Mar 25, 2024

Uh oh!

KlausDornsbach commented Mar 25, 2024

Uh oh!

JoaoJandre commented Mar 26, 2024

Uh oh!

blueorangutan commented Mar 26, 2024

Uh oh!

blueorangutan commented Mar 26, 2024

Uh oh!

DaanHoogland commented Mar 29, 2024

Uh oh!

blueorangutan commented Mar 29, 2024

Uh oh!

blueorangutan commented Mar 30, 2024

Uh oh!

JoaoJandre commented Apr 1, 2024

Uh oh!

boring-cyborg Bot commented Apr 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Feb 27, 2024 •

edited

Loading