Changed dbus add report interface
- metric parameters now take single sensor instead of list
- added interface support for new operation types
Tested:
- All telemetry tests are passing.
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: Id3a41c48e81a287e7d205ae1c747daa36d4cdb29
diff --git a/src/interfaces/metric.hpp b/src/interfaces/metric.hpp
index 5a960a4..deb3ed6 100644
--- a/src/interfaces/metric.hpp
+++ b/src/interfaces/metric.hpp
@@ -16,7 +16,7 @@
virtual ~Metric() = default;
virtual void initialize() = 0;
- virtual const std::vector<MetricValue>& getReadings() const = 0;
+ virtual const MetricValue& getReading() const = 0;
virtual LabeledMetricParameters dumpConfiguration() const = 0;
};
diff --git a/src/interfaces/types.hpp b/src/interfaces/types.hpp
index 3cc069e..a5ed0db 100644
--- a/src/interfaces/types.hpp
+++ b/src/interfaces/types.hpp
@@ -1,5 +1,6 @@
#pragma once
+#include "operation_type.hpp"
#include "utils/labeled_tuple.hpp"
#include "utils/tstring.hpp"
@@ -11,17 +12,17 @@
#include <vector>
using ReadingParameters =
- std::vector<std::tuple<std::vector<sdbusplus::message::object_path>,
- std::string, std::string, std::string>>;
+ std::vector<std::tuple<sdbusplus::message::object_path, std::string,
+ std::string, std::string>>;
using LabeledSensorParameters =
utils::LabeledTuple<std::tuple<std::string, std::string>,
utils::tstring::Service, utils::tstring::Path>;
using LabeledMetricParameters =
- utils::LabeledTuple<std::tuple<std::vector<LabeledSensorParameters>,
- std::string, std::string, std::string>,
- utils::tstring::SensorPaths,
+ utils::LabeledTuple<std::tuple<LabeledSensorParameters, OperationType,
+ std::string, std::string>,
+ utils::tstring::SensorPath,
utils::tstring::OperationType, utils::tstring::Id,
utils::tstring::MetricMetadata>;
diff --git a/src/metric.cpp b/src/metric.cpp
index b12f3a6..2a73536 100644
--- a/src/metric.cpp
+++ b/src/metric.cpp
@@ -5,58 +5,52 @@
#include <algorithm>
-Metric::Metric(std::vector<std::shared_ptr<interfaces::Sensor>> sensors,
- std::string operationType, std::string id,
+Metric::Metric(std::shared_ptr<interfaces::Sensor> sensor,
+ OperationType operationType, std::string id,
std::string metadata) :
- sensors(std::move(sensors)),
- operationType(std::move(operationType)), id(std::move(id)),
- metadata(std::move(metadata))
+ sensor(std::move(sensor)),
+ operationType(std::move(operationType)), reading{std::move(id),
+ std::move(metadata), 0.,
+ 0u}
{}
void Metric::initialize()
{
- readings = std::vector<MetricValue>(sensors.size(),
- MetricValue{id, metadata, 0., 0u});
-
- for (auto& sensor : sensors)
- {
- sensor->registerForUpdates(weak_from_this());
- }
+ sensor->registerForUpdates(weak_from_this());
}
-const std::vector<MetricValue>& Metric::getReadings() const
+const MetricValue& Metric::getReading() const
{
- return readings;
+ return reading;
}
-void Metric::sensorUpdated(interfaces::Sensor& sensor, uint64_t timestamp)
+void Metric::sensorUpdated(interfaces::Sensor& notifier, uint64_t timestamp)
{
- MetricValue& mv = findMetric(sensor);
+ MetricValue& mv = findMetric(notifier);
mv.timestamp = timestamp;
}
-void Metric::sensorUpdated(interfaces::Sensor& sensor, uint64_t timestamp,
+void Metric::sensorUpdated(interfaces::Sensor& notifier, uint64_t timestamp,
double value)
{
- MetricValue& mv = findMetric(sensor);
+ MetricValue& mv = findMetric(notifier);
mv.timestamp = timestamp;
mv.value = value;
}
-MetricValue& Metric::findMetric(interfaces::Sensor& sensor)
+MetricValue& Metric::findMetric(interfaces::Sensor& notifier)
{
- 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);
+ if (sensor.get() != ¬ifier)
+ {
+ throw std::out_of_range("unknown sensor");
+ }
+ return reading;
}
LabeledMetricParameters Metric::dumpConfiguration() const
{
- auto sensorPaths = utils::transform(sensors, [](const auto& sensor) {
- return LabeledSensorParameters(sensor->id().service, sensor->id().path);
- });
- return LabeledMetricParameters(std::move(sensorPaths), operationType, id,
- metadata);
+ auto sensorPath =
+ LabeledSensorParameters(sensor->id().service, sensor->id().path);
+ return LabeledMetricParameters(std::move(sensorPath), operationType,
+ reading.id, reading.metadata);
}
diff --git a/src/metric.hpp b/src/metric.hpp
index 39f525c..aab4d15 100644
--- a/src/metric.hpp
+++ b/src/metric.hpp
@@ -10,11 +10,11 @@
public std::enable_shared_from_this<Metric>
{
public:
- Metric(std::vector<std::shared_ptr<interfaces::Sensor>> sensors,
- std::string operationType, std::string id, std::string metadata);
+ Metric(std::shared_ptr<interfaces::Sensor> sensor,
+ OperationType operationType, std::string id, std::string metadata);
void initialize() override;
- const std::vector<MetricValue>& getReadings() const override;
+ const MetricValue& getReading() const override;
void sensorUpdated(interfaces::Sensor&, uint64_t) override;
void sensorUpdated(interfaces::Sensor&, uint64_t, double value) override;
LabeledMetricParameters dumpConfiguration() const override;
@@ -22,9 +22,7 @@
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;
+ std::shared_ptr<interfaces::Sensor> sensor;
+ OperationType operationType;
+ MetricValue reading;
};
diff --git a/src/operation_type.hpp b/src/operation_type.hpp
new file mode 100644
index 0000000..a4f085c
--- /dev/null
+++ b/src/operation_type.hpp
@@ -0,0 +1,51 @@
+#pragma once
+
+#include "utils/conversion.hpp"
+
+#include <array>
+#include <cstdint>
+#include <string_view>
+
+enum class OperationType : uint32_t
+{
+ single,
+ max,
+ min,
+ avg,
+ sum
+};
+
+namespace utils
+{
+
+constexpr std::array<std::pair<std::string_view, OperationType>, 5>
+ convDataOperationType = {
+ {std::make_pair<std::string_view, OperationType>("SINGLE",
+ OperationType::single),
+ std::make_pair<std::string_view, OperationType>("MAX",
+ OperationType::max),
+ std::make_pair<std::string_view, OperationType>("MIN",
+ OperationType::min),
+ std::make_pair<std::string_view, OperationType>("AVG",
+ OperationType::avg),
+ std::make_pair<std::string_view, OperationType>("SUM",
+ OperationType::sum)}};
+
+inline OperationType
+ toOperationType(std::underlying_type_t<OperationType> value)
+{
+ return toEnum<OperationType, OperationType::single, OperationType::sum>(
+ value);
+}
+
+inline OperationType stringToOperationType(const std::string& value)
+{
+ return stringToEnum(convDataOperationType, value);
+}
+
+inline std::string enumToString(OperationType value)
+{
+ return std::string(enumToString(convDataOperationType, value));
+}
+
+} // namespace utils
diff --git a/src/report.cpp b/src/report.cpp
index 59028f4..77189c1 100644
--- a/src/report.cpp
+++ b/src/report.cpp
@@ -143,23 +143,14 @@
void Report::updateReadings()
{
- auto numElements = std::accumulate(
- metrics.begin(), metrics.end(), 0u, [](auto sum, const auto& metric) {
- return sum + metric->getReadings().size();
- });
+ std::tuple_element_t<1, Readings> readingsCache(metrics.size());
- std::tuple_element_t<1, Readings> readingsCache(numElements);
-
- auto it = readingsCache.begin();
-
- for (const auto& metric : metrics)
- {
- for (const auto& reading : metric->getReadings())
- {
- *(it++) = std::make_tuple(reading.id, reading.metadata,
- reading.value, reading.timestamp);
- }
- }
+ std::transform(std::begin(metrics), std::end(metrics),
+ std::begin(readingsCache), [](const auto& metric) {
+ const auto& reading = metric->getReading();
+ return std::make_tuple(reading.id, reading.metadata,
+ reading.value, reading.timestamp);
+ });
std::get<0>(readings) = std::time(0);
std::get<1>(readings) = std::move(readingsCache);
diff --git a/src/report.hpp b/src/report.hpp
index 2f470d3..97afe03 100644
--- a/src/report.hpp
+++ b/src/report.hpp
@@ -75,5 +75,5 @@
"/xyz/openbmc_project/Telemetry/Reports/";
static constexpr const char* deleteIfaceName =
"xyz.openbmc_project.Object.Delete";
- static constexpr size_t reportVersion = 2;
+ static constexpr size_t reportVersion = 3;
};
diff --git a/src/report_factory.cpp b/src/report_factory.cpp
index 2cd5da3..9dfdbb0 100644
--- a/src/report_factory.cpp
+++ b/src/report_factory.cpp
@@ -3,6 +3,7 @@
#include "metric.hpp"
#include "report.hpp"
#include "sensor.hpp"
+#include "utils/conversion.hpp"
#include "utils/dbus_mapper.hpp"
#include "utils/transform.hpp"
@@ -41,7 +42,7 @@
[this](const LabeledMetricParameters& param)
-> std::shared_ptr<interfaces::Metric> {
return std::make_shared<Metric>(
- getSensors(param.at_index<0>()), param.at_index<1>(),
+ getSensor(param.at_index<0>()), param.at_index<1>(),
param.at_index<2>(), param.at_index<3>());
});
@@ -51,19 +52,14 @@
reportManager, reportStorage, std::move(metrics));
}
-std::vector<std::shared_ptr<interfaces::Sensor>> ReportFactory::getSensors(
- const std::vector<LabeledSensorParameters>& sensorPaths) const
+std::shared_ptr<interfaces::Sensor>
+ ReportFactory::getSensor(const LabeledSensorParameters& sensorPath) const
{
- return utils::transform(sensorPaths,
- [this](const LabeledSensorParameters& param)
- -> std::shared_ptr<interfaces::Sensor> {
- using namespace utils::tstring;
+ using namespace utils::tstring;
- return sensorCache.makeSensor<Sensor>(
- param.at_label<Service>(),
- param.at_label<Path>(),
- bus->get_io_context(), bus);
- });
+ return sensorCache.makeSensor<Sensor>(sensorPath.at_label<Service>(),
+ sensorPath.at_label<Path>(),
+ bus->get_io_context(), bus);
}
std::vector<LabeledMetricParameters> ReportFactory::convertMetricParams(
@@ -73,24 +69,22 @@
auto tree = utils::getSubTreeSensors(yield, bus);
return utils::transform(metricParams, [&tree](const auto& item) {
- std::vector<LabeledSensorParameters> sensors;
+ const auto& [sensorPath, operationType, id, metadata] = item;
- for (const auto& sensorPath : std::get<0>(item))
+ auto it = std::find_if(
+ tree.begin(), tree.end(),
+ [&sensorPath](const auto& v) { return v.first == sensorPath; });
+
+ if (it != tree.end() && it->second.size() == 1)
{
- auto it = std::find_if(
- tree.begin(), tree.end(),
- [&sensorPath](const auto& v) { return v.first == sensorPath; });
-
- if (it != tree.end())
- {
- for (const auto& [service, ifaces] : it->second)
- {
- sensors.emplace_back(service, sensorPath);
- }
- }
+ const auto& [service, ifaces] = it->second.front();
+ return LabeledMetricParameters(
+ LabeledSensorParameters(service, sensorPath),
+ utils::stringToOperationType(operationType), id, metadata);
}
- return LabeledMetricParameters(std::move(sensors), std::get<1>(item),
- std::get<2>(item), std::get<3>(item));
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument),
+ "Could not find service for provided sensors");
});
}
diff --git a/src/report_factory.hpp b/src/report_factory.hpp
index 6d90902..550eb81 100644
--- a/src/report_factory.hpp
+++ b/src/report_factory.hpp
@@ -34,8 +34,8 @@
const override;
private:
- std::vector<std::shared_ptr<interfaces::Sensor>> getSensors(
- const std::vector<LabeledSensorParameters>& sensorPaths) const;
+ std::shared_ptr<interfaces::Sensor>
+ getSensor(const LabeledSensorParameters& sensorPath) const;
std::vector<LabeledMetricParameters>
convertMetricParams(boost::asio::yield_context& yield,
const ReadingParameters& metricParams) const;
diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index 1d3e47b..9ea6026 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -2,6 +2,7 @@
#include "interfaces/types.hpp"
#include "report.hpp"
+#include "utils/conversion.hpp"
#include "utils/transform.hpp"
#include <phosphor-logging/log.hpp>
@@ -92,22 +93,26 @@
static_cast<int>(std::errc::invalid_argument), "Invalid interval");
}
- for (const auto& param : readingParams)
- {
- const auto& sensors = std::get<0>(param);
- if (sensors.size() != 1)
- {
- throw sdbusplus::exception::SdBusError(
- static_cast<int>(std::errc::not_supported),
- "Only single sensor per metric is allowed");
- }
- }
if (readingParams.size() > maxReadingParams)
+
{
throw sdbusplus::exception::SdBusError(
static_cast<int>(std::errc::argument_list_too_long),
"Too many reading parameters");
}
+
+ try
+ {
+ for (const auto& item : readingParams)
+ {
+ utils::stringToOperationType(std::get<1>(item));
+ }
+ }
+ catch (const std::exception& e)
+ {
+ throw sdbusplus::exception::SdBusError(
+ static_cast<int>(std::errc::invalid_argument), e.what());
+ }
}
interfaces::Report& ReportManager::addReport(
@@ -136,12 +141,10 @@
using namespace utils::tstring;
return ReadingParameters::value_type(
- utils::transform(param.at_index<0>(),
- [](const LabeledSensorParameters& p) {
- return sdbusplus::message::object_path(
- p.at_label<Path>());
- }),
- param.at_index<1>(), param.at_index<2>(), param.at_index<3>());
+ sdbusplus::message::object_path(
+ param.at_index<0>().at_label<Path>()),
+ utils::enumToString(param.at_index<1>()), param.at_index<2>(),
+ param.at_index<3>());
});
verifyAddReport(reportName, reportingType, interval, metricParams);
diff --git a/src/utils/conversion.hpp b/src/utils/conversion.hpp
index 7db4be8..5e8ef81 100644
--- a/src/utils/conversion.hpp
+++ b/src/utils/conversion.hpp
@@ -1,18 +1,57 @@
#pragma once
+#include <algorithm>
+#include <array>
#include <stdexcept>
+#include <string>
namespace utils
{
template <class T, T first, T last>
-inline T toEnum(int x)
+inline T toEnum(std::underlying_type_t<T> x)
{
- if (x < static_cast<decltype(x)>(first) ||
- x > static_cast<decltype(x)>(last))
+ if (x < static_cast<std::underlying_type_t<T>>(first) ||
+ x > static_cast<std::underlying_type_t<T>>(last))
{
throw std::out_of_range("Value is not in range of enum");
}
return static_cast<T>(x);
}
+
+template <class T>
+inline std::underlying_type_t<T> toUnderlying(T value)
+{
+ return static_cast<std::underlying_type_t<T>>(value);
+}
+
+template <class T, size_t N>
+inline T stringToEnum(const std::array<std::pair<std::string_view, T>, N>& data,
+ const std::string& value)
+{
+ auto it = std::find_if(
+ std::begin(data), std::end(data),
+ [&value](const auto& item) { return item.first == value; });
+ if (it == std::end(data))
+ {
+ throw std::out_of_range("Value is not in range of enum");
+ }
+ return it->second;
+}
+
+template <class T, size_t N>
+inline std::string_view
+ enumToString(const std::array<std::pair<std::string_view, T>, N>& data,
+ T value)
+{
+ auto it = std::find_if(
+ std::begin(data), std::end(data),
+ [value](const auto& item) { return item.second == value; });
+ if (it == std::end(data))
+ {
+ throw std::out_of_range("Value is not in range of enum");
+ }
+ return it->first;
+}
+
} // namespace utils
diff --git a/src/utils/tstring.hpp b/src/utils/tstring.hpp
index f0a401f..a8d3e90 100644
--- a/src/utils/tstring.hpp
+++ b/src/utils/tstring.hpp
@@ -16,11 +16,11 @@
}
};
-struct SensorPaths
+struct SensorPath
{
static std::string str()
{
- return "sensorPaths";
+ return "sensorPath";
}
};