storage metric: add generic implementation
Add the generic implementation for the storage metric, so there is no
need to add a new enum value and new code, every time a new storage
type needs to be added.
Change-Id: Idcd99455ca559eb6274a71613684a99b4b63a6f0
Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
diff --git a/health_metric.cpp b/health_metric.cpp
index 8ee427b..e29954a 100644
--- a/health_metric.cpp
+++ b/health_metric.cpp
@@ -12,7 +12,8 @@
using association_t = std::tuple<std::string, std::string, std::string>;
-auto HealthMetric::getPath(SubType subType) -> std::string
+auto HealthMetric::getPath(phosphor::health::metric::Type type,
+ std::string name, SubType subType) -> std::string
{
std::string path;
switch (subType)
@@ -50,17 +51,28 @@
{
return std::string(BmcPath) + "/" + PathIntf::total_memory;
}
- case SubType::storageReadWrite:
+ case SubType::NA:
{
- return std::string(BmcPath) + "/" + PathIntf::read_write_storage;
- }
- case SubType::storageTmp:
- {
- return std::string(BmcPath) + "/" + PathIntf::tmp_storage;
+ if (type == phosphor::health::metric::Type::storage)
+ {
+ static constexpr auto nameDelimiter = "_";
+ auto storageType = name.substr(
+ name.find_last_of(nameDelimiter) + 1, name.length());
+ std::ranges::for_each(storageType,
+ [](auto& c) { c = std::tolower(c); });
+ return std::string(BmcPath) + "/" + PathIntf::storage + "/" +
+ storageType;
+ }
+ else
+ {
+ error("Invalid metric {SUBTYPE} for metric {TYPE}", "SUBTYPE",
+ subType, "TYPE", type);
+ return "";
+ }
}
default:
{
- error("Invalid Memory metric {TYPE}", "TYPE", subType);
+ error("Invalid metric {SUBTYPE}", "SUBTYPE", subType);
return "";
}
}
@@ -68,27 +80,27 @@
void HealthMetric::initProperties()
{
- switch (config.subType)
+ switch (type)
{
- case SubType::cpuTotal:
- case SubType::cpuKernel:
- case SubType::cpuUser:
+ case MType::cpu:
{
ValueIntf::unit(ValueIntf::Unit::Percent, true);
ValueIntf::minValue(0.0, true);
ValueIntf::maxValue(100.0, true);
break;
}
- case SubType::memoryAvailable:
- case SubType::memoryBufferedAndCached:
- case SubType::memoryFree:
- case SubType::memoryShared:
- case SubType::memoryTotal:
- case SubType::storageReadWrite:
- default:
+ case MType::memory:
+ case MType::storage:
{
ValueIntf::unit(ValueIntf::Unit::Bytes, true);
ValueIntf::minValue(0.0, true);
+ break;
+ }
+ case MType::inode:
+ case MType::unknown:
+ default:
+ {
+ throw std::invalid_argument("Invalid metric type");
}
}
ValueIntf::value(std::numeric_limits<double>::quiet_NaN(), true);
diff --git a/health_metric.hpp b/health_metric.hpp
index e79fce1..8d3dee4 100644
--- a/health_metric.hpp
+++ b/health_metric.hpp
@@ -25,6 +25,7 @@
using BmcIntf = sdbusplus::xyz::openbmc_project::Inventory::Item::server::Bmc;
using MetricIntf =
sdbusplus::server::object_t<ValueIntf, ThresholdIntf, AssociationIntf>;
+using MType = phosphor::health::metric::Type;
struct MValue
{
@@ -42,9 +43,10 @@
HealthMetric(HealthMetric&&) = delete;
virtual ~HealthMetric() = default;
- HealthMetric(sdbusplus::bus_t& bus, phosphor::health::metric::Type type,
+ HealthMetric(sdbusplus::bus_t& bus, MType type,
const config::HealthMetric& config, const paths_t& bmcPaths) :
- MetricIntf(bus, getPath(config.subType).c_str(), action::defer_emit),
+ MetricIntf(bus, getPath(type, config.name, config.subType).c_str(),
+ action::defer_emit),
bus(bus), type(type), config(config)
{
create(bmcPaths);
@@ -64,12 +66,13 @@
MValue value);
/** @brief Check all thresholds for the given value */
void checkThresholds(MValue value);
- /** @brief Get the object path for the given subtype */
- auto getPath(SubType subType) -> std::string;
+ /** @brief Get the object path for the given type, name and subtype */
+ auto getPath(phosphor::health::metric::Type type, std::string name,
+ SubType subType) -> std::string;
/** @brief D-Bus bus connection */
sdbusplus::bus_t& bus;
/** @brief Metric type */
- phosphor::health::metric::Type type;
+ MType type;
/** @brief Metric configuration */
const config::HealthMetric config;
/** @brief Window for metric history */
diff --git a/health_metric_collection.cpp b/health_metric_collection.cpp
index ea587ac..d83f8d0 100644
--- a/health_metric_collection.cpp
+++ b/health_metric_collection.cpp
@@ -102,7 +102,7 @@
debug("CPU Metric {SUBTYPE}: {VALUE}", "SUBTYPE", config.subType,
"VALUE", (double)activePercValue);
/* For CPU, both user and monitor uses percentage values */
- metrics[config.subType]->update(MValue(activePercValue, 100));
+ metrics[config.name]->update(MValue(activePercValue, 100));
}
return true;
}
@@ -159,7 +159,7 @@
auto total = memoryValues.at(MetricIntf::SubType::memoryTotal) * 1024;
debug("Memory Metric {SUBTYPE}: {VALUE}, {TOTAL}", "SUBTYPE",
config.subType, "VALUE", value, "TOTAL", total);
- metrics[config.subType]->update(MValue(value, total));
+ metrics[config.name]->update(MValue(value, total));
}
return true;
}
@@ -180,7 +180,7 @@
double total = buffer.f_blocks * buffer.f_frsize;
debug("Storage Metric {SUBTYPE}: {VALUE}, {TOTAL}", "SUBTYPE",
config.subType, "VALUE", value, "TOTAL", total);
- metrics[config.subType]->update(MValue(value, total));
+ metrics[config.name]->update(MValue(value, total));
}
return true;
}
@@ -227,12 +227,7 @@
for (auto& config : configs)
{
- /* TODO: Remove this after adding iNode support */
- if (config.subType == MetricIntf::SubType::NA)
- {
- continue;
- }
- metrics[config.subType] = std::make_unique<MetricIntf::HealthMetric>(
+ metrics[config.name] = std::make_unique<MetricIntf::HealthMetric>(
bus, type, config, bmcPaths);
}
}
diff --git a/health_metric_collection.hpp b/health_metric_collection.hpp
index 51a92e3..d1fd4fb 100644
--- a/health_metric_collection.hpp
+++ b/health_metric_collection.hpp
@@ -25,7 +25,7 @@
void read();
private:
- using map_t = std::unordered_map<MetricIntf::SubType,
+ using map_t = std::unordered_map<std::string,
std::unique_ptr<MetricIntf::HealthMetric>>;
using time_map_t = std::unordered_map<MetricIntf::SubType, uint64_t>;
/** @brief Create a new health metric collection object */
diff --git a/health_metric_config.cpp b/health_metric_config.cpp
index 144b49f..1570f12 100644
--- a/health_metric_config.cpp
+++ b/health_metric_config.cpp
@@ -59,8 +59,8 @@
{"Memory_Available", SubType::memoryAvailable},
{"Memory_Shared", SubType::memoryShared},
{"Memory_Buffered_And_Cached", SubType::memoryBufferedAndCached},
- {"Storage_RW", SubType::storageReadWrite},
- {"Storage_TMP", SubType::storageTmp}};
+ {"Storage_RW", SubType::NA},
+ {"Storage_TMP", SubType::NA}};
/** Deserialize a Threshold from JSON. */
void from_json(const json& j, Threshold& self)
diff --git a/health_metric_config.hpp b/health_metric_config.hpp
index 6391462..dedf2f4 100644
--- a/health_metric_config.hpp
+++ b/health_metric_config.hpp
@@ -36,9 +36,7 @@
memoryFree,
memoryShared,
memoryTotal,
- // Storage subtypes
- storageReadWrite,
- storageTmp,
+ // Types for which subtype is not applicable
NA
};
diff --git a/test/test_health_metric_collection.cpp b/test/test_health_metric_collection.cpp
index 19c2d49..7086369 100644
--- a/test/test_health_metric_collection.cpp
+++ b/test/test_health_metric_collection.cpp
@@ -45,8 +45,7 @@
for (auto& config : values)
{
config.windowSize = 1;
- if (key == MetricIntf::Type::storage &&
- config.subType == MetricIntf::SubType::storageReadWrite)
+ if (key == MetricIntf::Type::storage)
{
config.path = "/tmp";
}
diff --git a/test/test_health_metric_config.cpp b/test/test_health_metric_config.cpp
index 2670dab..dd92296 100644
--- a/test/test_health_metric_config.cpp
+++ b/test/test_health_metric_config.cpp
@@ -43,10 +43,6 @@
.contains(subType);
case metric::Type::storage:
- return set_t{metric::SubType::storageReadWrite,
- metric::SubType::storageTmp}
- .contains(subType);
-
case metric::Type::inode:
return set_t{metric::SubType::NA}.contains(subType);