Redfish non-compliance: Telemetry: MetricProperties accept full Sensor resources instead of arbitrary Redfish properties

[FHomps]

Is this the right place to submit this?

• This is not a security vulnerability or a crashing bug
• This is not a question about how to use OpenBMC
• This is not a bug in an OpenBMC fork or a bug in code still under code review.
• This is not a request for a new feature.

Bug Description

Hi,

The current telemetry implementation is not compliant with the redfish standard.

When creating a MetricReportDefinition, BMCWeb expects any MetricProperty to link to a Sensor resource. When POSTed to the TelemetryService, the following json works:

{
  "Id": "SampleMetric",
  "Metrics": [
    {
      "MetricProperties": [
        "/redfish/v1/Chassis/power_board/Sensors/power_C3PB7_HSW_PIN"
      ]
    }
  ],
  "MetricReportDefinitionType": "Periodic",
  "ReportActions": [
    "RedfishEvent",
    "LogToMetricReportsCollection"
  ],
  "Schedule": {
    "RecurrenceInterval": "PT5S"
  }
}

DSP0268 explains that a MetricProperty must point to a resource property (e.g. the Reading property of a Sensor):

This allows for Metric Reports to be defined for any property, not only the Reading of Sensors. E.g., the example of MetricReportDefinition in DSP0268 points to the PowerConsumedWatts property of the Thermal schema:

A properly Redfish-compliant MetricReportDefinition request on the previous sensor should thus be:

{
  "Id": "SampleMetric",
  "Metrics": [
    {
      "MetricProperties": [
        "/redfish/v1/Chassis/power_board/Sensors/power_C3PB7_HSW_PIN/Reading"
      ]
    }
  ],
  "MetricReportDefinitionType": "Periodic",
  "ReportActions": [
    "RedfishEvent",
    "LogToMetricReportsCollection"
  ],
  "Schedule": {
    "RecurrenceInterval": "PT5S"
  }
}

Which fails against bmcweb on OpenBMC 2.14 with the following reply:

{
  "error": {
    "@Message.ExtendedInfo": [
      {
        "@odata.type": "#Message.v1_1_1.Message",
        "Message": "The property '/redfish/v1/Chassis/power_board/Sensors/power_C3PB7_HSW_PIN/Reading' with the requested value of 'MetricProperties/0' could not be written because the value does not meet the constraints of the implementation.",
        "MessageArgs": [
          "/redfish/v1/Chassis/power_board/Sensors/power_C3PB7_HSW_PIN/Reading",
          "MetricProperties/0"
        ],
        "MessageId": "Base.1.16.0.PropertyValueIncorrect",
        "MessageSeverity": "Warning",
        "Resolution": "No resolution is required."
      }
    ],
    "code": "Base.1.16.0.PropertyValueIncorrect",
    "message": "The property '/redfish/v1/Chassis/power_board/Sensors/power_C3PB7_HSW_PIN/Reading' with the requested value of 'MetricProperties/0' could not be written because the value does not meet the constraints of the implementation."
  }

Version

Internal Eviden fork of OpenBMC based on OpenBMC 2.14.
Offending code is from upstream:
https://github.com/openbmc/bmcweb/blob/4d7b5ddb3a2b6cc42b7bbc0c710f297e6df4fd55/redfish-core/lib/metric_report_definition.hpp#L666

Additional Information

No response

[edtanous]

This allows for Metric Reports to be defined for any property

I don't see anything in the above that requires that an implementation accept this for ANY property. Is there something that specifies that any property in the tree be required to work for a conforming implementation?

With that said, if someone wanted to contribute support for what you're asking, I as the maintainer would be in full support.

[FHomps]

Hi Ed,

It would absolutely be fine for OpenBMC to only limit support of telemetry to specific properties in the redfish tree because of implementation limitations (e.g. in a first order of business, only the Reading property of Sensors). Certainly a limitation (reports on, for example, health / status values would be great!), but fine.

The big problem here is that MetricProperty should never point to a Sensor resource per the standard.

In the use case of using Metric Reports to gather sensor data, MetricProperties should point to the Sensors' underlying Reading properties. All metric report definitions done with the current implementation are non-compliant with Redfish, which is going to cause big issues with higher-level software.

[edtanous]

Technically a compliant redfish property would be:
/redfish/v1/Chassis/power_board/Sensors/power_C3PB7_HSW_PIN#/Reading
(note the added # compared to what you had).

Correct?

This seems like a pretty reasonable change to effect. Please send a patchset if this is something you want to improve, but no disagreement that our handling of this looks incorrect.

[FHomps]

Yes, good point about the # 🙂

We've done the tests again and get a different response:

{
  "MetricProperties/0@Message.ExtendedInfo": [
    {
      "@odata.type": "#Message.v1_1_1.Message",
      "Message": "The value '/redfish/v1/Chassis/power_board/Sensors/power_C3PB7_HSW_PIN#/Reading' for the property MetricProperties/0 is not in the list of acceptable values.",
      "MessageArgs": [
        "/redfish/v1/Chassis/power_board/Sensors/power_C3PB7_HSW_PIN#/Reading",
        "MetricProperties/0"
      ],
      "MessageId": "Base.1.16.0.PropertyValueNotInList",
      "MessageSeverity": "Warning",
      "Resolution": "Choose a value from the enumeration list that the implementation can support and resubmit the request if the operation failed."
    }
  ]
}

Looking at the bmcweb error log we are hitting this codepoint:

bmcweb/redfish-core/lib/metric_report_definition.hpp

Line 827 in c76f964

"Failed to find DBus sensor corresponding to URI {}",

I'm in architecture, so I can't take engagements with regards to potential contributions done by the dev team, but we'll be looking into it. Thanks for the help.