path: root/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb/telemetry/0003-Switched-bmcweb-to-use-new-telemetry-service-API.patch

From 41ee20f46f2c6d2e9e5418128993cd176d03a927 Mon Sep 17 00:00:00 2001
From: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
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 <krzysztof.grobelny@intel.com>
Signed-off-by: Lukasz Kazmierczak <lukasz.kazmierczak@intel.com>
---
 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::tuple<std::string, std::string>>,
     std::vector<std::tuple<uint32_t, std::vector<uint32_t>>>,
     std::vector<std::tuple<uint32_t, size_t>>,
-    std::vector<std::tuple<sdbusplus::message::object_path, std::string,
-                           std::string, std::string>>
+    std::vector<std::tuple<
+        std::vector<std::tuple<sdbusplus::message::object_path, std::string>>,
+            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 <boost/container/flat_map.hpp>
 #include <dbus_utility.hpp>
 #include <registries/privilege_registry.hpp>
+#include <sdbusplus/asio/property.hpp>
+#include <sdbusplus/unpack_properties.hpp>
 
 #include <tuple>
 #include <variant>
@@ -18,119 +20,156 @@ namespace redfish
 namespace telemetry
 {
 
-using ReadingParameters =
-    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>>;
 
-inline void fillReportDefinition(
-    const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, const std::string& id,
-    const std::vector<std::pair<std::string, dbus::utility::DbusVariantType>>&
-        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<bool>(&var);
-        }
-        else if (key == "LogToMetricReportsCollection")
-        {
-            logToMetricReportsCollection = std::get_if<bool>(&var);
-        }
-        else if (key == "ReadingParameters")
-        {
-            readingParams = std::get_if<ReadingParameters>(&var);
-        }
-        else if (key == "ReportingType")
-        {
-            reportingType = std::get_if<std::string>(&var);
-        }
-        else if (key == "Interval")
-        {
-            interval = std::get_if<uint64_t>(&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<std::string> 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<bmcweb::AsyncResp>& asyncResp, const std::string& id,
+    const std::vector<std::pair<std::string, dbus::utility::DbusVariantType>>&
+        properties)
+{
+    try
     {
-        metrics.push_back({
-            {"MetricId", id},
-            {"MetricProperties", {metadata}},
-        });
+        std::vector<std::string> 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<std::string> 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<std::string> uris;
+        std::optional<std::string> collectionFunction;
+        std::optional<std::string> collectionTimeScope;
+        std::optional<uint64_t> collectionDuration;
+    };
+
+    std::optional<std::string> id;
+    std::optional<std::string> name;
     std::string reportingType;
-    bool emitsReadingsUpdate = false;
-    bool logToMetricReportsCollection = false;
+    std::optional<std::string> reportUpdates;
+    std::optional<uint64_t> appendLimit;
+    std::vector<std::string> reportActions;
     uint64_t interval = 0;
-    std::vector<std::pair<std::string, std::vector<std::string>>> metrics;
+    std::vector<MetricArgs> metrics;
 };
 
 inline bool toDbusReportActions(crow::Response& res,
-                                std::vector<std::string>& actions,
+                                const std::vector<std::string>& 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<nlohmann::json> metrics;
     std::vector<std::string> reportActions;
     std::optional<nlohmann::json> 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<std::string> uris;
-        if (!json_util::readJson(m, res, "MetricId", id, "MetricProperties",
-                                 uris))
+        std::optional<std::string> 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<std::chrono::milliseconds> duration =
+                time_utils::fromDurationString(*collectionDurationStr);
+
+            if (!duration || duration->count() < 0)
+            {
+                messages::propertyValueIncorrect(res, "CollectionDuration",
+                                                 *collectionDurationStr);
+                return false;
+            }
+
+            metricArgs.collectionDuration =
+                static_cast<uint64_t>(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<bmcweb::AsyncResp>& asyncResp,
-    const std::vector<std::pair<std::string, std::vector<std::string>>>&
-        metrics,
+    const std::vector<AddReportArgs::MetricArgs>& metrics,
     boost::container::flat_set<std::pair<std::string, std::string>>& 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<sdbusplus::message::object_path, std::string>>
+                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<bmcweb::AsyncResp> 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<std::string, std::string>& el)
@@ -399,12 +459,15 @@ inline void requestRoutesMetricReportDefinition(App& app)
             [](const crow::Request&,
                const std::shared_ptr<bmcweb::AsyncResp>& 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<std::pair<
                              std::string, dbus::utility::DbusVariantType>>&
-                             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/<str>/")
         .privileges(redfish::privileges::deleteMetricReportDefinitionCollection)
-- 
2.25.1