-
Couldn't load subscription status.
- Fork 70
1120: Add support for MetricDefinition scheme (#1088) #1284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 1120
Are you sure you want to change the base?
Conversation
* 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"
}
],
"[email protected]": 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 <[email protected]>
Signed-off-by: Krzysztof Grobelny <[email protected]>
Change-Id: I3086e1302e1ba2e5442d1367939fd5507a0cbc00
Signed-off-by: Ali Ahmed <[email protected]>
* Review feedback changes
* review updates 2
---------
Signed-off-by: Wludzik, Jozef <[email protected]>
Signed-off-by: Krzysztof Grobelny <[email protected]>
Signed-off-by: Ali Ahmed <[email protected]>
Co-authored-by: Krzysztof Grobelny <[email protected]>
0c0291e to
d3e8d6e
Compare
836ac1a to
b29d627
Compare
b29d627 to
84fefdc
Compare
|
|
||
| inline void requestRoutesMetricDefinition(App& app) | ||
| { | ||
| BMCWEB_ROUTE(app, "/redfish/v1/TelemetryService/MetricDefinitions/<str>/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
|
|
||
| inline void requestRoutesMetricDefinitionCollection(App& app) | ||
| { | ||
| BMCWEB_ROUTE(app, "/redfish/v1/TelemetryService/MetricDefinitions/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
|
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. |
| { | ||
|
|
||
| template <typename F> | ||
| inline void getChassisNames(F&& cb) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
{
...
}
|
|
||
| crow::connections::systemBus->async_method_call( | ||
| [callback = std::forward<F>( | ||
| cb)](const boost::system::error_code ec, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const boost::system::error_code& ec
| }, | ||
| "xyz.openbmc_project.ObjectMapper", | ||
| "/xyz/openbmc_project/object_mapper", | ||
| "xyz.openbmc_project.ObjectMapper", "GetSubTreePaths", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dbus::utility::getSubTreePaths()
|
|
||
| callback(ec, value); | ||
| }, | ||
| service2, path, "org.freedesktop.DBus.Properties", "Get", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dbus::utility::getProperty(std::string>(...)
| }, | ||
| "xyz.openbmc_project.ObjectMapper", | ||
| "/xyz/openbmc_project/object_mapper", | ||
| "xyz.openbmc_project.ObjectMapper", "GetSubTree", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dbus::utility::getSubTree()
| { | ||
| sdbusplus::message::object_path sensorObjectPath{sensorPath}; | ||
|
|
||
| const auto* const it = std::find_if( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::ranges::find_if(metricDefinitionMapping,
[&sensorObjectPath](const auto& item) {
return item.first == sensorObjectPath.parent_path().filename();
});
| [](const crow::Request&, | ||
| const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) { | ||
| telemetry::mapRedfishUriToDbusPath( | ||
| [asyncResp](boost::system::error_code ec, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const boost::system::error_code& ec,
| } | ||
|
|
||
| template <class Callback> | ||
| inline void mapRedfishUriToDbusPath(Callback&& callback) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
{
...
}
| const std::shared_ptr< | ||
| bmcweb::AsyncResp>& asyncResp, | ||
| const std::string& name) { | ||
| telemetry::mapRedfishUriToDbusPath( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lambda body is long. It may be better to make it as a separate function
|
@rfrandse This something we are planning on doing for 1120?? What if we left them off? Is the HMC broke ? |
1120: Add support for MetricDefinition scheme (1110 PR: #1088)