Created metric class
Metric collects updates from sensor. Report displays metric readings
depending on reportingType.
Tested:
- Added new units tests for Metric class
- All other unit tests are passing
Change-Id: I19f4831fab163a4f9540cef7bb23e903ae90fddf
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
diff --git a/src/interfaces/metric.hpp b/src/interfaces/metric.hpp
index 50bf5d5..d63c979 100644
--- a/src/interfaces/metric.hpp
+++ b/src/interfaces/metric.hpp
@@ -14,6 +14,7 @@
public:
virtual ~Metric() = default;
+ virtual void initialize() = 0;
virtual const std::vector<MetricValue>& getReadings() const = 0;
virtual nlohmann::json to_json() const = 0;
};
diff --git a/src/metric.cpp b/src/metric.cpp
index b40da5a..8d9f19b 100644
--- a/src/metric.cpp
+++ b/src/metric.cpp
@@ -1 +1,64 @@
#include "metric.hpp"
+
+#include "interfaces/types.hpp"
+#include "utils/transform.hpp"
+
+#include <algorithm>
+
+Metric::Metric(std::vector<std::shared_ptr<interfaces::Sensor>> sensors,
+ std::string operationType, std::string id,
+ std::string metadata) :
+ sensors(std::move(sensors)),
+ operationType(std::move(operationType)), id(std::move(id)),
+ metadata(std::move(metadata))
+{}
+
+void Metric::initialize()
+{
+ readings = std::vector<MetricValue>(sensors.size(),
+ MetricValue{id, metadata, 0., 0u});
+
+ for (auto& sensor : sensors)
+ {
+ sensor->registerForUpdates(weak_from_this());
+ }
+}
+
+const std::vector<MetricValue>& Metric::getReadings() const
+{
+ return readings;
+}
+
+void Metric::sensorUpdated(interfaces::Sensor& sensor, uint64_t timestamp)
+{
+ MetricValue& mv = findMetric(sensor);
+ mv.timestamp = timestamp;
+}
+
+void Metric::sensorUpdated(interfaces::Sensor& sensor, uint64_t timestamp,
+ double value)
+{
+ MetricValue& mv = findMetric(sensor);
+ mv.timestamp = timestamp;
+ mv.value = value;
+}
+
+MetricValue& Metric::findMetric(interfaces::Sensor& sensor)
+{
+ auto it =
+ std::find_if(sensors.begin(), sensors.end(),
+ [&sensor](const auto& s) { return s.get() == &sensor; });
+ auto index = std::distance(sensors.begin(), it);
+ return readings.at(index);
+}
+
+nlohmann::json Metric::to_json() const
+{
+ auto sensorPaths = utils::transform(
+ sensors, [](const auto& sensor) -> sdbusplus::message::object_path {
+ return sdbusplus::message::object_path(sensor->id().service + ":" +
+ sensor->id().path);
+ });
+ return LabeledReadingParameter::to_json(ReadingParameters::value_type(
+ std::move(sensorPaths), operationType, id, metadata));
+}
diff --git a/src/metric.hpp b/src/metric.hpp
index 76a60fb..6e27446 100644
--- a/src/metric.hpp
+++ b/src/metric.hpp
@@ -1,21 +1,30 @@
#pragma once
#include "interfaces/metric.hpp"
+#include "interfaces/sensor.hpp"
#include "interfaces/sensor_listener.hpp"
-class Metric : public interfaces::Metric, public interfaces::SensorListener
+class Metric :
+ public interfaces::Metric,
+ public interfaces::SensorListener,
+ public std::enable_shared_from_this<Metric>
{
public:
- const std::vector<MetricValue>& getReadings() const override
- {
- return readings;
- }
+ Metric(std::vector<std::shared_ptr<interfaces::Sensor>> sensors,
+ std::string operationType, std::string id, std::string metadata);
- void sensorUpdated(interfaces::Sensor&, uint64_t) override
- {}
- void sensorUpdated(interfaces::Sensor&, uint64_t, double value) override
- {}
+ void initialize() override;
+ const std::vector<MetricValue>& getReadings() const override;
+ void sensorUpdated(interfaces::Sensor&, uint64_t) override;
+ void sensorUpdated(interfaces::Sensor&, uint64_t, double value) override;
+ nlohmann::json to_json() const override;
private:
+ MetricValue& findMetric(interfaces::Sensor&);
+
+ std::vector<std::shared_ptr<interfaces::Sensor>> sensors;
+ std::string operationType;
+ std::string id;
+ std::string metadata;
std::vector<MetricValue> readings;
};
diff --git a/src/report.cpp b/src/report.cpp
index 6698ec3..c1b7d5f 100644
--- a/src/report.cpp
+++ b/src/report.cpp
@@ -27,6 +27,11 @@
fileName(std::to_string(std::hash<std::string>{}(name))),
reportStorage(reportStorageIn)
{
+ for (auto& metric : this->metrics)
+ {
+ metric->initialize();
+ }
+
deleteIface = objServer->add_unique_interface(
path, deleteIfaceName, [this, &ioc, &reportManager](auto& dbusIface) {
dbusIface.register_method("Delete", [this, &ioc, &reportManager] {
@@ -137,7 +142,7 @@
return sum + metric->getReadings().size();
});
- readingsCache.resize(numElements);
+ std::tuple_element_t<1, Readings> readingsCache(numElements);
auto it = readingsCache.begin();
@@ -151,7 +156,7 @@
}
std::get<0>(readings) = std::time(0);
- std::get<1>(readings) = readingsCache;
+ std::get<1>(readings) = std::move(readingsCache);
reportIface->signal_property("Readings");
}
diff --git a/src/report.hpp b/src/report.hpp
index 2020f99..b227392 100644
--- a/src/report.hpp
+++ b/src/report.hpp
@@ -59,7 +59,6 @@
ReadingParameters readingParameters;
bool persistency;
Readings readings = {};
- std::tuple_element_t<1, Readings> readingsCache = {};
std::shared_ptr<sdbusplus::asio::object_server> objServer;
std::unique_ptr<sdbusplus::asio::dbus_interface> reportIface;
std::unique_ptr<sdbusplus::asio::dbus_interface> deleteIface;
diff --git a/src/report_factory.cpp b/src/report_factory.cpp
index 5ac32f3..e4689aa 100644
--- a/src/report_factory.cpp
+++ b/src/report_factory.cpp
@@ -1,5 +1,6 @@
#include "report_factory.hpp"
+#include "metric.hpp"
#include "report.hpp"
#include "sensor.hpp"
#include "utils/transform.hpp"
@@ -19,10 +20,84 @@
interfaces::ReportManager& reportManager,
interfaces::JsonStorage& reportStorage) const
{
+ std::optional<std::vector<ReportFactory::SensorTree>> sensorTree;
+
std::vector<std::shared_ptr<interfaces::Metric>> metrics;
+ metrics.reserve(metricParams.size());
+
+ for (const auto& [sensorPaths, op, id, metadata] : metricParams)
+ {
+ if (!sensorTree && yield && sensorPaths.size() > 0)
+ {
+ sensorTree = getSensorTree(*yield);
+ }
+
+ std::vector<std::shared_ptr<interfaces::Sensor>> sensors =
+ getSensors(sensorTree, sensorPaths);
+
+ metrics.emplace_back(
+ std::make_shared<Metric>(std::move(sensors), op, id, metadata));
+ }
return std::make_unique<Report>(
bus->get_io_context(), objServer, name, reportingType,
emitsReadingsSignal, logToMetricReportsCollection, period, metricParams,
reportManager, reportStorage, std::move(metrics));
}
+
+std::vector<std::shared_ptr<interfaces::Sensor>> ReportFactory::getSensors(
+ const std::optional<std::vector<ReportFactory::SensorTree>>& tree,
+ const std::vector<sdbusplus::message::object_path>& sensorPaths) const
+{
+ if (tree)
+ {
+ std::vector<std::shared_ptr<interfaces::Sensor>> sensors;
+
+ for (const auto& [sensor, ifacesMap] : *tree)
+ {
+ auto it = std::find(sensorPaths.begin(), sensorPaths.end(), sensor);
+ if (it != sensorPaths.end())
+ {
+ for (const auto& [service, ifaces] : ifacesMap)
+ {
+ sensors.emplace_back(sensorCache.makeSensor<Sensor>(
+ service, sensor, bus->get_io_context(), bus));
+ }
+ }
+ }
+
+ return sensors;
+ }
+ else
+ {
+ return utils::transform(
+ sensorPaths,
+ [this](const std::string& sensor)
+ -> std::shared_ptr<interfaces::Sensor> {
+ std::string::size_type pos = sensor.find_first_of(":");
+ auto service = sensor.substr(0, pos);
+ auto path = sensor.substr(pos + 1);
+ return sensorCache.makeSensor<Sensor>(
+ service, path, bus->get_io_context(), bus);
+ });
+ }
+}
+
+std::vector<ReportFactory::SensorTree>
+ ReportFactory::getSensorTree(boost::asio::yield_context& yield) const
+{
+ std::array<const char*, 1> interfaces = {
+ "xyz.openbmc_project.Sensor.Value"};
+ boost::system::error_code ec;
+
+ auto result = 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");
+ }
+ return result;
+}
diff --git a/src/report_factory.hpp b/src/report_factory.hpp
index bdf9268..0346441 100644
--- a/src/report_factory.hpp
+++ b/src/report_factory.hpp
@@ -2,6 +2,7 @@
#include "interfaces/report_factory.hpp"
#include "interfaces/sensor.hpp"
+#include "sensor_cache.hpp"
#include <boost/asio/io_context.hpp>
#include <sdbusplus/asio/object_server.hpp>
@@ -22,6 +23,19 @@
interfaces::JsonStorage& reportStorage) 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::optional<std::vector<SensorTree>>& tree,
+ const std::vector<sdbusplus::message::object_path>& sensorPaths) const;
+ std::vector<SensorTree>
+ getSensorTree(boost::asio::yield_context& yield) const;
+
std::shared_ptr<sdbusplus::asio::connection> bus;
std::shared_ptr<sdbusplus::asio::object_server> objServer;
+ mutable SensorCache sensorCache;
};
diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index d23b589..eb2d767 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -16,6 +16,7 @@
reportStorage(std::move(reportStorageIn)), objServer(objServerIn)
{
reports.reserve(maxReports);
+
loadFromPersistent();
reportManagerIface = objServer->add_unique_interface(
diff --git a/src/telemetry.hpp b/src/telemetry.hpp
index 4a6a6b0..4cc138e 100644
--- a/src/telemetry.hpp
+++ b/src/telemetry.hpp
@@ -3,6 +3,7 @@
#include "persistent_json_storage.hpp"
#include "report_factory.hpp"
#include "report_manager.hpp"
+#include "sensor_cache.hpp"
#include <sdbusplus/asio/connection.hpp>
#include <sdbusplus/asio/object_server.hpp>
diff --git a/tests/meson.build b/tests/meson.build
index 2bf9dfb..54d79e2 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -53,5 +53,6 @@
sdbusplus,
],
include_directories: ['../src', 'src']
- )
+ ),
+ timeout: 120,
)
diff --git a/tests/src/helpers/metric_value_helpers.hpp b/tests/src/helpers/metric_value_helpers.hpp
new file mode 100644
index 0000000..f21d42a
--- /dev/null
+++ b/tests/src/helpers/metric_value_helpers.hpp
@@ -0,0 +1,17 @@
+#pragma once
+
+#include "metric_value.hpp"
+
+#include <gmock/gmock.h>
+
+inline void PrintTo(const MetricValue& o, std::ostream* os)
+{
+ (*os) << "{ id: " << o.id << ", metadata: " << o.metadata
+ << ", value: " << o.value << ", timestamp: " << o.timestamp << " }";
+}
+
+inline bool operator==(const MetricValue& left, const MetricValue& right)
+{
+ return std::tie(left.id, left.metadata, left.value, left.timestamp) ==
+ std::tie(right.id, right.metadata, right.value, right.timestamp);
+}
\ No newline at end of file
diff --git a/tests/src/helpers/sensor_id_helpers.hpp b/tests/src/helpers/sensor_id_helpers.hpp
new file mode 100644
index 0000000..9b59d22
--- /dev/null
+++ b/tests/src/helpers/sensor_id_helpers.hpp
@@ -0,0 +1,22 @@
+#pragma once
+
+#include "interfaces/sensor.hpp"
+
+#include <gmock/gmock.h>
+
+namespace interfaces
+{
+
+inline void PrintTo(const Sensor::Id& o, std::ostream* os)
+{
+ (*os) << "{ type: " << o.type << ", service: " << o.service
+ << ", path: " << o.path << " }";
+}
+
+inline bool operator==(const Sensor::Id& left, const Sensor::Id& right)
+{
+ return std::tie(left.type, left.service, left.path) ==
+ std::tie(right.type, right.service, right.path);
+}
+
+} // namespace interfaces
\ No newline at end of file
diff --git a/tests/src/mocks/metric_mock.hpp b/tests/src/mocks/metric_mock.hpp
index d3d9e64..21f7733 100644
--- a/tests/src/mocks/metric_mock.hpp
+++ b/tests/src/mocks/metric_mock.hpp
@@ -15,6 +15,7 @@
.WillByDefault(ReturnRefOfCopy(std::vector<MetricValue>()));
}
+ MOCK_METHOD(void, initialize, (), (override));
MOCK_METHOD(const std::vector<MetricValue>&, getReadings, (),
(const, override));
MOCK_METHOD(nlohmann::json, to_json, (), (const, override));
diff --git a/tests/src/mocks/sensor_mock.hpp b/tests/src/mocks/sensor_mock.hpp
index 0dcb2b5..d71d77e 100644
--- a/tests/src/mocks/sensor_mock.hpp
+++ b/tests/src/mocks/sensor_mock.hpp
@@ -8,11 +8,14 @@
class SensorMock : public interfaces::Sensor
{
public:
+ explicit SensorMock()
+ {
+ initialize();
+ }
+
explicit SensorMock(Id sensorId) : mockSensorId(sensorId)
{
- ON_CALL(*this, id()).WillByDefault(testing::Invoke([this] {
- return this->mockSensorId;
- }));
+ initialize();
}
static Id makeId(std::string_view service, std::string_view path)
@@ -27,4 +30,12 @@
const uint64_t mockId = generateUniqueMockId();
Id mockSensorId = Id("SensorMock", "", "");
+
+ private:
+ void initialize()
+ {
+ ON_CALL(*this, id()).WillByDefault(testing::Invoke([this] {
+ return this->mockSensorId;
+ }));
+ }
};
diff --git a/tests/src/test_metric.cpp b/tests/src/test_metric.cpp
index b40da5a..ff3d120 100644
--- a/tests/src/test_metric.cpp
+++ b/tests/src/test_metric.cpp
@@ -1 +1,115 @@
+#include "helpers/metric_value_helpers.hpp"
+#include "interfaces/sensor.hpp"
#include "metric.hpp"
+#include "mocks/sensor_mock.hpp"
+#include "printers.hpp"
+#include "utils/conv_container.hpp"
+
+#include <gmock/gmock.h>
+
+using namespace testing;
+
+using Timestamp = uint64_t;
+
+class TestMetric : public Test
+{
+ public:
+ std::vector<std::shared_ptr<SensorMock>> sensorMocks = {
+ std::make_shared<NiceMock<SensorMock>>(),
+ std::make_shared<NiceMock<SensorMock>>(),
+ std::make_shared<NiceMock<SensorMock>>()};
+
+ std::shared_ptr<Metric> sut = std::make_shared<Metric>(
+ utils::convContainer<std::shared_ptr<interfaces::Sensor>>(sensorMocks),
+ "op", "id", "metadata");
+};
+
+TEST_F(TestMetric, subscribesForSensorDuringInitialization)
+{
+ for (auto& sensor : sensorMocks)
+ {
+ EXPECT_CALL(*sensor,
+ registerForUpdates(Truly([sut = sut.get()](const auto& a0) {
+ return a0.lock().get() == sut;
+ })));
+ }
+
+ sut->initialize();
+}
+
+TEST_F(TestMetric, containsNoReadingsWhenNotInitialized)
+{
+ ASSERT_THAT(sut->getReadings(), ElementsAre());
+}
+
+TEST_F(TestMetric, containsEmptyReadingsAfterInitialize)
+{
+ sut->initialize();
+
+ ASSERT_THAT(sut->getReadings(),
+ ElementsAre(MetricValue{"id", "metadata", 0., 0u},
+ MetricValue{"id", "metadata", 0., 0u},
+ MetricValue{"id", "metadata", 0., 0u}));
+}
+
+TEST_F(TestMetric, throwsWhenUpdateIsPerformedWhenNotInitialized)
+{
+ EXPECT_THROW(sut->sensorUpdated(*sensorMocks[0], Timestamp{10}),
+ std::out_of_range);
+ EXPECT_THROW(sut->sensorUpdated(*sensorMocks[1], Timestamp{10}, 20.0),
+ std::out_of_range);
+}
+
+TEST_F(TestMetric, updatesMetricValuesOnSensorUpdate)
+{
+ sut->initialize();
+
+ sut->sensorUpdated(*sensorMocks[2], Timestamp{18}, 31.0);
+ sut->sensorUpdated(*sensorMocks[0], Timestamp{21});
+
+ ASSERT_THAT(sut->getReadings(),
+ ElementsAre(MetricValue{"id", "metadata", 0., 21u},
+ MetricValue{"id", "metadata", 0., 0u},
+ MetricValue{"id", "metadata", 31., 18u}));
+}
+
+TEST_F(TestMetric, throwsWhenUpdateIsPerformedOnUnknownSensor)
+{
+ sut->initialize();
+
+ auto sensor = std::make_shared<StrictMock<SensorMock>>();
+ EXPECT_THROW(sut->sensorUpdated(*sensor, Timestamp{10}), std::out_of_range);
+ EXPECT_THROW(sut->sensorUpdated(*sensor, Timestamp{10}, 20.0),
+ std::out_of_range);
+}
+
+TEST_F(TestMetric, containsIdInJsonDump)
+{
+ ASSERT_THAT(sut->to_json().at("id"), Eq(nlohmann::json("id")));
+}
+
+TEST_F(TestMetric, containsOpInJsonDump)
+{
+ ASSERT_THAT(sut->to_json().at("operationType"), Eq(nlohmann::json("op")));
+}
+
+TEST_F(TestMetric, containsMetadataInJsonDump)
+{
+ ASSERT_THAT(sut->to_json().at("metricMetadata"),
+ Eq(nlohmann::json("metadata")));
+}
+
+TEST_F(TestMetric, containsSensorPathsInJsonDump)
+{
+ for (size_t i = 0; i < sensorMocks.size(); ++i)
+ {
+ const auto no = std::to_string(i);
+ ON_CALL(*sensorMocks[i], id())
+ .WillByDefault(
+ Return(SensorMock::makeId("service" + no, "path" + no)));
+ }
+
+ ASSERT_THAT(sut->to_json().at("sensorPaths"),
+ Eq(nlohmann::json(
+ {"service0:path0", "service1:path1", "service2:path2"})));
+}
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index 40e85da..ca556b0 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -477,6 +477,16 @@
MockFunction<void()> readingsUpdated;
};
+TEST_F(TestReportInitialization, metricsAreInitializedWhenConstructed)
+{
+ for (auto& metric : metricMocks)
+ {
+ EXPECT_CALL(*metric, initialize());
+ }
+
+ sut = makeReport(ReportParams());
+}
+
TEST_F(TestReportInitialization, readingsPropertiesChangedSingalEmits)
{
sut = makeReport(defaultParams.reportingType("Periodic"));
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index 813fa19..02bf3ec 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -2,6 +2,7 @@
#include "mocks/json_storage_mock.hpp"
#include "mocks/report_factory_mock.hpp"
#include "params/report_params.hpp"
+#include "printers.hpp"
#include "report.hpp"
#include "report_manager.hpp"
#include "utils/transform.hpp"
@@ -214,7 +215,7 @@
ON_CALL(storageMock, list())
.WillByDefault(Return(std::vector<FilePath>{FilePath("report1")}));
ON_CALL(storageMock, load(FilePath("report1")))
- .WillByDefault(Return(data));
+ .WillByDefault(InvokeWithoutArgs([this] { return data; }));
}
void makeReportManager()
@@ -256,7 +257,6 @@
{
data["Version"] = Report::reportVersion - 1;
- ON_CALL(storageMock, load(FilePath("report1"))).WillByDefault(Return(data));
EXPECT_CALL(storageMock, remove(FilePath("report1")));
makeReportManager();
@@ -267,7 +267,6 @@
{
data["Interval"] = "1000";
- ON_CALL(storageMock, load(FilePath("report1"))).WillByDefault(Return(data));
EXPECT_CALL(storageMock, remove(FilePath("report1")));
makeReportManager();
diff --git a/tests/src/test_sensor.cpp b/tests/src/test_sensor.cpp
index 83151b1..1398738 100644
--- a/tests/src/test_sensor.cpp
+++ b/tests/src/test_sensor.cpp
@@ -1,10 +1,10 @@
#include "dbus_environment.hpp"
+#include "helpers/sensor_id_helpers.hpp"
#include "mocks/sensor_listener_mock.hpp"
#include "printers.hpp"
#include "sensor.hpp"
#include "sensor_cache.hpp"
#include "stubs/dbus_sensor_object.hpp"
-#include "utils/sensor_id_eq.hpp"
#include <sdbusplus/asio/property.hpp>
@@ -56,8 +56,8 @@
TEST_F(TestSensor, createsCorretlyViaSensorCache)
{
ASSERT_THAT(sut->id(),
- sensorIdEq(Sensor::Id("Sensor", DbusEnvironment::serviceName(),
- sensorObject.path())));
+ Eq(Sensor::Id("Sensor", DbusEnvironment::serviceName(),
+ sensorObject.path())));
}
TEST_F(TestSensor, notifiesWithValueAfterRegister)
diff --git a/tests/src/test_sensor_cache.cpp b/tests/src/test_sensor_cache.cpp
index cf31add..0db3bc2 100644
--- a/tests/src/test_sensor_cache.cpp
+++ b/tests/src/test_sensor_cache.cpp
@@ -1,7 +1,7 @@
+#include "helpers/sensor_id_helpers.hpp"
#include "mocks/sensor_mock.hpp"
#include "printers.hpp"
#include "sensor_cache.hpp"
-#include "utils/sensor_id_eq.hpp"
#include <initializer_list>
@@ -100,5 +100,5 @@
auto expected = SensorMock::makeId("sensor-service", "sensor-path");
- ASSERT_THAT(id, sensorIdEq(expected));
+ ASSERT_THAT(id, Eq(expected));
}
diff --git a/tests/src/utils/sensor_id_eq.hpp b/tests/src/utils/sensor_id_eq.hpp
deleted file mode 100644
index f952504..0000000
--- a/tests/src/utils/sensor_id_eq.hpp
+++ /dev/null
@@ -1,14 +0,0 @@
-#pragma once
-
-#include "interfaces/sensor.hpp"
-
-#include <gmock/gmock.h>
-
-inline auto sensorIdEq(interfaces::Sensor::Id id)
-{
- using namespace testing;
-
- return AllOf(Field(&interfaces::Sensor::Id::type, Eq(id.type)),
- Field(&interfaces::Sensor::Id::service, Eq(id.service)),
- Field(&interfaces::Sensor::Id::path, Eq(id.path)));
-}