Make SEL logger messages more informative
Currently log entries are created using the "report" function from the
phosphor-logging repo. The problem is that function uses YAML file
"SEL.errors.yaml" for the message content. This leaves no space for the
message customization. And because of that webui displays all SEL
messages with a description
"xyz.openbmc_project.Logging.SEL.Error.Created" which is not very
informative.
To correct that switch to using "xyz.openbmc_project.Logging.Create"
function directly and create message content dynamically from all of
the incoming parameters.
Example of a SEL message description in webui:
- Before: "xyz.openbmc_project.Logging.SEL.Error.Created"
- After: "SEL Entry from System: RecordType=2, GeneratorID=65,
EventDir=1, EventData=01"
Change-Id: Iba17bd4c1834dd804c2eec04879fe226de2dfef7
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
diff --git a/include/host_error_event_monitor.hpp b/include/host_error_event_monitor.hpp
index 5f6242d..9375800 100644
--- a/include/host_error_event_monitor.hpp
+++ b/include/host_error_event_monitor.hpp
@@ -28,7 +28,8 @@
{"ThermalTrip", thermTripEventMatcher}, {"IERR", ierrEventMatcher}};
static boost::container::flat_set<std::string> hostErrorEvents;
-void hostErrorEventMonitor(sdbusplus::message_t& msg)
+void hostErrorEventMonitor(std::shared_ptr<sdbusplus::asio::connection> conn,
+ sdbusplus::message_t& msg)
{
std::string msgInterface;
boost::container::flat_map<std::string, std::variant<bool>> values;
@@ -71,7 +72,7 @@
uint8_t selType = (msgInterface.ends_with("ThermalTrip")) ? 0x01 : 0x00;
std::vector<uint8_t> selData{selType, 0xff, 0xff};
- selAddSystemRecord(message, objectPath, selData, assert, selBMCGenID);
+ selAddSystemRecord(conn, message, objectPath, selData, assert, selBMCGenID);
}
inline static void startHostErrorEventMonitor(
@@ -87,7 +88,7 @@
"HostErrorMonitor.Processor." +
iter->first + "'",
[conn, iter](sdbusplus::message_t& msg) {
- hostErrorEventMonitor(msg);
+ hostErrorEventMonitor(conn, msg);
});
}
}
diff --git a/include/sel_logger.hpp b/include/sel_logger.hpp
index 3d2e13c..1c9bc47 100644
--- a/include/sel_logger.hpp
+++ b/include/sel_logger.hpp
@@ -39,6 +39,7 @@
template <typename... T>
static void
- selAddSystemRecord(const std::string& message, const std::string& path,
+ selAddSystemRecord(std::shared_ptr<sdbusplus::asio::connection> conn,
+ const std::string& message, const std::string& path,
const std::vector<uint8_t>& selData, const bool& assert,
const uint16_t& genId, T&&... metadata);
diff --git a/include/threshold_alarm_event_monitor.hpp b/include/threshold_alarm_event_monitor.hpp
index bdfd3a9..0010d4b 100644
--- a/include/threshold_alarm_event_monitor.hpp
+++ b/include/threshold_alarm_event_monitor.hpp
@@ -238,7 +238,7 @@
". Reading=" + std::to_string(assertValue) +
" Threshold=" + std::to_string(thresholdVal) + ".");
- selAddSystemRecord(journalMsg, std::string(msg.get_path()), eventData,
+ selAddSystemRecord(conn, journalMsg, std::string(msg.get_path()), eventData,
assert, selBMCGenID, "REDFISH_MESSAGE_ID=%s",
redfishMessageID.c_str(),
"REDFISH_MESSAGE_ARGS=%.*s,%f,%f", sensorName.length(),
diff --git a/include/threshold_event_monitor.hpp b/include/threshold_event_monitor.hpp
index 61e4690..5b79a7a 100644
--- a/include/threshold_event_monitor.hpp
+++ b/include/threshold_event_monitor.hpp
@@ -325,7 +325,7 @@
}
#else
selAddSystemRecord(
- journalMsg, std::string(msg.get_path()), eventData, assert,
+ conn, journalMsg, std::string(msg.get_path()), eventData, assert,
selBMCGenID, "REDFISH_MESSAGE_ID=%s", redfishMessageID.c_str(),
"REDFISH_MESSAGE_ARGS=%.*s,%f,%f", sensorName.length(),
sensorName.data(), assertValue, thresholdVal);
diff --git a/include/watchdog_event_monitor.hpp b/include/watchdog_event_monitor.hpp
index facb282..777ac16 100644
--- a/include/watchdog_event_monitor.hpp
+++ b/include/watchdog_event_monitor.hpp
@@ -227,10 +227,10 @@
std::string redfishMessageID = "OpenBMC.0.1.IPMIWatchdog";
- selAddSystemRecord(journalMsg, std::string(msg.get_path()), eventData,
- assert, selBMCGenID, "REDFISH_MESSAGE_ID=%s",
- redfishMessageID.c_str(), "REDFISH_MESSAGE_ARGS=%s",
- eventMessageArgs.c_str(), NULL);
+ selAddSystemRecord(
+ conn, journalMsg, std::string(msg.get_path()), eventData, assert,
+ selBMCGenID, "REDFISH_MESSAGE_ID=%s", redfishMessageID.c_str(),
+ "REDFISH_MESSAGE_ARGS=%s", eventMessageArgs.c_str(), NULL);
}
}
diff --git a/src/sel_logger.cpp b/src/sel_logger.cpp
index cebc154..98c0530 100644
--- a/src/sel_logger.cpp
+++ b/src/sel_logger.cpp
@@ -37,17 +37,6 @@
#include <iostream>
#include <sstream>
-#ifdef SEL_LOGGER_SEND_TO_LOGGING_SERVICE
-#include <phosphor-logging/elog-errors.hpp>
-#include <phosphor-logging/elog.hpp>
-#include <phosphor-logging/log.hpp>
-#include <xyz/openbmc_project/Logging/SEL/error.hpp>
-
-using namespace phosphor::logging;
-using SELCreated =
- sdbusplus::xyz::openbmc_project::Logging::SEL::Error::Created;
-#endif
-
struct DBusInternalError final : public sdbusplus::exception_t
{
const char* name() const noexcept override
@@ -191,11 +180,11 @@
}
template <typename... T>
-static void selAddSystemRecord([[maybe_unused]] const std::string& message,
- const std::string& path,
- const std::vector<uint8_t>& selData,
- const bool& assert, const uint16_t& genId,
- [[maybe_unused]] T&&... metadata)
+static void selAddSystemRecord(
+ [[maybe_unused]] std::shared_ptr<sdbusplus::asio::connection> conn,
+ [[maybe_unused]] const std::string& message, const std::string& path,
+ const std::vector<uint8_t>& selData, const bool& assert,
+ const uint16_t& genId, [[maybe_unused]] T&&... metadata)
{
// Only 3 bytes of SEL event data are allowed in a system record
if (selData.size() > selEvtDataMaxSize)
@@ -206,11 +195,25 @@
toHexStr(selData, selDataStr);
#ifdef SEL_LOGGER_SEND_TO_LOGGING_SERVICE
- using namespace xyz::openbmc_project::Logging::SEL;
- report<SELCreated>(
- Created::RECORD_TYPE(selSystemType), Created::GENERATOR_ID(genId),
- Created::SENSOR_DATA(selDataStr.c_str()), Created::EVENT_DIR(assert),
- Created::SENSOR_PATH(path.c_str()));
+ sdbusplus::message_t AddToLog = conn->new_method_call(
+ "xyz.openbmc_project.Logging", "/xyz/openbmc_project/logging",
+ "xyz.openbmc_project.Logging.Create", "Create");
+
+ std::string journalMsg(message + " from " + path + ": " +
+ " RecordType=" + std::to_string(selSystemType) +
+ ", GeneratorID=" + std::to_string(genId) +
+ ", EventDir=" + std::to_string(assert) +
+ ", EventData=" + selDataStr);
+
+ AddToLog.append(journalMsg,
+ "xyz.openbmc_project.Logging.Entry.Level.Informational",
+ std::map<std::string, std::string>(
+ {{"SENSOR_PATH", path},
+ {"GENERATOR_ID", std::to_string(genId)},
+ {"RECORD_TYPE", std::to_string(selSystemType)},
+ {"EVENT_DIR", std::to_string(assert)},
+ {"SENSOR_DATA", selDataStr}}));
+ conn->call(AddToLog);
#else
unsigned int recordId = getNewRecordId();
sd_journal_send("MESSAGE=%s", message.c_str(), "PRIORITY=%i", selPriority,
@@ -223,9 +226,10 @@
#endif
}
-static void selAddOemRecord([[maybe_unused]] const std::string& message,
- const std::vector<uint8_t>& selData,
- const uint8_t& recordType)
+static void selAddOemRecord(
+ [[maybe_unused]] std::shared_ptr<sdbusplus::asio::connection> conn,
+ [[maybe_unused]] const std::string& message,
+ const std::vector<uint8_t>& selData, const uint8_t& recordType)
{
// A maximum of 13 bytes of SEL event data are allowed in an OEM record
if (selData.size() > selOemDataMaxSize)
@@ -236,11 +240,24 @@
toHexStr(selData, selDataStr);
#ifdef SEL_LOGGER_SEND_TO_LOGGING_SERVICE
- using namespace xyz::openbmc_project::Logging::SEL;
- report<SELCreated>(Created::RECORD_TYPE(recordType),
- Created::GENERATOR_ID(0),
- Created::SENSOR_DATA(selDataStr.c_str()),
- Created::EVENT_DIR(0), Created::SENSOR_PATH(""));
+ sdbusplus::message_t AddToLog = conn->new_method_call(
+ "xyz.openbmc_project.Logging", "/xyz/openbmc_project/logging",
+ "xyz.openbmc_project.Logging.Create", "Create");
+
+ std::string journalMsg(
+ message + ": " + " RecordType=" + std::to_string(recordType) +
+ ", GeneratorID=" + std::to_string(0) +
+ ", EventDir=" + std::to_string(0) + ", EventData=" + selDataStr);
+
+ AddToLog.append(journalMsg,
+ "xyz.openbmc_project.Logging.Entry.Level.Informational",
+ std::map<std::string, std::string>(
+ {{"SENSOR_PATH", ""},
+ {"GENERATOR_ID", std::to_string(0)},
+ {"RECORD_TYPE", std::to_string(recordType)},
+ {"EVENT_DIR", std::to_string(0)},
+ {"SENSOR_DATA", selDataStr}}));
+ conn->call(AddToLog);
#else
unsigned int recordId = getNewRecordId();
sd_journal_send("MESSAGE=%s", message.c_str(), "PRIORITY=%i", selPriority,
@@ -266,17 +283,18 @@
// Add a new SEL entry
ifaceAddSel->register_method(
- "IpmiSelAdd", [](const std::string& message, const std::string& path,
- const std::vector<uint8_t>& selData,
- const bool& assert, const uint16_t& genId) {
- return selAddSystemRecord(message, path, selData, assert, genId);
+ "IpmiSelAdd",
+ [conn](const std::string& message, const std::string& path,
+ const std::vector<uint8_t>& selData, const bool& assert,
+ const uint16_t& genId) {
+ return selAddSystemRecord(conn, message, path, selData, assert, genId);
});
// Add a new OEM SEL entry
ifaceAddSel->register_method("IpmiSelAddOem",
- [](const std::string& message,
- const std::vector<uint8_t>& selData,
- const uint8_t& recordType) {
- return selAddOemRecord(message, selData, recordType);
+ [conn](const std::string& message,
+ const std::vector<uint8_t>& selData,
+ const uint8_t& recordType) {
+ return selAddOemRecord(conn, message, selData, recordType);
});
#ifdef SEL_LOGGER_CLEARS_SEL