From 4bd7f53cc01315774eedef637f5d1824cd7f662a Mon Sep 17 00:00:00 2001 From: Krzysztof Grobelny Date: Thu, 17 Jun 2021 13:37:57 +0000 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 | 330 ++++++++++++------ redfish-core/lib/telemetry_service.hpp | 13 + 3 files changed, 240 insertions(+), 108 deletions(-) diff --git a/include/dbus_utility.hpp b/include/dbus_utility.hpp index 481b33d..32153f8 100644 --- a/include/dbus_utility.hpp +++ b/include/dbus_utility.hpp @@ -50,8 +50,9 @@ using DbusVariantType = std::variant< 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 1a520f3..ac980aa 100644 --- a/redfish-core/lib/metric_report_definition.hpp +++ b/redfish-core/lib/metric_report_definition.hpp @@ -17,50 +17,68 @@ namespace redfish namespace telemetry { - constexpr const char* metricReportDefinitionUri = "/redfish/v1/TelemetryService/MetricReportDefinitions"; -using ReadingParameters = - std::vector>; +using ReadingParameters = std::vector>, + std::string, std::string, std::string, uint64_t>>; + +std::string toReadfishReportAction(std::string_view action) +{ + if (action == "EmitsReadingsUpdate") + { + return "RedfishEvent"; + } + if (action == "LogToMetricReportsCollection") + { + return "LogToMetricReportsCollection"; + } + return ""; +} + +std::string toDbusReportAction(std::string_view action) +{ + if (action == "RedfishEvent") + { + return "EmitsReadingsUpdate"; + } + if (action == "LogToMetricReportsCollection") + { + return "LogToMetricReportsCollection"; + } + return ""; +} inline void fillReportDefinition( const std::shared_ptr& asyncResp, const std::string& id, const std::vector>& - ret) + properties) { - asyncResp->res.jsonValue["@odata.type"] = - "#MetricReportDefinition.v1_3_0.MetricReportDefinition"; - asyncResp->res.jsonValue["@odata.id"] = - crow::utility::urlFromPieces("redfish", "v1", "TelemetryService", - "MetricReportDefinitions", id) - .string(); - asyncResp->res.jsonValue["Id"] = id; - asyncResp->res.jsonValue["Name"] = id; - asyncResp->res.jsonValue["MetricReport"]["@odata.id"] = - crow::utility::urlFromPieces("redfish", "v1", "TelemetryService", - "MetricReports", id) - .string(); - asyncResp->res.jsonValue["Status"]["State"] = "Enabled"; - asyncResp->res.jsonValue["ReportUpdates"] = "Overwrite"; - - const bool* emitsReadingsUpdate = nullptr; - const bool* logToMetricReportsCollection = nullptr; + const std::vector* reportActions = nullptr; const ReadingParameters* readingParams = nullptr; const std::string* reportingType = nullptr; + const std::string* reportUpdates = nullptr; + const std::string* name = nullptr; + const uint64_t* appendLimit = nullptr; const uint64_t* interval = nullptr; - for (const auto& [key, var] : ret) + const bool* enabled = nullptr; + + for (const auto& [key, var] : properties) { - if (key == "EmitsReadingsUpdate") + if (key == "ReportActions") + { + reportActions = std::get_if>(&var); + } + else if (key == "ReportUpdates") { - emitsReadingsUpdate = std::get_if(&var); + reportUpdates = std::get_if(&var); } - else if (key == "LogToMetricReportsCollection") + else if (key == "AppendLimit") { - logToMetricReportsCollection = std::get_if(&var); + appendLimit = std::get_if(&var); } - else if (key == "ReadingParameters") + else if (key == "ReadingParametersFutureVersion") { readingParams = std::get_if(&var); } @@ -72,73 +90,149 @@ inline void fillReportDefinition( { interval = std::get_if(&var); } + else if (key == "Name") + { + name = std::get_if(&var); + } + else if (key == "Enabled") + { + enabled = std::get_if(&var); + } } - if (emitsReadingsUpdate == nullptr || - logToMetricReportsCollection == nullptr || readingParams == nullptr || - reportingType == nullptr || interval == nullptr) + + std::vector redfishReportActions; + if (reportActions != nullptr) { - BMCWEB_LOG_ERROR << "Property type mismatch or property is missing"; - messages::internalError(asyncResp->res); - return; + for (const std::string& action : *reportActions) + { + std::string redfishAction = toReadfishReportAction(action); + + if (redfishAction.empty()) + { + BMCWEB_LOG_ERROR << "Unknown ReportActions element: " << action; + messages::internalError(asyncResp->res); + return; + } + + redfishReportActions.emplace_back(std::move(redfishAction)); + } } - std::vector redfishReportActions; - redfishReportActions.reserve(2); - if (*emitsReadingsUpdate) + asyncResp->res.jsonValue["@odata.type"] = + "#MetricReportDefinition.v1_3_0.MetricReportDefinition"; + asyncResp->res.jsonValue["@odata.id"] = + crow::utility::urlFromPieces("redfish", "v1", "TelemetryService", + "MetricReportDefinitions", id) + .string(); + asyncResp->res.jsonValue["Id"] = id; + asyncResp->res.jsonValue["MetricReport"]["@odata.id"] = + crow::utility::urlFromPieces("redfish", "v1", "TelemetryService", + "MetricReports", id) + .string(); + + if (enabled != nullptr) { - redfishReportActions.emplace_back("RedfishEvent"); + asyncResp->res.jsonValue["Status"]["State"] = + *enabled ? "Enabled" : "Disabled"; + asyncResp->res.jsonValue["MetricReportDefinitionEnabled"] = *enabled; } - if (*logToMetricReportsCollection) + + if (appendLimit != nullptr) { - redfishReportActions.emplace_back("LogToMetricReportsCollection"); + asyncResp->res.jsonValue["AppendLimit"] = *appendLimit; } - nlohmann::json metrics = nlohmann::json::array(); - for (const auto& [sensorPath, operationType, id, metadata] : *readingParams) + if (reportUpdates != nullptr) { - metrics.push_back({ - {"MetricId", id}, - {"MetricProperties", {metadata}}, - }); + asyncResp->res.jsonValue["ReportUpdates"] = *reportUpdates; + } + + if (name != nullptr) + { + asyncResp->res.jsonValue["Name"] = *name; + } + + if (reportActions != nullptr) + { + asyncResp->res.jsonValue["ReportActions"] = + std::move(redfishReportActions); + } + + if (reportingType != nullptr) + { + asyncResp->res.jsonValue["MetricReportDefinitionType"] = *reportingType; + } + + if (interval != nullptr) + { + asyncResp->res.jsonValue["Schedule"]["RecurrenceInterval"] = + time_utils::toDurationString(std::chrono::milliseconds(*interval)); + } + + if (readingParams != nullptr) + { + nlohmann::json& metrics = asyncResp->res.jsonValue["Metrics"]; + metrics = nlohmann::json::array(); + for (auto& [sensorData, collectionFunction, id, collectionTimeScope, + collectionDuration] : *readingParams) + { + std::vector metricProperties; + + for (const auto& [sensorPath, sensorMetadata] : sensorData) + { + metricProperties.emplace_back(sensorMetadata); + } + + metrics.push_back( + {{"MetricId", id}, + {"MetricProperties", std::move(metricProperties)}, + {"CollectionFunction", collectionFunction}, + {"CollectionDuration", + time_utils::toDurationString( + std::chrono::milliseconds(collectionDuration))}, + {"CollectionTimeScope", collectionTimeScope}}); + } } - 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; @@ -150,27 +244,17 @@ inline bool getUserParameters(crow::Response& res, const crow::Request& req, std::vector metrics; std::vector reportActions; std::optional schedule; - if (!json_util::readJsonPatch(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::readJsonPatch( + 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; } - if (args.reportingType != "Periodic" && args.reportingType != "OnRequest") + if (args.reportingType != "Periodic" && args.reportingType != "OnRequest" && + args.reportingType != "OnChange") { messages::propertyValueNotInList(res, args.reportingType, "MetricReportDefinitionType"); @@ -211,15 +295,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; @@ -227,15 +331,14 @@ inline bool getUserParameters(crow::Response& res, const crow::Request& req, inline bool getChassisSensorNode( 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) { - for (size_t i = 0; i < uris.size(); i++) + for (size_t i = 0; i < metric.uris.size(); i++) { - const std::string& uri = uris[i]; + const std::string& uri = metric.uris[i]; std::string chassis; std::string node; @@ -280,11 +383,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()) { @@ -298,17 +406,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) @@ -338,10 +452,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); } AddReport(const AddReport&) = delete; @@ -436,10 +552,10 @@ inline void requestRoutesMetricReportDefinition(App& app) const std::string& id) { crow::connections::systemBus->async_method_call( [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) { @@ -454,12 +570,14 @@ inline void requestRoutesMetricReportDefinition(App& app) return; } - telemetry::fillReportDefinition(asyncResp, id, ret); + telemetry::fillReportDefinition(asyncResp, id, + properties); }, telemetry::service, telemetry::getDbusReportPath(id), "org.freedesktop.DBus.Properties", "GetAll", telemetry::reportInterface); }); + BMCWEB_ROUTE(app, "/redfish/v1/TelemetryService/MetricReportDefinitions//") .privileges(redfish::privileges::deleteMetricReportDefinitionCollection) diff --git a/redfish-core/lib/telemetry_service.hpp b/redfish-core/lib/telemetry_service.hpp index c1fe7d0..64d712b 100644 --- a/redfish-core/lib/telemetry_service.hpp +++ b/redfish-core/lib/telemetry_service.hpp @@ -49,6 +49,8 @@ inline void handleTelemetryServiceGet( const size_t* maxReports = nullptr; const uint64_t* minInterval = nullptr; + const std::vector* supportedCollectionFunction = + nullptr; for (const auto& [key, var] : ret) { if (key == "MaxReports") @@ -59,6 +61,11 @@ inline void handleTelemetryServiceGet( { minInterval = std::get_if(&var); } + else if (key == "SupportedOperationTypes") + { + supportedCollectionFunction = + std::get_if>(&var); + } } if (maxReports == nullptr || minInterval == nullptr) { @@ -72,6 +79,12 @@ inline void handleTelemetryServiceGet( asyncResp->res.jsonValue["MinCollectionInterval"] = time_utils::toDurationString(std::chrono::milliseconds( static_cast(*minInterval))); + + if (supportedCollectionFunction != nullptr) + { + asyncResp->res.jsonValue["SupportedCollectionFunction"] = + *supportedCollectionFunction; + } }, telemetry::service, "/xyz/openbmc_project/Telemetry/Reports", "org.freedesktop.DBus.Properties", "GetAll", -- 2.25.1