1120: Add support for MetricDefinition scheme (#1088) #1284

rfrandse · 2025-05-09T18:16:05Z

1120: Add support for MetricDefinition scheme (1110 PR: #1088)

* Add support for MetricDefinition scheme

Added MetricDefinition node to Redfish code. Now user is able to list
all available metrics in OpenBMC that are supported by Telemetry
service. Metrics are grouped by reading type.

MetricDefinitions contains all physical sensors supported by redfish,
algorithm iterates through all chassis and collects results for each
node available in that chassis (Power, Thermal, Sensors).

When BMCWEB_NEW_POWERSUBSYSTEM_THERMALSUBSYSTEM will be enabled by
default (meson option redfish-new-powersubsystem-thermalsubsystem) it
will be possible to optimize this algorithm to only get sensors from
Sensors node. Currently Sensors node doesn't contain all available
sensors.

Removal of ResourceNotFound in sensors.hpp to fix MetricDefinition
Collection REST Get call.

Tested:
 - MetricDefinitions response is filled with existing sensors, it works
   with and without Telemetry service
 - Validated a presence of MetricDefinition members and its attributes
 - Successfully passed RedfishServiceValidator.py using witherspoon
   image on QEMU
 - Tested using following GET,POST requests

GET /redfish/v1/TelemetryService/MetricDefinitions
{
  "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions",
  "@odata.type": "#MetricDefinitionCollection.MetricDefinitionCollection",
  "Members": [
    {
      "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/Fan_Pwm"
    },
    {
      "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/Fan_Tach"
    },
    {
      "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/HostCpuUtilization"
    },
    {
      "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/HostMemoryBandwidthUtilization"
    },
    {
      "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/HostPciBandwidthUtilization"
    },
    {
      "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/Inlet_BRD_Temp"
    },
    {
      "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/Left_Rear_Board_Temp"
    }
  ],
  "Members@odata.count": 7,
  "Name": "Metric Definition Collection"
}

GET /redfish/v1/TelemetryService/MetricDefinitions/Fan_Tach
{
  "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/Fan_Tach",
  "@odata.type": "#MetricDefinition.v1_0_3.MetricDefinition",
  "Id": "Fan_Tach",
  "IsLinear": true,
  "MaxReadingRange": 25000.0,
  "MetricDataType": "Decimal",
  "MetricProperties": [
    "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/0/Reading",
    "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/1/Reading",
    "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/2/Reading",
    "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/3/Reading",
    "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/4/Reading",
    "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/5/Reading",
    "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/6/Reading",
    "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/7/Reading",
    "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/8/Reading",
    "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/9/Reading"
  ],
  "MetricType": "Gauge",
  "MinReadingRange": 0.0,
  "Name": "Fan_Tach",
  "Units": "RPM"
}

POST redfish/v1/TelemetryService/MetricReportDefinitions, body:
{
  "Id": "TestReport",
  "Metrics": [
    {
      "MetricId": "TestMetric",
      "MetricProperties": [
        "/redfish/v1/Chassis/Chassis0/Thermal#/Fans/3/Reading",
      ]
    }
  ],
  "MetricReportDefinitionType": "OnRequest",
  "ReportActions": [
    "RedfishEvent",
    "LogToMetricReportsCollection"
  ]
}
{
  "@Message.ExtendedInfo": [
    {
      "@odata.type": "#Message.v1_1_1.Message",
      "Message": "The resource has been created successfully",
      "MessageArgs": [],
      "MessageId": "Base.1.8.1.Created",
      "MessageSeverity": "OK",
      "Resolution": "None"
    }
  ]
}

Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: I3086e1302e1ba2e5442d1367939fd5507a0cbc00
Signed-off-by: Ali Ahmed <ama213000@gmail.com>

* Review feedback changes

* review updates 2

---------

Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Signed-off-by: Ali Ahmed <ama213000@gmail.com>
Co-authored-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>```

* Add support for MetricDefinition scheme Added MetricDefinition node to Redfish code. Now user is able to list all available metrics in OpenBMC that are supported by Telemetry service. Metrics are grouped by reading type. MetricDefinitions contains all physical sensors supported by redfish, algorithm iterates through all chassis and collects results for each node available in that chassis (Power, Thermal, Sensors). When BMCWEB_NEW_POWERSUBSYSTEM_THERMALSUBSYSTEM will be enabled by default (meson option redfish-new-powersubsystem-thermalsubsystem) it will be possible to optimize this algorithm to only get sensors from Sensors node. Currently Sensors node doesn't contain all available sensors. Removal of ResourceNotFound in sensors.hpp to fix MetricDefinition Collection REST Get call. Tested: - MetricDefinitions response is filled with existing sensors, it works with and without Telemetry service - Validated a presence of MetricDefinition members and its attributes - Successfully passed RedfishServiceValidator.py using witherspoon image on QEMU - Tested using following GET,POST requests GET /redfish/v1/TelemetryService/MetricDefinitions { "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions", "@odata.type": "#MetricDefinitionCollection.MetricDefinitionCollection", "Members": [ { "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/Fan_Pwm" }, { "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/Fan_Tach" }, { "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/HostCpuUtilization" }, { "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/HostMemoryBandwidthUtilization" }, { "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/HostPciBandwidthUtilization" }, { "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/Inlet_BRD_Temp" }, { "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/Left_Rear_Board_Temp" } ], "Members@odata.count": 7, "Name": "Metric Definition Collection" } GET /redfish/v1/TelemetryService/MetricDefinitions/Fan_Tach { "@odata.id": "/redfish/v1/TelemetryService/MetricDefinitions/Fan_Tach", "@odata.type": "#MetricDefinition.v1_0_3.MetricDefinition", "Id": "Fan_Tach", "IsLinear": true, "MaxReadingRange": 25000.0, "MetricDataType": "Decimal", "MetricProperties": [ "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/0/Reading", "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/1/Reading", "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/2/Reading", "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/3/Reading", "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/4/Reading", "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/5/Reading", "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/6/Reading", "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/7/Reading", "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/8/Reading", "/redfish/v1/Chassis/AC_Baseboard/Thermal#/Fans/9/Reading" ], "MetricType": "Gauge", "MinReadingRange": 0.0, "Name": "Fan_Tach", "Units": "RPM" } POST redfish/v1/TelemetryService/MetricReportDefinitions, body: { "Id": "TestReport", "Metrics": [ { "MetricId": "TestMetric", "MetricProperties": [ "/redfish/v1/Chassis/Chassis0/Thermal#/Fans/3/Reading", ] } ], "MetricReportDefinitionType": "OnRequest", "ReportActions": [ "RedfishEvent", "LogToMetricReportsCollection" ] } { "@Message.ExtendedInfo": [ { "@odata.type": "#Message.v1_1_1.Message", "Message": "The resource has been created successfully", "MessageArgs": [], "MessageId": "Base.1.8.1.Created", "MessageSeverity": "OK", "Resolution": "None" } ] } Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com> Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com> Change-Id: I3086e1302e1ba2e5442d1367939fd5507a0cbc00 Signed-off-by: Ali Ahmed <ama213000@gmail.com> * Review feedback changes * review updates 2 --------- Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com> Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com> Signed-off-by: Ali Ahmed <ama213000@gmail.com> Co-authored-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>

gtmills · 2025-05-19T16:43:37Z

redfish-core/lib/metric_definition.hpp

+
+inline void requestRoutesMetricDefinition(App& app)
+{
+    BMCWEB_ROUTE(app, "/redfish/v1/TelemetryService/MetricDefinitions/<str>/")


The new way to do this is these ROUTEs sit below and then a function for each route
e.g. https://github.com/openbmc/bmcweb/blob/master/redfish-core/lib/power_supply.hpp#L594

It would make this more readable

gtmills · 2025-05-19T16:44:50Z

redfish-core/lib/metric_definition.hpp

+
+inline void requestRoutesMetricDefinitionCollection(App& app)
+{
+    BMCWEB_ROUTE(app, "/redfish/v1/TelemetryService/MetricDefinitions/")


The new way to do this is these ROUTEs sit below and then a function for each route
e.g. https://github.com/openbmc/bmcweb/blob/master/redfish-core/lib/power_supply.hpp#L594

It would make this more readable

gtmills · 2025-05-19T19:15:47Z

Might also be good to combine those 3 commits into 1 for tracking later in the spreadsheet

rfrandse · 2025-05-19T19:29:01Z

Might also be good to combine those 3 commits into 1 for tracking later in the spreadsheet

I plan on merging the 3 commits together when the review is done. I thought it would be helpful to leave them separate for the review.

baemyung · 2025-05-19T19:18:26Z

redfish-core/include/utils/get_chassis_names.hpp

+{
+
+template <typename F>
+inline void getChassisNames(F&& cb)


Can we add this function in redfish-core/include/utils/chassis_utils.hpp, and make the function without template ?

inline void getChassisNames(const boost::system::error_code& ec, const std::vector<std::string>& chassisNames) { ... }

baemyung · 2025-05-19T19:18:58Z

redfish-core/include/utils/get_chassis_names.hpp

+
+    crow::connections::systemBus->async_method_call(
+        [callback = std::forward<F>(
+             cb)](const boost::system::error_code ec,


const boost::system::error_code& ec

baemyung · 2025-05-19T19:19:43Z

redfish-core/include/utils/get_chassis_names.hpp

+        },
+        "xyz.openbmc_project.ObjectMapper",
+        "/xyz/openbmc_project/object_mapper",
+        "xyz.openbmc_project.ObjectMapper", "GetSubTreePaths",


dbus::utility::getSubTreePaths()

baemyung · 2025-05-19T19:22:15Z

redfish-core/lib/metric_definition.hpp

+
+            callback(ec, value);
+        },
+        service2, path, "org.freedesktop.DBus.Properties", "Get",


dbus::utility::getProperty(std::string>(...)

baemyung · 2025-05-19T19:22:52Z

redfish-core/lib/metric_definition.hpp

+        },
+        "xyz.openbmc_project.ObjectMapper",
+        "/xyz/openbmc_project/object_mapper",
+        "xyz.openbmc_project.ObjectMapper", "GetSubTree",


dbus::utility::getSubTree()

baemyung · 2025-05-19T19:23:49Z

redfish-core/lib/metric_definition.hpp

+{
+    sdbusplus::message::object_path sensorObjectPath{sensorPath};
+
+    const auto* const it = std::find_if(


std::ranges::find_if(metricDefinitionMapping, [&sensorObjectPath](const auto& item) { return item.first == sensorObjectPath.parent_path().filename(); });

baemyung · 2025-05-19T19:24:38Z

redfish-core/lib/metric_definition.hpp

+            [](const crow::Request&,
+               const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) {
+                telemetry::mapRedfishUriToDbusPath(
+                    [asyncResp](boost::system::error_code ec,


const boost::system::error_code& ec,

baemyung · 2025-05-19T19:28:37Z

redfish-core/lib/metric_definition.hpp

+}
+
+template <class Callback>
+inline void mapRedfishUriToDbusPath(Callback&& callback)


I think this can be more clearer without using template like

inline void mapRedfishUriToDbusPath( std::function<void (const boost::system::error_code& ec, const boost::container::flat_map< std::string, std::string>& uriToDbus)>&& callback) { ... }

baemyung · 2025-05-19T19:29:37Z

redfish-core/lib/metric_definition.hpp

+                                              const std::shared_ptr<
+                                                  bmcweb::AsyncResp>& asyncResp,
+                                              const std::string& name) {
+            telemetry::mapRedfishUriToDbusPath(


lambda body is long. It may be better to make it as a separate function

rfrandse force-pushed the 1120-metricDefinition branch 9 times, most recently from 0c0291e to d3e8d6e Compare May 15, 2025 19:26

clang-format

a293f6f

rfrandse force-pushed the 1120-metricDefinition branch 4 times, most recently from 836ac1a to b29d627 Compare May 16, 2025 22:38

sensorsNodeStr change

84fefdc

rfrandse force-pushed the 1120-metricDefinition branch from b29d627 to 84fefdc Compare May 17, 2025 00:10

gtmills changed the title ~~1120: 1110: Add support for MetricDefinition scheme (#1088)~~ 1120: Add support for MetricDefinition scheme (#1088) May 19, 2025

rfrandse requested a review from gtmills May 19, 2025 16:00

gtmills requested a review from baemyung May 19, 2025 16:41

gtmills reviewed May 19, 2025

View reviewed changes

baemyung reviewed May 19, 2025

View reviewed changes

review comment changes

c573478

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

1120: Add support for MetricDefinition scheme (#1088) #1284

1120: Add support for MetricDefinition scheme (#1088) #1284

Uh oh!

rfrandse commented May 9, 2025 •

edited by gtmills

Loading

Uh oh!

gtmills May 19, 2025

Uh oh!

gtmills May 19, 2025

Uh oh!

gtmills commented May 19, 2025

Uh oh!

rfrandse commented May 19, 2025 •

edited

Loading

Uh oh!

baemyung May 19, 2025

Uh oh!

baemyung May 19, 2025

Uh oh!

baemyung May 19, 2025

Uh oh!

baemyung May 19, 2025

Uh oh!

baemyung May 19, 2025

Uh oh!

baemyung May 19, 2025

Uh oh!

baemyung May 19, 2025

Uh oh!

baemyung May 19, 2025

Uh oh!

baemyung May 19, 2025

Uh oh!

Uh oh!

1120: Add support for MetricDefinition scheme (#1088) #1284

Are you sure you want to change the base?

1120: Add support for MetricDefinition scheme (#1088) #1284

Uh oh!

Conversation

rfrandse commented May 9, 2025 • edited by gtmills Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1120: Add support for MetricDefinition scheme (1110 PR: #1088)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gtmills commented May 19, 2025

Uh oh!

rfrandse commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rfrandse commented May 9, 2025 •

edited by gtmills

Loading

rfrandse commented May 19, 2025 •

edited

Loading