JoaoJandre commented on code in PR #8714:
URL: https://github.com/apache/cloudstack/pull/8714#discussion_r1506412659
##########
agent/src/main/java/com/cloud/agent/Agent.java:
##########
@@ -312,7 +313,7 @@ public void start() {
}
public void stop(final String reason, final String detail) {
- logger.info("Stopping the agent: Reason = " + reason + (detail != null
? ": Detail = " + detail : ""));
+ logger.info("Stopping the agent: Reason = {}", reason +
ObjectUtils.defaultIfNull(": Detail = " + detail, ""));
Review Comment:
This defaultIfNull does not make sense, `": Detail = " + detail` will never
be null. Something like this makes more sense, but maybe using string format
would be better for the second parameter.
```suggestion
logger.info("Stopping the agent: Reason = {} {}", reason, () -> ":
Detail = " + ObjectUtils.defaultIfNull(detail, ""));
```
##########
agent/src/main/java/com/cloud/agent/Agent.java:
##########
@@ -396,17 +397,16 @@ private synchronized void scheduleHostLBCheckerTask(final
long checkInterval) {
hostLBTimer.cancel();
}
if (checkInterval > 0L) {
- logger.info("Scheduling preferred host timer task with
host.lb.interval=" + checkInterval + "ms");
+ logger.info("Scheduling preferred host timer task with
host.lb.interval={}ms", checkInterval);
hostLBTimer = new Timer("Host LB Timer");
hostLBTimer.scheduleAtFixedRate(new PreferredHostCheckerTask(),
checkInterval, checkInterval);
}
}
public void scheduleWatch(final Link link, final Request request, final
long delay, final long period) {
synchronized (_watchList) {
- if (logger.isDebugEnabled()) {
- logger.debug("Adding a watch list");
- }
+ logger.debug("Adding a watch list");
Review Comment:
We could rewrite this log to add the task that is being added on the
watchList.
##########
agent/src/main/java/com/cloud/agent/Agent.java:
##########
@@ -377,7 +378,7 @@ public Long getId() {
}
public void setId(final Long id) {
- logger.info("Set agent id " + id);
+ logger.info("Set agent id {}", id);
Review Comment:
```suggestion
logger.debug("Set agent id {}", id);
```
##########
agent/src/main/java/com/cloud/agent/Agent.java:
##########
@@ -432,9 +432,7 @@ protected void cancelTasks() {
for (final WatchTask task : _watchList) {
task.cancel();
}
- if (logger.isDebugEnabled()) {
- logger.debug("Clearing watch list: " + _watchList.size());
- }
+ logger.debug("Clearing watch list: {}", _watchList.size());
Review Comment:
Either log the entire list, or change the log to specify that the size is
being logged
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
