Switched bmcweb to use new telemetry service API

Added support for multiple MetricProperties. Added support for new
parameters: CollectionTimeScope, CollectionDuration. ReadingParameters
was not yet changed in telemetry backend, instead temporary property
ReadingParametersFutureVersion was introduced. Once bmcweb is adapted to
use ReadingParametersFutureVersion this property will be renamed in
backend to ReadingParameters. Then bmcweb will change to use
ReadingParameters. Then ReadingParametersFutureVersion will be removed
from backend and everything will be exactly like described in
phosphor-dbus-interfaces without introducing breaking changes.

Related change in phosphor-dbus-interfaces [1], [2]. This change needs
to be bumped together with [3].

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.

[1]: https://github.com/openbmc/phosphor-dbus-interfaces/commit/4f9c09144b60edc015291d2c120fc5b33aa0bec2
[2]: https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/60750
[3]: https://gerrit.openbmc.org/c/openbmc/telemetry/+/58229

Change-Id: I2cd17069e3ea015c8f5571c29278f1d50536272a
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Signed-off-by: Lukasz Kazmierczak <lukasz.kazmierczak@intel.com>
diff --git a/redfish-core/include/utils/json_utils.hpp b/redfish-core/include/utils/json_utils.hpp
index 774ab7c..fa49d3f 100644
--- a/redfish-core/include/utils/json_utils.hpp
+++ b/redfish-core/include/utils/json_utils.hpp
@@ -589,6 +589,5 @@
     }
     return readJson(jsonRequest, res, key, std::forward<UnpackTypes&&>(in)...);
 }
-
 } // namespace json_util
 } // namespace redfish
diff --git a/redfish-core/include/utils/telemetry_utils.hpp b/redfish-core/include/utils/telemetry_utils.hpp
index f5f9360..fc045f6 100644
--- a/redfish-core/include/utils/telemetry_utils.hpp
+++ b/redfish-core/include/utils/telemetry_utils.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include "dbus_utility.hpp"
+#include "generated/enums/metric_report_definition.hpp"
 #include "http/utility.hpp"
 #include "logging.hpp"
 #include "utility.hpp"
@@ -40,7 +41,7 @@
 };
 
 inline std::optional<IncorrectMetricUri> getChassisSensorNode(
-    const std::vector<std::string>& uris,
+    std::span<const std::string> uris,
     boost::container::flat_set<std::pair<std::string, std::string>>& matched)
 {
     size_t uriIdx = 0;
@@ -89,5 +90,74 @@
     return std::nullopt;
 }
 
+inline metric_report_definition::CalculationAlgorithmEnum
+    toRedfishCollectionFunction(std::string_view dbusValue)
+{
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.OperationType.Maximum")
+    {
+        return metric_report_definition::CalculationAlgorithmEnum::Maximum;
+    }
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.OperationType.Minimum")
+    {
+        return metric_report_definition::CalculationAlgorithmEnum::Minimum;
+    }
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.OperationType.Average")
+    {
+        return metric_report_definition::CalculationAlgorithmEnum::Average;
+    }
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.OperationType.Summation")
+    {
+        return metric_report_definition::CalculationAlgorithmEnum::Summation;
+    }
+    return metric_report_definition::CalculationAlgorithmEnum::Invalid;
+}
+
+inline std::string toDbusCollectionFunction(std::string_view redfishValue)
+{
+    if (redfishValue == "Maximum")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.OperationType.Maximum";
+    }
+    if (redfishValue == "Minimum")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.OperationType.Minimum";
+    }
+    if (redfishValue == "Average")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.OperationType.Average";
+    }
+    if (redfishValue == "Summation")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.OperationType.Summation";
+    }
+    return "";
+}
+
+inline std::optional<nlohmann::json::array_t>
+    toRedfishCollectionFunctions(std::span<const std::string> dbusEnums)
+{
+    nlohmann::json::array_t redfishEnums;
+    redfishEnums.reserve(dbusEnums.size());
+
+    for (const auto& dbusValue : dbusEnums)
+    {
+        metric_report_definition::CalculationAlgorithmEnum redfishValue =
+            toRedfishCollectionFunction(dbusValue);
+
+        if (redfishValue ==
+            metric_report_definition::CalculationAlgorithmEnum::Invalid)
+        {
+            return std::nullopt;
+        }
+
+        redfishEnums.emplace_back(redfishValue);
+    }
+    return redfishEnums;
+}
+
 } // namespace telemetry
 } // namespace redfish
