Implement Trigger Actions
Now action is triggered when threshold is crossed. There are 3
actions: write message to journal, write message to journal as
Redfish Event and update an existing Report.
To update an existing Report updateReading method is changed to
public. Added UpdateReport to ReportManager, now object is able
to update readings of any report using report name.
Tested:
- Unit tests passed
- Verified that logs are propagated to journal when threshold
is crossed
Change-Id: Iebca7c8b9ab9b50b4c401877ccf8c2f01f1e6f36
Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
diff --git a/meson.build b/meson.build
index fa857ad..1fc346d 100644
--- a/meson.build
+++ b/meson.build
@@ -89,6 +89,7 @@
'src/sensor.cpp',
'src/sensor_cache.cpp',
'src/trigger.cpp',
+ 'src/trigger_actions.cpp',
'src/trigger_factory.cpp',
'src/trigger_manager.cpp',
],
diff --git a/src/interfaces/report.hpp b/src/interfaces/report.hpp
index 98421be..a299dac 100644
--- a/src/interfaces/report.hpp
+++ b/src/interfaces/report.hpp
@@ -12,5 +12,6 @@
virtual std::string getName() const = 0;
virtual std::string getPath() const = 0;
+ virtual void updateReadings() = 0;
};
} // namespace interfaces
diff --git a/src/interfaces/report_manager.hpp b/src/interfaces/report_manager.hpp
index 80e0c82..910e439 100644
--- a/src/interfaces/report_manager.hpp
+++ b/src/interfaces/report_manager.hpp
@@ -11,6 +11,7 @@
virtual ~ReportManager() = default;
virtual void removeReport(const interfaces::Report* report) = 0;
+ virtual void updateReport(const std::string& name) = 0;
};
} // namespace interfaces
diff --git a/src/interfaces/trigger_factory.hpp b/src/interfaces/trigger_factory.hpp
index 2de5ae8..7d448d0 100644
--- a/src/interfaces/trigger_factory.hpp
+++ b/src/interfaces/trigger_factory.hpp
@@ -5,6 +5,7 @@
#include "interfaces/trigger_types.hpp"
#include <boost/asio/spawn.hpp>
+#include <sdbusplus/message/types.hpp>
#include <memory>
#include <utility>
diff --git a/src/interfaces/trigger_types.hpp b/src/interfaces/trigger_types.hpp
index ef61aa6..eb0f7c4 100644
--- a/src/interfaces/trigger_types.hpp
+++ b/src/interfaces/trigger_types.hpp
@@ -1,10 +1,11 @@
#pragma once
-#include <sdbusplus/message/types.hpp>
+#include "utils/conversion.hpp"
#include <string>
#include <tuple>
#include <utility>
+#include <variant>
#include <vector>
namespace discrete
@@ -17,6 +18,11 @@
critical
};
+inline Severity toSeverity(int x)
+{
+ return utils::toEnum<Severity, Severity::ok, Severity::critical>(x);
+}
+
using ThresholdParam = std::tuple<std::string, std::underlying_type_t<Severity>,
std::variant<double>, uint64_t>;
} // namespace discrete
@@ -39,6 +45,17 @@
increasing
};
+inline Type toType(int x)
+{
+ return utils::toEnum<Type, Type::lowerCritical, Type::upperCritical>(x);
+}
+
+inline Direction toDirection(int x)
+{
+ return utils::toEnum<Direction, Direction::either, Direction::increasing>(
+ x);
+}
+
using ThresholdParam = std::tuple<std::underlying_type_t<Type>, uint64_t,
std::underlying_type_t<Direction>, double>;
} // namespace numeric
diff --git a/src/report.hpp b/src/report.hpp
index c58d38b..2f470d3 100644
--- a/src/report.hpp
+++ b/src/report.hpp
@@ -43,12 +43,12 @@
return path;
}
+ void updateReadings() override;
bool storeConfiguration() const;
private:
static void timerProc(boost::system::error_code, Report& self);
void scheduleTimer(std::chrono::milliseconds interval);
- void updateReadings();
const std::string name;
const std::string path;
diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index 951aa5f..1d3e47b 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -197,3 +197,15 @@
}
}
}
+
+void ReportManager::updateReport(const std::string& name)
+{
+ for (auto& report : reports)
+ {
+ if (report->getName() == name)
+ {
+ report->updateReadings();
+ return;
+ }
+ }
+}
diff --git a/src/report_manager.hpp b/src/report_manager.hpp
index a1a9007..bb4d395 100644
--- a/src/report_manager.hpp
+++ b/src/report_manager.hpp
@@ -27,6 +27,7 @@
ReportManager& operator=(ReportManager&&) = delete;
void removeReport(const interfaces::Report* report) override;
+ void updateReport(const std::string& name) override;
private:
std::unique_ptr<interfaces::ReportFactory> reportFactory;
diff --git a/src/telemetry.hpp b/src/telemetry.hpp
index 8361936..fb7f2a3 100644
--- a/src/telemetry.hpp
+++ b/src/telemetry.hpp
@@ -23,9 +23,9 @@
interfaces::JsonStorage::DirectoryPath(
"/var/lib/telemetry/Reports")),
objServer),
- triggerManager(
- std::make_unique<TriggerFactory>(bus, objServer, sensorCache),
- objServer)
+ triggerManager(std::make_unique<TriggerFactory>(
+ bus, objServer, sensorCache, reportManager),
+ objServer)
{}
private:
diff --git a/src/trigger_actions.cpp b/src/trigger_actions.cpp
new file mode 100644
index 0000000..26fa703
--- /dev/null
+++ b/src/trigger_actions.cpp
@@ -0,0 +1,95 @@
+#include "trigger_actions.hpp"
+
+#include <phosphor-logging/log.hpp>
+
+#include <ctime>
+
+namespace action
+{
+
+static const char* getDirection(double value, double threshold)
+{
+ if (value < threshold)
+ {
+ return "decreasing";
+ }
+ if (value > threshold)
+ {
+ return "increasing";
+ }
+ throw std::runtime_error("Invalid value");
+}
+
+const char* LogToJournal::getType() const
+{
+ switch (type)
+ {
+ case numeric::Type::upperCritical:
+ return "UpperCritical";
+ case numeric::Type::lowerCritical:
+ return "LowerCritical";
+ case numeric::Type::upperWarning:
+ return "UpperWarning";
+ case numeric::Type::lowerWarning:
+ return "LowerWarning";
+ }
+ throw std::runtime_error("Invalid type");
+}
+
+void LogToJournal::commit(const std::string& sensorName, uint64_t timestamp,
+ double value)
+{
+ std::time_t t = static_cast<time_t>(timestamp);
+ std::array<char, sizeof("YYYY-MM-DDThh:mm:ssZ")> buf = {};
+ size_t size =
+ std::strftime(buf.data(), buf.size(), "%FT%TZ", std::gmtime(&t));
+ if (size == 0)
+ {
+ throw std::runtime_error("Failed to parse timestamp to string");
+ }
+
+ std::string msg = std::string(getType()) +
+ " numeric threshold condition is met on sensor " +
+ sensorName + ", recorded value " + std::to_string(value) +
+ ", timestamp " + std::string(buf.data(), size) +
+ ", direction " +
+ std::string(getDirection(value, threshold));
+
+ phosphor::logging::log<phosphor::logging::level::INFO>(msg.c_str());
+}
+
+const char* LogToRedfish::getMessageId() const
+{
+ switch (type)
+ {
+ case numeric::Type::upperCritical:
+ return "OpenBMC.0.1.0.NumericThresholdUpperCritical";
+ case numeric::Type::lowerCritical:
+ return "OpenBMC.0.1.0.NumericThresholdLowerCritical";
+ case numeric::Type::upperWarning:
+ return "OpenBMC.0.1.0.NumericThresholdUpperWarning";
+ case numeric::Type::lowerWarning:
+ return "OpenBMC.0.1.0.NumericThresholdLowerWarning";
+ }
+ throw std::runtime_error("Invalid type");
+}
+
+void LogToRedfish::commit(const std::string& sensorName, uint64_t timestamp,
+ double value)
+{
+ phosphor::logging::log<phosphor::logging::level::INFO>(
+ "Threshold value is exceeded",
+ phosphor::logging::entry("REDFISH_MESSAGE_ID=%s", getMessageId()),
+ phosphor::logging::entry("REDFISH_MESSAGE_ARGS=%s,%f,%llu,%s",
+ sensorName.c_str(), value, timestamp,
+ getDirection(value, threshold)));
+}
+
+void UpdateReport::commit(const std::string&, uint64_t, double)
+{
+ for (const auto& name : reportNames)
+ {
+ reportManager.updateReport(name);
+ }
+}
+} // namespace action
diff --git a/src/trigger_actions.hpp b/src/trigger_actions.hpp
new file mode 100644
index 0000000..2c8db69
--- /dev/null
+++ b/src/trigger_actions.hpp
@@ -0,0 +1,58 @@
+#pragma once
+
+#include "interfaces/report_manager.hpp"
+#include "interfaces/trigger_action.hpp"
+#include "interfaces/trigger_types.hpp"
+
+namespace action
+{
+
+class LogToJournal : public interfaces::TriggerAction
+{
+ public:
+ LogToJournal(numeric::Type type, double val) : type(type), threshold(val)
+ {}
+
+ void commit(const std::string& id, uint64_t timestamp,
+ double value) override;
+
+ private:
+ numeric::Type type;
+ double threshold;
+
+ const char* getType() const;
+};
+
+class LogToRedfish : public interfaces::TriggerAction
+{
+ public:
+ LogToRedfish(numeric::Type type, double val) : type(type), threshold(val)
+ {}
+
+ void commit(const std::string& id, uint64_t timestamp,
+ double value) override;
+
+ private:
+ numeric::Type type;
+ double threshold;
+
+ const char* getMessageId() const;
+};
+
+class UpdateReport : public interfaces::TriggerAction
+{
+ public:
+ UpdateReport(interfaces::ReportManager& reportManager,
+ std::vector<std::string> names) :
+ reportManager(reportManager),
+ reportNames(std::move(names))
+ {}
+
+ void commit(const std::string& id, uint64_t timestamp,
+ double value) override;
+
+ private:
+ interfaces::ReportManager& reportManager;
+ std::vector<std::string> reportNames;
+};
+} // namespace action
diff --git a/src/trigger_factory.cpp b/src/trigger_factory.cpp
index 709b5d6..d8484c2 100644
--- a/src/trigger_factory.cpp
+++ b/src/trigger_factory.cpp
@@ -3,14 +3,16 @@
#include "numeric_threshold.hpp"
#include "sensor.hpp"
#include "trigger.hpp"
+#include "trigger_actions.hpp"
#include "utils/dbus_mapper.hpp"
TriggerFactory::TriggerFactory(
std::shared_ptr<sdbusplus::asio::connection> bus,
std::shared_ptr<sdbusplus::asio::object_server> objServer,
- SensorCache& sensorCache) :
+ SensorCache& sensorCache, interfaces::ReportManager& reportManager) :
bus(std::move(bus)),
- objServer(std::move(objServer)), sensorCache(sensorCache)
+ objServer(std::move(objServer)), sensorCache(sensorCache),
+ reportManager(reportManager)
{}
std::unique_ptr<interfaces::Trigger> TriggerFactory::make(
@@ -35,11 +37,26 @@
for (const auto& [type, dwellTime, direction, value] : params)
{
std::vector<std::unique_ptr<interfaces::TriggerAction>> actions;
+ if (logToJournal)
+ {
+ actions.emplace_back(std::make_unique<action::LogToJournal>(
+ numeric::toType(type), value));
+ }
+ if (logToRedfish)
+ {
+ actions.emplace_back(std::make_unique<action::LogToRedfish>(
+ numeric::toType(type), value));
+ }
+ if (updateReport)
+ {
+ actions.emplace_back(std::make_unique<action::UpdateReport>(
+ reportManager, reportNames));
+ }
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));
+ numeric::toDirection(direction), value));
}
return std::make_unique<Trigger>(
diff --git a/src/trigger_factory.hpp b/src/trigger_factory.hpp
index 6d8f4f4..c1f2473 100644
--- a/src/trigger_factory.hpp
+++ b/src/trigger_factory.hpp
@@ -1,5 +1,6 @@
#pragma once
+#include "interfaces/report_manager.hpp"
#include "interfaces/sensor.hpp"
#include "interfaces/trigger_factory.hpp"
#include "sensor_cache.hpp"
@@ -11,7 +12,8 @@
public:
TriggerFactory(std::shared_ptr<sdbusplus::asio::connection> bus,
std::shared_ptr<sdbusplus::asio::object_server> objServer,
- SensorCache& sensorCache);
+ SensorCache& sensorCache,
+ interfaces::ReportManager& reportManager);
std::unique_ptr<interfaces::Trigger> make(
boost::asio::yield_context& yield, const std::string& name,
@@ -27,6 +29,7 @@
std::shared_ptr<sdbusplus::asio::connection> bus;
std::shared_ptr<sdbusplus::asio::object_server> objServer;
SensorCache& sensorCache;
+ interfaces::ReportManager& reportManager;
std::pair<std::vector<std::shared_ptr<interfaces::Sensor>>,
std::vector<std::string>>
diff --git a/src/trigger_manager.hpp b/src/trigger_manager.hpp
index b5b4a22..41257eb 100644
--- a/src/trigger_manager.hpp
+++ b/src/trigger_manager.hpp
@@ -1,5 +1,6 @@
#pragma once
+#include "interfaces/report_manager.hpp"
#include "interfaces/trigger_factory.hpp"
#include "interfaces/trigger_manager.hpp"
diff --git a/src/utils/conversion.hpp b/src/utils/conversion.hpp
new file mode 100644
index 0000000..7db4be8
--- /dev/null
+++ b/src/utils/conversion.hpp
@@ -0,0 +1,18 @@
+#pragma once
+
+#include <stdexcept>
+
+namespace utils
+{
+
+template <class T, T first, T last>
+inline T toEnum(int x)
+{
+ if (x < static_cast<decltype(x)>(first) ||
+ x > static_cast<decltype(x)>(last))
+ {
+ throw std::out_of_range("Value is not in range of enum");
+ }
+ return static_cast<T>(x);
+}
+} // namespace utils
diff --git a/tests/meson.build b/tests/meson.build
index ee57387..393f73f 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -32,11 +32,13 @@
'../src/sensor.cpp',
'../src/sensor_cache.cpp',
'../src/trigger.cpp',
+ '../src/trigger_actions.cpp',
'../src/trigger_factory.cpp',
'../src/trigger_manager.cpp',
'src/dbus_environment.cpp',
'src/main.cpp',
'src/stubs/dbus_sensor_object.cpp',
+ 'src/test_conversion.cpp',
'src/test_detached_timer.cpp',
'src/test_metric.cpp',
'src/test_numeric_threshold.cpp',
@@ -47,6 +49,7 @@
'src/test_sensor_cache.cpp',
'src/test_transform.cpp',
'src/test_trigger.cpp',
+ 'src/test_trigger_actions.cpp',
'src/test_trigger_manager.cpp',
'src/test_unique_call.cpp',
'src/utils/generate_unique_mock_id.cpp',
diff --git a/tests/src/mocks/report_manager_mock.hpp b/tests/src/mocks/report_manager_mock.hpp
index 872a6e6..df79e12 100644
--- a/tests/src/mocks/report_manager_mock.hpp
+++ b/tests/src/mocks/report_manager_mock.hpp
@@ -8,4 +8,5 @@
{
public:
MOCK_METHOD(void, removeReport, (const interfaces::Report*), (override));
+ MOCK_METHOD(void, updateReport, (const std::string& name), (override));
};
diff --git a/tests/src/mocks/report_mock.hpp b/tests/src/mocks/report_mock.hpp
index f3eafba..726d53c 100644
--- a/tests/src/mocks/report_mock.hpp
+++ b/tests/src/mocks/report_mock.hpp
@@ -23,5 +23,6 @@
MOCK_METHOD(std::string, getName, (), (override, const));
MOCK_METHOD(std::string, getPath, (), (override, const));
+ MOCK_METHOD(void, updateReadings, (), (override));
MOCK_METHOD(void, Die, ());
};
diff --git a/tests/src/test_conversion.cpp b/tests/src/test_conversion.cpp
new file mode 100644
index 0000000..e3984ca
--- /dev/null
+++ b/tests/src/test_conversion.cpp
@@ -0,0 +1,33 @@
+#include "utils/conversion.hpp"
+
+#include <gmock/gmock.h>
+
+using namespace testing;
+
+class TestEnum : public Test
+{
+ public:
+ enum class Enum
+ {
+ zero = 0,
+ one,
+ two
+ };
+
+ Enum toEnum(int x)
+ {
+ return utils::toEnum<Enum, Enum::zero, Enum::two>(x);
+ }
+};
+
+TEST_F(TestEnum, passValueInRangeExpectToGetValidOutput)
+{
+ EXPECT_EQ(toEnum(0), Enum::zero);
+ EXPECT_EQ(toEnum(2), Enum::two);
+}
+
+TEST_F(TestEnum, passInvalidValueExpectToThrowOutOfRangeException)
+{
+ EXPECT_THROW(toEnum(-1), std::out_of_range);
+ EXPECT_THROW(toEnum(3), std::out_of_range);
+}
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index 8f2228e..f6c4ee7 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -375,6 +375,18 @@
Eq(GetParam().reportingType()));
}
+TEST_P(TestReportAllReportTypes, updateReadingsCallUpdateReadingsProperty)
+{
+ const uint64_t expectedTime = std::time(0);
+
+ sut->updateReadings();
+
+ const auto [timestamp, readings] =
+ getProperty<Readings>(sut->getPath(), "Readings");
+
+ EXPECT_THAT(timestamp, Ge(expectedTime));
+}
+
class TestReportOnRequestType : public TestReport
{
void SetUp() override
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index ac947c4..30628d6 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -226,6 +226,26 @@
checkPoint.Call("end");
}
+TEST_F(TestReportManager, updateReportCallsUpdateReadingsForExistReport)
+{
+ reportFactoryMock.expectMake(_, reportParams, Ref(*sut), Ref(storageMock))
+ .WillOnce(Return(ByMove(std::move(reportMockPtr))));
+ EXPECT_CALL(reportMock, updateReadings());
+
+ addReport(reportParams);
+ sut->updateReport(reportParams.reportName());
+}
+
+TEST_F(TestReportManager, updateReportDoNothingIfReportDoesNotExist)
+{
+ reportFactoryMock.expectMake(_, reportParams, Ref(*sut), Ref(storageMock))
+ .WillOnce(Return(ByMove(std::move(reportMockPtr))));
+ EXPECT_CALL(reportMock, updateReadings()).Times(0);
+
+ addReport(reportParams);
+ sut->updateReport("NotAReport");
+}
+
class TestReportManagerStorage : public TestReportManager
{
public:
diff --git a/tests/src/test_trigger_actions.cpp b/tests/src/test_trigger_actions.cpp
new file mode 100644
index 0000000..2fa220a
--- /dev/null
+++ b/tests/src/test_trigger_actions.cpp
@@ -0,0 +1,121 @@
+#include "mocks/report_manager_mock.hpp"
+#include "trigger_actions.hpp"
+
+#include <stdexcept>
+
+using namespace testing;
+
+namespace action
+{
+
+using LogParam = std::pair<numeric::Type, double>;
+
+class TestLogToJournal : public Test, public WithParamInterface<LogParam>
+{
+ public:
+ void SetUp() override
+ {
+ auto [type, threshold] = GetParam();
+ sut = std::make_unique<LogToJournal>(type, threshold);
+ }
+
+ std::unique_ptr<LogToJournal> sut;
+};
+
+INSTANTIATE_TEST_SUITE_P(
+ LogToJournalParams, TestLogToJournal,
+ Values(std::make_pair(numeric::Type::upperCritical, 91.1),
+ std::make_pair(numeric::Type::lowerCritical, 91.2),
+ std::make_pair(numeric::Type::upperWarning, 88.5),
+ std::make_pair(numeric::Type::lowerWarning, 88.6)));
+
+TEST_P(TestLogToJournal, commitAnActionDoesNotThrow)
+{
+ EXPECT_NO_THROW(sut->commit("Test", 100'000, 90.0));
+}
+
+class TestLogToJournalThrow : public TestLogToJournal
+{};
+
+INSTANTIATE_TEST_SUITE_P(
+ _, TestLogToJournalThrow,
+ Values(std::make_pair(numeric::Type::upperCritical, 90.0),
+ std::make_pair(static_cast<numeric::Type>(-1), 88.0),
+ std::make_pair(static_cast<numeric::Type>(123), 123.0)));
+
+TEST_P(TestLogToJournalThrow, commitAnActionExpectThrow)
+{
+ EXPECT_THROW(sut->commit("Test", 100'000, 90.0), std::runtime_error);
+}
+
+class TestLogToRedfish : public Test, public WithParamInterface<LogParam>
+{
+ public:
+ void SetUp() override
+ {
+ auto [type, threshold] = GetParam();
+ sut = std::make_unique<LogToRedfish>(type, threshold);
+ }
+
+ std::unique_ptr<LogToRedfish> sut;
+};
+
+INSTANTIATE_TEST_SUITE_P(
+ LogToRedfishParams, TestLogToRedfish,
+ Values(std::make_pair(numeric::Type::upperCritical, 91.1),
+ std::make_pair(numeric::Type::lowerCritical, 91.4),
+ std::make_pair(numeric::Type::upperWarning, 88.6),
+ std::make_pair(numeric::Type::lowerWarning, 88.5)));
+
+TEST_P(TestLogToRedfish, commitExpectNoThrow)
+{
+ EXPECT_NO_THROW(sut->commit("Test", 100'000, 90.0));
+}
+
+class TestLogToRedfishThrow : public TestLogToRedfish
+{};
+
+INSTANTIATE_TEST_SUITE_P(
+ _, TestLogToRedfishThrow,
+ Values(std::make_pair(numeric::Type::upperCritical, 90.0),
+ std::make_pair(static_cast<numeric::Type>(-1), 88.5),
+ std::make_pair(static_cast<numeric::Type>(123), 123.6)));
+
+TEST_P(TestLogToRedfishThrow, commitExpectToThrow)
+{
+ EXPECT_THROW(sut->commit("Test", 100'000, 90.0), std::runtime_error);
+}
+
+class TestUpdateReport : public Test
+{
+ public:
+ void make(std::vector<std::string> names)
+ {
+ sut = std::make_unique<UpdateReport>(reportManager, std::move(names));
+ }
+
+ NiceMock<ReportManagerMock> reportManager;
+ std::unique_ptr<UpdateReport> sut;
+};
+
+TEST_F(TestUpdateReport, commitWhenReportNameIsEmptyExpectNoReportUpdate)
+{
+ EXPECT_CALL(reportManager, updateReport(_)).Times(0);
+
+ make({});
+ sut->commit("Test", 100'000, 90.0);
+}
+
+TEST_F(TestUpdateReport, commitExpectReportUpdate)
+{
+ std::vector<std::string> names = {"Report1", "Report2", "Report3"};
+ for (const auto& name : names)
+ {
+ EXPECT_CALL(reportManager, updateReport(name));
+ }
+
+ make(names);
+ sut->commit("Test", 100'000, 90.0);
+}
+
+} // namespace action