Break out Dbus events into class
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.
There are a number of call sites, like cacheRedfishLogFile, that are
obsoleted when the class is raii. The file will be re-cached on
creation.
Tested: WIP
No TelemetryService tests exist in Redfish.
Change-Id: Ibc91cd1496edf4a080e2d60bfc1a32e00a6c74b8
Signed-off-by: Ed Tanous <etanous@nvidia.com>
diff --git a/redfish-core/include/dbus_log_watcher.hpp b/redfish-core/include/dbus_log_watcher.hpp
index 86e4a9d..b47fca7 100644
--- a/redfish-core/include/dbus_log_watcher.hpp
+++ b/redfish-core/include/dbus_log_watcher.hpp
@@ -1,9 +1,13 @@
#pragma once
-#include <sdbusplus/message.hpp>
+#include <sdbusplus/bus/match.hpp>
namespace redfish
{
+class DbusTelemetryMonitor
+{
+ public:
+ DbusTelemetryMonitor();
-void getReadingsForReport(sdbusplus::message_t& msg);
-
+ sdbusplus::bus::match_t matchTelemetryMonitor;
+};
} // namespace redfish
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index f3fb11c..4197cda 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -25,16 +25,16 @@
#include "ossl_random.hpp"
#include "persistent_data.hpp"
#include "subscription.hpp"
+#include "utility.hpp"
+#include "utils/dbus_event_log_entry.hpp"
+#include "utils/json_utils.hpp"
#include "utils/time_utils.hpp"
-#include <sys/inotify.h>
-
#include <boost/asio/io_context.hpp>
#include <boost/circular_buffer.hpp>
#include <boost/container/flat_map.hpp>
#include <boost/url/format.hpp>
#include <boost/url/url_view_base.hpp>
-#include <sdbusplus/bus/match.hpp>
#include <algorithm>
#include <cstdlib>
@@ -45,6 +45,7 @@
#include <string>
#include <string_view>
#include <utility>
+#include <variant>
namespace redfish
{
@@ -67,7 +68,7 @@
std::streampos redfishLogFilePosition{0};
size_t noOfEventLogSubscribers{0};
size_t noOfMetricReportSubscribers{0};
- std::shared_ptr<sdbusplus::bus::match_t> matchTelemetryMonitor;
+ std::optional<DbusTelemetryMonitor> matchTelemetryMonitor;
boost::container::flat_map<std::string, std::shared_ptr<Subscription>>
subscriptionsMap;
@@ -264,17 +265,28 @@
bool updateConfig = false;
bool updateRetryCfg = false;
- if (serviceEnabled != cfg.enabled)
+ if (serviceEnabled)
{
- serviceEnabled = cfg.enabled;
- if (serviceEnabled && noOfMetricReportSubscribers != 0U)
+ if (noOfMetricReportSubscribers > 0U)
{
- registerMetricReportSignal();
+ if (!matchTelemetryMonitor)
+ {
+ matchTelemetryMonitor.emplace();
+ }
}
else
{
- unregisterMetricReportSignal();
+ matchTelemetryMonitor.reset();
}
+ }
+ else
+ {
+ matchTelemetryMonitor.reset();
+ }
+
+ if (serviceEnabled != cfg.enabled)
+ {
+ serviceEnabled = cfg.enabled;
updateConfig = true;
}
@@ -332,11 +344,14 @@
noOfMetricReportSubscribers = metricReportSubCount;
if (noOfMetricReportSubscribers != 0U)
{
- registerMetricReportSignal();
+ if (!matchTelemetryMonitor)
+ {
+ matchTelemetryMonitor.emplace();
+ }
}
else
{
- unregisterMetricReportSignal();
+ matchTelemetryMonitor.reset();
}
}
}
@@ -399,6 +414,7 @@
cacheRedfishLogFile();
}
}
+
// Update retry configuration.
subValue->updateRetryConfig(retryAttempts, retryTimeoutInterval);
@@ -698,33 +714,6 @@
}
}
}
-
- void unregisterMetricReportSignal()
- {
- if (matchTelemetryMonitor)
- {
- BMCWEB_LOG_DEBUG("Metrics report signal - Unregister");
- matchTelemetryMonitor.reset();
- matchTelemetryMonitor = nullptr;
- }
- }
-
- void registerMetricReportSignal()
- {
- if (!serviceEnabled || matchTelemetryMonitor)
- {
- BMCWEB_LOG_DEBUG("Not registering metric report signal.");
- return;
- }
-
- BMCWEB_LOG_DEBUG("Metrics report signal - Register");
- std::string matchStr = "type='signal',member='PropertiesChanged',"
- "interface='org.freedesktop.DBus.Properties',"
- "arg0=xyz.openbmc_project.Telemetry.Report";
-
- matchTelemetryMonitor = std::make_shared<sdbusplus::bus::match_t>(
- *crow::connections::systemBus, matchStr, getReadingsForReport);
- }
};
} // namespace redfish
diff --git a/redfish-core/src/dbus_log_watcher.cpp b/redfish-core/src/dbus_log_watcher.cpp
index 77f1f02..c2eab64 100644
--- a/redfish-core/src/dbus_log_watcher.cpp
+++ b/redfish-core/src/dbus_log_watcher.cpp
@@ -1,10 +1,12 @@
#include "dbus_log_watcher.hpp"
+#include "dbus_singleton.hpp"
#include "dbus_utility.hpp"
#include "event_service_manager.hpp"
#include "logging.hpp"
#include "metric_report.hpp"
+#include <sdbusplus/bus/match.hpp>
#include <sdbusplus/message.hpp>
#include <sdbusplus/message/native_types.hpp>
@@ -15,7 +17,7 @@
namespace redfish
{
-void getReadingsForReport(sdbusplus::message_t& msg)
+static void getReadingsForReport(sdbusplus::message_t& msg)
{
if (msg.is_method_error())
{
@@ -54,4 +56,14 @@
}
EventServiceManager::sendTelemetryReportToSubs(id, *readings);
}
+
+const std::string telemetryMatchStr =
+ "type='signal',member='PropertiesChanged',"
+ "interface='org.freedesktop.DBus.Properties',"
+ "arg0=xyz.openbmc_project.Telemetry.Report";
+
+DbusTelemetryMonitor::DbusTelemetryMonitor() :
+ matchTelemetryMonitor(*crow::connections::systemBus, telemetryMatchStr,
+ getReadingsForReport)
+{}
} // namespace redfish
diff --git a/src/webserver_run.cpp b/src/webserver_run.cpp
index dfed2b7..eddfc5b 100644
--- a/src/webserver_run.cpp
+++ b/src/webserver_run.cpp
@@ -6,7 +6,6 @@
#include "dbus_monitor.hpp"
#include "dbus_singleton.hpp"
#include "event_service_manager.hpp"
-#include "filesystem_log_watcher.hpp"
#include "google/google_service_root.hpp"
#include "hostname_monitor.hpp"
#include "ibm/management_console_rest.hpp"
@@ -79,9 +78,10 @@
if constexpr (BMCWEB_REDFISH)
{
redfish::RedfishService redfish(app);
-
- // Create EventServiceManager instance and initialize Config
- redfish::EventServiceManager::getInstance(&*io);
+ if constexpr (!BMCWEB_REDFISH_DBUS_LOG)
+ {
+ redfish::EventServiceManager::getInstance(&*io);
+ }
if constexpr (BMCWEB_REDFISH_AGGREGATION)
{
@@ -116,16 +116,6 @@
crow::login_routes::requestRoutes(app);
- if constexpr (!BMCWEB_REDFISH_DBUS_LOG)
- {
- int rc = redfish::startEventLogMonitor(*io);
- if (rc != 0)
- {
- BMCWEB_LOG_ERROR("Redfish event handler setup failed...");
- return rc;
- }
- }
-
if constexpr (!BMCWEB_INSECURE_DISABLE_SSL)
{
BMCWEB_LOG_INFO("Start Hostname Monitor Service...");
@@ -142,8 +132,5 @@
crow::connections::systemBus = nullptr;
- // TODO(ed) Make event log monitor an RAII object instead of global vars
- redfish::stopEventLogMonitor();
-
return 0;
}