diff --git a/redfish-core/lib/metric_report.hpp b/redfish-core/lib/metric_report.hpp
index 1f21c91..6f26d8a 100644
--- a/redfish-core/lib/metric_report.hpp
+++ b/redfish-core/lib/metric_report.hpp
@@ -20,18 +20,16 @@
 namespace telemetry
 {
 
-using Readings =
-    std::vector<std::tuple<std::string, std::string, double, uint64_t>>;
+using Readings = std::vector<std::tuple<std::string, double, uint64_t>>;
 using TimestampReadings = std::tuple<uint64_t, Readings>;
 
 inline nlohmann::json toMetricValues(const Readings& readings)
 {
     nlohmann::json metricValues = nlohmann::json::array_t();
 
-    for (const auto& [id, metadata, sensorValue, timestamp] : readings)
+    for (const auto& [metadata, sensorValue, timestamp] : readings)
     {
         nlohmann::json::object_t metricReport;
-        metricReport["MetricId"] = id;
         metricReport["MetricProperty"] = metadata;
         metricReport["MetricValue"] = std::to_string(sensorValue);
         metricReport["Timestamp"] =
diff --git a/redfish-core/lib/metric_report_definition.hpp b/redfish-core/lib/metric_report_definition.hpp
index 7f8a29f..185c298 100644
--- a/redfish-core/lib/metric_report_definition.hpp
+++ b/redfish-core/lib/metric_report_definition.hpp
@@ -2,6 +2,7 @@
 
 #include "app.hpp"
 #include "dbus_utility.hpp"
+#include "generated/enums/metric_report_definition.hpp"
 #include "query.hpp"
 #include "registries/privilege_registry.hpp"
 #include "sensors.hpp"
@@ -27,37 +28,173 @@
 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, uint64_t>>;
+
+inline metric_report_definition::ReportActionsEnum
+    toRedfishReportAction(std::string_view dbusValue)
+{
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.ReportActions.EmitsReadingsUpdate")
+    {
+        return metric_report_definition::ReportActionsEnum::RedfishEvent;
+    }
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.ReportActions.LogToMetricReportsCollection")
+    {
+        return metric_report_definition::ReportActionsEnum::
+            LogToMetricReportsCollection;
+    }
+    return metric_report_definition::ReportActionsEnum::Invalid;
+}
+
+inline std::string toDbusReportAction(std::string_view redfishValue)
+{
+    if (redfishValue == "RedfishEvent")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.ReportActions.EmitsReadingsUpdate";
+    }
+    if (redfishValue == "LogToMetricReportsCollection")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.ReportActions.LogToMetricReportsCollection";
+    }
+    return "";
+}
+
+inline metric_report_definition::MetricReportDefinitionType
+    toRedfishReportingType(std::string_view dbusValue)
+{
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.ReportingType.OnChange")
+    {
+        return metric_report_definition::MetricReportDefinitionType::OnChange;
+    }
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.ReportingType.OnRequest")
+    {
+        return metric_report_definition::MetricReportDefinitionType::OnRequest;
+    }
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.ReportingType.Periodic")
+    {
+        return metric_report_definition::MetricReportDefinitionType::Periodic;
+    }
+    return metric_report_definition::MetricReportDefinitionType::Invalid;
+}
+
+inline std::string toDbusReportingType(std::string_view redfishValue)
+{
+    if (redfishValue == "OnChange")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.ReportingType.OnChange";
+    }
+    if (redfishValue == "OnRequest")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.ReportingType.OnRequest";
+    }
+    if (redfishValue == "Periodic")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.ReportingType.Periodic";
+    }
+    return "";
+}
+
+inline metric_report_definition::CollectionTimeScope
+    toRedfishCollectionTimeScope(std::string_view dbusValue)
+{
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.CollectionTimescope.Point")
+    {
+        return metric_report_definition::CollectionTimeScope::Point;
+    }
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.CollectionTimescope.Interval")
+    {
+        return metric_report_definition::CollectionTimeScope::Interval;
+    }
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.CollectionTimescope.StartupInterval")
+    {
+        return metric_report_definition::CollectionTimeScope::StartupInterval;
+    }
+    return metric_report_definition::CollectionTimeScope::Invalid;
+}
+
+inline std::string toDbusCollectionTimeScope(std::string_view redfishValue)
+{
+    if (redfishValue == "Point")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.CollectionTimescope.Point";
+    }
+    if (redfishValue == "Interval")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.CollectionTimescope.Interval";
+    }
+    if (redfishValue == "StartupInterval")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.CollectionTimescope.StartupInterval";
+    }
+    return "";
+}
+
+inline metric_report_definition::ReportUpdatesEnum
+    toRedfishReportUpdates(std::string_view dbusValue)
+{
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.ReportUpdates.Overwrite")
+    {
+        return metric_report_definition::ReportUpdatesEnum::Overwrite;
+    }
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.ReportUpdates.AppendWrapsWhenFull")
+    {
+        return metric_report_definition::ReportUpdatesEnum::AppendWrapsWhenFull;
+    }
+    if (dbusValue ==
+        "xyz.openbmc_project.Telemetry.Report.ReportUpdates.AppendStopsWhenFull")
+    {
+        return metric_report_definition::ReportUpdatesEnum::AppendStopsWhenFull;
+    }
+    return metric_report_definition::ReportUpdatesEnum::Invalid;
+}
+
+inline std::string toDbusReportUpdates(std::string_view redfishValue)
+{
+    if (redfishValue == "Overwrite")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.ReportUpdates.Overwrite";
+    }
+    if (redfishValue == "AppendWrapsWhenFull")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.ReportUpdates.AppendWrapsWhenFull";
+    }
+    if (redfishValue == "AppendStopsWhenFull")
+    {
+        return "xyz.openbmc_project.Telemetry.Report.ReportUpdates.AppendStopsWhenFull";
+    }
+    return "";
+}
 
 inline void
     fillReportDefinition(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
                          const std::string& id,
-                         const dbus::utility::DBusPropertiesMap& ret)
+                         const dbus::utility::DBusPropertiesMap& properties)
 {
-    asyncResp->res.jsonValue["@odata.type"] =
-        "#MetricReportDefinition.v1_3_0.MetricReportDefinition";
-    asyncResp->res.jsonValue["@odata.id"] = boost::urls::format(
-        "/redfish/v1/TelemetryService/MetricReportDefinitions/{}", id);
-    asyncResp->res.jsonValue["Id"] = id;
-    asyncResp->res.jsonValue["Name"] = id;
-    asyncResp->res.jsonValue["MetricReport"]["@odata.id"] = boost::urls::format(
-        "/redfish/v1/TelemetryService/MetricReports/{}", id);
-    asyncResp->res.jsonValue["Status"]["State"] = "Enabled";
-    asyncResp->res.jsonValue["ReportUpdates"] = "Overwrite";
-
-    const bool* emitsReadingsUpdate = nullptr;
-    const bool* logToMetricReportsCollection = nullptr;
-    const ReadingParameters* readingParameters = nullptr;
-    const std::string* reportingType = nullptr;
-    const uint64_t* interval = nullptr;
+    std::vector<std::string> reportActions;
+    ReadingParameters readingParams;
+    std::string reportingType;
+    std::string reportUpdates;
+    std::string name;
+    uint64_t appendLimit = 0;
+    uint64_t interval = 0;
+    bool enabled = false;
 
     const bool success = sdbusplus::unpackPropertiesNoThrow(
-        dbus_utils::UnpackErrorPrinter(), ret, "EmitsReadingsUpdate",
-        emitsReadingsUpdate, "LogToMetricReportsCollection",
-        logToMetricReportsCollection, "ReadingParameters", readingParameters,
-        "ReportingType", reportingType, "Interval", interval);
+        dbus_utils::UnpackErrorPrinter(), properties, "ReportingType",
+        reportingType, "Interval", interval, "ReportActions", reportActions,
+        "ReportUpdates", reportUpdates, "AppendLimit", appendLimit,
+        "ReadingParameters", readingParams, "Name", name, "Enabled", enabled);
 
     if (!success)
     {
@@ -65,122 +202,324 @@
         return;
     }
 
