Telemetry: Support AppendWrapsWhenFull

[gtmills]

Currently, the telemetry service only supports ReportUpdates = Overwrite

https://github.com/openbmc/bmcweb/blob/081ebf06b4c947e828408029273699ff2d49a54f/redfish-core/lib/metric_report_definition.hpp#L35

This story is to support AppendWrapsWhenFull which allows for more than 1 metric report.

On the mailing list, Intel mentioned this work was on their roadmap so please work with them and the community
https://lists.ozlabs.org/pipermail/openbmc/2021-February/025141.html

These changes will need to happen in bmcweb and the telemetry repo.

When this work is complete, the client should be able to set Metric Definitions and store multiple (I would think at least 20) Metrics reports.

        "ReportUpdatesEnum": {
            "description": "Handling of subsequent metric reports when a metric report exists.",
            "enum": [
                "Overwrite",
                "AppendWrapsWhenFull",
                "AppendStopsWhenFull",
                "NewReport"
            ],
            "enumDescriptions": {
                "AppendStopsWhenFull": "New information is appended to the metric report.  The service stops adding entries when the metric report has reached its maximum capacity.",
                "AppendWrapsWhenFull": "New information is appended to the metric report.  The metric report entries are overwritten with new entries when the metric report has reached its maximum capacity.",
                "NewReport": "A new metric report is created, whose resource name is the metric report resource name concatenated with the timestamp.",
                "Overwrite": "Overwrite the metric report."
            },
            "enumLongDescriptions": {
                "AppendStopsWhenFull": "This value shall indicate the service appends new information to the metric report referenced by the MetricReport property.  The service shall stop adding entries when the metric report has reached its maximum capacity.  The State property within Status should be set to `Disabled` and the MetricReportDefinitionEnabled property should be set to `false` when the append limit is reached.",
                "AppendWrapsWhenFull": "This value shall indicate the service appends new information to the metric report referenced by the MetricReport property.  The service shall overwrite entries in the metric report with new entries when the metric report has reached its maximum capacity.",
                "NewReport": "This value shall indicate the service creates a new metric report resource, whose resource name is the metric report resource name concatenated with the timestamp.",
                "Overwrite": "This value shall indicate the service overwrites the metric report referenced by the MetricReport property."
            },

Additional references:
https://www.dmtf.org/documents/redfish-spmf/redfish-telemetry-white-paper-0
https://redfish.dmtf.org/redfish/mockups/v1/1027

[lxwinspur]

@gtmills @mzipse

I have read this issue, and have a question:

Since this issue requires changes to the bmeweb and the telemetry repo, and I got some information by the mailing list:

"Append" is on the list of to dos and should be ready till summer
(might be ready). Other feature that are described by Redfish Telemetry
Model should be ready till the end of summer too -> triggers and
collection functions.

Do we only need to implement the bmcweb repo? Or also need to implement telemetry repo?

Same question as #2968

Thanks

[lxwinspur]

Also, I do not know how to test?

Right now, I just see using Unit Test and redfish-test in the Telemetry repo.

So I did not found how to trigger the AddReport method in xyz.openbmc_project.Telemetry.ReportManager interface and the AddTrigger method in xyz.openbmc_project.Telemetry.TriggerManager interface.

[lxwinspur]

how to trigger the AddReport method in xyz.openbmc_project.Telemetry.ReportManager interface

I found it in bmcweb repo:
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/32536/59/redfish-core/lib/metric_report_definition.hpp#330

For the AppendWrapsWhenFull property, do we need to implement an interface in phosphor-dbus-interface? and change the Telemetry repo?

Sorry, I am not familiar with the Telemetry service, Please correct me if I am wrong, Thanks.

[gtmills]

Do we only need to implement the bmcweb repo? Or also need to implement telemetry repo?

This work should span both repos.

Also, I do not know how to test?

Unit tests and redfish-test are great for the telemetry repo.
To test end to end, you will want to create a metric definition with ReportUpdatesEnum=AppendWrapsWhenFull and ensure the behavior is as the API describes. E.g. you have multiple metricValues from different times and that the metricsValues grow until they reach the size of AppendLimit then they wrap.

Note: AppendLimit is ReadOnly, so would hardcoded to some value the community is agreeable to.

[gtmills]

For the AppendWrapsWhenFull property, do we need to implement an interface in phosphor-dbus-interface? and change the Telemetry repo?

Yes, I believe we need an addition to the phosphor-dbus-interface here. Please be sure to add myself, Ed, and Jozef to any reviews.

Please be sure to work with the community here. Intel is the telemetry expert having written this code. I think it would be reasonable after the phosphor-dbus-interface is pushed to reply to the above mail with "Here is the phosphor-dbus-interface to add support for "AppendWrapsWhenFull".

Thanks!
Keep up the great work.
Gunnar

[lxwinspur]

@gtmills

Thanks for the reply.
Right now, I have added a new property to the Report and ReportManager interfaces.
review by: https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/41010

I will update the telemetry service and bmcweb after this commit merge.

[gtmills]

I see https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/41010 merged. Any more progress on this?

[lxwinspur]

I see https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/41010 merged. Any more progress on this?

Sorry, I will implement it, but I still read the Telemetry repo first.

[lxwinspur]

@gtmills @mzipse
This is Intel's patch and they have already pushed it to Gerrit, Let me review it
https://gerrit.openbmc-project.xyz/c/openbmc/telemetry/+/46111

[lxwinspur]

@gtmills
can we close this issue?

[gtmills]

@lxwinspur Has this been merged upstream ?

[lxwinspur]

@lxwinspur Has this been merged upstream ?

Yeah, I think so
https://gerrit.openbmc-project.xyz/c/openbmc/telemetry/+/46111

[gtmills]

Yeah, I think so
https://gerrit.openbmc-project.xyz/c/openbmc/telemetry/+/46111

And the bmcweb change as well?
If so, then yeah lets close. If not, let's use this to track getting that in. We might not have to do any coding, just reviews, etc.

[gtmills]

I don't see this in bmcweb today: https://github.com/openbmc/bmcweb/search?q=AppendWrapsWhenFull

[gtmills]

Need https://gerrit.openbmc.org/c/openbmc/bmcweb/+/44270
Then this can close :)

[gtmills]

Resolved by https://gerrit.openbmc.org/c/openbmc/bmcweb/+/44270

[gtmills]

This merged and I see it here so closing
https://github.com/search?q=repo%3Aopenbmc%2Fbmcweb+AppendWrapsWhenFull+language%3AC%2B%2B&type=code&l=C%2B%2B