Skip to content

Refactor cloud-agent module logs for Log4j2 #8714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JoaoJandre merged 11 commits into from
Apr 1, 2024
Merged

Refactor cloud-agent module logs for Log4j2 #8714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5ccbe3f
Refactor cloud-agent module logs for Log4j2
Feb 16, 2024
bec2d67
better log4j2 logs for cloud-agent module
Feb 19, 2024
849b749
erasing commented lines
Feb 19, 2024
8641c3d
erasing unused import
Feb 19, 2024
311706e
fixing mistakes
Feb 21, 2024
c552c97
more log refactor
Feb 27, 2024
191245d
fixing log refactor lambdas
Feb 27, 2024
09cc53c
adding suggestions to log refactor
Feb 29, 2024
7238f69
Apply suggestions from code review
KlausDornsbach Feb 29, 2024
dfc0f1c
Update Agent.java
KlausDornsbach Mar 4, 2024
396034a
Update Agent.java
KlausDornsbach Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 55 additions & 74 deletions agent/src/main/java/com/cloud/agent/Agent.java
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions agent/src/main/java/com/cloud/agent/AgentShell.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ void loadProperties() throws ConfigurationException {
throw new ConfigurationException("Unable to find agent.properties.");
}

LOGGER.info("agent.properties found at " + file.getAbsolutePath());
LOGGER.info("agent.properties found at {}", file.getAbsolutePath());

