Removed FutureVersion from API
Instead of using FutureVersion API currently used version is updated.
This change needs to be bumped together with [1]. API that utilized map
of variants to be more flexible and backwards compatible was reverted.
It was decided that straight forward API that is commonly used should be
used instead.
Removed MetricId property from Metric. In telemetry it was implemented
as a name for Metric, but it was supposed to work as described in [2].
Currently MetricId is not supported by telemetry service and property
was removed.
Tested:
- After chaging bmcweb to use new API old and new features are working
as expected
[1]: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/44270
[2]: https://redfish.dmtf.org/schemas/v1/MetricReportDefinition.v1_4_2.json
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: I5930a466a370f268d68f575a4a3db5ee9655e574
diff --git a/src/discrete_threshold.cpp b/src/discrete_threshold.cpp
index ad3b153..9195878 100644
--- a/src/discrete_threshold.cpp
+++ b/src/discrete_threshold.cpp
@@ -1,6 +1,7 @@
#include "discrete_threshold.hpp"
#include "utils/conversion_trigger.hpp"
+#include "utils/to_short_enum.hpp"
#include <phosphor-logging/log.hpp>
@@ -113,7 +114,8 @@
{
if (nameIn.empty())
{
- return discrete::severityToString(severity) + " condition";
+ return std::string(utils::toShortEnum(utils::enumToString(severity))) +
+ " condition";
}
return nameIn;
}
diff --git a/src/metric.cpp b/src/metric.cpp
index 222af3c..9350099 100644
--- a/src/metric.cpp
+++ b/src/metric.cpp
@@ -11,12 +11,12 @@
#include <algorithm>
Metric::Metric(Sensors sensorsIn, OperationType operationTypeIn,
- std::string idIn, CollectionTimeScope timeScopeIn,
+ CollectionTimeScope timeScopeIn,
CollectionDuration collectionDurationIn,
std::unique_ptr<interfaces::Clock> clockIn) :
- id(std::move(idIn)),
- sensors(std::move(sensorsIn)), operationType(operationTypeIn),
- collectionTimeScope(timeScopeIn), collectionDuration(collectionDurationIn),
+ sensors(std::move(sensorsIn)),
+ operationType(operationTypeIn), collectionTimeScope(timeScopeIn),
+ collectionDuration(collectionDurationIn),
collectionAlgorithms(
metrics::makeCollectionData(sensors.size(), operationType,
collectionTimeScope, collectionDuration)),
@@ -81,7 +81,7 @@
i = idx;
}
- readings.emplace_back(id, sensors[i]->metadata(), *value,
+ readings.emplace_back(sensors[i]->metadata(), *value,
systemTimestamp);
}
}
@@ -123,7 +123,7 @@
sensor->metadata());
});
- return LabeledMetricParameters(std::move(sensorPath), operationType, id,
+ return LabeledMetricParameters(std::move(sensorPath), operationType,
collectionTimeScope, collectionDuration);
}
diff --git a/src/metric.hpp b/src/metric.hpp
index e6c0ea1..fa462cb 100644
--- a/src/metric.hpp
+++ b/src/metric.hpp
@@ -14,9 +14,8 @@
public std::enable_shared_from_this<Metric>
{
public:
- Metric(Sensors sensors, OperationType operationType, std::string id,
- CollectionTimeScope, CollectionDuration,
- std::unique_ptr<interfaces::Clock>);
+ Metric(Sensors sensors, OperationType operationType, CollectionTimeScope,
+ CollectionDuration, std::unique_ptr<interfaces::Clock>);
void initialize() override;
void deinitialize() override;
@@ -34,7 +33,6 @@
metrics::CollectionData&
findAssociatedData(const interfaces::Sensor& notifier);
- std::string id;
std::vector<MetricValue> readings;
Sensors sensors;
OperationType operationType;
diff --git a/src/metric_value.hpp b/src/metric_value.hpp
index 6c1d3da..5e7c6df 100644
--- a/src/metric_value.hpp
+++ b/src/metric_value.hpp
@@ -5,14 +5,13 @@
struct MetricValue
{
- std::string id;
std::string metadata;
double value;
uint64_t timestamp;
- MetricValue(std::string_view idIn, std::string_view metadataIn,
- double valueIn, uint64_t timestampIn) :
- id(idIn),
- metadata(metadataIn), value(valueIn), timestamp(timestampIn)
+ MetricValue(std::string_view metadataIn, double valueIn,
+ uint64_t timestampIn) :
+ metadata(metadataIn),
+ value(valueIn), timestamp(timestampIn)
{}
};
diff --git a/src/report.cpp b/src/report.cpp
index dbc1a9a..26fcd0b 100644
--- a/src/report.cpp
+++ b/src/report.cpp
@@ -48,16 +48,6 @@
return metric->dumpConfiguration();
}));
- readingParametersPastVersion = utils::transform(readingParameters,
- [](const auto& item) {
- const auto& [sensorData, operationType, id, collectionTimeScope,
- collectionDuration] = item;
-
- return ReadingParametersPastVersion::value_type(
- std::get<0>(sensorData.front()), operationType, id,
- std::get<1>(sensorData.front()));
- });
-
reportActions.insert(ReportAction::logToMetricReportsCollection);
deleteIface = objServer->add_unique_interface(
@@ -77,7 +67,7 @@
});
});
- errorMessages = verify();
+ auto errorMessages = verify(reportingType, interval);
state.set<ReportFlags::enabled, ReportFlags::valid>(enabledIn,
errorMessages.empty());
@@ -135,17 +125,12 @@
void Report::activate()
{
- for (auto& metric : this->metrics)
+ for (auto& metric : metrics)
{
metric->initialize();
}
scheduleTimer();
-
- if (reportIface)
- {
- reportIface->signal_property("ErrorMessages");
- }
}
void Report::deactivate()
@@ -157,11 +142,6 @@
unregisterFromMetrics = nullptr;
timer.cancel();
-
- if (reportIface)
- {
- reportIface->signal_property("ErrorMessages");
- }
}
uint64_t Report::getMetricCount(
@@ -225,40 +205,59 @@
return 1;
},
[this](const auto&) { return state.get<ReportFlags::enabled>(); });
- dbusIface->register_property_r<
- std::vector<std::tuple<std::string, std::string>>>(
- "ErrorMessages", sdbusplus::vtable::property_::emits_change,
- [this](const auto&) {
- return utils::transform(errorMessages, [](const auto& em) {
- return std::tuple<std::string, std::string>(
- utils::enumToString(em.error), em.arg0);
- });
- });
- dbusIface->register_property_rw<uint64_t>(
- "Interval", sdbusplus::vtable::property_::emits_change,
- [this](uint64_t newVal, auto& oldVal) {
- const Milliseconds newValT{newVal};
- if (newValT < ReportManager::minInterval && newValT != Milliseconds{0})
+ dbusIface->register_method(
+ "SetReportingProperties",
+ [this](std::string newReportingType, uint64_t newInterval) {
+ ReportingType newReportingTypeT = reportingType;
+
+ if (!newReportingType.empty())
{
- throw errors::InvalidArgument("Interval");
+ newReportingTypeT = utils::toReportingType(newReportingType);
}
- if (newValT != interval)
- {
- oldVal = newVal;
- interval = newValT;
+ Milliseconds newIntervalT = interval;
- errorMessages = verify();
- if (state.set<ReportFlags::valid>(errorMessages.empty()) ==
- StateEvent::active)
+ if (newInterval != std::numeric_limits<uint64_t>::max())
+ {
+ newIntervalT = Milliseconds(newInterval);
+ }
+
+ auto errorMessages = verify(newReportingTypeT, newIntervalT);
+
+ if (!errorMessages.empty())
+ {
+ if (newIntervalT != interval)
{
- scheduleTimer();
+ throw errors::InvalidArgument("Interval");
}
- persistency = storeConfiguration();
+ throw errors::InvalidArgument("ReportingType");
}
- return 1;
- },
+
+ if (reportingType != newReportingTypeT)
+ {
+ reportingType = newReportingTypeT;
+ reportIface->signal_property("ReportingType");
+ }
+
+ if (interval != newIntervalT)
+ {
+ interval = newIntervalT;
+ reportIface->signal_property("Interval");
+ }
+
+ if (state.set<ReportFlags::valid>(errorMessages.empty()) ==
+ StateEvent::active)
+ {
+ scheduleTimer();
+ }
+
+ persistency = storeConfiguration();
+
+ setReadingBuffer(reportUpdates);
+ });
+ dbusIface->register_property_r<uint64_t>(
+ "Interval", sdbusplus::vtable::property_::emits_change,
[this](const auto&) { return interval.count(); });
dbusIface->register_property_rw<bool>(
"Persistency", sdbusplus::vtable::property_::emits_change,
@@ -283,39 +282,14 @@
dbusIface->register_property_r("Readings", readings,
sdbusplus::vtable::property_::emits_change,
[this](const auto&) { return readings; });
- dbusIface->register_property_rw<std::string>(
+ dbusIface->register_property_r<std::string>(
"ReportingType", sdbusplus::vtable::property_::emits_change,
- [this](auto newVal, auto& oldVal) {
- ReportingType tmp = utils::toReportingType(newVal);
- if (tmp != reportingType)
- {
- reportingType = tmp;
- oldVal = std::move(newVal);
-
- errorMessages = verify();
- if (state.set<ReportFlags::valid>(errorMessages.empty()) ==
- StateEvent::active)
- {
- scheduleTimer();
- }
-
- persistency = storeConfiguration();
-
- setReadingBuffer(reportUpdates);
- }
- return 1;
- },
[this](const auto&) { return utils::enumToString(reportingType); });
- dbusIface->register_property_r(
- "ReadingParameters", readingParametersPastVersion,
- sdbusplus::vtable::property_::const_,
- [this](const auto&) { return readingParametersPastVersion; });
dbusIface->register_property_rw(
- "ReadingParametersFutureVersion", readingParameters,
+ "ReadingParameters", readingParameters,
sdbusplus::vtable::property_::emits_change,
[this, &reportFactory](auto newVal, auto& oldVal) {
auto labeledMetricParams = reportFactory.convertMetricParams(newVal);
- ReportManager::verifyMetricParameters(labeledMetricParams);
reportFactory.updateMetrics(metrics, state.get<ReportFlags::enabled>(),
labeledMetricParams);
readingParameters = toReadingParameters(
@@ -471,7 +445,7 @@
break;
}
- for (const auto& [id, metadata, value, timestamp] :
+ for (const auto& [metadata, value, timestamp] :
metric->getUpdatedReadings())
{
if (reportUpdates == ReportUpdates::appendStopsWhenFull &&
@@ -481,7 +455,7 @@
reportIface->signal_property("Enabled");
break;
}
- readingsBuffer.emplace(id, metadata, value, timestamp);
+ readingsBuffer.emplace(metadata, value, timestamp);
}
}
@@ -628,8 +602,14 @@
}
}
-std::vector<ErrorMessage> Report::verify() const
+std::vector<ErrorMessage> Report::verify(ReportingType reportingType,
+ Milliseconds interval)
{
+ if (interval != Milliseconds{0} && interval < ReportManager::minInterval)
+ {
+ throw errors::InvalidArgument("Interval");
+ }
+
std::vector<ErrorMessage> result;
if ((reportingType == ReportingType::periodic &&
diff --git a/src/report.hpp b/src/report.hpp
index be22c4f..bb164d9 100644
--- a/src/report.hpp
+++ b/src/report.hpp
@@ -108,7 +108,7 @@
bool shouldStoreMetricValues() const;
void updateReadings();
void scheduleTimer();
- std::vector<ErrorMessage> verify() const;
+ static std::vector<ErrorMessage> verify(ReportingType, Milliseconds);
std::string id;
const sdbusplus::message::object_path path;
@@ -116,7 +116,6 @@
ReportingType reportingType;
Milliseconds interval;
std::unordered_set<ReportAction> reportActions;
- ReadingParametersPastVersion readingParametersPastVersion;
ReadingParameters readingParameters;
bool persistency = false;
uint64_t metricCount;
@@ -138,7 +137,6 @@
utils::Ensure<std::function<void()>> unregisterFromMetrics;
State<ReportFlags, Report, ReportFlags::enabled, ReportFlags::valid> state{
*this};
- std::vector<ErrorMessage> errorMessages;
public:
static constexpr const char* reportIfaceName =
diff --git a/src/report_factory.cpp b/src/report_factory.cpp
index d3a9bb0..5f80355 100644
--- a/src/report_factory.cpp
+++ b/src/report_factory.cpp
@@ -26,19 +26,19 @@
std::vector<LabeledMetricParameters> labeledMetricParams, bool enabled,
Readings readings) const
{
- auto metrics = utils::transform(
- labeledMetricParams,
- [this](const LabeledMetricParameters& param)
- -> std::shared_ptr<interfaces::Metric> {
- namespace ts = utils::tstring;
+ auto metrics =
+ utils::transform(labeledMetricParams,
+ [this](const LabeledMetricParameters& param)
+ -> std::shared_ptr<interfaces::Metric> {
+ namespace ts = utils::tstring;
- return std::make_shared<Metric>(
- getSensors(param.at_label<ts::SensorPath>()),
- param.at_label<ts::OperationType>(), param.at_label<ts::Id>(),
- param.at_label<ts::CollectionTimeScope>(),
- param.at_label<ts::CollectionDuration>(),
- std::make_unique<Clock>());
- });
+ return std::make_shared<Metric>(
+ getSensors(param.at_label<ts::SensorPath>()),
+ param.at_label<ts::OperationType>(),
+ param.at_label<ts::CollectionTimeScope>(),
+ param.at_label<ts::CollectionDuration>(),
+ std::make_unique<Clock>());
+ });
return std::make_unique<Report>(
bus->get_io_context(), objServer, id, name, reportingType,
@@ -72,7 +72,6 @@
newMetrics.emplace_back(std::make_shared<Metric>(
getSensors(labeledMetricParam.at_label<ts::SensorPath>()),
labeledMetricParam.at_label<ts::OperationType>(),
- labeledMetricParam.at_label<ts::Id>(),
labeledMetricParam.at_label<ts::CollectionTimeScope>(),
labeledMetricParam.at_label<ts::CollectionDuration>(),
std::make_unique<Clock>()));
@@ -142,7 +141,7 @@
try
{
return utils::transform(metricParams, [&tree](const auto& item) {
- auto [sensorPaths, operationType, id, collectionTimeScope,
+ auto [sensorPaths, operationType, collectionTimeScope,
collectionDuration] = item;
std::vector<LabeledSensorInfo> sensorParameters;
@@ -187,7 +186,7 @@
return LabeledMetricParameters(
std::move(sensorParameters),
- utils::toOperationType(operationType), id,
+ utils::toOperationType(operationType),
utils::toCollectionTimeScope(collectionTimeScope),
CollectionDuration(Milliseconds(collectionDuration)));
});
diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index 69a110c..0dcb6d5 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -15,23 +15,6 @@
#include <stdexcept>
#include <system_error>
-ReadingParameters
- convertToReadingParameters(ReadingParametersPastVersion params)
-{
- return utils::transform(params, [](const auto& param) {
- using namespace std::chrono_literals;
-
- const auto& [sensorPath, operationType, id, metadata] = param;
-
- return ReadingParameters::value_type(
- std::vector<
- std::tuple<sdbusplus::message::object_path, std::string>>{
- {sensorPath, metadata}},
- operationType, id, utils::enumToString(CollectionTimeScope::point),
- 0u);
- });
-}
-
ReportManager::ReportManager(
std::unique_ptr<interfaces::ReportFactory> reportFactoryIn,
std::unique_ptr<interfaces::JsonStorage> reportStorageIn,
@@ -60,94 +43,43 @@
utils::convDataOperationType,
[](const auto& item) { return std::string(item.first); });
});
-
dbusIface.register_method(
"AddReport",
- [this](boost::asio::yield_context& yield,
- const std::string& reportId,
- const std::string& reportingType,
- const bool emitsReadingsUpdate,
- const bool logToMetricReportsCollection,
- const uint64_t interval,
- ReadingParametersPastVersion metricParams) {
- constexpr auto enabledDefault = true;
- constexpr ReportUpdates reportUpdatesDefault =
- ReportUpdates::overwrite;
-
- std::vector<ReportAction> reportActions;
-
- if (emitsReadingsUpdate)
+ [this](boost::asio::yield_context& yield, std::string reportId,
+ std::string reportName, std::string reportingType,
+ std::string reportUpdates, uint64_t appendLimit,
+ std::vector<std::string> reportActions, uint64_t interval,
+ ReadingParameters readingParameters, bool enabled) {
+ if (reportingType.empty())
{
- reportActions.emplace_back(ReportAction::emitsReadingsUpdate);
- }
- if (logToMetricReportsCollection)
- {
- reportActions.emplace_back(
- ReportAction::logToMetricReportsCollection);
+ reportingType = utils::enumToString(ReportingType::onRequest);
}
- return addReport(
- yield, reportId, reportId,
- utils::toReportingType(reportingType), reportActions,
- Milliseconds(interval), maxAppendLimit,
- reportUpdatesDefault,
- convertToReadingParameters(std::move(metricParams)),
- enabledDefault)
- .getPath();
- });
-
- dbusIface.register_method(
- "AddReportFutureVersion",
- [this](boost::asio::yield_context& yield,
- const std::vector<
- std::pair<std::string, AddReportFutureVersionVariant>>&
- properties) {
- std::optional<std::string> reportId;
- std::optional<std::string> reportName;
- std::optional<std::string> reportingType;
- std::optional<std::vector<std::string>> reportActions;
- std::optional<uint64_t> interval;
- std::optional<uint64_t> appendLimit;
- std::optional<std::string> reportUpdates;
- std::optional<ReadingParameters> metricParams;
- std::optional<ReadingParameters> readingParameters;
- std::optional<bool> enabled;
-
- try
+ if (reportUpdates.empty())
{
- sdbusplus::unpackProperties(
- properties, "Id", reportId, "Name", reportName,
- "ReportingType", reportingType, "ReportActions",
- reportActions, "Interval", interval, "AppendLimit",
- appendLimit, "ReportUpdates", reportUpdates, "MetricParams",
- metricParams, "Enabled", enabled, "ReadingParameters",
- readingParameters);
- }
- catch (const sdbusplus::exception::UnpackPropertyError& e)
- {
- throw errors::InvalidArgument(e.propertyName);
+ reportUpdates = utils::enumToString(ReportUpdates::overwrite);
}
- if (readingParameters == std::nullopt)
+ if (appendLimit == std::numeric_limits<uint64_t>::max())
{
- readingParameters = metricParams;
+ appendLimit = maxAppendLimit;
}
- return addReport(
- yield, reportId.value_or(""), reportName.value_or(""),
- utils::toReportingType(reportingType.value_or(
- utils::enumToString(ReportingType::onRequest))),
- utils::transform(
- reportActions.value_or(std::vector<std::string>{}),
- [](const auto& reportAction) {
+ if (interval == std::numeric_limits<uint64_t>::max())
+ {
+ interval = 0;
+ }
+
+ return addReport(yield, reportId, reportName,
+ utils::toReportingType(reportingType),
+ utils::transform(
+ reportActions,
+ [](const auto& reportAction) {
return utils::toReportAction(reportAction);
- }),
- Milliseconds(interval.value_or(0)),
- appendLimit.value_or(maxAppendLimit),
- utils::toReportUpdates(reportUpdates.value_or(
- utils::enumToString(ReportUpdates::overwrite))),
- readingParameters.value_or(ReadingParameters{}),
- enabled.value_or(true))
+ }),
+ Milliseconds(interval), appendLimit,
+ utils::toReportUpdates(reportUpdates),
+ readingParameters, enabled)
.getPath();
});
});
@@ -161,21 +93,6 @@
reports.end());
}
-void ReportManager::verifyMetricParameters(
- const std::vector<LabeledMetricParameters>& readingParams)
-{
- namespace ts = utils::tstring;
-
- for (auto readingParam : readingParams)
- {
- if (readingParam.at_label<ts::Id>().length() >
- utils::constants::maxIdNameLength)
- {
- throw errors::InvalidArgument("ReadingParameters.Id", "Too long.");
- }
- }
-}
-
void ReportManager::verifyAddReport(
const std::string& reportId, const std::string& reportName,
const ReportingType reportingType, Milliseconds interval,
@@ -218,8 +135,6 @@
throw errors::InvalidArgument("MetricParams", "Too many.");
}
- verifyMetricParameters(readingParams);
-
for (const LabeledMetricParameters& item : readingParams)
{
utils::toOperationType(
diff --git a/src/report_manager.hpp b/src/report_manager.hpp
index 4882419..dcd2bfc 100644
--- a/src/report_manager.hpp
+++ b/src/report_manager.hpp
@@ -33,9 +33,6 @@
void removeReport(const interfaces::Report* report) override;
- static void verifyMetricParameters(
- const std::vector<LabeledMetricParameters>& readingParams);
-
private:
std::unique_ptr<interfaces::ReportFactory> reportFactory;
std::unique_ptr<interfaces::JsonStorage> reportStorage;
diff --git a/src/trigger_actions.cpp b/src/trigger_actions.cpp
index 75aab50..ef4a50b 100644
--- a/src/trigger_actions.cpp
+++ b/src/trigger_actions.cpp
@@ -4,6 +4,7 @@
#include "types/trigger_types.hpp"
#include "utils/clock.hpp"
#include "utils/messanger.hpp"
+#include "utils/to_short_enum.hpp"
#include <phosphor-logging/log.hpp>
@@ -53,11 +54,12 @@
std::string thresholdName = ::numeric::typeToString(type);
auto direction = getDirection(value, threshold);
- std::string msg = "Numeric threshold '" + thresholdName + "' of trigger '" +
- triggerId + "' is crossed on sensor " + sensorName +
- ", recorded value: " + std::to_string(value) +
- ", crossing direction: " + direction +
- ", timestamp: " + timestampToString(timestamp);
+ std::string msg =
+ "Numeric threshold '" + std::string(utils::toShortEnum(thresholdName)) +
+ "' of trigger '" + triggerId + "' is crossed on sensor " + sensorName +
+ ", recorded value: " + std::to_string(value) +
+ ", crossing direction: " + std::string(utils::toShortEnum(direction)) +
+ ", timestamp: " + timestampToString(timestamp);
phosphor::logging::log<phosphor::logging::level::INFO>(msg.c_str());
}
@@ -145,11 +147,12 @@
{
auto value = std::get<std::string>(triggerValue);
- std::string msg = "Discrete condition '" + thresholdNameIn->get() +
- "' of trigger '" + triggerId + "' is met on sensor " +
- sensorName + ", recorded value: " + value +
- ", severity: " + ::discrete::severityToString(severity) +
- ", timestamp: " + timestampToString(timestamp);
+ std::string msg =
+ "Discrete condition '" + thresholdNameIn->get() + "' of trigger '" +
+ triggerId + "' is met on sensor " + sensorName +
+ ", recorded value: " + value + ", severity: " +
+ std::string(utils::toShortEnum(utils::enumToString(severity))) +
+ ", timestamp: " + timestampToString(timestamp);
phosphor::logging::log<phosphor::logging::level::INFO>(msg.c_str());
}
diff --git a/src/types/collection_time_scope.hpp b/src/types/collection_time_scope.hpp
index 25e0744..79db567 100644
--- a/src/types/collection_time_scope.hpp
+++ b/src/types/collection_time_scope.hpp
@@ -26,11 +26,16 @@
constexpr std::array<std::pair<std::string_view, CollectionTimeScope>, 3>
convDataCollectionTimeScope = {
{std::make_pair<std::string_view, CollectionTimeScope>(
- "Point", CollectionTimeScope::point),
+ "xyz.openbmc_project.Telemetry.Report.CollectionTimescope.Point",
+ CollectionTimeScope::point),
std::make_pair<std::string_view, CollectionTimeScope>(
- "Interval", CollectionTimeScope::interval),
+ "xyz.openbmc_project.Telemetry.Report.CollectionTimescope."
+ "Interval",
+ CollectionTimeScope::interval),
std::make_pair<std::string_view, CollectionTimeScope>(
- "StartupInterval", CollectionTimeScope::startup)}};
+ "xyz.openbmc_project.Telemetry.Report.CollectionTimescope."
+ "StartupInterval",
+ CollectionTimeScope::startup)}};
inline CollectionTimeScope
toCollectionTimeScope(std::underlying_type_t<CollectionTimeScope> value)
diff --git a/src/types/operation_type.hpp b/src/types/operation_type.hpp
index cca8b23..e7f6ed2 100644
--- a/src/types/operation_type.hpp
+++ b/src/types/operation_type.hpp
@@ -24,14 +24,19 @@
};
constexpr std::array<std::pair<std::string_view, OperationType>, 4>
- convDataOperationType = {{std::make_pair<std::string_view, OperationType>(
- "Maximum", OperationType::max),
- std::make_pair<std::string_view, OperationType>(
- "Minimum", OperationType::min),
- std::make_pair<std::string_view, OperationType>(
- "Average", OperationType::avg),
- std::make_pair<std::string_view, OperationType>(
- "Summation", OperationType::sum)}};
+ convDataOperationType = {
+ {std::make_pair<std::string_view, OperationType>(
+ "xyz.openbmc_project.Telemetry.Report.OperationType.Maximum",
+ OperationType::max),
+ std::make_pair<std::string_view, OperationType>(
+ "xyz.openbmc_project.Telemetry.Report.OperationType.Minimum",
+ OperationType::min),
+ std::make_pair<std::string_view, OperationType>(
+ "xyz.openbmc_project.Telemetry.Report.OperationType.Average",
+ OperationType::avg),
+ std::make_pair<std::string_view, OperationType>(
+ "xyz.openbmc_project.Telemetry.Report.OperationType.Summation",
+ OperationType::sum)}};
inline OperationType
toOperationType(std::underlying_type_t<OperationType> value)
diff --git a/src/types/readings.hpp b/src/types/readings.hpp
index 6749bbf..d5fe359 100644
--- a/src/types/readings.hpp
+++ b/src/types/readings.hpp
@@ -3,12 +3,11 @@
#include "utils/labeled_tuple.hpp"
#include "utils/tstring.hpp"
-using ReadingData = std::tuple<std::string, std::string, double, uint64_t>;
+using ReadingData = std::tuple<std::string, double, uint64_t>;
using Readings = std::tuple<uint64_t, std::vector<ReadingData>>;
using LabeledReadingData =
- utils::LabeledTuple<ReadingData, utils::tstring::MetricId,
- utils::tstring::MetricProperty,
+ utils::LabeledTuple<ReadingData, utils::tstring::MetricProperty,
utils::tstring::MetricValue, utils::tstring::Timestamp>;
using LabeledReadings =
diff --git a/src/types/report_action.hpp b/src/types/report_action.hpp
index 3a114e3..1ec74f9 100644
--- a/src/types/report_action.hpp
+++ b/src/types/report_action.hpp
@@ -23,12 +23,14 @@
};
constexpr std::array<std::pair<std::string_view, ReportAction>, 2>
- convDataReportAction = {
- {std::make_pair<std::string_view, ReportAction>(
- "EmitsReadingsUpdate", ReportAction::emitsReadingsUpdate),
- std::make_pair<std::string_view, ReportAction>(
- "LogToMetricReportsCollection",
- ReportAction::logToMetricReportsCollection)}};
+ convDataReportAction = {{std::make_pair<std::string_view, ReportAction>(
+ "xyz.openbmc_project.Telemetry.Report."
+ "ReportActions.EmitsReadingsUpdate",
+ ReportAction::emitsReadingsUpdate),
+ std::make_pair<std::string_view, ReportAction>(
+ "xyz.openbmc_project.Telemetry.Report."
+ "ReportActions.LogToMetricReportsCollection",
+ ReportAction::logToMetricReportsCollection)}};
inline ReportAction toReportAction(std::underlying_type_t<ReportAction> value)
{
diff --git a/src/types/report_types.cpp b/src/types/report_types.cpp
index 4fb0b4e..8e31ae6 100644
--- a/src/types/report_types.cpp
+++ b/src/types/report_types.cpp
@@ -18,7 +18,6 @@
sensorParameters.at_label<ts::Metadata>());
}),
utils::enumToString(metricParams.at_label<ts::OperationType>()),
- metricParams.at_label<ts::Id>(),
utils::enumToString(
metricParams.at_label<ts::CollectionTimeScope>()),
metricParams.at_label<ts::CollectionDuration>().t.count());
diff --git a/src/types/report_types.hpp b/src/types/report_types.hpp
index 49dca73..c40c76d 100644
--- a/src/types/report_types.hpp
+++ b/src/types/report_types.hpp
@@ -16,22 +16,17 @@
#include <variant>
#include <vector>
-using ReadingParametersPastVersion =
- std::vector<std::tuple<sdbusplus::message::object_path, std::string,
- std::string, std::string>>;
-
using ReadingParameters = std::vector<std::tuple<
std::vector<std::tuple<sdbusplus::message::object_path, std::string>>,
- std::string, std::string, std::string, uint64_t>>;
+ std::string, std::string, uint64_t>>;
using LabeledMetricParameters = utils::LabeledTuple<
- std::tuple<std::vector<LabeledSensorInfo>, OperationType, std::string,
+ std::tuple<std::vector<LabeledSensorInfo>, OperationType,
CollectionTimeScope, CollectionDuration>,
utils::tstring::SensorPath, utils::tstring::OperationType,
- utils::tstring::Id, utils::tstring::CollectionTimeScope,
- utils::tstring::CollectionDuration>;
+ utils::tstring::CollectionTimeScope, utils::tstring::CollectionDuration>;
-using AddReportFutureVersionVariant =
+using AddReportVariant =
std::variant<std::monostate, bool, uint64_t, std::string,
std::vector<std::string>, ReadingParameters>;
diff --git a/src/types/report_updates.hpp b/src/types/report_updates.hpp
index a3dfe81..9286e24 100644
--- a/src/types/report_updates.hpp
+++ b/src/types/report_updates.hpp
@@ -26,12 +26,17 @@
};
constexpr auto convDataReportUpdates = std::array{
- std::make_pair<std::string_view, ReportUpdates>("Overwrite",
- ReportUpdates::overwrite),
std::make_pair<std::string_view, ReportUpdates>(
- "AppendStopsWhenFull", ReportUpdates::appendStopsWhenFull),
+ "xyz.openbmc_project.Telemetry.Report.ReportUpdates.Overwrite",
+ ReportUpdates::overwrite),
std::make_pair<std::string_view, ReportUpdates>(
- "AppendWrapsWhenFull", ReportUpdates::appendWrapsWhenFull)};
+ "xyz.openbmc_project.Telemetry.Report.ReportUpdates."
+ "AppendStopsWhenFull",
+ ReportUpdates::appendStopsWhenFull),
+ std::make_pair<std::string_view, ReportUpdates>(
+ "xyz.openbmc_project.Telemetry.Report.ReportUpdates."
+ "AppendWrapsWhenFull",
+ ReportUpdates::appendWrapsWhenFull)};
inline ReportUpdates
toReportUpdates(std::underlying_type_t<ReportUpdates> value)
diff --git a/src/types/reporting_type.hpp b/src/types/reporting_type.hpp
index 2c9f500..f179eb7 100644
--- a/src/types/reporting_type.hpp
+++ b/src/types/reporting_type.hpp
@@ -26,12 +26,16 @@
};
constexpr std::array<std::pair<std::string_view, ReportingType>, 3>
- convDataReportingType = {{std::make_pair<std::string_view, ReportingType>(
- "Periodic", ReportingType::periodic),
- std::make_pair<std::string_view, ReportingType>(
- "OnRequest", ReportingType::onRequest),
- std::make_pair<std::string_view, ReportingType>(
- "OnChange", ReportingType::onChange)}};
+ convDataReportingType = {
+ {std::make_pair<std::string_view, ReportingType>(
+ "xyz.openbmc_project.Telemetry.Report.ReportingType.OnChange",
+ ReportingType::onChange),
+ std::make_pair<std::string_view, ReportingType>(
+ "xyz.openbmc_project.Telemetry.Report.ReportingType.OnRequest",
+ ReportingType::onRequest),
+ std::make_pair<std::string_view, ReportingType>(
+ "xyz.openbmc_project.Telemetry.Report.ReportingType.Periodic",
+ ReportingType::periodic)}};
inline ReportingType
toReportingType(std::underlying_type_t<ReportingType> value)
diff --git a/src/types/trigger_types.hpp b/src/types/trigger_types.hpp
index 7ba0768..38da4f8 100644
--- a/src/types/trigger_types.hpp
+++ b/src/types/trigger_types.hpp
@@ -23,10 +23,15 @@
{
constexpr std::array<std::pair<std::string_view, TriggerAction>, 3>
convDataTriggerAction = {
- std::make_pair("LogToJournal", TriggerAction::LogToJournal),
- std::make_pair("LogToRedfishEventLog",
+ std::make_pair(
+ "xyz.openbmc_project.Telemetry.Trigger.TriggerAction.LogToJournal",
+ TriggerAction::LogToJournal),
+ std::make_pair("xyz.openbmc_project.Telemetry.Trigger.TriggerAction."
+ "LogToRedfishEventLog",
TriggerAction::LogToRedfishEventLog),
- std::make_pair("UpdateReport", TriggerAction::UpdateReport)};
+ std::make_pair(
+ "xyz.openbmc_project.Telemetry.Trigger.TriggerAction.UpdateReport",
+ TriggerAction::UpdateReport)};
}
inline TriggerAction toTriggerAction(const std::string& str)
@@ -49,25 +54,6 @@
critical
};
-namespace details
-{
-constexpr std::array<std::pair<std::string_view, Severity>, 3>
- convDataSeverity = {std::make_pair("OK", Severity::ok),
- std::make_pair("Warning", Severity::warning),
- std::make_pair("Critical", Severity::critical)};
-
-} // namespace details
-
-inline Severity toSeverity(const std::string& str)
-{
- return utils::toEnum(details::convDataSeverity, str);
-}
-
-inline std::string severityToString(Severity v)
-{
- return std::string(utils::enumToString(details::convDataSeverity, v));
-}
-
using ThresholdParam =
std::tuple<std::string, std::string, uint64_t, std::string>;
@@ -99,15 +85,25 @@
{
constexpr std::array<std::pair<std::string_view, Type>, 4> convDataType = {
- std::make_pair("LowerCritical", Type::lowerCritical),
- std::make_pair("LowerWarning", Type::lowerWarning),
- std::make_pair("UpperWarning", Type::upperWarning),
- std::make_pair("UpperCritical", Type::upperCritical)};
+ std::make_pair("xyz.openbmc_project.Telemetry.Trigger.Type.LowerCritical",
+ Type::lowerCritical),
+ std::make_pair("xyz.openbmc_project.Telemetry.Trigger.Type.LowerWarning",
+ Type::lowerWarning),
+ std::make_pair("xyz.openbmc_project.Telemetry.Trigger.Type.UpperWarning",
+ Type::upperWarning),
+ std::make_pair("xyz.openbmc_project.Telemetry.Trigger.Type.UpperCritical",
+ Type::upperCritical)};
constexpr std::array<std::pair<std::string_view, Direction>, 3>
- convDataDirection = {std::make_pair("Either", Direction::either),
- std::make_pair("Decreasing", Direction::decreasing),
- std::make_pair("Increasing", Direction::increasing)};
+ convDataDirection = {
+ std::make_pair("xyz.openbmc_project.Telemetry.Trigger.Direction.Either",
+ Direction::either),
+ std::make_pair(
+ "xyz.openbmc_project.Telemetry.Trigger.Direction.Decreasing",
+ Direction::decreasing),
+ std::make_pair(
+ "xyz.openbmc_project.Telemetry.Trigger.Direction.Increasing",
+ Direction::increasing)};
} // namespace details
@@ -181,6 +177,26 @@
namespace utils
{
+constexpr std::array<std::pair<std::string_view, discrete::Severity>, 3>
+ convDataSeverity = {
+ std::make_pair("xyz.openbmc_project.Telemetry.Trigger.Severity.OK",
+ discrete::Severity::ok),
+ std::make_pair("xyz.openbmc_project.Telemetry.Trigger.Severity.Warning",
+ discrete::Severity::warning),
+ std::make_pair(
+ "xyz.openbmc_project.Telemetry.Trigger.Severity.Critical",
+ discrete::Severity::critical)};
+
+inline discrete::Severity toSeverity(const std::string& str)
+{
+ return utils::toEnum(convDataSeverity, str);
+}
+
+inline std::string enumToString(discrete::Severity v)
+{
+ return std::string(enumToString(convDataSeverity, v));
+}
+
template <>
struct EnumTraits<TriggerAction>
{
diff --git a/src/utils/conversion_trigger.cpp b/src/utils/conversion_trigger.cpp
index a8417e5..98ae2a5 100644
--- a/src/utils/conversion_trigger.cpp
+++ b/src/utils/conversion_trigger.cpp
@@ -36,7 +36,7 @@
const auto& [userId, severity, dwellTime,
thresholdValue] = thresholdParam;
return discrete::LabeledThresholdParam(
- userId, discrete::toSeverity(severity), dwellTime, thresholdValue);
+ userId, utils::toSeverity(severity), dwellTime, thresholdValue);
});
}
@@ -59,13 +59,13 @@
const std::vector<discrete::LabeledThresholdParam>& arg) const
{
return utils::transform(
- arg, [](const discrete::LabeledThresholdParam& labeledThresholdParam) {
- return discrete::ThresholdParam(
- labeledThresholdParam.at_label<ts::UserId>(),
- discrete::severityToString(
- labeledThresholdParam.at_label<ts::Severity>()),
- labeledThresholdParam.at_label<ts::DwellTime>(),
- labeledThresholdParam.at_label<ts::ThresholdValue>());
+ arg,
+ [](const discrete::LabeledThresholdParam& labeledThresholdParam) {
+ return discrete::ThresholdParam(
+ labeledThresholdParam.at_label<ts::UserId>(),
+ utils::enumToString(labeledThresholdParam.at_label<ts::Severity>()),
+ labeledThresholdParam.at_label<ts::DwellTime>(),
+ labeledThresholdParam.at_label<ts::ThresholdValue>());
});
}
@@ -104,8 +104,7 @@
}
return discrete::ThresholdParam(
paramUnpacked->at_label<ts::UserId>(),
- discrete::severityToString(
- paramUnpacked->at_label<ts::Severity>()),
+ utils::enumToString(paramUnpacked->at_label<ts::Severity>()),
paramUnpacked->at_label<ts::DwellTime>(),
paramUnpacked->at_label<ts::ThresholdValue>());
});
diff --git a/src/utils/to_short_enum.hpp b/src/utils/to_short_enum.hpp
new file mode 100644
index 0000000..56ea88d
--- /dev/null
+++ b/src/utils/to_short_enum.hpp
@@ -0,0 +1,18 @@
+#pragma once
+
+#include <string_view>
+
+namespace utils
+{
+
+inline std::string_view toShortEnum(std::string_view name)
+{
+ auto pos = name.find_last_of(".");
+ if (pos != std::string_view::npos && pos + 1 < name.size())
+ {
+ return name.substr(pos + 1);
+ }
+ return name;
+}
+
+} // namespace utils
diff --git a/tests/src/dbus_environment.hpp b/tests/src/dbus_environment.hpp
index 558445e..1006323 100644
--- a/tests/src/dbus_environment.hpp
+++ b/tests/src/dbus_environment.hpp
@@ -131,6 +131,23 @@
return DbusEnvironment::waitForFuture(std::move(future));
}
+ template <class... Args>
+ static boost::system::error_code
+ callMethod(const std::string& path, const std::string& interface,
+ const std::string& method, Args&&... args)
+ {
+ auto promise = std::promise<boost::system::error_code>();
+ auto future = promise.get_future();
+ DbusEnvironment::getBus()->async_method_call(
+ [promise = std::move(promise)](
+ boost::system::error_code ec) mutable {
+ promise.set_value(ec);
+ },
+ DbusEnvironment::serviceName(), path, interface, method,
+ std::forward<Args>(args)...);
+ return DbusEnvironment::waitForFuture(std::move(future));
+ }
+
private:
static std::future<bool> getFuture(std::string_view name);
diff --git a/tests/src/helpers/metric_value_helpers.hpp b/tests/src/helpers/metric_value_helpers.hpp
index 9634de3..dee48bd 100644
--- a/tests/src/helpers/metric_value_helpers.hpp
+++ b/tests/src/helpers/metric_value_helpers.hpp
@@ -6,12 +6,12 @@
inline void PrintTo(const MetricValue& o, std::ostream* os)
{
- (*os) << "{ id: " << o.id << ", metadata: " << o.metadata
- << ", value: " << o.value << ", timestamp: " << o.timestamp << " }";
+ (*os) << "{ 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);
+ return std::tie(left.metadata, left.value, left.timestamp) ==
+ std::tie(right.metadata, right.value, right.timestamp);
}
diff --git a/tests/src/mocks/report_factory_mock.hpp b/tests/src/mocks/report_factory_mock.hpp
index 8af6d5c..deed82d 100644
--- a/tests/src/mocks/report_factory_mock.hpp
+++ b/tests/src/mocks/report_factory_mock.hpp
@@ -21,9 +21,8 @@
std::get<1>(sensorData));
}),
utils::toOperationType(std::get<1>(params)),
- std::get<2>(params),
- utils::toCollectionTimeScope(std::get<3>(params)),
- CollectionDuration(Milliseconds(std::get<4>(params))));
+ utils::toCollectionTimeScope(std::get<2>(params)),
+ CollectionDuration(Milliseconds(std::get<3>(params))));
});
}
diff --git a/tests/src/params/metric_params.hpp b/tests/src/params/metric_params.hpp
index cc5519d..cef5f87 100644
--- a/tests/src/params/metric_params.hpp
+++ b/tests/src/params/metric_params.hpp
@@ -24,17 +24,6 @@
return operationTypeProperty;
}
- MetricParams& id(std::string val)
- {
- idProperty = std::move(val);
- return *this;
- }
-
- const std::string& id() const
- {
- return idProperty;
- }
-
MetricParams& collectionTimeScope(CollectionTimeScope val)
{
collectionTimeScopeProperty = val;
@@ -92,7 +81,6 @@
private:
OperationType operationTypeProperty = {};
- std::string idProperty = "MetricId";
CollectionTimeScope collectionTimeScopeProperty = {};
CollectionDuration collectionDurationProperty =
CollectionDuration(Milliseconds(0u));
diff --git a/tests/src/params/report_params.hpp b/tests/src/params/report_params.hpp
index 458e11b..a10c783 100644
--- a/tests/src/params/report_params.hpp
+++ b/tests/src/params/report_params.hpp
@@ -134,7 +134,6 @@
"/xyz/openbmc_project/sensors/power/p1",
"metadata1"}},
OperationType::avg,
- "MetricId1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))},
LabeledMetricParameters{
@@ -142,7 +141,6 @@
"/xyz/openbmc_project/sensors/power/p2",
"metadata2"}},
OperationType::avg,
- "MetricId2",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}};
bool enabledProperty = true;
diff --git a/tests/src/test_metric.cpp b/tests/src/test_metric.cpp
index 952651c..6410337 100644
--- a/tests/src/test_metric.cpp
+++ b/tests/src/test_metric.cpp
@@ -45,12 +45,11 @@
return std::make_shared<Metric>(
utils::convContainer<std::shared_ptr<interfaces::Sensor>>(
sensorMocks),
- p.operationType(), p.id(), p.collectionTimeScope(),
- p.collectionDuration(), std::move(clockFakePtr));
+ p.operationType(), p.collectionTimeScope(), p.collectionDuration(),
+ std::move(clockFakePtr));
}
MetricParams params = MetricParams()
- .id("id")
.operationType(OperationType::avg)
.collectionTimeScope(CollectionTimeScope::point)
.collectionDuration(CollectionDuration(0ms));
@@ -141,7 +140,7 @@
ASSERT_THAT(
sut->getUpdatedReadings(),
- ElementsAre(MetricValue{"id", "metadata0", 31.2,
+ ElementsAre(MetricValue{"metadata0", 31.2,
std::chrono::duration_cast<Milliseconds>(
clockFake.system.timestamp())
.count()}));
@@ -167,7 +166,6 @@
const auto conf = sut->dumpConfiguration();
LabeledMetricParameters expected = {};
- expected.at_label<ts::Id>() = params.id();
expected.at_label<ts::OperationType>() = params.operationType();
expected.at_label<ts::CollectionTimeScope>() = params.collectionTimeScope();
expected.at_label<ts::CollectionDuration>() = params.collectionDuration();
@@ -357,9 +355,8 @@
expectedReading] = GetParam().expectedReading();
const auto readings = sut->getUpdatedReadings();
- EXPECT_THAT(readings,
- ElementsAre(MetricValue{"id", "metadata0", expectedReading,
- expectedTimestamp.count()}));
+ EXPECT_THAT(readings, ElementsAre(MetricValue{"metadata0", expectedReading,
+ expectedTimestamp.count()}));
}
TEST_P(TestMetricCalculationFunctions,
@@ -377,9 +374,8 @@
expectedReading] = GetParam().expectedReading();
const auto readings = sut->getUpdatedReadings();
- EXPECT_THAT(readings,
- ElementsAre(MetricValue{"id", "metadata0", expectedReading,
- expectedTimestamp.count()}));
+ EXPECT_THAT(readings, ElementsAre(MetricValue{"metadata0", expectedReading,
+ expectedTimestamp.count()}));
}
TEST_P(TestMetricCalculationFunctions, returnsIsTimerRequired)
@@ -411,13 +407,13 @@
clockFake.system.set(Milliseconds{72});
EXPECT_THAT(sut->getUpdatedReadings(),
- ElementsAre(MetricValue{"id", "metadata0", 10.0, 72},
- MetricValue{"id", "metadata1", 11.0, 72},
- MetricValue{"id", "metadata2", 12.0, 72},
- MetricValue{"id", "metadata3", 13.0, 72},
- MetricValue{"id", "metadata4", 14.0, 72},
- MetricValue{"id", "metadata5", 15.0, 72},
- MetricValue{"id", "metadata6", 16.0, 72}));
+ ElementsAre(MetricValue{"metadata0", 10.0, 72},
+ MetricValue{"metadata1", 11.0, 72},
+ MetricValue{"metadata2", 12.0, 72},
+ MetricValue{"metadata3", 13.0, 72},
+ MetricValue{"metadata4", 14.0, 72},
+ MetricValue{"metadata5", 15.0, 72},
+ MetricValue{"metadata6", 16.0, 72}));
}
TEST_F(TestMetricWithMultipleSensors, readingsContainOnlyAvailableSensors)
@@ -431,10 +427,10 @@
clockFake.system.set(Milliseconds{62});
EXPECT_THAT(sut->getUpdatedReadings(),
- ElementsAre(MetricValue{"id", "metadata5", 15.0, 62},
- MetricValue{"id", "metadata3", 13.0, 62},
- MetricValue{"id", "metadata6", 16.0, 62},
- MetricValue{"id", "metadata0", 10.0, 62}));
+ ElementsAre(MetricValue{"metadata5", 15.0, 62},
+ MetricValue{"metadata3", 13.0, 62},
+ MetricValue{"metadata6", 16.0, 62},
+ MetricValue{"metadata0", 10.0, 62}));
}
TEST_F(TestMetricWithMultipleSensors, readingsContainsAllReadingsOutOfOrder)
@@ -448,11 +444,11 @@
clockFake.system.set(Milliseconds{52});
EXPECT_THAT(sut->getUpdatedReadings(),
- ElementsAre(MetricValue{"id", "metadata6", 16.0, 52},
- MetricValue{"id", "metadata5", 15.0, 52},
- MetricValue{"id", "metadata3", 13.0, 52},
- MetricValue{"id", "metadata4", 14.0, 52},
- MetricValue{"id", "metadata0", 10.0, 52},
- MetricValue{"id", "metadata2", 12.0, 52},
- MetricValue{"id", "metadata1", 11.0, 52}));
+ ElementsAre(MetricValue{"metadata6", 16.0, 52},
+ MetricValue{"metadata5", 15.0, 52},
+ MetricValue{"metadata3", 13.0, 52},
+ MetricValue{"metadata4", 14.0, 52},
+ MetricValue{"metadata0", 10.0, 52},
+ MetricValue{"metadata2", 12.0, 52},
+ MetricValue{"metadata1", 11.0, 52}));
}
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index 1d0f0bb..accd112 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -83,8 +83,8 @@
}
metricMocks.resize(metricParameters.size());
- std::vector<MetricValue> readings{{MetricValue{"a", "b", 17.1, 114},
- MetricValue{"aa", "bb", 42.0, 74}}};
+ std::vector<MetricValue> readings{
+ {MetricValue{"b", 17.1, 114}, MetricValue{"bb", 42.0, 74}}};
ASSERT_THAT(readings.size(), Ge(metricParameters.size()));
@@ -161,6 +161,16 @@
property, newValue);
}
+ template <class... Args>
+ static boost::system::error_code callMethod(const std::string& path,
+ const std::string& method,
+ Args&&... args)
+ {
+ return DbusEnvironment::callMethod(path, Report::reportIfaceName,
+ "SetReportingProperties",
+ std::forward<Args>(args)...);
+ }
+
template <class T>
struct ChangePropertyParams
{
@@ -242,17 +252,14 @@
getProperty<bool>(sut->getPath(), "LogToMetricReportsCollection"),
Eq(utils::contains(defaultParams().reportActions(),
ReportAction::logToMetricReportsCollection)));
- EXPECT_THAT(getProperty<ReadingParameters>(
- sut->getPath(), "ReadingParametersFutureVersion"),
- Eq(toReadingParameters(defaultParams().metricParameters())));
+ EXPECT_THAT(
+ getProperty<ReadingParameters>(sut->getPath(), "ReadingParameters"),
+ Eq(toReadingParameters(defaultParams().metricParameters())));
EXPECT_THAT(getProperty<std::string>(sut->getPath(), "Name"),
Eq(defaultParams().reportName()));
EXPECT_THAT(
getProperty<std::vector<object_path>>(sut->getPath(), "Triggers"),
IsEmpty());
- EXPECT_THAT(
- getProperty<ErrorMessagesDbusType>(sut->getPath(), "ErrorMessages"),
- IsEmpty());
}
TEST_F(TestReport, readingsAreInitialyEmpty)
@@ -269,7 +276,6 @@
"/xyz/openbmc_project/sensors/power/psu",
"NewMetadata123"}},
OperationType::avg,
- "NewMetricId123",
CollectionTimeScope::startup,
CollectionDuration(250ms)}}});
auto metrics = getMetricsFromReadingParams(newParams);
@@ -277,61 +283,17 @@
EXPECT_CALL(*reportFactoryMock, updateMetrics(_, _, _))
.WillOnce(SetArgReferee<0>(metrics));
EXPECT_THAT(
- setProperty(sut->getPath(), "ReadingParametersFutureVersion", newParams)
- .value(),
+ setProperty(sut->getPath(), "ReadingParameters", newParams).value(),
Eq(boost::system::errc::success));
- EXPECT_THAT(getProperty<ReadingParameters>(
- sut->getPath(), "ReadingParametersFutureVersion"),
- Eq(newParams));
-}
-
-TEST_F(TestReport, setReadingParametersWithTooLongMetricId)
-{
- const ReadingParameters currentValue =
- toReadingParameters(defaultParams().metricParameters());
-
- ReadingParameters newParams = toReadingParameters(
- std::vector<LabeledMetricParameters>{{LabeledMetricParameters{
- {LabeledSensorInfo{"Service",
- "/xyz/openbmc_project/sensors/power/psu",
- "NewMetadata123"}},
- OperationType::avg,
- utils::string_utils::getTooLongId(),
- CollectionTimeScope::startup,
- CollectionDuration(250ms)}}});
-
- changeProperty<ReadingParameters>(
- sut->getPath(), "ReadingParametersFutureVersion",
- {.valueBefore = Eq(currentValue),
- .newValue = newParams,
- .ec = Eq(boost::system::errc::invalid_argument),
- .valueAfter = Eq(currentValue)});
-}
-
-TEST_F(TestReport, setReportingTypeWithValidNewType)
-{
- changeProperty<std::string>(
- sut->getPath(), "ReportingType",
- {.valueBefore = Not(Eq(utils::enumToString(ReportingType::onRequest))),
- .newValue = utils::enumToString(ReportingType::onRequest)});
-}
-
-TEST_F(TestReport, setReportingTypeWithInvalidType)
-{
- const std::string currentValue =
- utils::enumToString(defaultParams().reportingType());
-
- changeProperty<std::string>(
- sut->getPath(), "ReportingType",
- {.valueBefore = Eq(currentValue),
- .newValue = "Periodic_ABC",
- .ec = Eq(boost::system::errc::invalid_argument),
- .valueAfter = Eq(currentValue)});
+ EXPECT_THAT(
+ getProperty<ReadingParameters>(sut->getPath(), "ReadingParameters"),
+ Eq(newParams));
}
TEST_F(TestReport, setReportActionsWithValidNewActions)
{
- std::vector<std::string> newActions = {"EmitsReadingsUpdate"};
+ std::vector<std::string> newActions = {
+ utils::enumToString(ReportAction::emitsReadingsUpdate)};
std::vector<std::string> currActions =
utils::transform(defaultParams().reportActions(),
[](const auto v) { return utils::enumToString(v); });
@@ -342,16 +304,19 @@
Eq(boost::system::errc::success));
EXPECT_THAT(
getProperty<std::vector<std::string>>(sut->getPath(), "ReportActions"),
- UnorderedElementsAre("EmitsReadingsUpdate",
- "LogToMetricReportsCollection"));
+ UnorderedElementsAre(
+ utils::enumToString(ReportAction::emitsReadingsUpdate),
+ utils::enumToString(ReportAction::logToMetricReportsCollection)));
}
TEST_F(TestReport, setReportActionsWithValidUnsortedActions)
{
- std::vector<std::string> newActions = {"LogToMetricReportsCollection",
- "EmitsReadingsUpdate"};
- std::vector<std::string> expectedActions = {"EmitsReadingsUpdate",
- "LogToMetricReportsCollection"};
+ std::vector<std::string> newActions = {
+ utils::enumToString(ReportAction::logToMetricReportsCollection),
+ utils::enumToString(ReportAction::emitsReadingsUpdate)};
+ std::vector<std::string> expectedActions = {
+ utils::enumToString(ReportAction::emitsReadingsUpdate),
+ utils::enumToString(ReportAction::logToMetricReportsCollection)};
std::vector<std::string> currActions =
utils::transform(defaultParams().reportActions(),
[](const auto v) { return utils::enumToString(v); });
@@ -368,7 +333,8 @@
TEST_F(TestReport, setReportActionsWithEmptyActions)
{
std::vector<std::string> newActions = {};
- std::vector<std::string> expectedActions = {"LogToMetricReportsCollection"};
+ std::vector<std::string> expectedActions = {
+ utils::enumToString(ReportAction::logToMetricReportsCollection)};
std::vector<std::string> currActions =
utils::transform(defaultParams().reportActions(),
[](const auto v) { return utils::enumToString(v); });
@@ -397,7 +363,8 @@
TEST_F(TestReport, createReportWithEmptyActions)
{
- std::vector<std::string> expectedActions = {"LogToMetricReportsCollection"};
+ std::vector<std::string> expectedActions = {
+ utils::enumToString(ReportAction::logToMetricReportsCollection)};
sut = makeReport(ReportParams().reportId("TestId_1").reportActions({}));
EXPECT_THAT(
@@ -407,10 +374,12 @@
TEST_F(TestReport, createReportWithValidUnsortedActions)
{
- std::vector<std::string> newActions = {"LogToMetricReportsCollection",
- "EmitsReadingsUpdate"};
- std::vector<std::string> expectedActions = {"EmitsReadingsUpdate",
- "LogToMetricReportsCollection"};
+ std::vector<std::string> newActions = {
+ utils::enumToString(ReportAction::logToMetricReportsCollection),
+ utils::enumToString(ReportAction::emitsReadingsUpdate)};
+ std::vector<std::string> expectedActions = {
+ utils::enumToString(ReportAction::emitsReadingsUpdate),
+ utils::enumToString(ReportAction::logToMetricReportsCollection)};
sut = makeReport(
ReportParams()
@@ -431,113 +400,75 @@
EXPECT_THAT(getProperty<bool>(sut->getPath(), "Enabled"), Eq(newValue));
}
-TEST_F(TestReport, setIntervalWithValidValue)
+TEST_F(TestReport, setReportingPropertiesWithValidValues)
{
uint64_t newValue = ReportManager::minInterval.count() * 42;
- EXPECT_THAT(setProperty(sut->getPath(), "Interval", newValue).value(),
+ EXPECT_THAT(callMethod(sut->getPath(), "SetReportingProperties",
+ utils::enumToString(ReportingType::periodic),
+ newValue),
Eq(boost::system::errc::success));
EXPECT_THAT(getProperty<uint64_t>(sut->getPath(), "Interval"),
Eq(newValue));
}
-TEST_F(
- TestReport,
- settingIntervalWithInvalidValueDoesNotChangePropertyAndReturnsInvalidArgument)
+TEST_F(TestReport, failsToSetInvalidInterval)
{
uint64_t newValue = ReportManager::minInterval.count() - 1;
- EXPECT_THAT(setProperty(sut->getPath(), "Interval", newValue).value(),
- Eq(boost::system::errc::invalid_argument));
+
+ EXPECT_THAT(
+ callMethod(sut->getPath(), "SetReportingProperties", "", newValue),
+ Eq(boost::system::errc::invalid_argument));
+
EXPECT_THAT(getProperty<uint64_t>(sut->getPath(), "Interval"),
Eq(defaultParams().interval().count()));
}
-TEST_F(TestReport, settingInvalidReportingTypeCreatesErrorMessage)
+TEST_F(TestReport, failsToSetIncompatibleInterval)
{
auto report = makeReport(defaultParams()
.reportId("report2")
.reportingType(ReportingType::onRequest)
.interval(Milliseconds{0}));
- EXPECT_THAT(
- setProperty<std::string>(report->getPath(), "ReportingType", "Periodic")
- .value(),
- Eq(boost::system::errc::success));
-
- EXPECT_THAT(getProperty<std::string>(report->getPath(), "ReportingType"),
- Eq("Periodic"));
- EXPECT_THAT(
- getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
- UnorderedElementsAre(
- ErrorMessageDbusType(
- utils::enumToString(ErrorType::propertyConflict), "Interval"),
- ErrorMessageDbusType(
- utils::enumToString(ErrorType::propertyConflict),
- "ReportingType")));
-}
-
-TEST_F(TestReport, settingValidReportingTypeRemovesErrors)
-{
- auto report = makeReport(defaultParams()
- .reportId("report2")
- .reportingType(ReportingType::onRequest)
- .interval(Milliseconds{0}));
+ uint64_t newValue = ReportManager::minInterval.count();
EXPECT_THAT(
- setProperty<std::string>(report->getPath(), "ReportingType", "Periodic")
- .value(),
- Eq(boost::system::errc::success));
- EXPECT_THAT(setProperty<std::string>(report->getPath(), "ReportingType",
- "OnRequest")
- .value(),
- Eq(boost::system::errc::success));
-
- EXPECT_THAT(getProperty<std::string>(report->getPath(), "ReportingType"),
- Eq("OnRequest"));
- EXPECT_THAT(
- getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
- IsEmpty());
-}
-
-TEST_F(TestReport, settingInvalidIntervalDisablesReport)
-{
- auto report = makeReport(defaultParams()
- .reportId("report2")
- .reportingType(ReportingType::periodic)
- .interval(ReportManager::minInterval));
-
- EXPECT_THAT(setProperty<uint64_t>(report->getPath(), "Interval", 0).value(),
- Eq(boost::system::errc::success));
-
- EXPECT_THAT(getProperty<uint64_t>(report->getPath(), "Interval"), Eq(0u));
- EXPECT_THAT(
- getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
- UnorderedElementsAre(
- ErrorMessageDbusType(
- utils::enumToString(ErrorType::propertyConflict), "Interval"),
- ErrorMessageDbusType(
- utils::enumToString(ErrorType::propertyConflict),
- "ReportingType")));
-}
-
-TEST_F(TestReport, settingValidIntervalEnablesReport)
-{
- auto report = makeReport(defaultParams()
- .reportId("report2")
- .reportingType(ReportingType::periodic)
- .interval(ReportManager::minInterval));
-
- EXPECT_THAT(setProperty<uint64_t>(report->getPath(), "Interval", 0).value(),
- Eq(boost::system::errc::success));
- EXPECT_THAT(setProperty<uint64_t>(report->getPath(), "Interval",
- ReportManager::minInterval.count())
- .value(),
- Eq(boost::system::errc::success));
+ callMethod(report->getPath(), "SetReportingProperties", "", newValue),
+ Eq(boost::system::errc::invalid_argument));
EXPECT_THAT(getProperty<uint64_t>(report->getPath(), "Interval"),
- Eq(ReportManager::minInterval.count()));
- EXPECT_THAT(
- getProperty<ErrorMessagesDbusType>(report->getPath(), "ErrorMessages"),
- IsEmpty());
+ Eq(defaultParams().interval().count()));
+}
+
+TEST_F(TestReport, failsToSetInvalidReportingType)
+{
+ auto report = makeReport(defaultParams()
+ .reportId("report2")
+ .reportingType(ReportingType::onRequest)
+ .interval(Milliseconds{0}));
+
+ EXPECT_THAT(callMethod(sut->getPath(), "SetReportingProperties", "XYZ",
+ std::numeric_limits<uint64_t>::max()),
+ Eq(boost::system::errc::invalid_argument));
+
+ EXPECT_THAT(getProperty<std::string>(report->getPath(), "ReportingType"),
+ Eq(utils::enumToString(ReportingType::onRequest)));
+}
+
+TEST_F(TestReport, failsToSetIncompatibleReportingType)
+{
+ auto report = makeReport(defaultParams()
+ .reportId("report2")
+ .reportingType(ReportingType::onRequest)
+ .interval(Milliseconds{0}));
+
+ EXPECT_THAT(callMethod(sut->getPath(), "SetReportingProperties",
+ utils::enumToString(ReportingType::periodic),
+ std::numeric_limits<uint64_t>::max()),
+ Eq(boost::system::errc::invalid_argument));
+
+ EXPECT_THAT(getProperty<std::string>(report->getPath(), "ReportingType"),
+ Eq(utils::enumToString(ReportingType::onRequest)));
}
TEST_F(TestReport, settingEmitsReadingsUpdateHaveNoEffect)
@@ -701,7 +632,6 @@
"/xyz/openbmc_project/sensors/power/p1"},
{tstring::Metadata::str(), "metadata1"}}}},
{tstring::OperationType::str(), OperationType::avg},
- {tstring::Id::str(), "MetricId1"},
{tstring::CollectionTimeScope::str(),
CollectionTimeScope::point},
{tstring::CollectionDuration::str(), 0}},
@@ -711,7 +641,6 @@
"/xyz/openbmc_project/sensors/power/p2"},
{tstring::Metadata::str(), "metadata2"}}}},
{tstring::OperationType::str(), OperationType::avg},
- {tstring::Id::str(), "MetricId2"},
{tstring::CollectionTimeScope::str(),
CollectionTimeScope::point},
{tstring::CollectionDuration::str(), 0}}}))));
@@ -877,9 +806,8 @@
const auto [timestamp, readings] = getProperty<Readings>(sut->getPath(),
"Readings");
- EXPECT_THAT(readings,
- ElementsAre(std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)));
+ EXPECT_THAT(readings, ElementsAre(std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)));
}
class TestReportNonOnRequestType :
@@ -892,10 +820,13 @@
}
};
-INSTANTIATE_TEST_SUITE_P(
- _, TestReportNonOnRequestType,
- Values(defaultParams().reportingType(ReportingType::periodic),
- defaultParams().reportingType(ReportingType::onChange)));
+INSTANTIATE_TEST_SUITE_P(_, TestReportNonOnRequestType,
+ Values(defaultParams()
+ .reportingType(ReportingType::periodic)
+ .interval(ReportManager::minInterval * 10),
+ defaultParams()
+ .reportingType(ReportingType::onChange)
+ .interval(Milliseconds(0))));
TEST_P(TestReportNonOnRequestType, readingsAreNotUpdateOnUpdateCall)
{
@@ -958,9 +889,8 @@
const auto [timestamp, readings] = getProperty<Readings>(sut->getPath(),
"Readings");
- EXPECT_THAT(readings,
- ElementsAre(std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)));
+ EXPECT_THAT(readings, ElementsAre(std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)));
}
struct ReportUpdatesReportParams
@@ -979,9 +909,8 @@
void changeReport(ReportingType rt, Milliseconds interval)
{
- setProperty<std::string>(sut->getPath(), "ReportingType",
- utils::enumToString(rt));
- setProperty<uint64_t>(sut->getPath(), "Interval", interval.count());
+ callMethod(sut->getPath(), "SetReportingProperties",
+ utils::enumToString(rt), interval.count());
}
auto readings()
@@ -1007,21 +936,20 @@
defaultParams()
.reportUpdates(ReportUpdates::appendWrapsWhenFull)
.appendLimit(5),
- std::vector<ReadingData>{{std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u)}},
+ std::vector<ReadingData>{{std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u)}},
true},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::appendWrapsWhenFull)
.appendLimit(4),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
true},
ReportUpdatesReportParams{
defaultParams()
@@ -1032,35 +960,33 @@
defaultParams()
.reportUpdates(ReportUpdates::appendStopsWhenFull)
.appendLimit(10),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
true},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::appendStopsWhenFull)
.appendLimit(5),
- std::vector<ReadingData>{{std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u)}},
false},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::appendStopsWhenFull)
.appendLimit(4),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u),
- std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u),
+ std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
false},
ReportUpdatesReportParams{
defaultParams()
@@ -1071,33 +997,29 @@
defaultParams()
.reportUpdates(ReportUpdates::overwrite)
.appendLimit(500),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
true},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::overwrite)
.appendLimit(1),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
true},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::overwrite)
.appendLimit(0),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
true},
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::appendStopsWhenFull)
.appendLimit(2u),
- std::vector<ReadingData>{
- {std::make_tuple("a"s, "b"s, 17.1, 114u),
- std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
+ std::vector<ReadingData>{{std::make_tuple("b"s, 17.1, 114u),
+ std::make_tuple("bb"s, 42.0, 74u)}},
false}));
TEST_P(TestReportWithReportUpdatesAndLimit,
@@ -1270,7 +1192,8 @@
sut = makeReport(defaultParams()
.reportingType(ReportingType::periodic)
- .reportActions({}));
+ .reportActions({})
+ .interval(Milliseconds(1000)));
makeMonitor();
DbusEnvironment::sleepFor(defaultParams().interval() * 2);
}
diff --git a/tests/src/test_report_manager.cpp b/tests/src/test_report_manager.cpp
index fc394ae..295d966 100644
--- a/tests/src/test_report_manager.cpp
+++ b/tests/src/test_report_manager.cpp
@@ -18,8 +18,7 @@
using namespace std::string_literals;
using namespace std::chrono_literals;
-using AddReportFutureVersionVariantForSet =
- utils::WithoutMonostate<AddReportFutureVersionVariant>;
+using AddReportVariantForSet = utils::WithoutMonostate<AddReportVariant>;
class TestReportManager : public Test
{
@@ -57,10 +56,9 @@
DbusEnvironment::synchronizeIoc();
}
- std::pair<boost::system::error_code, std::string>
- addReport(const std::vector<
- std::pair<std::string, AddReportFutureVersionVariantForSet>>&
- properties)
+ template <class... Args>
+ requires(sizeof...(Args) > 1)
+ std::pair<boost::system::error_code, std::string> addReport(Args&&... args)
{
std::promise<std::pair<boost::system::error_code, std::string>>
addReportPromise;
@@ -70,34 +68,22 @@
addReportPromise.set_value({ec, path});
},
DbusEnvironment::serviceName(), ReportManager::reportManagerPath,
- ReportManager::reportManagerIfaceName, "AddReportFutureVersion",
- properties);
+ ReportManager::reportManagerIfaceName, "AddReport",
+ std::forward<Args>(args)...);
return DbusEnvironment::waitForFuture(addReportPromise.get_future());
}
auto addReport(const ReportParams& params)
{
- std::vector<std::pair<std::string, AddReportFutureVersionVariantForSet>>
- properties;
-
- properties.emplace_back("Id", params.reportId());
- properties.emplace_back("Name", params.reportName());
- properties.emplace_back("ReportingType",
- utils::enumToString(params.reportingType()));
- properties.emplace_back("ReportUpdates",
- utils::enumToString(params.reportUpdates()));
- properties.emplace_back("AppendLimit", params.appendLimit());
- properties.emplace_back("Enabled", params.enabled());
- properties.emplace_back("ReportActions",
- utils::transform(params.reportActions(),
- [](const auto v) {
- return utils::enumToString(v);
- }));
- properties.emplace_back("Interval", params.interval().count());
- properties.emplace_back("ReadingParameters",
- toReadingParameters(params.metricParameters()));
-
- return addReport(properties);
+ return addReport(
+ params.reportId(), params.reportName(),
+ utils::enumToString(params.reportingType()),
+ utils::enumToString(params.reportUpdates()), params.appendLimit(),
+ utils::transform(
+ params.reportActions(),
+ [](const auto v) { return utils::enumToString(v); }),
+ params.interval().count(),
+ toReadingParameters(params.metricParameters()), params.enabled());
}
template <class T>
@@ -125,7 +111,10 @@
{
EXPECT_THAT(
getProperty<std::vector<std::string>>("SupportedOperationTypes"),
- UnorderedElementsAre("Maximum", "Minimum", "Average", "Summation"));
+ UnorderedElementsAre(utils::enumToString(OperationType::max),
+ utils::enumToString(OperationType::min),
+ utils::enumToString(OperationType::avg),
+ utils::enumToString(OperationType::sum)));
}
TEST_F(TestReportManager, addReport)
@@ -163,8 +152,10 @@
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
auto [ec, path] = addReport(
- std::vector<
- std::pair<std::string, AddReportFutureVersionVariantForSet>>{});
+ "", "", "", "", std::numeric_limits<uint64_t>::max(),
+ std::vector<std::string>(), std::numeric_limits<uint64_t>::max(),
+ ReadingParameters(), true);
+
EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
EXPECT_THAT(path, Eq(reportMock.getPath()));
}
@@ -243,28 +234,6 @@
EXPECT_THAT(path, Eq("/"s + reportParams.reportId()));
}
-TEST_F(TestReportManager, addReportWithMaxLengthMetricId)
-{
- namespace ts = utils::tstring;
- std::vector<LabeledMetricParameters> newMetricParams{
- {LabeledMetricParameters{
- {LabeledSensorInfo{"Service",
- "/xyz/openbmc_project/sensors/power/p1",
- "metadata1"}},
- OperationType::avg,
- utils::string_utils::getMaxId(),
- CollectionTimeScope::point,
- CollectionDuration(Milliseconds(0u))}}};
-
- reportParams.metricParameters(newMetricParams);
- reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock));
-
- auto [ec, path] = addReport(reportParams);
-
- EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
- EXPECT_THAT(path, Eq("/"s + reportParams.reportId()));
-}
-
TEST_F(TestReportManager, DISABLED_failToAddReportWithTooLongFullId)
{
reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
@@ -338,31 +307,6 @@
EXPECT_THAT(path, Eq(std::string()));
}
-TEST_F(TestReportManager, DISABLED_failToAddReportWithTooLongMetricId)
-{
- namespace ts = utils::tstring;
-
- std::vector<LabeledMetricParameters> newMetricParams{
- {LabeledMetricParameters{
- {LabeledSensorInfo{"Service",
- "/xyz/openbmc_project/sensors/power/p1",
- "metadata1"}},
- OperationType::avg,
- utils::string_utils::getTooLongId(),
- CollectionTimeScope::point,
- CollectionDuration(Milliseconds(0u))}}};
-
- reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
- .Times(0);
-
- reportParams.metricParameters(newMetricParams);
-
- auto [ec, path] = addReport(reportParams);
-
- EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
- EXPECT_THAT(path, Eq(std::string()));
-}
-
TEST_F(TestReportManager, DISABLED_failToAddReportTwice)
{
reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
@@ -395,8 +339,10 @@
reportFactoryMock.expectMake(std::nullopt, Ref(*sut), Ref(storageMock))
.Times(0);
- auto [ec, path] = addReport({{"Name", reportParams.reportName()},
- {"ReportingType", "InvalidReportingType"}});
+ auto [ec, path] = addReport(
+ "", "", "InvalidReportingType", "",
+ std::numeric_limits<uint64_t>::max(), std::vector<std::string>(),
+ std::numeric_limits<uint64_t>::max(), ReadingParameters(), false);
EXPECT_THAT(ec.value(), Eq(boost::system::errc::invalid_argument));
EXPECT_THAT(path, Eq(std::string()));
@@ -414,7 +360,6 @@
"/xyz/openbmc_project/sensors/power/p1",
"Metadata1"}},
OperationType::avg,
- "MetricId1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}});
@@ -448,7 +393,6 @@
metricParams.emplace_back(
LabeledMetricParameters{{},
OperationType::avg,
- "MetricId1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))});
}
@@ -476,13 +420,14 @@
TEST_F(TestReportManager, addReportWithAppendLimitEqualToUint64MaxIsAllowed)
{
- reportParams.appendLimit(std::numeric_limits<uint64_t>::max());
-
EXPECT_CALL(reportFactoryMock, convertMetricParams(_, _));
- reportFactoryMock.expectMake(reportParams, Ref(*sut), Ref(storageMock))
+ reportFactoryMock
+ .expectMake(reportParams.appendLimit(ReportManager::maxAppendLimit),
+ Ref(*sut), Ref(storageMock))
.WillOnce(Return(ByMove(std::move(reportMockPtr))));
- auto [ec, path] = addReport(reportParams);
+ auto [ec, path] = addReport(
+ reportParams.appendLimit(std::numeric_limits<uint64_t>::max()));
EXPECT_THAT(ec.value(), Eq(boost::system::errc::success));
EXPECT_THAT(path, Eq(reportMock.getPath()));
}
@@ -574,7 +519,6 @@
"/xyz/openbmc_project/sensors/power/p1",
"Metadata1"}},
operationType,
- "MetricId1",
CollectionTimeScope::point,
CollectionDuration(Milliseconds(0u))}}});
diff --git a/tests/src/test_trigger.cpp b/tests/src/test_trigger.cpp
index f55dec1..6cccb4a 100644
--- a/tests/src/test_trigger.cpp
+++ b/tests/src/test_trigger.cpp
@@ -315,8 +315,9 @@
{
EXPECT_CALL(*triggerFactoryMockPtr, updateThresholds(_, _, _, _, _, _));
TriggerThresholdParams newThresholds =
- std::vector<discrete::ThresholdParam>(
- {std::make_tuple("discrete threshold", "OK", 10, "12.3")});
+ std::vector<discrete::ThresholdParam>({std::make_tuple(
+ "discrete threshold", utils::enumToString(discrete::Severity::ok),
+ 10, "12.3")});
EXPECT_THAT(setProperty(sut->getPath(), "Thresholds", newThresholds),
Eq(boost::system::errc::success));
}
@@ -329,7 +330,8 @@
TriggerThresholdParams newThresholds =
std::vector<discrete::ThresholdParam>({std::make_tuple(
- utils::string_utils::getTooLongName(), "OK", 10, "12.3")});
+ utils::string_utils::getTooLongName(),
+ utils::enumToString(discrete::Severity::ok), 10, "12.3")});
changeProperty<TriggerThresholdParams>(
sut->getPath(), "Thresholds",