-    std::vector<std::string> redfishReportActions;
-    redfishReportActions.reserve(2);
-    if (emitsReadingsUpdate != nullptr && *emitsReadingsUpdate)
+    metric_report_definition::MetricReportDefinitionType redfishReportingType =
+        toRedfishReportingType(reportingType);
+    if (redfishReportingType ==
+        metric_report_definition::MetricReportDefinitionType::Invalid)
     {
-        redfishReportActions.emplace_back("RedfishEvent");
+        messages::internalError(asyncResp->res);
+        return;
     }
 
-    if (logToMetricReportsCollection != nullptr &&
-        *logToMetricReportsCollection)
-    {
-        redfishReportActions.emplace_back("LogToMetricReportsCollection");
-    }
+    asyncResp->res.jsonValue["MetricReportDefinitionType"] =
+        redfishReportingType;
 
-    nlohmann::json metrics = nlohmann::json::array();
-    if (readingParameters != nullptr)
+    nlohmann::json::array_t redfishReportActions;
+    for (const std::string& action : reportActions)
     {
-        for (const auto& [sensorPath, operationType, metricId, metadata] :
-             *readingParameters)
+        metric_report_definition::ReportActionsEnum redfishAction =
+            toRedfishReportAction(action);
+        if (redfishAction ==
+            metric_report_definition::ReportActionsEnum::Invalid)
         {
-            nlohmann::json::object_t metric;
-            metric["MetricId"] = metricId;
-            metric["MetricProperties"] = nlohmann::json::array_t({metadata});
-            metrics.emplace_back(std::move(metric));
+            messages::internalError(asyncResp->res);
+            return;
         }
+
+        redfishReportActions.emplace_back(redfishAction);
     }
 
