simplified appendLimit implementation
AppendLimit is now set to max value by default. This simplifies the code
while keeping most of the feature functionality. This change increases
report version and will cause older report to be deleted.
Tested:
- AppendLimit is set by default to max value
- Telemetry features are working as expected
Change-Id: I94c85393a9601c90c00776bf0bc814d85cbf006a
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
diff --git a/src/interfaces/metric.hpp b/src/interfaces/metric.hpp
index c52dd45..63f4cf7 100644
--- a/src/interfaces/metric.hpp
+++ b/src/interfaces/metric.hpp
@@ -21,7 +21,7 @@
virtual void deinitialize() = 0;
virtual const std::vector<MetricValue>& getUpdatedReadings() = 0;
virtual LabeledMetricParameters dumpConfiguration() const = 0;
- virtual uint64_t sensorCount() const = 0;
+ virtual uint64_t metricCount() const = 0;
virtual void registerForUpdates(interfaces::MetricListener& listener) = 0;
virtual void
unregisterFromUpdates(interfaces::MetricListener& listener) = 0;
diff --git a/src/metric.cpp b/src/metric.cpp
index d6b62d8..f7a0642 100644
--- a/src/metric.cpp
+++ b/src/metric.cpp
@@ -126,7 +126,7 @@
collectionTimeScope, collectionDuration);
}
-uint64_t Metric::sensorCount() const
+uint64_t Metric::metricCount() const
{
return sensors.size();
}
diff --git a/src/metric.hpp b/src/metric.hpp
index 8b367b3..e6c0ea1 100644
--- a/src/metric.hpp
+++ b/src/metric.hpp
@@ -24,7 +24,7 @@
void sensorUpdated(interfaces::Sensor&, Milliseconds,
double value) override;
LabeledMetricParameters dumpConfiguration() const override;
- uint64_t sensorCount() const override;
+ uint64_t metricCount() const override;
void registerForUpdates(interfaces::MetricListener& listener) override;
void unregisterFromUpdates(interfaces::MetricListener& listener) override;
void updateReadings(Milliseconds) override;
diff --git a/src/report.cpp b/src/report.cpp
index 6fc6aaf..9e69b62 100644
--- a/src/report.cpp
+++ b/src/report.cpp
@@ -35,8 +35,7 @@
path(utils::pathAppend(utils::constants::reportDirPath, id)),
name(reportName), reportingType(reportingTypeIn), interval(intervalIn),
reportActions(reportActionsIn.begin(), reportActionsIn.end()),
- sensorCount(getSensorCount(metricsIn)),
- appendLimit(deduceAppendLimit(appendLimitIn)),
+ metricCount(getMetricCount(metricsIn)), appendLimit(appendLimitIn),
reportUpdates(reportUpdatesIn), readings(std::move(readingsIn)),
readingsBuffer(std::get<1>(readings),
deduceBufferSize(reportUpdates, reportingType)),
@@ -165,28 +164,15 @@
}
}
-uint64_t Report::getSensorCount(
+uint64_t Report::getMetricCount(
const std::vector<std::shared_ptr<interfaces::Metric>>& metrics)
{
- uint64_t sensorCount = 0;
+ uint64_t metricCount = 0;
for (auto& metric : metrics)
{
- sensorCount += metric->sensorCount();
+ metricCount += metric->metricCount();
}
- return sensorCount;
-}
-
-std::optional<uint64_t>
- Report::deduceAppendLimit(const uint64_t appendLimitIn) const
-{
- if (appendLimitIn == std::numeric_limits<uint64_t>::max())
- {
- return std::nullopt;
- }
- else
- {
- return appendLimitIn;
- }
+ return metricCount;
}
uint64_t Report::deduceBufferSize(const ReportUpdates reportUpdatesIn,
@@ -195,11 +181,11 @@
if (reportUpdatesIn == ReportUpdates::overwrite ||
reportingTypeIn == ReportingType::onRequest)
{
- return sensorCount;
+ return metricCount;
}
else
{
- return appendLimit.value_or(sensorCount);
+ return appendLimit;
}
}
@@ -222,18 +208,6 @@
}
}
-void Report::updateSensorCount(const uint64_t newSensorCount)
-{
- if (sensorCount != newSensorCount)
- {
- sensorCount = newSensorCount;
- if (!appendLimit.has_value())
- {
- reportIface->signal_property("AppendLimit");
- }
- }
-}
-
std::unique_ptr<sdbusplus::asio::dbus_interface>
Report::makeReportInterface(const interfaces::ReportFactory& reportFactory)
{
@@ -351,7 +325,7 @@
utils::transform(metrics, [](const auto& metric) {
return metric->dumpConfiguration();
}));
- updateSensorCount(getSensorCount(metrics));
+ metricCount = getMetricCount(metrics);
setReadingBuffer(reportUpdates);
persistency = storeConfiguration();
oldVal = std::move(newVal);
@@ -395,10 +369,9 @@
return utils::enumToString(reportAction);
});
});
- dbusIface->register_property_r(
- "AppendLimit", appendLimit.value_or(sensorCount),
- sdbusplus::vtable::property_::emits_change,
- [this](const auto&) { return appendLimit.value_or(sensorCount); });
+ dbusIface->register_property_r<uint64_t>(
+ "AppendLimit", sdbusplus::vtable::property_::emits_change,
+ [this](const auto&) { return appendLimit; });
dbusIface->register_property_rw(
"ReportUpdates", std::string(),
sdbusplus::vtable::property_::emits_change,
@@ -547,8 +520,7 @@
return utils::toUnderlying(reportAction);
});
data["Interval"] = interval.count();
- data["AppendLimit"] =
- appendLimit.value_or(std::numeric_limits<uint64_t>::max());
+ data["AppendLimit"] = appendLimit;
data["ReportUpdates"] = utils::toUnderlying(reportUpdates);
data["ReadingParameters"] =
utils::transform(metrics, [](const auto& metric) {
diff --git a/src/report.hpp b/src/report.hpp
index 856e369..d3c88ec 100644
--- a/src/report.hpp
+++ b/src/report.hpp
@@ -96,14 +96,11 @@
Report& self);
void scheduleTimerForPeriodicReport(Milliseconds interval);
void scheduleTimerForOnChangeReport();
- std::optional<uint64_t>
- deduceAppendLimit(const uint64_t appendLimitIn) const;
uint64_t deduceBufferSize(const ReportUpdates reportUpdatesIn,
const ReportingType reportingTypeIn) const;
void setReadingBuffer(const ReportUpdates newReportUpdates);
void setReportUpdates(const ReportUpdates newReportUpdates);
- void updateSensorCount(const uint64_t newSensorCount);
- static uint64_t getSensorCount(
+ static uint64_t getMetricCount(
const std::vector<std::shared_ptr<interfaces::Metric>>& metrics);
interfaces::JsonStorage::FilePath reportFileName() const;
std::unordered_set<std::string>
@@ -123,8 +120,8 @@
ReadingParametersPastVersion readingParametersPastVersion;
ReadingParameters readingParameters;
bool persistency = false;
- uint64_t sensorCount;
- std::optional<uint64_t> appendLimit;
+ uint64_t metricCount;
+ uint64_t appendLimit;
ReportUpdates reportUpdates;
Readings readings = {};
CircularVector<ReadingData> readingsBuffer;
@@ -149,5 +146,5 @@
"xyz.openbmc_project.Telemetry.Report";
static constexpr const char* deleteIfaceName =
"xyz.openbmc_project.Object.Delete";
- static constexpr size_t reportVersion = 6;
+ static constexpr size_t reportVersion = 7;
};
diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index 932ffe0..d12e6aa 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -70,7 +70,6 @@
const uint64_t interval,
ReadingParametersPastVersion metricParams) {
constexpr auto enabledDefault = true;
- constexpr uint64_t appendLimitDefault = 0;
constexpr ReportUpdates reportUpdatesDefault =
ReportUpdates::overwrite;
@@ -90,7 +89,7 @@
return addReport(yield, reportId, reportId,
utils::toReportingType(reportingType),
reportActions, Milliseconds(interval),
- appendLimitDefault, reportUpdatesDefault,
+ maxAppendLimit, reportUpdatesDefault,
convertToReadingParameters(
std::move(metricParams)),
enabledDefault)
diff --git a/tests/src/mocks/metric_mock.hpp b/tests/src/mocks/metric_mock.hpp
index 6dfe660..dc0309b 100644
--- a/tests/src/mocks/metric_mock.hpp
+++ b/tests/src/mocks/metric_mock.hpp
@@ -13,7 +13,7 @@
ON_CALL(*this, getUpdatedReadings())
.WillByDefault(ReturnRefOfCopy(std::vector<MetricValue>()));
- ON_CALL(*this, sensorCount).WillByDefault(InvokeWithoutArgs([this] {
+ ON_CALL(*this, metricCount).WillByDefault(InvokeWithoutArgs([this] {
return getUpdatedReadings().size();
}));
}
@@ -24,7 +24,7 @@
(override));
MOCK_METHOD(LabeledMetricParameters, dumpConfiguration, (),
(const, override));
- MOCK_METHOD(uint64_t, sensorCount, (), (const, override));
+ MOCK_METHOD(uint64_t, metricCount, (), (const, override));
MOCK_METHOD(void, registerForUpdates, (interfaces::MetricListener&),
(override));
MOCK_METHOD(void, unregisterFromUpdates, (interfaces::MetricListener&),
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index ea3b4f8..875bd4b 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -109,7 +109,7 @@
metricMocks.emplace_back(std::make_shared<NiceMock<MetricMock>>());
ON_CALL(*metricMocks[i], dumpConfiguration())
.WillByDefault(Return(metricParameters[i]));
- ON_CALL(*metricMocks[i], sensorCount())
+ ON_CALL(*metricMocks[i], metricCount())
.WillByDefault(Return(metricParameters[i]
.at_label<tstring::SensorPath>()
.size()));
@@ -285,34 +285,6 @@
Eq(newParams));
}
-TEST_F(TestReport, setReadingParametersWithNewParamsUpdatesSensorCount)
-{
- auto report =
- makeReport(ReportParams()
- .appendLimit(std::numeric_limits<uint64_t>::max())
- .reportId("DefaultAppendLimit"));
-
- ReadingParameters newParams = toReadingParameters(
- std::vector<LabeledMetricParameters>{{LabeledMetricParameters{
- {LabeledSensorInfo{"Service",
- "/xyz/openbmc_project/sensors/power/psu",
- "NewMetadata123"}},
- OperationType::avg,
- "NewMetricId123",
- CollectionTimeScope::startup,
- CollectionDuration(250ms)}}});
- auto metrics = getMetricsFromReadingParams(newParams);
-
- EXPECT_CALL(*reportFactoryMock, updateMetrics(_, _, _))
- .WillOnce(SetArgReferee<0>(metrics));
- EXPECT_THAT(setProperty(report->getPath(), "ReadingParametersFutureVersion",
- newParams)
- .value(),
- Eq(boost::system::errc::success));
- EXPECT_THAT(getProperty<uint64_t>(report->getPath(), "AppendLimit"),
- Eq(1ull));
-}
-
TEST_F(TestReport, setReadingParametersWithTooLongMetricId)
{
const ReadingParameters currentValue =
@@ -707,7 +679,7 @@
_, TestReportStore,
Values(
std::make_pair("Enabled"s, nlohmann::json(defaultParams().enabled())),
- std::make_pair("Version"s, nlohmann::json(6)),
+ std::make_pair("Version"s, nlohmann::json(7)),
std::make_pair("Id"s, nlohmann::json(defaultParams().reportId())),
std::make_pair("Name"s, nlohmann::json(defaultParams().reportName())),
std::make_pair("ReportingType",
@@ -965,6 +937,7 @@
void SetUp() override
{
sut = makeReport(defaultParams()
+ .appendLimit(2u)
.reportingType(ReportingType::periodic)
.interval(ReportManager::minInterval));
}
@@ -1125,7 +1098,7 @@
ReportUpdatesReportParams{
defaultParams()
.reportUpdates(ReportUpdates::appendStopsWhenFull)
- .appendLimit(std::numeric_limits<uint64_t>::max()),
+ .appendLimit(2u),
std::vector<ReadingData>{
{std::make_tuple("a"s, "b"s, 17.1, 114u),
std::make_tuple("aa"s, "bb"s, 42.0, 74u)}},
@@ -1149,6 +1122,7 @@
appendLimitIsRespectedAfterChangingToPeriodic)
{
sut = makeReport(ReportParams(GetParam().reportParams)
+ .appendLimit(GetParam().expectedReadings.size())
.reportingType(ReportingType::onRequest)
.interval(std::chrono::hours(0)));
@@ -1305,23 +1279,6 @@
DbusEnvironment::sleepFor(defaultParams().interval() * 2);
}
-TEST_F(TestReportInitialization, appendLimitDeducedProperly)
-{
- sut = makeReport(
- defaultParams().appendLimit(std::numeric_limits<uint64_t>::max()));
- auto appendLimit = getProperty<uint64_t>(sut->getPath(), "AppendLimit");
- EXPECT_EQ(appendLimit, 2ull);
-}
-
-TEST_F(TestReportInitialization, appendLimitSetToUintMaxIsStoredCorrectly)
-{
- sut = makeReport(
- ReportParams().appendLimit(std::numeric_limits<uint64_t>::max()));
-
- ASSERT_THAT(storedConfiguration.at("AppendLimit"),
- Eq(std::numeric_limits<uint64_t>::max()));
-}
-
TEST_F(TestReportInitialization, triggerIdsPropertyIsInitialzed)
{
for (const auto& triggerId : {"trigger1", "trigger2"})