try {
PropertiesUtil.loadFromFile(_properties, file);
Expand Down Expand Up @@ -382,15 +382,15 @@ public void init(String[] args) throws ConfigurationException {
if (_version == null) {
throw new CloudRuntimeException("Unable to find the implementation version of this agent");
}
LOGGER.info("Implementation Version is " + _version);
LOGGER.info("Implementation Version is {}", _version);

loadProperties();
parseCommand(args);

if (LOGGER.isDebugEnabled()) {
List<String> properties = Collections.list((Enumeration<String>)_properties.propertyNames());
for (String property : properties) {
LOGGER.debug("Found property: " + property);
LOGGER.debug("Found property: {}", property);
}
}

Expand All @@ -411,7 +411,7 @@ public void init(String[] args) throws ConfigurationException {

private void launchAgent() throws ConfigurationException {
String resourceClassNames = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.RESOURCE);
LOGGER.trace("resource=" + resourceClassNames);
LOGGER.trace("resource={}", resourceClassNames);
if (resourceClassNames != null) {
launchAgentFromClassInfo(resourceClassNames);
return;
Expand Down Expand Up @@ -444,7 +444,7 @@ private void launchAgentFromTypeInfo() throws ConfigurationException {
LOGGER.error("Unable to retrieve the type");
throw new ConfigurationException("Unable to retrieve the type of this agent.");
}
LOGGER.trace("Launching agent based on type=" + typeInfo);
LOGGER.trace("Launching agent based on type={}", typeInfo);
}

public void launchNewAgent(ServerResource resource) throws ConfigurationException {
Expand Down Expand Up @@ -506,7 +506,7 @@ public void start() {
String pidDir = getProperty(null, "piddir");

final String run = "agent." + instance + "pid";
LOGGER.debug("Checking to see if " + run + " exists.");
LOGGER.debug("Checking to see if {} exists.", run);
ProcessUtil.pidCheck(pidDir, run);

launchAgent();
Expand Down
8 changes: 3 additions & 5 deletions agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,12 @@ public synchronized boolean configure(String name, Map<String, Object> params) {
file = new File(path);
try {
if (!file.createNewFile()) {
logger.error(String.format("Unable to create _file: %s", file.getAbsolutePath()));
logger.error("Unable to create _file: {}", file.getAbsolutePath());
return false;
}
} catch (IOException e) {
logger.error(String.format("Unable to create file: %s", file.getAbsolutePath()));
if (logger.isDebugEnabled()) {
logger.debug(String.format("IOException while trying to create file: %s", file.getAbsolutePath()), e);
}
logger.error("Unable to create file: {}", file.getAbsolutePath());
logger.debug("IOException while trying to create file: {}", file.getAbsolutePath(), e);
return false;
}
}
Expand Down
9 changes: 3 additions & 6 deletions agent/src/main/java/com/cloud/agent/mockvm/MockVmMgr.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ public String startVM(String vmName, String vnetId, String gateway, String dns,

@Override
public String stopVM(String vmName, boolean force) {
if (logger.isInfoEnabled())
logger.info("Stop VM. name: " + vmName);
logger.info("Stop VM. name: {}", vmName);

synchronized (this) {
MockVm vm = vms.get(vmName);
Expand All @@ -103,8 +102,7 @@ public String stopVM(String vmName, boolean force) {

@Override
public String rebootVM(String vmName) {
if (logger.isInfoEnabled())
logger.info("Reboot VM. name: " + vmName);
logger.info("Reboot VM. name: {}", vmName);

synchronized (this) {
MockVm vm = vms.get(vmName);
Expand All @@ -116,8 +114,7 @@ public String rebootVM(String vmName) {

@Override
public boolean migrate(String vmName, String params) {
if (logger.isInfoEnabled())
logger.info("Migrate VM. name: " + vmName);
logger.info("Migrate VM. name: {}", vmName);

synchronized (this) {
MockVm vm = vms.get(vmName);
Expand Down
8 changes: 4 additions & 4 deletions agent/src/main/java/com/cloud/agent/properties/AgentPropertiesFileHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,15 @@ public static <T> T getPropertyValue(AgentProperties.Property<T> property) {
File agentPropertiesFile = PropertiesUtil.findConfigFile(KeyStoreUtils.AGENT_PROPSFILE);

if (agentPropertiesFile == null) {
LOGGER.debug(String.format("File [%s] was not found, we will use default defined values. Property [%s]: [%s].", KeyStoreUtils.AGENT_PROPSFILE, name, defaultValue));
LOGGER.debug("File [{}] was not found, we will use default defined values. Property [{}]: [{}].", KeyStoreUtils.AGENT_PROPSFILE, name, defaultValue);

return defaultValue;
}

try {
String configValue = PropertiesUtil.loadFromFile(agentPropertiesFile).getProperty(name);
if (StringUtils.isBlank(configValue)) {
LOGGER.debug(String.format("Property [%s] has empty or null value. Using default value [%s].", name, defaultValue));
LOGGER.debug("Property [{}] has empty or null value. Using default value [{}].", name, defaultValue);
return defaultValue;
}

Expand All @@ -68,11 +68,11 @@ public static <T> T getPropertyValue(AgentProperties.Property<T> property) {
ConvertUtils.register(new LongConverter(defaultValue), Long.class);
}

LOGGER.debug(String.format("Property [%s] was altered. Now using the value [%s].", name, configValue));
LOGGER.debug("Property [{}] was altered. Now using the value [{}].", name, configValue);
return (T)ConvertUtils.convert(configValue, property.getTypeClass());

} catch (IOException ex) {
LOGGER.debug(String.format("Failed to get property [%s]. Using default value [%s].", name, defaultValue), ex);
LOGGER.debug("Failed to get property [{}]. Using default value [{}].", name, defaultValue, ex);
}

return defaultValue;
Expand Down
38 changes: 17 additions & 21 deletions agent/src/main/java/com/cloud/agent/resource/consoleproxy/ConsoleProxyResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,12 @@ private Answer executeProxyLoadScan(final Command cmd, final long proxyVmId, fin
try {
is.close();
} catch (final IOException e) {
logger.warn("Exception when closing , console proxy address : " + proxyManagementIp);
logger.warn("Exception when closing , console proxy address : {}", proxyManagementIp);
success = false;
}
}
} catch (final IOException e) {
logger.warn("Unable to open console proxy command port url, console proxy address : " + proxyManagementIp);
logger.warn("Unable to open console proxy command port url, console proxy address : {}", proxyManagementIp);
success = false;
}

Expand Down Expand Up @@ -278,34 +278,33 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
disableRpFilter();
}

if (logger.isInfoEnabled())
logger.info("Receive proxyVmId in ConsoleProxyResource configuration as " + proxyVmId);
logger.info("Receive proxyVmId in ConsoleProxyResource configuration as {}", proxyVmId);

return true;
}

private void addRouteToInternalIpOrCidr(String localgw, String eth1ip, String eth1mask, String destIpOrCidr) {
logger.debug("addRouteToInternalIp: localgw=" + localgw + ", eth1ip=" + eth1ip + ", eth1mask=" + eth1mask + ",destIp=" + destIpOrCidr);
logger.debug("addRouteToInternalIp: localgw={}, eth1ip={}, eth1mask={}, destIp={}", localgw, eth1ip, eth1mask, destIpOrCidr);
if (destIpOrCidr == null) {
logger.debug("addRouteToInternalIp: destIp is null");
return;
}
if (!NetUtils.isValidIp4(destIpOrCidr) && !NetUtils.isValidIp4Cidr(destIpOrCidr)) {
logger.warn(" destIp is not a valid ip address or cidr destIp=" + destIpOrCidr);
logger.warn(" destIp is not a valid ip address or cidr destIp={}", destIpOrCidr);
return;
}
boolean inSameSubnet = false;
if (NetUtils.isValidIp4(destIpOrCidr)) {
if (eth1ip != null && eth1mask != null) {
inSameSubnet = NetUtils.sameSubnet(eth1ip, destIpOrCidr, eth1mask);
} else {
logger.warn("addRouteToInternalIp: unable to determine same subnet: eth1ip=" + eth1ip + ", dest ip=" + destIpOrCidr + ", eth1mask=" + eth1mask);
logger.warn("addRouteToInternalIp: unable to determine same subnet: eth1ip={}, dest ip={}, eth1mask={}", eth1ip, destIpOrCidr, eth1mask);
}
} else {
inSameSubnet = NetUtils.isNetworkAWithinNetworkB(destIpOrCidr, NetUtils.ipAndNetMaskToCidr(eth1ip, eth1mask));
}
if (inSameSubnet) {
logger.debug("addRouteToInternalIp: dest ip " + destIpOrCidr + " is in the same subnet as eth1 ip " + eth1ip);
logger.debug("addRouteToInternalIp: dest ip {} is in the same subnet as eth1 ip {}", destIpOrCidr, eth1ip);
return;
}
Script command = new Script("/bin/bash", logger);
Expand All @@ -317,9 +316,9 @@ private void addRouteToInternalIpOrCidr(String localgw, String eth1ip, String et
command.add("ip route add " + destIpOrCidr + " via " + localgw);
String result = command.execute();
if (result != null) {
logger.warn("Error in configuring route to internal ip err=" + result);
logger.warn("Error in configuring route to internal ip err={}", result);
} else {
logger.debug("addRouteToInternalIp: added route to internal ip=" + destIpOrCidr + " via " + localgw);
logger.debug("addRouteToInternalIp: added route to internal ip={} via {}", destIpOrCidr, localgw);
}
}

Expand All @@ -332,7 +331,7 @@ private void launchConsoleProxy(final byte[] ksBits, final String ksPassword, fi
final Object resource = this;
logger.info("Building class loader for com.cloud.consoleproxy.ConsoleProxy");
if (consoleProxyMain == null) {
logger.info("Running com.cloud.consoleproxy.ConsoleProxy with encryptor password=" + encryptorPassword);
logger.info("Running com.cloud.consoleproxy.ConsoleProxy with encryptor password={}", encryptorPassword);
consoleProxyMain = new Thread(new ManagedContextRunnable() {
@Override
protected void runInContext() {
Expand All @@ -355,7 +354,7 @@ protected void runInContext() {
logger.error("Unable to launch console proxy due to IllegalAccessException", e);
System.exit(ExitStatus.Error.value());
} catch (InvocationTargetException e) {
logger.error("Unable to launch console proxy due to InvocationTargetException " + e.getTargetException().toString(), e);
logger.error("Unable to launch console proxy due to InvocationTargetException {}", e.getTargetException().toString(), e);
System.exit(ExitStatus.Error.value());
}
} catch (final ClassNotFoundException e) {
Expand Down Expand Up @@ -418,10 +417,10 @@ public String authenticateConsoleAccess(String host, String port, String vmId, S
result.setTunnelUrl(authAnswer.getTunnelUrl());
result.setTunnelSession(authAnswer.getTunnelSession());
} else {
logger.error("Authentication failed for vm: " + vmId + " with sid: " + sid);
logger.error("Authentication failed for vm: {} with sid: {}", vmId, sid);
}
} catch (AgentControlChannelException e) {
logger.error("Unable to send out console access authentication request due to " + e.getMessage(), e);
logger.error("Unable to send out console access authentication request due to {}", e.getMessage(), e);
}

return new Gson().toJson(result);
Expand All @@ -431,26 +430,23 @@ public void reportLoadInfo(String gsonLoadInfo) {
ConsoleProxyLoadReportCommand cmd = new ConsoleProxyLoadReportCommand(proxyVmId, gsonLoadInfo);
try {
getAgentControl().postRequest(cmd);

if (logger.isDebugEnabled())
logger.debug("Report proxy load info, proxy : " + proxyVmId + ", load: " + gsonLoadInfo);
logger.debug("Report proxy load info, proxy : {}, load: {}", proxyVmId, gsonLoadInfo);
} catch (AgentControlChannelException e) {
logger.error("Unable to send out load info due to " + e.getMessage(), e);
logger.error("Unable to send out load info due to {}", e.getMessage(), e);
}
}

public void ensureRoute(String address) {
if (localGateway != null) {
if (logger.isDebugEnabled())
logger.debug("Ensure route for " + address + " via " + localGateway);
logger.debug("Ensure route for {} via {}", address, localGateway);

// this method won't be called in high frequency, serialize access
// to script execution
synchronized (this) {
try {
addRouteToInternalIpOrCidr(localGateway, eth1Ip, eth1Mask, address);
} catch (Throwable e) {
logger.warn("Unexpected exception while adding internal route to " + address, e);
logger.warn("Unexpected exception while adding internal route to {}", address, e);
}
}
}
Expand Down