bernardodemarco commented on code in PR #8924:
URL: https://github.com/apache/cloudstack/pull/8924#discussion_r1578507302
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java:
##########
@@ -104,20 +105,20 @@ public void onConnect(final Session session) throws
IOException, InterruptedExce
try {
port = Integer.parseInt(portStr);
} catch (NumberFormatException e) {
- logger.error("Invalid port value in query string: {}. Expected a
number.", portStr, e);
+ logger.error(String.format("Invalid port value in query string:
%s. Expected a number.", portStr), e);
Review Comment:
I'm not really sure why, but the `logger` fields of
`com.cloud.consoleproxy.ConsoleProxyNoVNCHandler` and
`com.cloud.consoleproxy.vnc.NoVncClient` are not instantiated directly from
`log4j`. They're generated from a static method from the
`com.cloud.consoleproxy.util.Logger` helper class.
The problem is that this helper class does not implement the new APIs
provided by the new version of `log4j`, especially those that accept multiple
parameters. As a consequence of that, `String.format()` must be used to log
messages in those two classes.
IMO, it should be evaluated if the helper class is still needed. If so, we
need to implement new methods to be able to use the new syntax provided by
`log4j2`.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