-    if (reportingType != nullptr)
+    asyncResp->res.jsonValue["ReportActions"] = std::move(redfishReportActions);
+
+    nlohmann::json::array_t metrics = nlohmann::json::array();
+    for (const auto& [sensorData, collectionFunction, collectionTimeScope,
+                      collectionDuration] : readingParams)
     {
-        asyncResp->res.jsonValue["MetricReportDefinitionType"] = *reportingType;
-    }
+        nlohmann::json::array_t metricProperties;
 
-    if (interval != nullptr)
+        for (const auto& [sensorPath, sensorMetadata] : sensorData)
+        {
+            metricProperties.emplace_back(sensorMetadata);
+        }
+
+        nlohmann::json::object_t metric;
+
+        metric_report_definition::CalculationAlgorithmEnum
+            redfishCollectionFunction =
+                telemetry::toRedfishCollectionFunction(collectionFunction);
+        if (redfishCollectionFunction ==
+            metric_report_definition::CalculationAlgorithmEnum::Invalid)
+        {
+            messages::internalError(asyncResp->res);
+            return;
+        }
+        metric["CollectionFunction"] = redfishCollectionFunction;
+
+        metric_report_definition::CollectionTimeScope
+            redfishCollectionTimeScope =
+                toRedfishCollectionTimeScope(collectionTimeScope);
+        if (redfishCollectionTimeScope ==
+            metric_report_definition::CollectionTimeScope::Invalid)
+        {
+            messages::internalError(asyncResp->res);
+            return;
+        }
+        metric["CollectionTimeScope"] = redfishCollectionTimeScope;
+
+        metric["MetricProperties"] = std::move(metricProperties);
+        metric["CollectionDuration"] = time_utils::toDurationString(
+            std::chrono::milliseconds(collectionDuration));
+        metrics.emplace_back(std::move(metric));
+    }
+    asyncResp->res.jsonValue["Metrics"] = std::move(metrics);
+
+    if (enabled)
     {
-        asyncResp->res.jsonValue["Schedule"]["RecurrenceInterval"] =
-            time_utils::toDurationString(std::chrono::milliseconds(*interval));
+        asyncResp->res.jsonValue["Status"]["State"] = "Enabled";
+    }
+    else
+    {
+        asyncResp->res.jsonValue["Status"]["State"] = "Disabled";
     }
 
