You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "KlausDornsbach (via GitHub)" <gi...@apache.org> on 2024/02/29 13:24:05 UTC

[PR] Log4j2 refactor cloud api module [cloudstack]

KlausDornsbach opened a new pull request, #8728:
URL: https://github.com/apache/cloudstack/pull/8728

   ### Description
   
   This PR...
   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-api 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)
   - [X] Cleanup (Code refactoring and cleanup, that may add test cases)
   - [ ] build/CI
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [X] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   ### How Has This Been Tested?
   
   I have done some operations on cloudstack to induce the generation of the logs
   No problems or changes to the log output were found
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3670558935

   Left a minor nit


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r1507664135


##########
api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java:
##########
@@ -119,8 +120,7 @@ protected OVFPropertyTO createOVFPropertyFromNode(Node node, int index, String c
         boolean password = StringUtils.isNotBlank(passStr) && passStr.equalsIgnoreCase("true");
         String label = ovfParser.getChildNodeValue(node, "Label");
         String description = ovfParser.getChildNodeValue(node, "Description");
-        logger.debug("Creating OVF property index " + index + (category == null ? "" : " for category " + category)
-                + " with key = " + key);
+        logger.debug("Creating OVF property index {} {} with key = {}", index, (category == null ? "" : " for category " + category), key);

Review Comment:
   You could use lambda here



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r1507917240


##########
api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java:
##########
@@ -37,6 +37,7 @@
 
 import org.apache.cloudstack.utils.security.ParserUtils;
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang.ObjectUtils;

Review Comment:
   Build failing due to this unused import.
   ```suggestion
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2306493574

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2314580704

   @blueorangutan LLtest


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2314596746

   @vishesh92 is this one good to go?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "KlausDornsbach (via GitHub)" <gi...@apache.org>.
KlausDornsbach commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r1507801056


##########
api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java:
##########
@@ -119,8 +120,7 @@ protected OVFPropertyTO createOVFPropertyFromNode(Node node, int index, String c
         boolean password = StringUtils.isNotBlank(passStr) && passStr.equalsIgnoreCase("true");
         String label = ovfParser.getChildNodeValue(node, "Label");
         String description = ovfParser.getChildNodeValue(node, "Description");
-        logger.debug("Creating OVF property index " + index + (category == null ? "" : " for category " + category)
-                + " with key = " + key);
+        logger.debug("Creating OVF property index {} {} with key = {}", index, (category == null ? "" : " for category " + category), key);

Review Comment:
   should I use lambdas even when there is no big structure being read from memory or costly function being called?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3662986334

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3669490554

   @GutoVeronezi , are you satisfied like this or do you want more testing? `test_03_deploy_and_scale_kubernetes_cluster` needs attention but seems unrelated.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-1978428601

   > > clgtm, I think we need to implement some policies on when to log stack traces though. cc @JoaoJandre
   > 
   > I think that some guidelines would be good, sadly the discussion regarding this never got traction on the dev mailing list.
   
   maybe this will work better : #8746 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r1508946404


##########
api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java:
##########
@@ -119,8 +120,7 @@ protected OVFPropertyTO createOVFPropertyFromNode(Node node, int index, String c
         boolean password = StringUtils.isNotBlank(passStr) && passStr.equalsIgnoreCase("true");
         String label = ovfParser.getChildNodeValue(node, "Label");
         String description = ovfParser.getChildNodeValue(node, "Description");
-        logger.debug("Creating OVF property index " + index + (category == null ? "" : " for category " + category)
-                + " with key = " + key);
+        logger.debug("Creating OVF property index {} {} with key = {}", index, (category == null ? "" : " for category " + category), key);

Review Comment:
   We could avoid the if else and the string concatenation when debug is not turned on. But on a second thought, these are indeed not very costly considering the scope of ACS, so we can keep as is.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2312386671

   <b>[SF] Trillian test result (tid-11179)</b>
   Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
   Total time taken: 95346 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8728-t11179-kvm-ol8.zip
   Smoke tests completed. 123 look OK, 16 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_CRUD_operations_userdata | `Error` | 1549.62 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata | `Error` | 5.69 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_allow | `Error` | 5.38 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_append | `Error` | 5.40 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_deny | `Error` | 5.42 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_params | `Error` | 5.35 | test_register_userdata.py
   test_link_and_unlink_userdata_to_template | `Error` | 5.07 | test_register_userdata.py
   test_user_userdata_crud | `Error` | 5.30 | test_register_userdata.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   test_03_register_template | `Error` | 1.14 | test_resource_names.py
   ContextSuite context=TestRestoreVM>:setup | `Error` | 0.00 | test_restore_vm.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.26 | test_service_offerings.py
   ContextSuite context=TestSetSourceNatIp>:setup | `Error` | 0.00 | test_set_sourcenat.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   ContextSuite context=TestSnapshotStandaloneBackup>:setup | `Error` | 0.00 | test_snapshots.py
   test_10_reboot_cpvm_forced | `Error` | 9.43 | test_ssvm.py
   test_05_vmschedule_test_e2e | `Failure` | 362.62 | test_vm_schedule.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2164659301

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2309693126

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2314585005

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3664180990

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 merged PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-1985459548

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2304735525

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3662981266

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3664533002

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3670399563

   > @GutoVeronezi , are you satisfied like this or do you want more testing? `test_03_deploy_and_scale_kubernetes_cluster` needs attention but seems unrelated.
   
   It is fine for me. 
   
   @JoaoJandre, could you review it again as well?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2307877351

   <b>[SF] Trillian test result (tid-11156)</b>
   Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
   Total time taken: 49854 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8728-t11156-kvm-ol8.zip
   Smoke tests completed. 137 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_arping_in_ssvm | `Failure` | 5.25 | test_diagnostics.py
   test_13_retrieve_vr_default_files | `Failure` | 129.40 | test_diagnostics.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 179.10 | test_routers_network_ops.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2309696782

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3663232758

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r2618541144


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java:
##########
@@ -224,10 +224,8 @@ public Map<String, List<String>> getServiceProviders() {
             Iterator<? extends Map<String, String>> iter = servicesCollection.iterator();
             while (iter.hasNext()) {
                 Map<String, String> obj = iter.next();
-                if (logger.isTraceEnabled()) {
-                    logger.trace("service provider entry specified: " + obj);
-                }
-                HashMap<String, String> services = (HashMap<String, String>) obj;
+                logger.trace("Service provider entry specified: ", obj);

Review Comment:
   ```suggestion
                   logger.trace("Service provider entry specified: {}", obj);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r2619000358


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java:
##########
@@ -224,10 +224,8 @@ public Map<String, List<String>> getServiceProviders() {
             Iterator<? extends Map<String, String>> iter = servicesCollection.iterator();
             while (iter.hasNext()) {
                 Map<String, String> obj = iter.next();
-                if (logger.isTraceEnabled()) {
-                    logger.trace("service provider entry specified: " + obj);
-                }
-                HashMap<String, String> services = (HashMap<String, String>) obj;
+                logger.trace("Service provider entry specified: ", obj);

Review Comment:
   Thank you @DaanHoogland 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3645537817

   @KlausDornsbach is this still relevant? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3664173381

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2304734091

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-1976623765

   > clgtm, I think we need to implement some policies on when to log stack traces though. cc @JoaoJandre
   
   I think that some guidelines would be good, sadly the discussion regarding this never got traction on the dev mailing list.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3649485571

   > @KlausDornsbach is this still relevant?
   
   Hello @DaanHoogland,
   
   @KlausDornsbach is no longer maintaining this PR; however, I will resolve the conflicts, review the changes, and address any other necessary changes so we can move forward.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r2625143704


##########
api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkACLListCmd.java:
##########
@@ -128,7 +128,7 @@ public long getEntityOwnerId() {
         } else {
             account = CallContext.current().getCallingAccount();
             if (!Account.Type.ADMIN.equals(account.getType())) {
-                logger.warn(String.format("Only Root Admin can create global ACLs. Account [%s] cannot create any global ACL.", account));
+                logger.error("Only Root Admin can create global ACLs. {} cannot create any global ACL.", account);

Review Comment:
   Example without the changes:
   
   ```
   2025-12-16 17:26:41,858 WARN  [o.a.c.a.c.u.n.CreateNetworkACLListCmd] (qtp2038105753-22:[ctx-a568fe7b, ctx-db4621bd]) (logid:ef24b1da) Only Root Admin can create global ACLs. Account [Account [{"accountName":"test","id":4,"uuid":"991ecfb6-b09a-4abf-9488-7e3d47b175fe"}]] cannot create any global ACL.
   ```
   
   ---
   
   Example with the changes:
   
   ```
   2025-12-16 19:04:43,593 ERROR [o.a.c.a.c.u.n.CreateNetworkACLListCmd] (qtp2038105753-27:[ctx-37d7f311, ctx-ddfdecd2]) (logid:62794f63) Only Root Admin can create global ACLs. Account [{"accountName":"test","id":4,"uuid":"991ecfb6-b09a-4abf-9488-7e3d47b175fe"}] cannot create any global ACL.
   
   ```



##########
api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotFromVMSnapshotCmd.java:
##########
@@ -165,7 +165,7 @@ public void create() throws ResourceAllocationException {
     @Override
     public void execute() {
         VMSnapshot vmSnapshot = _vmSnapshotService.getVMSnapshotById(getVMSnapshotId());
-        logger.info("CreateSnapshotFromVMSnapshotCmd with vm snapshot {} with id {} and snapshot [id: {}, uuid: {}]", vmSnapshot, getVMSnapshotId(), getEntityId(), getEntityUuid());
+        logger.info("CreateSnapshotFromVMSnapshotCmd with {} and snapshot [id: {}, uuid: {}]", vmSnapshot, getEntityId(), getEntityUuid());

Review Comment:
   Example without the changes:
   
   ```
   2025-12-16 18:57:48,937 INFO  [o.a.c.a.c.u.s.CreateSnapshotFromVMSnapshotCmd] (API-Job-Executor-5:[ctx-f3a19882, job-75, ctx-5649bc6b]) (logid:874d335b) CreateSnapshotFromVMSnapshotCmd with vm snapshot VMSnapshot {"id":2,"name":"i-2-3-VM_VS_20251216215732","uuid":"2038367a-3f0a-4931-bcd7-9faf927f75d4","vmId":3} with id 2 and snapshot [id: 2, uuid: 21d9aed7-65d3-47e3-a75c-fd96b9c97422]
   ```
   
   ---
   
   Example with the changes:
   
   ```
   2025-12-16 19:08:50,057 INFO  [o.a.c.a.c.u.s.CreateSnapshotFromVMSnapshotCmd] (API-Job-Executor-1:[ctx-7cf2cd28, job-78, ctx-088f4cdf]) (logid:4db7c046) CreateSnapshotFromVMSnapshotCmd with VMSnapshot {"id":2,"name":"i-2-3-VM_VS_20251216215732","uuid":"2038367a-3f0a-4931-bcd7-9faf927f75d4","vmId":3} and snapshot [id: 3, uuid: 1a89a444-b5f3-4ca2-9463-e32c89530aa9]
   ```
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r2625142139


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/host/UpdateHostCmd.java:
##########
@@ -147,7 +147,7 @@ public void execute() {
             this.setResponseObject(hostResponse);
         } catch (Exception e) {
             Host host = _entityMgr.findById(Host.class, getId());
-            logger.debug("Failed to update host: {} with id {}", host, getId(), e);
+            logger.debug("Failed to update {}", host, e);

Review Comment:
   Examples without the changes:
   
   ```
   2025-12-16 17:16:50,237 DEBUG [o.a.c.a.c.a.h.UpdateHostCmd] (qtp2038105753-23:[ctx-5063625c, ctx-1ffc3504]) (logid:8d27d46e) Failed to update host: null with id 20 com.cloud.exception.InvalidParameterValueException: Host with id 20 doesn't exist
   ```
   ```
   2025-12-16 17:22:17,341 DEBUG [o.a.c.a.c.a.h.UpdateHostCmd] (qtp2038105753-23:[ctx-9cf64008, ctx-8c4ea9d8]) (logid:8b67772c) Failed to update host: Host {"id":2,"name":"v-1-VM","type":"ConsoleProxy","uuid":"8d403878-97ec-432c-b7b2-48dc4fc1ec34"} with id 2 com.cloud.exception.InvalidParameterValueException: Please specify a valid guest OS category.
   ```
   
   ---
   
   Examples with the change:
   
   ```
   2025-12-16 19:11:05,173 DEBUG [o.a.c.a.c.a.h.UpdateHostCmd] (qtp2038105753-381:[ctx-ecf1c194, ctx-ae65f974]) (logid:bf96b30d) Failed to update null com.cloud.exception.InvalidParameterValueException: Host with id 20 doesn't exist
   ```
   
   ```
   2025-12-16 19:09:49,348 DEBUG [o.a.c.a.c.a.h.UpdateHostCmd] (qtp2038105753-28:[ctx-0e6a09ab, ctx-9a5fdab5]) (logid:43ca681c) Failed to update Host {"id":2,"name":"v-1-VM","type":"ConsoleProxy","uuid":"8d403878-97ec-432c-b7b2-48dc4fc1ec34"} com.cloud.exception.InvalidParameterValueException: Please specify a valid guest OS category.
   
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3665142496

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3665136106

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2309692756

   trying insanity, all failures seem internet connectivity related.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2577663280

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2305011328

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-2306496084

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-1971600620

   ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8728?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   All modified and coverable lines are covered by tests :white_check_mark:
   > Project coverage is 4.36%. Comparing base [(`a31449b`)](https://app.codecov.io/gh/apache/cloudstack/commit/a31449b10490cb386d60fc961f6d3bf3bdb09ccd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`a0f8ce9`)](https://app.codecov.io/gh/apache/cloudstack/pull/8728?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 30 commits behind head on main.
   
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@             Coverage Diff              @@
   ##               main   #8728       +/-   ##
   ============================================
   - Coverage     30.76%   4.36%   -26.40%     
   ============================================
     Files          5353     361     -4992     
     Lines        374607   28831   -345776     
     Branches      54635    5037    -49598     
   ============================================
   - Hits         115238    1259   -113979     
   + Misses       244099   27433   -216666     
   + Partials      15270     139    -15131     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8728/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8728/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8728/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.36% <ø> (-0.02%)` | :arrow_down: |
   | [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8728/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8728?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "KlausDornsbach (via GitHub)" <gi...@apache.org>.
KlausDornsbach commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r1508990476


##########
api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java:
##########
@@ -37,6 +37,7 @@
 
 import org.apache.cloudstack.utils.security.ParserUtils;
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang.ObjectUtils;

Review Comment:
   Thanks



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3665404903

   <b>[SF] Trillian Build Failed (tid-15014)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#issuecomment-3668507697

   <b>[SF] Trillian test result (tid-15016)</b>
   Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
   Total time taken: 54993 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8728-t15016-kvm-ol8.zip
   Smoke tests completed. 149 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 28.76 | test_kubernetes_clusters.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r2631272782


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java:
##########
@@ -244,9 +240,7 @@ public Map<String, Long> getDataDiskToDiskOfferingList() {
             for (Map<String, String> entry : (Collection<Map<String, String>>)dataDiskToDiskOfferingList.values()) {
                 String disk = entry.get(VmDetailConstants.DISK);
                 String offeringUuid = entry.get(VmDetailConstants.DISK_OFFERING);
-                if (logger.isTraceEnabled()) {
-                    logger.trace(String.format("disk, '%s', gets offering, '%s'", disk, offeringUuid));
-                }
+                logger.trace("Checking if offering '{}' can be user on disk '{}'", offeringUuid, disk);

Review Comment:
   ```suggestion
                   logger.trace("Checking if offering '{}' can be used on disk '{}'", offeringUuid, disk);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Log4j2 refactor cloud api module [cloudstack]

Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on code in PR #8728:
URL: https://github.com/apache/cloudstack/pull/8728#discussion_r2618995375


##########
api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotFromVMSnapshotCmd.java:
##########
@@ -165,7 +165,7 @@ public void create() throws ResourceAllocationException {
     @Override
     public void execute() {
         VMSnapshot vmSnapshot = _vmSnapshotService.getVMSnapshotById(getVMSnapshotId());
-        logger.info("CreateSnapshotFromVMSnapshotCmd with vm snapshot {} with id {} and snapshot [id: {}, uuid: {}]", vmSnapshot, getVMSnapshotId(), getEntityId(), getEntityUuid());
+        logger.info("CreateSnapshotFromVMSnapshotCmd with {} and snapshot [id: {}, uuid: {}]", vmSnapshot, getEntityId(), getEntityUuid());

Review Comment:
   I left only the snapshot object to be logged because its `toString()` already prints the necessary data; soon I'll post the tests.



##########
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java:
##########
@@ -255,11 +255,8 @@ public void create() {
             }
         } catch (NetworkRuleConflictException ex) {
             String message = "Network rule conflict: ";
-            if (!logger.isTraceEnabled()) {
-                logger.info(message + ex.getMessage());
-            } else {
-                logger.trace(message, ex);
-            }
+            logger.error( "Network rule conflict: {}", ex.getMessage());

Review Comment:
   ```suggestion
               logger.error("{}{}", message, ex.getMessage());
   ```



##########
api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java:
##########
@@ -540,19 +533,15 @@ public List<OVFNetworkTO> getNetPrerequisitesFromDocument(Document doc) throws I
     private void matchNicsToNets(Map<String, OVFNetworkTO> nets, Node systemElement) {
         final DocumentTraversal traversal = (DocumentTraversal) systemElement;
         final NodeIterator iterator = traversal.createNodeIterator(systemElement, NodeFilter.SHOW_ELEMENT, null, true);
-        if (logger.isTraceEnabled()) {
-            logger.trace(String.format("starting out with %d network-prerequisites, parsing hardware",nets.size()));
-        }
+        logger.trace("Starting out with {} network-prerequisites, parsing hardware", nets.size());
         int nicCount = 0;
         for (Node n = iterator.nextNode(); n != null; n = iterator.nextNode()) {
             final Element e = (Element) n;
             if ("rasd:Connection".equals(e.getTagName())) {
                 nicCount++;
                 String name = e.getTextContent(); // should be in our nets
                 if(nets.get(name) == null) {
-                    if(logger.isInfoEnabled()) {
-                        logger.info(String.format("found a nic definition without a network definition byname %s, adding it to the list.", name));
-                    }
+                    logger.info("Found a nic definition without a network definition byname {}, adding it to the list.", name);

Review Comment:
   ```suggestion
                       logger.info("Found a nic definition without a network definition by name {}, adding it to the list.", name);
   ```



##########
api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkACLListCmd.java:
##########
@@ -128,7 +128,7 @@ public long getEntityOwnerId() {
         } else {
             account = CallContext.current().getCallingAccount();
             if (!Account.Type.ADMIN.equals(account.getType())) {
-                logger.warn(String.format("Only Root Admin can create global ACLs. Account [%s] cannot create any global ACL.", account));
+                logger.error("Only Root Admin can create global ACLs. {} cannot create any global ACL.", account);

Review Comment:
   I left only the account object to be logged because its `toString()` already prints the necessary data; soon I'll post the tests.



##########
api/src/main/java/org/apache/cloudstack/api/command/admin/host/UpdateHostCmd.java:
##########
@@ -147,7 +147,7 @@ public void execute() {
             this.setResponseObject(hostResponse);
         } catch (Exception e) {
             Host host = _entityMgr.findById(Host.class, getId());
-            logger.debug("Failed to update host: {} with id {}", host, getId(), e);
+            logger.debug("Failed to update {}", host, e);

Review Comment:
   I left only the host object to be logged because its `toString()` already prints the necessary data; soon I'll post the tests.



##########
api/src/main/java/org/apache/cloudstack/context/LogContext.java:
##########
@@ -207,9 +205,7 @@ public static void unregister() {
         LogContext context = s_currentContext.get();
         if (context != null) {
             s_currentContext.remove();
-            if (LOGGER.isTraceEnabled()) {
-                LOGGER.trace("Unregistered: " + context);
-            }
+            LOGGER.trace("Unregistered: {}", () -> context);

Review Comment:
   ```suggestion
               LOGGER.trace("Unregistered: {}", context);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org