Implement NumericThreshold for Trigger
Implemented NumericThreshold for Trigger that allows to set
expected threshold value for collection of sensors and monitors
if this value is crossed. Implemented detection of direction
when threshold value is crossed. Added initial interface that
is used to commit an action when threshold value is crossed.
Moved Sensor Cache from Report Factory to Telemetry class, now
Sensor Cache is shared between Report Factory and Trigger Factory.
Moved fetching sensor from Dbus to seperate header to have single
implementation for factories.
Disabled tests that uses boost coroutine because of false positive
that is catched by address sanitizer.
Tested:
- Passed unit tests
- Built in yocto envrionment with success
- Telemetry service started successfully in OpenBMC
Change-Id: I1ff7ab96174a27576786f0b9a71554fe1eeff436
Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
diff --git a/src/interfaces/threshold.hpp b/src/interfaces/threshold.hpp
new file mode 100644
index 0000000..23ff9d9
--- /dev/null
+++ b/src/interfaces/threshold.hpp
@@ -0,0 +1,14 @@
+#pragma once
+
+namespace interfaces
+{
+
+class Threshold
+{
+ public:
+ virtual ~Threshold() = default;
+
+ virtual void initialize() = 0;
+};
+
+} // namespace interfaces
diff --git a/src/interfaces/trigger_action.hpp b/src/interfaces/trigger_action.hpp
new file mode 100644
index 0000000..d21f238
--- /dev/null
+++ b/src/interfaces/trigger_action.hpp
@@ -0,0 +1,17 @@
+#pragma once
+
+#include <cstdint>
+#include <string>
+
+namespace interfaces
+{
+
+class TriggerAction
+{
+ public:
+ virtual ~TriggerAction() = default;
+
+ virtual void commit(const std::string& id, uint64_t timestamp,
+ double value) = 0;
+};
+} // namespace interfaces
diff --git a/src/interfaces/trigger_factory.hpp b/src/interfaces/trigger_factory.hpp
index 91f7f4b..2de5ae8 100644
--- a/src/interfaces/trigger_factory.hpp
+++ b/src/interfaces/trigger_factory.hpp
@@ -4,6 +4,8 @@
#include "interfaces/trigger_manager.hpp"
#include "interfaces/trigger_types.hpp"
+#include <boost/asio/spawn.hpp>
+
#include <memory>
#include <utility>
@@ -16,8 +18,9 @@
virtual ~TriggerFactory() = default;
virtual std::unique_ptr<interfaces::Trigger> make(
- const std::string& name, bool isDiscrete, bool logToJournal,
- bool logToRedfish, bool updateReport,
+ boost::asio::yield_context& yield, const std::string& name,
+ bool isDiscrete, bool logToJournal, bool logToRedfish,
+ bool updateReport,
const std::vector<
std::pair<sdbusplus::message::object_path, std::string>>& sensors,
const std::vector<std::string>& reportNames,
diff --git a/src/interfaces/types.hpp b/src/interfaces/types.hpp
index de97353..3cc069e 100644
--- a/src/interfaces/types.hpp
+++ b/src/interfaces/types.hpp
@@ -7,6 +7,7 @@
#include <string>
#include <tuple>
+#include <type_traits>
#include <vector>
using ReadingParameters =
diff --git a/src/numeric_threshold.cpp b/src/numeric_threshold.cpp
new file mode 100644
index 0000000..5cc2be5
--- /dev/null
+++ b/src/numeric_threshold.cpp
@@ -0,0 +1,104 @@
+#include "numeric_threshold.hpp"
+
+#include <phosphor-logging/log.hpp>
+
+NumericThreshold::NumericThreshold(
+ boost::asio::io_context& ioc,
+ std::vector<std::shared_ptr<interfaces::Sensor>> sensorsIn,
+ std::vector<std::string> sensorNames,
+ std::vector<std::unique_ptr<interfaces::TriggerAction>> actionsIn,
+ std::chrono::milliseconds dwellTimeIn, numeric::Direction direction,
+ double thresholdValueIn) :
+ ioc(ioc),
+ sensors(std::move(sensorsIn)), actions(std::move(actionsIn)),
+ dwellTime(dwellTimeIn), direction(direction),
+ thresholdValue(thresholdValueIn)
+{
+ details.reserve(sensors.size());
+ for (size_t i = 0; i < sensors.size(); i++)
+ {
+ details.emplace_back(sensorNames[i], thresholdValue, false, ioc);
+ }
+}
+
+NumericThreshold::~NumericThreshold()
+{}
+
+void NumericThreshold::initialize()
+{
+ for (auto& sensor : sensors)
+ {
+ sensor->registerForUpdates(weak_from_this());
+ }
+}
+
+NumericThreshold::ThresholdDetail&
+ NumericThreshold::getDetails(interfaces::Sensor& sensor)
+{
+ auto it =
+ std::find_if(sensors.begin(), sensors.end(),
+ [&sensor](const auto& x) { return &sensor == x.get(); });
+ auto index = std::distance(sensors.begin(), it);
+ return details.at(index);
+}
+
+void NumericThreshold::sensorUpdated(interfaces::Sensor& sensor,
+ uint64_t timestamp)
+{}
+
+void NumericThreshold::sensorUpdated(interfaces::Sensor& sensor,
+ uint64_t timestamp, double value)
+{
+ auto& [sensorName, prevValue, dwell, timer] = getDetails(sensor);
+ bool decreasing = thresholdValue < prevValue && thresholdValue > value;
+ bool increasing = thresholdValue > prevValue && thresholdValue < value;
+
+ if (dwell && (increasing || decreasing))
+ {
+ timer.cancel();
+ dwell = false;
+ }
+ if ((direction == numeric::Direction::decreasing && decreasing) ||
+ (direction == numeric::Direction::increasing && increasing) ||
+ (direction == numeric::Direction::either && (increasing || decreasing)))
+ {
+ startTimer(sensorName, timestamp, value, dwell, timer);
+ }
+
+ prevValue = value;
+}
+
+void NumericThreshold::startTimer(const std::string& sensorName,
+ uint64_t timestamp, double value, bool& dwell,
+ boost::asio::steady_timer& timer)
+{
+ if (dwellTime == std::chrono::milliseconds::zero())
+ {
+ commit(sensorName, timestamp, value);
+ }
+ else
+ {
+ dwell = true;
+ timer.expires_after(dwellTime);
+ timer.async_wait([this, sensorName, timestamp, value,
+ &dwell](const boost::system::error_code ec) {
+ if (ec)
+ {
+ phosphor::logging::log<phosphor::logging::level::DEBUG>(
+ "Timer has been canceled");
+ return;
+ }
+ commit(sensorName, timestamp, value);
+ dwell = false;
+ });
+ }
+}
+
+void NumericThreshold::commit(const std::string& sensorName, uint64_t timestamp,
+ double value)
+{
+ for (const auto& action : actions)
+ {
+ action->commit(sensorName, timestamp, value);
+ }
+}
diff --git a/src/numeric_threshold.hpp b/src/numeric_threshold.hpp
new file mode 100644
index 0000000..0951956
--- /dev/null
+++ b/src/numeric_threshold.hpp
@@ -0,0 +1,61 @@
+#pragma once
+
+#include "interfaces/sensor.hpp"
+#include "interfaces/sensor_listener.hpp"
+#include "interfaces/threshold.hpp"
+#include "interfaces/trigger_action.hpp"
+#include "interfaces/trigger_types.hpp"
+
+#include <boost/asio/steady_timer.hpp>
+
+#include <chrono>
+#include <memory>
+#include <vector>
+
+class NumericThreshold :
+ public interfaces::Threshold,
+ public interfaces::SensorListener,
+ public std::enable_shared_from_this<NumericThreshold>
+{
+ public:
+ NumericThreshold(
+ boost::asio::io_context& ioc,
+ std::vector<std::shared_ptr<interfaces::Sensor>> sensors,
+ std::vector<std::string> sensorNames,
+ std::vector<std::unique_ptr<interfaces::TriggerAction>> actions,
+ std::chrono::milliseconds dwellTime, numeric::Direction direction,
+ double thresholdValue);
+ ~NumericThreshold();
+
+ void initialize() override;
+ void sensorUpdated(interfaces::Sensor&, uint64_t) override;
+ void sensorUpdated(interfaces::Sensor&, uint64_t, double) override;
+
+ private:
+ boost::asio::io_context& ioc;
+ const std::vector<std::shared_ptr<interfaces::Sensor>> sensors;
+ const std::vector<std::unique_ptr<interfaces::TriggerAction>> actions;
+ const std::chrono::milliseconds dwellTime;
+ const numeric::Direction direction;
+ const double thresholdValue;
+
+ struct ThresholdDetail
+ {
+ std::string sensorName;
+ double prevValue;
+ bool dwell;
+ boost::asio::steady_timer timer;
+
+ ThresholdDetail(const std::string& name, double prevValue, bool dwell,
+ boost::asio::io_context& ioc) :
+ sensorName(name),
+ prevValue(prevValue), dwell(dwell), timer(ioc)
+ {}
+ };
+ std::vector<ThresholdDetail> details;
+
+ void startTimer(const std::string&, uint64_t, double, bool&,
+ boost::asio::steady_timer&);
+ void commit(const std::string&, uint64_t, double);
+ ThresholdDetail& getDetails(interfaces::Sensor& sensor);
+};
diff --git a/src/report_factory.cpp b/src/report_factory.cpp
index b8b5518..2cd5da3 100644
--- a/src/report_factory.cpp
+++ b/src/report_factory.cpp
@@ -3,13 +3,15 @@
#include "metric.hpp"
#include "report.hpp"
#include "sensor.hpp"
+#include "utils/dbus_mapper.hpp"
#include "utils/transform.hpp"
ReportFactory::ReportFactory(
std::shared_ptr<sdbusplus::asio::connection> bus,
- const std::shared_ptr<sdbusplus::asio::object_server>& objServer) :
+ const std::shared_ptr<sdbusplus::asio::object_server>& objServer,
+ SensorCache& sensorCache) :
bus(std::move(bus)),
- objServer(objServer)
+ objServer(objServer), sensorCache(sensorCache)
{}
std::unique_ptr<interfaces::Report> ReportFactory::make(
@@ -68,19 +70,7 @@
boost::asio::yield_context& yield,
const ReadingParameters& metricParams) const
{
- std::array<const char*, 1> interfaces = {
- "xyz.openbmc_project.Sensor.Value"};
- boost::system::error_code ec;
-
- auto tree = bus->yield_method_call<std::vector<SensorTree>>(
- yield, ec, "xyz.openbmc_project.ObjectMapper",
- "/xyz/openbmc_project/object_mapper",
- "xyz.openbmc_project.ObjectMapper", "GetSubTree",
- "/xyz/openbmc_project/sensors", 2, interfaces);
- if (ec)
- {
- throw std::runtime_error("Failed to query ObjectMapper!");
- }
+ auto tree = utils::getSubTreeSensors(yield, bus);
return utils::transform(metricParams, [&tree](const auto& item) {
std::vector<LabeledSensorParameters> sensors;
diff --git a/src/report_factory.hpp b/src/report_factory.hpp
index de2bc02..6d90902 100644
--- a/src/report_factory.hpp
+++ b/src/report_factory.hpp
@@ -12,7 +12,8 @@
public:
ReportFactory(
std::shared_ptr<sdbusplus::asio::connection> bus,
- const std::shared_ptr<sdbusplus::asio::object_server>& objServer);
+ const std::shared_ptr<sdbusplus::asio::object_server>& objServer,
+ SensorCache& sensorCache);
std::unique_ptr<interfaces::Report>
make(boost::asio::yield_context& yield, const std::string& name,
@@ -33,12 +34,6 @@
const override;
private:
- using SensorPath = std::string;
- using ServiceName = std::string;
- using Ifaces = std::vector<std::string>;
- using SensorIfaces = std::vector<std::pair<ServiceName, Ifaces>>;
- using SensorTree = std::pair<SensorPath, SensorIfaces>;
-
std::vector<std::shared_ptr<interfaces::Sensor>> getSensors(
const std::vector<LabeledSensorParameters>& sensorPaths) const;
std::vector<LabeledMetricParameters>
@@ -47,5 +42,5 @@
std::shared_ptr<sdbusplus::asio::connection> bus;
std::shared_ptr<sdbusplus::asio::object_server> objServer;
- mutable SensorCache sensorCache;
+ SensorCache& sensorCache;
};
diff --git a/src/telemetry.hpp b/src/telemetry.hpp
index 20749e4..8361936 100644
--- a/src/telemetry.hpp
+++ b/src/telemetry.hpp
@@ -17,17 +17,20 @@
public:
Telemetry(std::shared_ptr<sdbusplus::asio::connection> bus) :
objServer(std::make_shared<sdbusplus::asio::object_server>(bus)),
- reportManager(std::make_unique<ReportFactory>(bus, objServer),
- std::make_unique<PersistentJsonStorage>(
- interfaces::JsonStorage::DirectoryPath(
- "/var/lib/telemetry/Reports")),
- objServer),
- triggerManager(std::make_unique<TriggerFactory>(bus, objServer),
- objServer)
+ reportManager(
+ std::make_unique<ReportFactory>(bus, objServer, sensorCache),
+ std::make_unique<PersistentJsonStorage>(
+ interfaces::JsonStorage::DirectoryPath(
+ "/var/lib/telemetry/Reports")),
+ objServer),
+ triggerManager(
+ std::make_unique<TriggerFactory>(bus, objServer, sensorCache),
+ objServer)
{}
private:
std::shared_ptr<sdbusplus::asio::object_server> objServer;
+ mutable SensorCache sensorCache;
ReportManager reportManager;
TriggerManager triggerManager;
};
diff --git a/src/trigger.cpp b/src/trigger.cpp
index 4152dd1..471ad10 100644
--- a/src/trigger.cpp
+++ b/src/trigger.cpp
@@ -11,10 +11,12 @@
sensorsIn,
const std::vector<std::string>& reportNamesIn,
const TriggerThresholdParams& thresholdParamsIn,
+ std::vector<std::shared_ptr<interfaces::Threshold>>&& thresholdsIn,
interfaces::TriggerManager& triggerManager) :
name(nameIn),
path(triggerDir + name), persistent(false), sensors(sensorsIn),
- reportNames(reportNamesIn), thresholdParams(thresholdParamsIn)
+ reportNames(reportNamesIn), thresholdParams(thresholdParamsIn),
+ thresholds(std::move(thresholdsIn))
{
deleteIface = objServer->add_unique_interface(
path, deleteIfaceName, [this, &ioc, &triggerManager](auto& dbusIface) {
@@ -57,4 +59,9 @@
sdbusplus::vtable::property_::const_,
[](const auto& x) { return x; });
});
+
+ for (const auto& threshold : thresholds)
+ {
+ threshold->initialize();
+ }
}
diff --git a/src/trigger.hpp b/src/trigger.hpp
index 5092b6d..3405c5b 100644
--- a/src/trigger.hpp
+++ b/src/trigger.hpp
@@ -1,5 +1,6 @@
#pragma once
+#include "interfaces/threshold.hpp"
#include "interfaces/trigger.hpp"
#include "interfaces/trigger_manager.hpp"
#include "interfaces/trigger_types.hpp"
@@ -20,7 +21,8 @@
const std::vector<
std::pair<sdbusplus::message::object_path, std::string>>& sensorsIn,
const std::vector<std::string>& reportNames,
- const TriggerThresholdParams& thresholds,
+ const TriggerThresholdParams& thresholdParams,
+ std::vector<std::shared_ptr<interfaces::Threshold>>&& thresholds,
interfaces::TriggerManager& triggerManager);
Trigger(const Trigger&) = delete;
@@ -46,9 +48,9 @@
sensors;
std::vector<std::string> reportNames;
TriggerThresholdParams thresholdParams;
-
std::unique_ptr<sdbusplus::asio::dbus_interface> deleteIface;
std::unique_ptr<sdbusplus::asio::dbus_interface> triggerIface;
+ std::vector<std::shared_ptr<interfaces::Threshold>> thresholds;
public:
static constexpr const char* triggerIfaceName =
diff --git a/src/trigger_factory.cpp b/src/trigger_factory.cpp
index 0a45dec..709b5d6 100644
--- a/src/trigger_factory.cpp
+++ b/src/trigger_factory.cpp
@@ -1,25 +1,86 @@
#include "trigger_factory.hpp"
+#include "numeric_threshold.hpp"
+#include "sensor.hpp"
#include "trigger.hpp"
+#include "utils/dbus_mapper.hpp"
TriggerFactory::TriggerFactory(
std::shared_ptr<sdbusplus::asio::connection> bus,
- std::shared_ptr<sdbusplus::asio::object_server> objServer) :
+ std::shared_ptr<sdbusplus::asio::object_server> objServer,
+ SensorCache& sensorCache) :
bus(std::move(bus)),
- objServer(std::move(objServer))
+ objServer(std::move(objServer)), sensorCache(sensorCache)
{}
std::unique_ptr<interfaces::Trigger> TriggerFactory::make(
- const std::string& name, bool isDiscrete, bool logToJournal,
- bool logToRedfish, bool updateReport,
+ boost::asio::yield_context& yield, const std::string& name, bool isDiscrete,
+ bool logToJournal, bool logToRedfish, bool updateReport,
const std::vector<std::pair<sdbusplus::message::object_path, std::string>>&
- sensors,
+ sensorPaths,
const std::vector<std::string>& reportNames,
const TriggerThresholdParams& thresholdParams,
- interfaces::TriggerManager& reportManager) const
+ interfaces::TriggerManager& triggerManager) const
{
- return std::make_unique<Trigger>(bus->get_io_context(), objServer, name,
- isDiscrete, logToJournal, logToRedfish,
- updateReport, sensors, reportNames,
- thresholdParams, reportManager);
+ if (isDiscrete)
+ {
+ throw std::runtime_error("Not implemented!");
+ }
+
+ auto [sensors, sensorNames] = getSensors(yield, sensorPaths);
+ std::vector<std::shared_ptr<interfaces::Threshold>> thresholds;
+
+ const auto& params =
+ std::get<std::vector<numeric::ThresholdParam>>(thresholdParams);
+ for (const auto& [type, dwellTime, direction, value] : params)
+ {
+ std::vector<std::unique_ptr<interfaces::TriggerAction>> actions;
+
+ thresholds.emplace_back(std::make_shared<NumericThreshold>(
+ bus->get_io_context(), sensors, sensorNames, std::move(actions),
+ std::chrono::milliseconds(dwellTime),
+ static_cast<numeric::Direction>(direction), value));
+ }
+
+ return std::make_unique<Trigger>(
+ bus->get_io_context(), objServer, name, isDiscrete, logToJournal,
+ logToRedfish, updateReport, sensorPaths, reportNames, thresholdParams,
+ std::move(thresholds), triggerManager);
+}
+
+std::pair<std::vector<std::shared_ptr<interfaces::Sensor>>,
+ std::vector<std::string>>
+ TriggerFactory::getSensors(
+ boost::asio::yield_context& yield,
+ const std::vector<std::pair<sdbusplus::message::object_path,
+ std::string>>& sensorPaths) const
+{
+ auto tree = utils::getSubTreeSensors(yield, bus);
+
+ std::vector<std::shared_ptr<interfaces::Sensor>> sensors;
+ std::vector<std::string> sensorNames;
+ for (const auto& [sensorPath, metadata] : sensorPaths)
+ {
+ auto found = std::find_if(
+ tree.begin(), tree.end(),
+ [&sensorPath](const auto& x) { return x.first == sensorPath; });
+ if (found == tree.end())
+ {
+ throw std::runtime_error("Not found");
+ }
+
+ const auto& service = found->second[0].first;
+ const auto& path = found->first;
+ sensors.emplace_back(sensorCache.makeSensor<Sensor>(
+ service, path, bus->get_io_context(), bus));
+ if (metadata.empty())
+ {
+ sensorNames.emplace_back(sensorPath);
+ }
+ else
+ {
+ sensorNames.emplace_back(metadata);
+ }
+ }
+ return {sensors, sensorNames};
}
diff --git a/src/trigger_factory.hpp b/src/trigger_factory.hpp
index 60a9fb4..6d8f4f4 100644
--- a/src/trigger_factory.hpp
+++ b/src/trigger_factory.hpp
@@ -1,6 +1,8 @@
#pragma once
+#include "interfaces/sensor.hpp"
#include "interfaces/trigger_factory.hpp"
+#include "sensor_cache.hpp"
#include <sdbusplus/asio/object_server.hpp>
@@ -8,11 +10,13 @@
{
public:
TriggerFactory(std::shared_ptr<sdbusplus::asio::connection> bus,
- std::shared_ptr<sdbusplus::asio::object_server> objServer);
+ std::shared_ptr<sdbusplus::asio::object_server> objServer,
+ SensorCache& sensorCache);
std::unique_ptr<interfaces::Trigger> make(
- const std::string& name, bool isDiscrete, bool logToJournal,
- bool logToRedfish, bool updateReport,
+ boost::asio::yield_context& yield, const std::string& name,
+ bool isDiscrete, bool logToJournal, bool logToRedfish,
+ bool updateReport,
const std::vector<
std::pair<sdbusplus::message::object_path, std::string>>& sensors,
const std::vector<std::string>& reportNames,
@@ -22,4 +26,12 @@
private:
std::shared_ptr<sdbusplus::asio::connection> bus;
std::shared_ptr<sdbusplus::asio::object_server> objServer;
+ SensorCache& sensorCache;
+
+ std::pair<std::vector<std::shared_ptr<interfaces::Sensor>>,
+ std::vector<std::string>>
+ getSensors(boost::asio::yield_context& yield,
+ const std::vector<
+ std::pair<sdbusplus::message::object_path, std::string>>&
+ sensorPaths) const;
};
diff --git a/src/trigger_manager.cpp b/src/trigger_manager.cpp
index cc164e4..4191738 100644
--- a/src/trigger_manager.cpp
+++ b/src/trigger_manager.cpp
@@ -10,12 +10,20 @@
iface.register_method(
"AddTrigger",
[this](
- const std::string& name, bool isDiscrete, bool logToJournal,
- bool logToRedfish, bool updateReport,
+ boost::asio::yield_context& yield, const std::string& name,
+ bool isDiscrete, bool logToJournal, bool logToRedfish,
+ bool updateReport,
const std::vector<std::pair<sdbusplus::message::object_path,
std::string>>& sensors,
const std::vector<std::string>& reportNames,
const TriggerThresholdParams& thresholds) {
+ if (isDiscrete)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::not_supported),
+ "Only numeric threshold is supported");
+ }
+
if (triggers.size() >= maxTriggers)
{
throw sdbusplus::exception::SdBusError(
@@ -34,7 +42,7 @@
}
triggers.emplace_back(triggerFactory->make(
- name, isDiscrete, logToJournal, logToRedfish,
+ yield, name, isDiscrete, logToJournal, logToRedfish,
updateReport, sensors, reportNames, thresholds, *this));
return triggers.back()->getPath();
});
diff --git a/src/utils/dbus_mapper.hpp b/src/utils/dbus_mapper.hpp
new file mode 100644
index 0000000..78cf7b2
--- /dev/null
+++ b/src/utils/dbus_mapper.hpp
@@ -0,0 +1,40 @@
+#pragma once
+
+#include <boost/asio/spawn.hpp>
+#include <sdbusplus/asio/object_server.hpp>
+
+#include <array>
+#include <string>
+#include <utility>
+#include <vector>
+
+namespace utils
+{
+
+using SensorPath = std::string;
+using ServiceName = std::string;
+using Ifaces = std::vector<std::string>;
+using SensorIfaces = std::vector<std::pair<ServiceName, Ifaces>>;
+using SensorTree = std::pair<SensorPath, SensorIfaces>;
+
+inline std::vector<SensorTree>
+ getSubTreeSensors(boost::asio::yield_context& yield,
+ const std::shared_ptr<sdbusplus::asio::connection>& bus)
+{
+ std::array<const char*, 1> interfaces = {
+ "xyz.openbmc_project.Sensor.Value"};
+ boost::system::error_code ec;
+
+ auto tree = bus->yield_method_call<std::vector<SensorTree>>(
+ yield, ec, "xyz.openbmc_project.ObjectMapper",
+ "/xyz/openbmc_project/object_mapper",
+ "xyz.openbmc_project.ObjectMapper", "GetSubTree",
+ "/xyz/openbmc_project/sensors", 2, interfaces);
+ if (ec)
+ {
+ throw std::runtime_error("Failed to query ObjectMapper!");
+ }
+ return tree;
+}
+
+} // namespace utils