commit 89644441c01beaee1db50fa3c4ed9ad6607b0261
author Riya Dixit <riyadixitagra@gmail.com> Sun Mar 31 05:39:59 2024 -0500
committer ManojKiran Eda <manojkiran.eda@gmail.com> Thu May 30 04:39:47 2024 +0000
tree 5e1726c2545dbfdfe5cee8dbe05213c61808adaf
parent a85c69d3f9ce22ee906155d7c59ef997af0100fb

libpldmresponder: 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: I7dc5c308a8cd76573995e07d01d1a6037bca31ba
Signed-off-by: Riya Dixit <riyadixitagra@gmail.com>

libpldmresponder/platform_state_effecter.hpp

diff --git a/libpldmresponder/platform_state_effecter.hpp b/libpldmresponder/platform_state_effecter.hpp
index 247dbfd..a23732b 100644
--- a/libpldmresponder/platform_state_effecter.hpp
+++ b/libpldmresponder/platform_state_effecter.hpp
@@ -60,7 +60,7 @@
                   PLDM_STATE_EFFECTER_PDR);
     if (stateEffecterPDRs.empty())
     {
-        error("Failed to get record by PDR type");
+        error("Failed to get StateEffecterPDR record");
         return PLDM_PLATFORM_INVALID_EFFECTER_ID;
     }
 
@@ -81,8 +81,9 @@
         if (compEffecterCnt > pdr->composite_effecter_count)
         {
             error(
-                "The requester sent wrong composite effecter count for the effecter, EFFECTER_ID={EFFECTER_ID} COMP_EFF_CNT={COMP_EFF_CNT}",
-                "EFFECTER_ID", effecterId, "COMP_EFF_CNT", compEffecterCnt);
+                "The requester sent wrong composite effecter count '{COMPOSITE_EFFECTER_COUNT}' for the effecter ID '{EFFECTERID}'",
+                "EFFECTERID", effecterId, "COMPOSITE_EFFECTER_COUNT",
+                compEffecterCnt);
             return PLDM_ERROR_INVALID_DATA;
         }
         break;
@@ -100,7 +101,7 @@
                      dbusValMaps] = handler.getDbusObjMaps(effecterId);
         if (dbusMappings.empty() || dbusValMaps.empty())
         {
-            error("dbusMappings for effecter id : {EFFECTER_ID} is missing",
+            error("DbusMappings for effecter ID '{EFFECTER_ID}' is missing",
                   "EFFECTER_ID", effecterId);
             return PLDM_ERROR;
         }
@@ -118,13 +119,13 @@
             if (states->possible_states_size < bitfieldIndex ||
                 !(states->states[bitfieldIndex].byte & (1 << bit)))
             {
-                error(
-                    "Invalid state set value, EFFECTER_ID={EFFECTER_ID} VALUE={EFFECTER_STATE} COMPOSITE_EFFECTER_ID={CURR_STATE} DBUS_PATH={DBUS_OBJ_PATH}",
-                    "EFFECTER_ID", effecterId, "EFFECTER_STATE",
-                    stateField[currState].effecter_state, "CURR_STATE",
-                    currState, "DBUS_OBJ_PATH",
-                    dbusMappings[currState].objectPath.c_str());
                 rc = PLDM_PLATFORM_SET_EFFECTER_UNSUPPORTED_SENSORSTATE;
+                error(
+                    "Invalid state set value for effecter ID '{EFFECTER_ID}', effecter state '{EFFECTER_STATE}', composite effecter ID '{COMPOSITE_EFFECTER_ID}' and path '{PATH}', response code '{RC}'",
+                    "EFFECTER_ID", effecterId, "EFFECTER_STATE",
+                    stateField[currState].effecter_state,
+                    "COMPOSITE_EFFECTER_ID", currState, "PATH",
+                    dbusMappings[currState].objectPath.c_str(), "RC", rc);
                 break;
             }
             const DBusMapping& dbusMapping = dbusMappings[currState];
@@ -142,11 +143,10 @@
                 catch (const std::exception& e)
                 {
                     error(
-                        "Error setting property, ERROR={ERR_EXCEP} PROPERTY={DBUS_PROP} INTERFACE={DBUS_INTF} PATH={DBUS_OBJ_PATH}",
-                        "ERR_EXCEP", e.what(), "DBUS_PROP",
-                        dbusMapping.propertyName, "DBUS_INTF",
-                        dbusMapping.interface, "DBUS_OBJ_PATH",
-                        dbusMapping.objectPath.c_str());
+                        "Failed to set property '{PROPERTY}', interface '{INTERFACE}' and path '{PATH}', error - '{ERROR}'",
+                        "PROPERTY", dbusMapping.propertyName, "INTERFACE",
+                        dbusMapping.interface, "PATH",
+                        dbusMapping.objectPath.c_str(), "ERROR", e);
                     return PLDM_ERROR;
                 }
             }
@@ -161,8 +161,8 @@
     }
     catch (const std::out_of_range& e)
     {
-        error("Unknown effecter ID : {EFFECTER_ID} {ERR_EXCEP}", "EFFECTER_ID",
-              effecterId, "ERR_EXCEP", e.what());
+        error("Unknown effecter ID '{EFFECTERID}', error - {ERROR}",
+              "EFFECTERID", effecterId, "ERROR", e);
         return PLDM_ERROR;
     }