Break out Filesystem events into classes
EventServiceManager is already too large. Implement the TODO from run
that these should be classes, and fix the issue where events are being
registered on startup, not on a subscription being created.
To accomplish this, this patch takes global state and breaks them out
into RAII classes from EventServiceManager, one for monitoring DBus
matches, and one for monitoring filesystem log events using inotify.
Each of these connect to static methods on EventService that can send
the relevant events to the user.
Fundamentally, no code within the two new classes is changed, and the
only changes to event service are made to support creation and
destruction of the RAII classes.
Tested: WIP
No TelemetryService tests exist in Redfish.
Change-Id: I74210a10002eb39fddc9e42b0690a7c3d42fbd4c
Signed-off-by: Ed Tanous <etanous@nvidia.com>
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index 37d047e..78ecae9 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -56,8 +56,6 @@
static constexpr const char* eventServiceFile =
"/var/lib/bmcweb/eventservice_config.json";
-static constexpr const char* redfishEventLogFile = "/var/log/redfish";
-
class EventServiceManager
{
private:
@@ -65,10 +63,10 @@
uint32_t retryAttempts = 0;
uint32_t retryTimeoutInterval = 0;
- std::streampos redfishLogFilePosition{0};
size_t noOfEventLogSubscribers{0};
size_t noOfMetricReportSubscribers{0};
std::optional<DbusTelemetryMonitor> matchTelemetryMonitor;
+ std::optional<FilesystemLogWatcher> filesystemLogMonitor;
boost::container::flat_map<std::string, std::shared_ptr<Subscription>>
subscriptionsMap;
@@ -143,11 +141,6 @@
updateNoOfSubscribersCount();
- if constexpr (!BMCWEB_REDFISH_DBUS_LOG)
- {
- cacheRedfishLogFile();
- }
-
// Update retry configuration.
subValue->updateRetryConfig(retryAttempts, retryTimeoutInterval);
}
@@ -267,6 +260,21 @@
if (serviceEnabled)
{
+ if (noOfEventLogSubscribers > 0U)
+ {
+ if constexpr (!BMCWEB_REDFISH_DBUS_LOG)
+ {
+ if (!filesystemLogMonitor)
+ {
+ filesystemLogMonitor.emplace(ioc);
+ }
+ }
+ }
+ else
+ {
+ filesystemLogMonitor.reset();
+ }
+
if (noOfMetricReportSubscribers > 0U)
{
if (!matchTelemetryMonitor)
@@ -282,6 +290,7 @@
else
{
matchTelemetryMonitor.reset();
+ filesystemLogMonitor.reset();
}
if (serviceEnabled != cfg.enabled)
@@ -407,14 +416,6 @@
updateNoOfSubscribersCount();
- if constexpr (!BMCWEB_REDFISH_DBUS_LOG)
- {
- if (redfishLogFilePosition != 0)
- {
- cacheRedfishLogFile();
- }
- }
-
// Update retry configuration.
subValue->updateRetryConfig(retryAttempts, retryTimeoutInterval);
@@ -620,103 +621,6 @@
eventId++; // increment the eventId
}
}
-
- void resetRedfishFilePosition()
- {
- // Control would be here when Redfish file is created.
- // Reset File Position as new file is created
- redfishLogFilePosition = 0;
- }
-
- void cacheRedfishLogFile()
- {
- // Open the redfish file and read till the last record.
-
- std::ifstream logStream(redfishEventLogFile);
- if (!logStream.good())
- {
- BMCWEB_LOG_ERROR(" Redfish log file open failed ");
- return;
- }
- std::string logEntry;
- while (std::getline(logStream, logEntry))
- {
- redfishLogFilePosition = logStream.tellg();
- }
- }
-
- void readEventLogsFromFile()
- {
- std::ifstream logStream(redfishEventLogFile);
- if (!logStream.good())
- {
- BMCWEB_LOG_ERROR(" Redfish log file open failed");
- return;
- }
-
- std::vector<EventLogObjectsType> eventRecords;
-
- std::string logEntry;
-
- BMCWEB_LOG_DEBUG("Redfish log file: seek to {}",
- static_cast<int>(redfishLogFilePosition));
-
- // Get the read pointer to the next log to be read.
- logStream.seekg(redfishLogFilePosition);
-
- while (std::getline(logStream, logEntry))
- {
- BMCWEB_LOG_DEBUG("Redfish log file: found new event log entry");
- // Update Pointer position
- redfishLogFilePosition = logStream.tellg();
-
- std::string idStr;
- if (!event_log::getUniqueEntryID(logEntry, idStr))
- {
- BMCWEB_LOG_DEBUG(
- "Redfish log file: could not get unique entry id for {}",
- logEntry);
- continue;
- }
-
- if (!serviceEnabled || noOfEventLogSubscribers == 0)
- {
- // If Service is not enabled, no need to compute
- // the remaining items below.
- // But, Loop must continue to keep track of Timestamp
- BMCWEB_LOG_DEBUG(
- "Redfish log file: no subscribers / event service not enabled");
- continue;
- }
-
- std::string timestamp;
- std::string messageID;
- std::vector<std::string> messageArgs;
- if (event_log::getEventLogParams(logEntry, timestamp, messageID,
- messageArgs) != 0)
- {
- BMCWEB_LOG_DEBUG("Read eventLog entry params failed for {}",
- logEntry);
- continue;
- }
-
- eventRecords.emplace_back(idStr, timestamp, messageID, messageArgs);
- }
-
- if (!serviceEnabled || noOfEventLogSubscribers == 0)
- {
- BMCWEB_LOG_DEBUG("EventService disabled or no Subscriptions.");
- return;
- }
-
- if (eventRecords.empty())
- {
- // No Records to send
- BMCWEB_LOG_DEBUG("No log entries available to be transferred.");
- return;
- }
- EventServiceManager::sendEventsToSubs(eventRecords);
- }
};
} // namespace redfish
diff --git a/redfish-core/include/filesystem_log_watcher.hpp b/redfish-core/include/filesystem_log_watcher.hpp
index d12af50..b89c9d2 100644
--- a/redfish-core/include/filesystem_log_watcher.hpp
+++ b/redfish-core/include/filesystem_log_watcher.hpp
@@ -1,9 +1,39 @@
#pragma once
-#include <boost/asio/io_context.hpp>
+#include <sys/inotify.h>
+#include <boost/asio/posix/stream_descriptor.hpp>
+
+#include <optional>
+#include <string_view>
namespace redfish
{
-int startEventLogMonitor(boost::asio::io_context& ioc);
-void stopEventLogMonitor();
+
+constexpr const char* redfishEventLogFile = "/var/log/redfish";
+
+class FilesystemLogWatcher
+{
+ private:
+ std::streampos redfishLogFilePosition{0};
+
+ int inotifyFd = -1;
+ int dirWatchDesc = -1;
+ int fileWatchDesc = -1;
+ boost::asio::posix::stream_descriptor inotifyConn;
+ void onINotify(const boost::system::error_code& ec,
+ std::size_t bytesTransferred);
+
+ void resetRedfishFilePosition();
+
+ void watchRedfishEventLogFile();
+
+ void readEventLogsFromFile();
+
+ void cacheRedfishLogFile();
+
+ std::array<char, 1024> readBuffer{};
+
+ public:
+ explicit FilesystemLogWatcher(boost::asio::io_context& iocIn);
+};
} // namespace redfish
diff --git a/redfish-core/src/filesystem_log_watcher.cpp b/redfish-core/src/filesystem_log_watcher.cpp
index a982783..2533d0c 100644
--- a/redfish-core/src/filesystem_log_watcher.cpp
+++ b/redfish-core/src/filesystem_log_watcher.cpp
@@ -1,5 +1,7 @@
#include "filesystem_log_watcher.hpp"
+#include "event_log.hpp"
+#include "event_logs_object_type.hpp"
#include "event_service_manager.hpp"
#include "logging.hpp"
@@ -13,30 +15,99 @@
#include <array>
#include <cstddef>
#include <cstring>
-#include <optional>
+#include <fstream>
+#include <functional>
#include <string>
+#include <vector>
namespace redfish
{
+void FilesystemLogWatcher::resetRedfishFilePosition()
+{
+ // Control would be here when Redfish file is created.
+ // Reset File Position as new file is created
+ redfishLogFilePosition = 0;
+}
-// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-static std::optional<boost::asio::posix::stream_descriptor> inotifyConn;
-// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-static int inotifyFd = -1;
-// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-static int dirWatchDesc = -1;
-// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-static int fileWatchDesc = -1;
-// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-static std::array<char, 1024> readBuffer{};
+void FilesystemLogWatcher::cacheRedfishLogFile()
+{
+ // Open the redfish file and read till the last record.
+
+ std::ifstream logStream(redfishEventLogFile);
+ if (!logStream.good())
+ {
+ BMCWEB_LOG_ERROR(" Redfish log file open failed ");
+ return;
+ }
+ std::string logEntry;
+ while (std::getline(logStream, logEntry))
+ {
+ redfishLogFilePosition = logStream.tellg();
+ }
+}
+
+void FilesystemLogWatcher::readEventLogsFromFile()
+{
+ std::ifstream logStream(redfishEventLogFile);
+ if (!logStream.good())
+ {
+ BMCWEB_LOG_ERROR(" Redfish log file open failed");
+ return;
+ }
+
+ std::vector<EventLogObjectsType> eventRecords;
+
+ std::string logEntry;
+
+ BMCWEB_LOG_DEBUG("Redfish log file: seek to {}",
+ static_cast<int>(redfishLogFilePosition));
+
+ // Get the read pointer to the next log to be read.
+ logStream.seekg(redfishLogFilePosition);
+
+ while (std::getline(logStream, logEntry))
+ {
+ BMCWEB_LOG_DEBUG("Redfish log file: found new event log entry");
+ // Update Pointer position
+ redfishLogFilePosition = logStream.tellg();
+
+ std::string idStr;
+ if (!event_log::getUniqueEntryID(logEntry, idStr))
+ {
+ BMCWEB_LOG_DEBUG(
+ "Redfish log file: could not get unique entry id for {}",
+ logEntry);
+ continue;
+ }
+
+ std::string timestamp;
+ std::string messageID;
+ std::vector<std::string> messageArgs;
+ if (event_log::getEventLogParams(logEntry, timestamp, messageID,
+ messageArgs) != 0)
+ {
+ BMCWEB_LOG_DEBUG("Read eventLog entry params failed for {}",
+ logEntry);
+ continue;
+ }
+
+ eventRecords.emplace_back(idStr, timestamp, messageID, messageArgs);
+ }
+
+ if (eventRecords.empty())
+ {
+ // No Records to send
+ BMCWEB_LOG_DEBUG("No log entries available to be transferred.");
+ return;
+ }
+ EventServiceManager::sendEventsToSubs(eventRecords);
+}
static constexpr const char* redfishEventLogDir = "/var/log";
static constexpr const size_t iEventSize = sizeof(inotify_event);
-static void watchRedfishEventLogFile();
-
-static void onINotify(const boost::system::error_code& ec,
- std::size_t bytesTransferred)
+void FilesystemLogWatcher::onINotify(const boost::system::error_code& ec,
+ std::size_t bytesTransferred)
{
if (ec == boost::asio::error::operation_aborted)
{
@@ -96,8 +167,8 @@
return;
}
- EventServiceManager::getInstance().resetRedfishFilePosition();
- EventServiceManager::getInstance().readEventLogsFromFile();
+ resetRedfishFilePosition();
+ readEventLogsFromFile();
}
else if ((event.mask == IN_DELETE) || (event.mask == IN_MOVED_TO))
{
@@ -112,7 +183,7 @@
{
if (event.mask == IN_MODIFY)
{
- EventServiceManager::getInstance().readEventLogsFromFile();
+ readEventLogsFromFile();
}
}
index += (iEventSize + event.len);
@@ -121,27 +192,22 @@
watchRedfishEventLogFile();
}
-static void watchRedfishEventLogFile()
+void FilesystemLogWatcher::watchRedfishEventLogFile()
{
- if (!inotifyConn)
- {
- BMCWEB_LOG_ERROR("inotify Connection is not present");
- return;
- }
-
- inotifyConn->async_read_some(boost::asio::buffer(readBuffer), onINotify);
+ inotifyConn.async_read_some(
+ boost::asio::buffer(readBuffer),
+ std::bind_front(&FilesystemLogWatcher::onINotify, this));
}
-int startEventLogMonitor(boost::asio::io_context& ioc)
+FilesystemLogWatcher::FilesystemLogWatcher(boost::asio::io_context& ioc) :
+ inotifyFd(inotify_init1(IN_NONBLOCK)), inotifyConn(ioc)
{
BMCWEB_LOG_DEBUG("starting Event Log Monitor");
- inotifyConn.emplace(ioc);
- inotifyFd = inotify_init1(IN_NONBLOCK);
if (inotifyFd == -1)
{
BMCWEB_LOG_ERROR("inotify_init1 failed.");
- return -1;
+ return;
}
// Add watch on directory to handle redfish event log file
@@ -151,7 +217,7 @@
if (dirWatchDesc == -1)
{
BMCWEB_LOG_ERROR("inotify_add_watch failed for event log directory.");
- return -1;
+ return;
}
// Watch redfish event log file for modifications.
@@ -165,14 +231,12 @@
}
// monitor redfish event log file
- inotifyConn->assign(inotifyFd);
+ inotifyConn.assign(inotifyFd);
watchRedfishEventLogFile();
- return 0;
-}
-
-void stopEventLogMonitor()
-{
- inotifyConn.reset();
+ if (redfishLogFilePosition != 0)
+ {
+ cacheRedfishLogFile();
+ }
}
} // namespace redfish