host-bmc: Improving Logs (lg2)
This commit corrects the severity level of logs and also formats the
message string, fixing the ill-defined message string of the logs as
mentioned in the anti-pattern document [1]. Additionally, based on the
requirement this commit adds more debug information to logs.
[1]: https://github.com/openbmc/docs/blob/master/anti-patterns.md#ill-defined-data-structuring-in-lg2-message-strings
Change-Id: I75a2b54ac1d363cfb675f29e811ab341046fbccd
Signed-off-by: Riya Dixit <riyadixitagra@gmail.com>
diff --git a/host-bmc/dbus_to_host_effecters.cpp b/host-bmc/dbus_to_host_effecters.cpp
index d5aeb58..0774685 100644
--- a/host-bmc/dbus_to_host_effecters.cpp
+++ b/host-bmc/dbus_to_host_effecters.cpp
@@ -40,16 +40,15 @@
fs::path jsonDir(jsonPath);
if (!fs::exists(jsonDir) || fs::is_empty(jsonDir))
{
- error("Host Effecter json path does not exist, DIR = {JSON_PATH}",
- "JSON_PATH", jsonPath.c_str());
+ error("Effecter json file for remote terminus '{PATH}' does not exist.",
+ "PATH", jsonPath);
return;
}
fs::path jsonFilePath = jsonDir / hostEffecterJson;
if (!fs::exists(jsonFilePath))
{
- error("json does not exist, PATH = {JSON_PATH}", "JSON_PATH",
- jsonFilePath.c_str());
+ error("Json at path '{PATH}' does not exist.", "PATH", jsonFilePath);
throw InternalFailure();
}
@@ -57,8 +56,7 @@
auto data = Json::parse(jsonFile, nullptr, false);
if (data.is_discarded())
{
- error("Parsing json file failed, FILE = {JSON_PATH}", "JSON_PATH",
- jsonFilePath.c_str());
+ error("Failed to parse json file {PATH}", "PATH", jsonFilePath);
throw InternalFailure();
}
const Json empty{};
@@ -101,9 +99,9 @@
if (dbusInfo.propertyValues.size() != states.size())
{
error(
- "Number of states do not match with number of D-Bus property values in the json. Object path {OBJ_PATH} and property {PROP_NAME} will not be monitored",
- "OBJ_PATH", dbusInfo.dbusMap.objectPath.c_str(),
- "PROP_NAME", dbusInfo.dbusMap.propertyName);
+ "Number of states do not match with number of D-Bus property values in the json. Object path at '{PATH}' and property '{PROPERTY}' will not be monitored",
+ "PATH", dbusInfo.dbusMap.objectPath, "PROPERTY",
+ dbusInfo.dbusMap.propertyName);
continue;
}
for (const auto& s : states)
@@ -153,7 +151,13 @@
localOrRemote);
if (effecterId == PLDM_INVALID_EFFECTER_ID)
{
- error("Effecter id not found in pdr repo");
+ error(
+ "Effecter ID '{EFFECTERID}' of entity type '{TYPE}', entityInstance '{INSTANCE}' and containerID '{CONTAINER_ID}' not found in pdr repo",
+ "EFFECTERID", effecterId, "TYPE",
+ hostEffecterInfo[effecterInfoIndex].entityType, "INSTANCE",
+ hostEffecterInfo[effecterInfoIndex].entityInstance,
+ "CONTAINER_ID",
+ hostEffecterInfo[effecterInfoIndex].containerId);
return;
}
}
@@ -173,16 +177,17 @@
currHostState != Stages::OSRunning &&
currHostState != Stages::SystemSetup)
{
- info("Host is not up. Current host state: {CUR_HOST_STATE}",
- "CUR_HOST_STATE", currHostState);
+ info(
+ "Remote terminus is not up/active, current remote terminus state is: '{CURRENT_HOST_STATE}'",
+ "CURRENT_HOST_STATE", currHostState);
return;
}
}
catch (const sdbusplus::exception_t& e)
{
error(
- "Error in getting current host state. Will still continue to set the host effecter - {ERR_EXCEP}",
- "ERR_EXCEP", e.what());
+ "Error in getting current remote terminus state. Will still continue to set the remote terminus effecter, error - {ERROR}",
+ "ERROR", e);
}
uint8_t newState{};
try
@@ -192,7 +197,8 @@
}
catch (const std::out_of_range& e)
{
- error("New state not found in json: {ERROR}", "ERROR", e);
+ error("Failed to find new state '{NEW_STATE}' in json, error - {ERROR}",
+ "ERROR", e, "NEW_STATE", newState);
return;
}
@@ -216,12 +222,16 @@
}
catch (const std::runtime_error& e)
{
- error("Could not set host state effecter");
+ error(
+ "Failed to set remote terminus state effecter for effecter ID '{EFFECTERID}', error - {ERROR}",
+ "ERROR", e, "EFFECTERID", effecterId);
return;
}
if (rc != PLDM_SUCCESS)
{
- error("Could not set the host state effecter, rc= {RC}", "RC", rc);
+ error(
+ "Failed to set the remote terminus state effecter for effecter ID '{EFFECTERID}', response code '{RC}'",
+ "EFFECTERID", effecterId, "RC", rc);
}
}
@@ -267,8 +277,10 @@
if (rc != PLDM_SUCCESS)
{
- error("Message encode failure. PLDM error code = {RC}", "RC", lg2::hex,
- rc);
+ error(
+ "Failed to encode set state effecter states message for effecter ID '{EFFECTERID}' and instanceID '{INSTANCE}' with response code '{RC}'",
+ "EFFECTERID", effecterId, "INSTANCE", instanceId, "RC", lg2::hex,
+ rc);
instanceIdDb->free(mctpEid, instanceId);
return rc;
}
@@ -278,7 +290,7 @@
if (response == nullptr || !respMsgLen)
{
error(
- "Failed to receive response for setStateEffecterStates command");
+ "Failed to receive response for setting state effecter states.");
return;
}
uint8_t completionCode{};
@@ -286,15 +298,17 @@
&completionCode);
if (rc)
{
- error("Failed to decode setStateEffecterStates response, rc {RC}",
- "RC", rc);
+ error(
+ "Failed to decode response of set state effecter states, response code '{RC}'",
+ "RC", rc);
pldm::utils::reportError(
"xyz.openbmc_project.bmc.pldm.SetHostEffecterFailed");
}
if (completionCode)
{
- error("Failed to set a Host effecter, cc = {CC}", "CC",
- static_cast<unsigned>(completionCode));
+ error(
+ "Failed to set a remote terminus effecter, completion code '{CC}'",
+ "CC", completionCode);
pldm::utils::reportError(
"xyz.openbmc_project.bmc.pldm.SetHostEffecterFailed");
}
@@ -305,7 +319,9 @@
std::move(requestMsg), std::move(setStateEffecterStatesRespHandler));
if (rc)
{
- error("Failed to send request to set an effecter on Host");
+ error(
+ "Failed to send request to set an effecter on remote terminus for effecter ID '{EFFECTERID}', response code '{RC}'",
+ "EFFECTERID", effecterId, "RC", rc);
}
return rc;
}