Add logs to script and command execution

[stephankruggg]

Description

This PR adds and improves existing logs when executing scripts and commands, since current information is not sufficient to enable troubleshooting.

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

How Has This Been Tested?

I applied the changes in a local lab and checked if the updated logs were successfully logged.

[codecov]

Codecov Report

Merging #6874 (809be7b) into main (1ee58ec) will increase coverage by 16.75%.
Report is 760 commits behind head on main.
The diff coverage is 53.33%.

@@              Coverage Diff              @@
##               main    #6874       +/-   ##
=============================================
+ Coverage     10.85%   27.61%   +16.75%     
- Complexity     7106    28818    +21712     
=============================================
  Files          2485     5121     +2636     
  Lines        245417   411899   +166482     
  Branches      38326    73699    +35373     
=============================================
+ Hits          26631   113726    +87095     
- Misses       215516   282737    +67221     
- Partials       3270    15436    +12166     

Flag	Coverage Δ
simulator-marvin-tests	25.42% <53.33%> (?)
uitests	5.56% <ø> (?)
unit-tests	14.50% <ø> (?)

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

Files	Coverage Δ
...s/src/main/java/com/cloud/utils/script/Script.java	29.09% <53.33%> (ø)

... and 4014 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

52.5% Coverage
0.0% Duplication

[BryanMLima]

LGTM

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland a 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]

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. LL-JID 222

[stephankruggg]

@DaanHoogland, the isEnabled check is detrimental in terms of code legibility. In my opinion, this check's barely noticeable performance gain is not sufficient to counter this negative impact, so it should not be used in this scenario.

[DaanHoogland]

@DaanHoogland, the isEnabled check is detrimental in terms of code legibility. In my opinion, this check's barely noticeable performance gain is not sufficient to counter this negative impact, so it should not be used in this scenario.

I don´t agree. we could change the log library to add variable parameters to the trace/debug/info messages to make the log line look cleaner. I´m very extrmely for legibility but not at cost of neat following of best practices.

[stephankruggg]

@DaanHoogland, the isEnabled check is detrimental in terms of code legibility. In my opinion, this check's barely noticeable performance gain is not sufficient to counter this negative impact, so it should not be used in this scenario.

I don´t agree. we could change the log library to add variable parameters to the trace/debug/info messages to make the log line look cleaner. I´m very extrmely for legibility but not at cost of neat following of best practices.

I understand; however, it is not consensus that the isEnabled check should be used, and the scenarios here do not require so much processing in order to make it necessary to validate the log level before creating the string, so I'd rather not add them.

[JoaoJandre]

CLGTM

[GaOrtiga]

As mentioned in comment #6947 (comment), I will be helping out @stephankruggg with some of his opened PRs.

@DaanHoogland given that the patch for upgrading Log4j is already ready, is guarding these logs still necessary? Could we move on with this PR?

[DaanHoogland]

As mentioned in comment #6947 (comment), I will be helping out @stephankruggg with some of his opened PRs.

@DaanHoogland given that the patch for upgrading Log4j is already ready, is guarding these logs still necessary? Could we move on with this PR?

@GaOrtiga , let me put it like this; I will not involve myself in these discussions anymore, nor have I changed my opinion. I will not (freudian error) stand in the way of it either.

[shwstppr]

@blueorangutan package

[blueorangutan]

@shwstppr a [SF] 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]

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

[DaanHoogland]

@blueorangutan test

[blueorangutan]

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

[blueorangutan]

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

Test	Result	Time (s)	Test File
test_08_upgrade_kubernetes_ha_cluster	Failure	665.97	test_kubernetes_clusters.py
test_05_vmschedule_test_e2e	Failure	270.97	test_vm_schedule.py

[shwstppr]

Code LGTM

[DaanHoogland]

I am not +1, more like -0, but too many do not agree with my stance on the logging issue, so I'll merge this anyway.