-    asyncResp->res.jsonValue["Metrics"] = metrics;
-    asyncResp->res.jsonValue["ReportActions"] = redfishReportActions;
+    metric_report_definition::ReportUpdatesEnum redfishReportUpdates =
+        toRedfishReportUpdates(reportUpdates);
+    if (redfishReportUpdates ==
+        metric_report_definition::ReportUpdatesEnum::Invalid)
+    {
+        messages::internalError(asyncResp->res);
+        return;
+    }
+    asyncResp->res.jsonValue["ReportUpdates"] = redfishReportUpdates;
+
+    asyncResp->res.jsonValue["MetricReportDefinitionEnabled"] = enabled;
+    asyncResp->res.jsonValue["AppendLimit"] = appendLimit;
+    asyncResp->res.jsonValue["Name"] = name;
+    asyncResp->res.jsonValue["Schedule"]["RecurrenceInterval"] =
+        time_utils::toDurationString(std::chrono::milliseconds(interval));
+    asyncResp->res.jsonValue["@odata.type"] =
+        "#MetricReportDefinition.v1_3_0.MetricReportDefinition";
+    asyncResp->res.jsonValue["@odata.id"] = boost::urls::format(
+        "/redfish/v1/TelemetryService/MetricReportDefinitions/{}", id);
+    asyncResp->res.jsonValue["Id"] = id;
+    asyncResp->res.jsonValue["MetricReport"]["@odata.id"] = boost::urls::format(
+        "/redfish/v1/TelemetryService/MetricReports/{}", id);
 }
 
 struct AddReportArgs
 {
+    struct MetricArgs
+    {
+        std::vector<std::string> uris;
+        std::string collectionFunction;
+        std::string collectionTimeScope;
+        uint64_t collectionDuration = 0;
+    };
+
+    std::string id;
     std::string name;
     std::string reportingType;
-    bool emitsReadingsUpdate = false;
-    bool logToMetricReportsCollection = false;
-    uint64_t interval = 0;
-    std::vector<std::pair<std::string, std::vector<std::string>>> metrics;
+    std::string reportUpdates;
+    uint64_t appendLimit = std::numeric_limits<uint64_t>::max();
+    std::vector<std::string> reportActions;
+    uint64_t interval = std::numeric_limits<uint64_t>::max();
+    std::vector<MetricArgs> metrics;
+    bool metricReportDefinitionEnabled = true;
 };
 
 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 std::string& action : actions)
     {
-        if (action == "RedfishEvent")
+        std::string dbusReportAction = toDbusReportAction(action);
+        if (dbusReportAction.empty())
         {
-            args.emitsReadingsUpdate = true;
-        }
-        else if (action == "LogToMetricReportsCollection")
-        {
-            args.logToMetricReportsCollection = true;
-        }
-        else
-        {
-            messages::propertyValueNotInList(
-                res, action, "ReportActions/" + std::to_string(index));
+            messages::propertyValueNotInList(res, nlohmann::json(action).dump(),
+                                             "ReportActions/" +
+                                                 std::to_string(index));
             return false;
         }
+
+        args.reportActions.emplace_back(std::move(dbusReportAction));
         index++;
     }
     return true;
 }
 
