Cleanup: Standardize logger message formatting in ApiServer.java #11188
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR standardizes logger message formatting in ApiServer.java by converting from string concatenation to parameterized logging. The changes improve logging performance and consistency by using placeholders ({}) instead of string concatenation with the + operator.
- Replaces string concatenation with parameterized logging format across multiple log statements
- Maintains the same log message content while improving performance and readability
- Updates debug, trace, info, and warn level logging statements
|
@blueorangutan package |
|
@sureshanaparti 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. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11188 +/- ##
=========================================
Coverage 17.56% 17.57%
- Complexity 15548 15550 +2
=========================================
Files 5913 5913
Lines 529440 529440
Branches 64670 64670
=========================================
+ Hits 93019 93025 +6
+ Misses 425963 425957 -6
Partials 10458 10458
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14161 |
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
clgtm, it is a trivial change, so can we consider if the message level needs adjusting in the same PR? cc @sureshanaparti @JoaoJandre @bernardodemarco @winterhazel
| final String keyStr = (String)key; | ||
| final String[] value = (String[])params.get(key); | ||
| logger.trace(" key: " + keyStr + ", value: " + ((value == null) ? "'null'" : value[0])); | ||
| logger.trace(" key: {}, value: {}", keyStr, (value == null) ? "'null'" : value[0]); |
There was a problem hiding this comment.
| logger.trace(" key: {}, value: {}", keyStr, (value == null) ? "'null'" : value[0]); | |
| logger.trace(" key: {}, value: {}", keyStr, () -> (value == null) ? "'null'" : value[0]); |
Since this is on trace (rarely used) It would be better to pass a lambda function that will me evaluated only if trace is on.
There was a problem hiding this comment.
Thanks for the suggestion! Just to confirm, I saw we are using Log4j 2.19.0, it doesn't support lambda directly, but it does support Supplier, which should work the same.
|
@JoaoJandre Sounds good! I will address them! |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
Hi @he1l0world, could you resolve the conflicts here? |
|
@JoaoJandre Updated |
5 participants
Description
This PR improves logging consistency across the logging statements in ApiServer.java
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?