From 41ee20f46f2c6d2e9e5418128993cd176d03a927 Mon Sep 17 00:00:00 2001 From: Krzysztof Grobelny Date: Wed, 5 Jan 2022 14:54:18 +0100 Subject: [PATCH] Switched bmcweb to use new telemetry service API Added support for multiple MetricProperties. Added support for new parameters: CollectionTimeScope, CollectionDuration. Tested: - It is possible to create MetricReportDefinitions with multiple MetricProperties. - Stub values for new parameters are correctly passed to telemetry service. - All existing telemetry service functionalities remain unchanged. Change-Id: I2cd17069e3ea015c8f5571c29278f1d50536272a Signed-off-by: Krzysztof Grobelny Signed-off-by: Lukasz Kazmierczak --- include/dbus_utility.hpp | 5 +- redfish-core/lib/metric_report_definition.hpp | 310 +++++++++++------- 2 files changed, 189 insertions(+), 126 deletions(-) diff --git a/include/dbus_utility.hpp b/include/dbus_utility.hpp index a971325..35109c4 100644 --- a/include/dbus_utility.hpp +++ b/include/dbus_utility.hpp @@ -51,8 +51,9 @@ using DbusVariantType = sdbusplus::utility::dedup_variant_t< std::vector>, std::vector>>, std::vector>, - std::vector> + std::vector>, + std::string, std::string, std::string, uint64_t>> >; // clang-format on diff --git a/redfish-core/lib/metric_report_definition.hpp b/redfish-core/lib/metric_report_definition.hpp index 2584afc..9512381 100644 --- a/redfish-core/lib/metric_report_definition.hpp +++ b/redfish-core/lib/metric_report_definition.hpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include @@ -18,119 +20,156 @@ namespace redfish namespace telemetry { -using ReadingParameters = - std::vector>; +using ReadingParameters = std::vector>, + std::string, std::string, std::string, uint64_t>>; -inline void fillReportDefinition( - const std::shared_ptr& asyncResp, const std::string& id, - const std::vector>& - ret) +std::string toReadfishReportAction(std::string_view action) { - asyncResp->res.jsonValue["@odata.type"] = - "#MetricReportDefinition.v1_3_0.MetricReportDefinition"; - asyncResp->res.jsonValue["@odata.id"] = - telemetry::metricReportDefinitionUri + std::string("/") + id; - asyncResp->res.jsonValue["Id"] = id; - asyncResp->res.jsonValue["Name"] = id; - asyncResp->res.jsonValue["MetricReport"]["@odata.id"] = - telemetry::metricReportUri + std::string("/") + id; - asyncResp->res.jsonValue["Status"]["State"] = "Enabled"; - asyncResp->res.jsonValue["ReportUpdates"] = "Overwrite"; - - const bool* emitsReadingsUpdate = nullptr; - const bool* logToMetricReportsCollection = nullptr; - const ReadingParameters* readingParams = nullptr; - const std::string* reportingType = nullptr; - const uint64_t* interval = nullptr; - for (const auto& [key, var] : ret) + if (action == "EmitsReadingsUpdate") { - if (key == "EmitsReadingsUpdate") - { - emitsReadingsUpdate = std::get_if(&var); - } - else if (key == "LogToMetricReportsCollection") - { - logToMetricReportsCollection = std::get_if(&var); - } - else if (key == "ReadingParameters") - { - readingParams = std::get_if(&var); - } - else if (key == "ReportingType") - { - reportingType = std::get_if(&var); - } - else if (key == "Interval") - { - interval = std::get_if(&var); - } + return "RedfishEvent"; } - if (!emitsReadingsUpdate || !logToMetricReportsCollection || - !readingParams || !reportingType || !interval) + if (action == "LogToMetricReportsCollection") { - BMCWEB_LOG_ERROR << "Property type mismatch or property is missing"; - messages::internalError(asyncResp->res); - return; + return "LogToMetricReportsCollection"; } + return ""; +} - std::vector redfishReportActions; - redfishReportActions.reserve(2); - if (*emitsReadingsUpdate) +std::string toDbusReportAction(std::string_view action) +{ + if (action == "RedfishEvent") { - redfishReportActions.emplace_back("RedfishEvent"); + return "EmitsReadingsUpdate"; } - if (*logToMetricReportsCollection) + if (action == "LogToMetricReportsCollection") { - redfishReportActions.emplace_back("LogToMetricReportsCollection"); + return "LogToMetricReportsCollection"; } + return ""; +} - nlohmann::json metrics = nlohmann::json::array(); - for (auto& [sensorPath, operationType, id, metadata] : *readingParams) +inline void fillReportDefinition( + const std::shared_ptr& asyncResp, const std::string& id, + const std::vector>& + properties) +{ + try { - metrics.push_back({ - {"MetricId", id}, - {"MetricProperties", {metadata}}, - }); + std::vector reportActions; + ReadingParameters readingParams; + std::string reportingType; + std::string reportUpdates; + std::string name; + uint64_t appendLimit = 0u; + uint64_t interval = 0u; + + sdbusplus::unpackProperties( + properties, "ReportActions", reportActions, "ReportUpdates", + reportUpdates, "AppendLimit", appendLimit, + "ReadingParametersFutureVersion", readingParams, "ReportingType", + reportingType, "Interval", interval, "Name", name); + + for (std::string& action : reportActions) + { + action = toReadfishReportAction(action); + + if (action.empty()) + { + messages::internalError(asyncResp->res); + return; + } + } + + nlohmann::json metrics = nlohmann::json::array(); + for (auto& [sensorData, collectionFunction, id, collectionTimeScope, + collectionDuration] : readingParams) + { + std::vector metricProperties; + + for (auto& [sensorPath, sensorMetadata] : sensorData) + { + metricProperties.emplace_back(std::move(sensorMetadata)); + } + + metrics.push_back( + {{"MetricId", std::move(id)}, + {"MetricProperties", std::move(metricProperties)}, + {"CollectionFunction", std::move(collectionFunction)}, + {"CollectionDuration", + time_utils::toDurationString( + std::chrono::milliseconds(collectionDuration))}, + {"CollectionTimeScope", std::move(collectionTimeScope)}}); + } + + asyncResp->res.jsonValue["@odata.type"] = + "#MetricReportDefinition.v1_3_0.MetricReportDefinition"; + asyncResp->res.jsonValue["@odata.id"] = + telemetry::metricReportDefinitionUri + std::string("/") + id; + asyncResp->res.jsonValue["Id"] = id; + asyncResp->res.jsonValue["Name"] = name; + asyncResp->res.jsonValue["MetricReport"]["@odata.id"] = + telemetry::metricReportUri + std::string("/") + id; + asyncResp->res.jsonValue["Status"]["State"] = "Enabled"; + asyncResp->res.jsonValue["ReportUpdates"] = reportUpdates; + asyncResp->res.jsonValue["AppendLimit"] = appendLimit; + asyncResp->res.jsonValue["Metrics"] = metrics; + asyncResp->res.jsonValue["MetricReportDefinitionType"] = reportingType; + asyncResp->res.jsonValue["ReportActions"] = reportActions; + asyncResp->res.jsonValue["Schedule"]["RecurrenceInterval"] = + time_utils::toDurationString(std::chrono::milliseconds(interval)); + } + catch (const sdbusplus::exception::UnpackPropertyError& error) + { + BMCWEB_LOG_ERROR << error.what() << ", property: " + << error.propertyName + ", reason: " << error.reason; + messages::queryParameterValueFormatError( + asyncResp->res, + std::string(error.propertyName) + " " + std::string(error.reason), + error.what()); + messages::internalError(asyncResp->res); } - asyncResp->res.jsonValue["Metrics"] = metrics; - asyncResp->res.jsonValue["MetricReportDefinitionType"] = *reportingType; - asyncResp->res.jsonValue["ReportActions"] = redfishReportActions; - asyncResp->res.jsonValue["Schedule"]["RecurrenceInterval"] = - time_utils::toDurationString(std::chrono::milliseconds(*interval)); } struct AddReportArgs { - std::string name; + struct MetricArgs + { + std::string id; + std::vector uris; + std::optional collectionFunction; + std::optional collectionTimeScope; + std::optional collectionDuration; + }; + + std::optional id; + std::optional name; std::string reportingType; - bool emitsReadingsUpdate = false; - bool logToMetricReportsCollection = false; + std::optional reportUpdates; + std::optional appendLimit; + std::vector reportActions; uint64_t interval = 0; - std::vector>> metrics; + std::vector metrics; }; inline bool toDbusReportActions(crow::Response& res, - std::vector& actions, + const std::vector& actions, AddReportArgs& args) { size_t index = 0; - for (auto& action : actions) + for (const auto& action : actions) { - if (action == "RedfishEvent") - { - args.emitsReadingsUpdate = true; - } - else if (action == "LogToMetricReportsCollection") - { - args.logToMetricReportsCollection = true; - } - else + std::string dbusReportAction = toDbusReportAction(action); + + if (dbusReportAction.empty()) { messages::propertyValueNotInList( res, action, "ReportActions/" + std::to_string(index)); return false; } + + args.reportActions.emplace_back(std::move(dbusReportAction)); index++; } return true; @@ -142,23 +181,12 @@ inline bool getUserParameters(crow::Response& res, const crow::Request& req, std::vector metrics; std::vector reportActions; std::optional schedule; - if (!json_util::readJson(req, res, "Id", args.name, "Metrics", metrics, - "MetricReportDefinitionType", args.reportingType, - "ReportActions", reportActions, "Schedule", - schedule)) - { - return false; - } - - constexpr const char* allowedCharactersInName = - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_"; - if (args.name.empty() || args.name.find_first_not_of( - allowedCharactersInName) != std::string::npos) + if (!json_util::readJson( + req, res, "Id", args.id, "Name", args.name, "Metrics", metrics, + "MetricReportDefinitionType", args.reportingType, "ReportUpdates", + args.reportUpdates, "AppendLimit", args.appendLimit, + "ReportActions", reportActions, "Schedule", schedule)) { - BMCWEB_LOG_ERROR << "Failed to match " << args.name - << " with allowed character " - << allowedCharactersInName; - messages::propertyValueIncorrect(res, "Id", args.name); return false; } @@ -203,15 +231,35 @@ inline bool getUserParameters(crow::Response& res, const crow::Request& req, args.metrics.reserve(metrics.size()); for (auto& m : metrics) { - std::string id; - std::vector uris; - if (!json_util::readJson(m, res, "MetricId", id, "MetricProperties", - uris)) + std::optional collectionDurationStr; + AddReportArgs::MetricArgs metricArgs; + if (!json_util::readJson( + m, res, "MetricId", metricArgs.id, "MetricProperties", + metricArgs.uris, "CollectionFunction", + metricArgs.collectionFunction, "CollectionTimeScope", + metricArgs.collectionTimeScope, "CollectionDuration", + collectionDurationStr)) { return false; } - args.metrics.emplace_back(std::move(id), std::move(uris)); + if (collectionDurationStr) + { + std::optional duration = + time_utils::fromDurationString(*collectionDurationStr); + + if (!duration || duration->count() < 0) + { + messages::propertyValueIncorrect(res, "CollectionDuration", + *collectionDurationStr); + return false; + } + + metricArgs.collectionDuration = + static_cast(duration->count()); + } + + args.metrics.emplace_back(std::move(metricArgs)); } return true; @@ -219,13 +267,12 @@ inline bool getUserParameters(crow::Response& res, const crow::Request& req, inline bool getChassisSensorNodeFromMetrics( const std::shared_ptr& asyncResp, - const std::vector>>& - metrics, + const std::vector& metrics, boost::container::flat_set>& matched) { - for (const auto& [id, uris] : metrics) + for (const auto& metric : metrics) { - if (!getChassisSensorNode(asyncResp, uris, matched)) + if (!getChassisSensorNode(asyncResp, metric.uris, matched)) { return false; } @@ -251,11 +298,16 @@ class AddReport telemetry::ReadingParameters readingParams; readingParams.reserve(args.metrics.size()); - for (const auto& [id, uris] : args.metrics) + for (auto& metric : args.metrics) { - for (size_t i = 0; i < uris.size(); i++) + std::vector< + std::tuple> + sensorParams; + sensorParams.reserve(metric.uris.size()); + + for (size_t i = 0; i < metric.uris.size(); i++) { - const std::string& uri = uris[i]; + const std::string& uri = metric.uris[i]; auto el = uriToDbus.find(uri); if (el == uriToDbus.end()) { @@ -269,17 +321,23 @@ class AddReport } const std::string& dbusPath = el->second; - readingParams.emplace_back(dbusPath, "SINGLE", id, uri); + sensorParams.emplace_back(dbusPath, uri); } + + readingParams.emplace_back( + std::move(sensorParams), metric.collectionFunction.value_or(""), + std::move(metric.id), metric.collectionTimeScope.value_or(""), + metric.collectionDuration.value_or(0u)); } const std::shared_ptr aResp = asyncResp; crow::connections::systemBus->async_method_call( - [aResp, name = args.name, uriToDbus = std::move(uriToDbus)]( + [aResp, id = args.id.value_or(""), + uriToDbus = std::move(uriToDbus)]( const boost::system::error_code ec, const std::string&) { if (ec == boost::system::errc::file_exists) { messages::resourceAlreadyExists( - aResp->res, "MetricReportDefinition", "Id", name); + aResp->res, "MetricReportDefinition", "Id", id); return; } if (ec == boost::system::errc::too_many_files_open) @@ -308,10 +366,12 @@ class AddReport messages::created(aResp->res); }, telemetry::service, "/xyz/openbmc_project/Telemetry/Reports", - "xyz.openbmc_project.Telemetry.ReportManager", "AddReport", - "TelemetryService/" + args.name, args.reportingType, - args.emitsReadingsUpdate, args.logToMetricReportsCollection, - args.interval, readingParams); + "xyz.openbmc_project.Telemetry.ReportManager", + "AddReportFutureVersion", + "TelemetryService/" + args.id.value_or(""), args.name.value_or(""), + args.reportingType, args.reportUpdates.value_or("Overwrite"), + args.appendLimit.value_or(0), args.reportActions, args.interval, + readingParams); } void insert(const boost::container::flat_map& el) @@ -399,12 +459,15 @@ inline void requestRoutesMetricReportDefinition(App& app) [](const crow::Request&, const std::shared_ptr& asyncResp, const std::string& id) { - crow::connections::systemBus->async_method_call( + sdbusplus::asio::getAllProperties( + *crow::connections::systemBus, telemetry::service, + telemetry::getDbusReportPath(id), + telemetry::reportInterface, [asyncResp, - id](const boost::system::error_code ec, + id](boost::system::error_code ec, const std::vector>& - ret) { + properties) { if (ec.value() == EBADR || ec == boost::system::errc::host_unreachable) { @@ -419,12 +482,11 @@ inline void requestRoutesMetricReportDefinition(App& app) return; } - telemetry::fillReportDefinition(asyncResp, id, ret); - }, - telemetry::service, telemetry::getDbusReportPath(id), - "org.freedesktop.DBus.Properties", "GetAll", - telemetry::reportInterface); + telemetry::fillReportDefinition(asyncResp, id, + properties); + }); }); + BMCWEB_ROUTE(app, "/redfish/v1/TelemetryService/MetricReportDefinitions//") .privileges(redfish::privileges::deleteMetricReportDefinitionCollection) -- 2.25.1