+inline bool getUserMetric(crow::Response& res, nlohmann::json& metric,
+                          AddReportArgs::MetricArgs& metricArgs)
+{
+    std::optional<std::vector<std::string>> uris;
+    std::optional<std::string> collectionDurationStr;
+    std::optional<std::string> collectionFunction;
+    std::optional<std::string> collectionTimeScopeStr;
+
+    if (!json_util::readJson(metric, res, "MetricProperties", uris,
+                             "CollectionFunction", collectionFunction,
+                             "CollectionTimeScope", collectionTimeScopeStr,
+                             "CollectionDuration", collectionDurationStr))
+    {
+        return false;
+    }
+
+    if (uris)
+    {
+        metricArgs.uris = std::move(*uris);
+    }
+
+    if (collectionFunction)
+    {
+        std::string dbusCollectionFunction =
+            telemetry::toDbusCollectionFunction(*collectionFunction);
+        if (dbusCollectionFunction.empty())
+        {
+            messages::propertyValueIncorrect(res, "CollectionFunction",
+                                             *collectionFunction);
+            return false;
+        }
+        metricArgs.collectionFunction = std::move(dbusCollectionFunction);
+    }
+
+    if (collectionTimeScopeStr)
+    {
+        std::string dbusCollectionTimeScope =
+            toDbusCollectionTimeScope(*collectionTimeScopeStr);
+        if (dbusCollectionTimeScope.empty())
+        {
+            messages::propertyValueIncorrect(res, "CollectionTimeScope",
+                                             *collectionTimeScopeStr);
+            return false;
+        }
+        metricArgs.collectionTimeScope = std::move(dbusCollectionTimeScope);
+    }
+
+    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());
+    }
+
+    return true;
+}
+
+inline bool getUserMetrics(crow::Response& res,
+                           std::span<nlohmann::json> metrics,
+                           std::vector<AddReportArgs::MetricArgs>& result)
+{
+    result.reserve(metrics.size());
+
+    for (nlohmann::json& m : metrics)
+    {
+        AddReportArgs::MetricArgs metricArgs;
+
+        if (!getUserMetric(res, m, metricArgs))
+        {
+            return false;
+        }
+
+        result.emplace_back(std::move(metricArgs));
+    }
+
+    return true;
+}
+
 inline bool getUserParameters(crow::Response& res, const crow::Request& req,
                               AddReportArgs& args)
 {
-    std::vector<nlohmann::json> metrics;
-    std::vector<std::string> reportActions;
+    std::optional<std::string> id;
+    std::optional<std::string> name;
+    std::optional<std::string> reportingTypeStr;
+    std::optional<std::string> reportUpdatesStr;
+    std::optional<uint64_t> appendLimit;
+    std::optional<bool> metricReportDefinitionEnabled;
+    std::optional<std::vector<nlohmann::json>> metrics;
+    std::optional<std::vector<std::string>> reportActionsStr;
     std::optional<nlohmann::json> schedule;
-    if (!json_util::readJsonPatch(req, res, "Id", args.name, "Metrics", metrics,
-                                  "MetricReportDefinitionType",
-                                  args.reportingType, "ReportActions",
-                                  reportActions, "Schedule", schedule))
+
+    if (!json_util::readJsonPatch(
+            req, res, "Id", id, "Name", name, "Metrics", metrics,
+            "MetricReportDefinitionType", reportingTypeStr, "ReportUpdates",
+            reportUpdatesStr, "AppendLimit", appendLimit, "ReportActions",
+            reportActionsStr, "Schedule", schedule,
+            "MetricReportDefinitionEnabled", metricReportDefinitionEnabled))
     {
         return false;
     }
 
-    constexpr const char* allowedCharactersInName =
-        "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_";
-    if (args.name.empty() || args.name.find_first_not_of(
-                                 allowedCharactersInName) != std::string::npos)
+    if (id)
     {
-        BMCWEB_LOG_ERROR << "Failed to match " << args.name
-                         << " with allowed character "
-                         << allowedCharactersInName;
-        messages::propertyValueIncorrect(res, "Id", args.name);
-        return false;
+        constexpr const char* allowedCharactersInId =
+            "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_";
+        if (id->empty() ||
+            id->find_first_not_of(allowedCharactersInId) != std::string::npos)
+        {
+            messages::propertyValueIncorrect(res, "Id", *id);
+            return false;
+        }
+        args.id = *id;
     }
 
-    if (args.reportingType != "Periodic" && args.reportingType != "OnRequest")
+    if (name)
     {
-        messages::propertyValueNotInList(res, args.reportingType,
-                                         "MetricReportDefinitionType");
-        return false;
+        args.name = *name;
     }
 
-    if (!toDbusReportActions(res, reportActions, args))
+    if (reportingTypeStr)
     {
-        return false;
+        std::string dbusReportingType = toDbusReportingType(*reportingTypeStr);
+        if (dbusReportingType.empty())
+        {
+            messages::propertyValueNotInList(res, *reportingTypeStr,
+                                             "MetricReportDefinitionType");
+            return false;
+        }
+        args.reportingType = dbusReportingType;
     }
 
-    if (args.reportingType == "Periodic")
+    if (reportUpdatesStr)
+    {
+        std::string dbusReportUpdates = toDbusReportUpdates(*reportUpdatesStr);
+        if (dbusReportUpdates.empty())
+        {
+            messages::propertyValueNotInList(res, *reportUpdatesStr,
+                                             "ReportUpdates");
+            return false;
+        }
+        args.reportUpdates = dbusReportUpdates;
+    }
+
+    if (appendLimit)
+    {
+        args.appendLimit = *appendLimit;
+    }
+
+    if (metricReportDefinitionEnabled)
+    {
+        args.metricReportDefinitionEnabled = *metricReportDefinitionEnabled;
+    }
+
+    if (reportActionsStr)
+    {
+        if (!toDbusReportActions(res, *reportActionsStr, args))
+        {
+            return false;
+        }
+    }
+
+    if (reportingTypeStr == "Periodic")
     {
         if (!schedule)
         {
@@ -197,7 +536,7 @@
 
         std::optional<std::chrono::milliseconds> durationNum =
             time_utils::fromDurationString(durationStr);
-        if (!durationNum)
+        if (!durationNum || durationNum->count() < 0)
         {
             messages::propertyValueIncorrect(res, "RecurrenceInterval",
                                              durationStr);
@@ -206,18 +545,12 @@
         args.interval = static_cast<uint64_t>(durationNum->count());
     }
 
-    args.metrics.reserve(metrics.size());
-    for (auto& m : metrics)
+    if (metrics)
     {
-        std::string id;
-        std::vector<std::string> uris;
-        if (!json_util::readJson(m, res, "MetricId", id, "MetricProperties",
-                                 uris))
+        if (!getUserMetrics(res, *metrics, args.metrics))
         {
             return false;
         }
-
-        args.metrics.emplace_back(std::move(id), std::move(uris));
     }
 
     return true;
@@ -225,16 +558,13 @@
 
 inline bool getChassisSensorNodeFromMetrics(
     const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
-    const std::vector<std::pair<std::string, std::vector<std::string>>>&
-        metrics,
+    std::span<const AddReportArgs::MetricArgs> metrics,
     boost::container::flat_set<std::pair<std::string, std::string>>& matched)
 {
     for (const auto& metric : metrics)
     {
-        const std::vector<std::string>& uris = metric.second;
-
-        std::optional<IncorrectMetricUri> error = getChassisSensorNode(uris,
-                                                                       matched);
+        std::optional<IncorrectMetricUri> error =
+            getChassisSensorNode(metric.uris, matched);
         if (error)
         {
             messages::propertyValueIncorrect(asyncResp->res, error->uri,
@@ -254,8 +584,19 @@
         asyncResp(asyncRespIn),
         args{std::move(argsIn)}
     {}
+
     ~AddReport()
     {
+        boost::asio::post(crow::connections::systemBus->get_io_context(),
+                          std::bind_front(&performAddReport, asyncResp, args,
+                                          std::move(uriToDbus)));
+    }
+
+    static void performAddReport(
+        const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+        const AddReportArgs& args,
+        const boost::container::flat_map<std::string, std::string>& uriToDbus)
+    {
         if (asyncResp->res.result() != boost::beast::http::status::ok)
         {
             return;
@@ -264,11 +605,16 @@
         telemetry::ReadingParameters readingParams;
         readingParams.reserve(args.metrics.size());
 
-        for (const auto& [id, uris] : args.metrics)
+        for (const 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())
                 {
@@ -282,22 +628,26 @@
                 }
 
                 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,
+                metric.collectionTimeScope, metric.collectionDuration);
         }
-        const std::shared_ptr<bmcweb::AsyncResp> aResp = asyncResp;
+
         crow::connections::systemBus->async_method_call(
-            [aResp, name = args.name, uriToDbus = std::move(uriToDbus)](
+            [asyncResp, id = args.id, uriToDbus](
                 const boost::system::error_code& ec, const std::string&) {
             if (ec == boost::system::errc::file_exists)
             {
                 messages::resourceAlreadyExists(
-                    aResp->res, "MetricReportDefinition", "Id", name);
+                    asyncResp->res, "MetricReportDefinition", "Id", id);
                 return;
             }
             if (ec == boost::system::errc::too_many_files_open)
             {
-                messages::createLimitReachedForResource(aResp->res);
+                messages::createLimitReachedForResource(asyncResp->res);
                 return;
             }
             if (ec == boost::system::errc::argument_list_too_long)
@@ -307,24 +657,25 @@
                 {
                     metricProperties.emplace_back(uri);
                 }
-                messages::propertyValueIncorrect(
-                    aResp->res, metricProperties.dump(), "MetricProperties");
+                messages::propertyValueIncorrect(asyncResp->res,
+                                                 metricProperties.dump(),
+                                                 "MetricProperties");
                 return;
             }
             if (ec)
             {
-                messages::internalError(aResp->res);
+                messages::internalError(asyncResp->res);
                 BMCWEB_LOG_ERROR << "respHandler DBus error " << ec;
                 return;
             }
 
-            messages::created(aResp->res);
+            messages::created(asyncResp->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);
+            "TelemetryService/" + args.id, args.name, args.reportingType,
+            args.reportUpdates, args.appendLimit, args.reportActions,
+            args.interval, readingParams, args.metricReportDefinitionEnabled);
     }
 
     AddReport(const AddReport&) = delete;
@@ -338,7 +689,7 @@
     }
 
   private:
-    const std::shared_ptr<bmcweb::AsyncResp> asyncResp;
+    std::shared_ptr<bmcweb::AsyncResp> asyncResp;
     AddReportArgs args;
     boost::container::flat_map<std::string, std::string> uriToDbus{};
 };
@@ -437,8 +788,7 @@
             telemetry::getDbusReportPath(id), telemetry::reportInterface,
             [asyncResp,
              id](const boost::system::error_code& ec,
-                 const std::vector<std::pair<
-                     std::string, dbus::utility::DbusVariantType>>& ret) {
+                 const dbus::utility::DBusPropertiesMap& properties) {
             if (ec.value() == EBADR ||
                 ec == boost::system::errc::host_unreachable)
             {
@@ -453,9 +803,10 @@
                 return;
             }
 
-            telemetry::fillReportDefinition(asyncResp, id, ret);
+            telemetry::fillReportDefinition(asyncResp, id, properties);
             });
         });
+
     BMCWEB_ROUTE(app,
                  "/redfish/v1/TelemetryService/MetricReportDefinitions/<str>/")
         .privileges(redfish::privileges::deleteMetricReportDefinitionCollection)
diff --git a/redfish-core/lib/telemetry_service.hpp b/redfish-core/lib/telemetry_service.hpp
index f9df850..9cc5ccb 100644
--- a/redfish-core/lib/telemetry_service.hpp
+++ b/redfish-core/lib/telemetry_service.hpp
@@ -79,6 +79,14 @@
                 time_utils::toDurationString(std::chrono::milliseconds(
                     static_cast<time_t>(*minInterval)));
         }
+        nlohmann::json::array_t supportedCollectionFunctions;
+        supportedCollectionFunctions.emplace_back("Maximum");
+        supportedCollectionFunctions.emplace_back("Minimum");
+        supportedCollectionFunctions.emplace_back("Average");
+        supportedCollectionFunctions.emplace_back("Summation");
+
+        asyncResp->res.jsonValue["SupportedCollectionFunctions"] =
+            std::move(supportedCollectionFunctions);
         });
 }