fixed issue with invalid readings being visible
Before this change readings for sensors that are not yet available were
displayed as value 0.0 with timestamp 1970-01-01 which was meant to
represent invalid readings. This behaviour is undesired in case of
periodic report with appendLimit set. After this change invalid readings
are no longer visible.
Tested:
- Readings for sensors that is currently not available are not present
on redfish.
- Confirmed in new unit tests that sensor readings can appear in any
order. And produce correct results.
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: I340b85017530d120f2da2cc8ac4ae1840177e78c
diff --git a/src/interfaces/metric.hpp b/src/interfaces/metric.hpp
index c40e960..c52dd45 100644
--- a/src/interfaces/metric.hpp
+++ b/src/interfaces/metric.hpp
@@ -19,7 +19,7 @@
virtual void initialize() = 0;
virtual void deinitialize() = 0;
- virtual std::vector<MetricValue> getReadings() const = 0;
+ virtual const std::vector<MetricValue>& getUpdatedReadings() = 0;
virtual LabeledMetricParameters dumpConfiguration() const = 0;
virtual uint64_t sensorCount() const = 0;
virtual void registerForUpdates(interfaces::MetricListener& listener) = 0;
diff --git a/src/metric.cpp b/src/metric.cpp
index a9cf45e..d6b62d8 100644
--- a/src/metric.cpp
+++ b/src/metric.cpp
@@ -21,11 +21,7 @@
metrics::makeCollectionData(sensors.size(), operationType,
collectionTimeScope, collectionDuration)),
clock(std::move(clockIn))
-{
- readings = utils::transform(sensors, [this](const auto& sensor) {
- return MetricValue{id, sensor->metadata(), 0.0, 0u};
- });
-}
+{}
void Metric::registerForUpdates(interfaces::MetricListener& listener)
{
@@ -58,25 +54,40 @@
}
}
-std::vector<MetricValue> Metric::getReadings() const
+const std::vector<MetricValue>& Metric::getUpdatedReadings()
{
const auto steadyTimestamp = clock->steadyTimestamp();
- const auto systemTimestamp = clock->systemTimestamp();
+ const auto systemTimestamp =
+ std::chrono::duration_cast<Milliseconds>(clock->systemTimestamp())
+ .count();
- auto resultReadings = readings;
-
- for (size_t i = 0; i < resultReadings.size(); ++i)
+ for (size_t i = 0; i < collectionAlgorithms.size(); ++i)
{
if (const auto value = collectionAlgorithms[i]->update(steadyTimestamp))
{
- resultReadings[i].timestamp =
- std::chrono::duration_cast<Milliseconds>(systemTimestamp)
- .count();
- resultReadings[i].value = *value;
+ if (i < readings.size())
+ {
+ readings[i].timestamp = systemTimestamp;
+ readings[i].value = *value;
+ }
+ else
+ {
+ if (i > readings.size())
+ {
+ const auto idx = readings.size();
+ std::swap(collectionAlgorithms[i],
+ collectionAlgorithms[idx]);
+ std::swap(sensors[i], sensors[idx]);
+ i = idx;
+ }
+
+ readings.emplace_back(id, sensors[i]->metadata(), *value,
+ systemTimestamp);
+ }
}
}
- return resultReadings;
+ return readings;
}
void Metric::sensorUpdated(interfaces::Sensor& notifier, Milliseconds timestamp,
diff --git a/src/metric.hpp b/src/metric.hpp
index b627b4e..8b367b3 100644
--- a/src/metric.hpp
+++ b/src/metric.hpp
@@ -20,7 +20,7 @@
void initialize() override;
void deinitialize() override;
- std::vector<MetricValue> getReadings() const override;
+ const std::vector<MetricValue>& getUpdatedReadings() override;
void sensorUpdated(interfaces::Sensor&, Milliseconds,
double value) override;
LabeledMetricParameters dumpConfiguration() const override;
diff --git a/src/report.cpp b/src/report.cpp
index 945d4e6..540418a 100644
--- a/src/report.cpp
+++ b/src/report.cpp
@@ -462,7 +462,7 @@
for (const auto& metric : metrics)
{
for (const auto& [id, metadata, value, timestamp] :
- metric->getReadings())
+ metric->getUpdatedReadings())
{
if (reportUpdates == ReportUpdates::appendStopsWhenFull &&
readingsBuffer.isFull())
diff --git a/tests/src/mocks/metric_mock.hpp b/tests/src/mocks/metric_mock.hpp
index a223734..6dfe660 100644
--- a/tests/src/mocks/metric_mock.hpp
+++ b/tests/src/mocks/metric_mock.hpp
@@ -11,16 +11,17 @@
{
using namespace testing;
- ON_CALL(*this, getReadings())
- .WillByDefault(Return(std::vector<MetricValue>()));
+ ON_CALL(*this, getUpdatedReadings())
+ .WillByDefault(ReturnRefOfCopy(std::vector<MetricValue>()));
ON_CALL(*this, sensorCount).WillByDefault(InvokeWithoutArgs([this] {
- return getReadings().size();
+ return getUpdatedReadings().size();
}));
}
MOCK_METHOD(void, initialize, (), (override));
MOCK_METHOD(void, deinitialize, (), (override));
- MOCK_METHOD(std::vector<MetricValue>, getReadings, (), (const, override));
+ MOCK_METHOD(const std::vector<MetricValue>&, getUpdatedReadings, (),
+ (override));
MOCK_METHOD(LabeledMetricParameters, dumpConfiguration, (),
(const, override));
MOCK_METHOD(uint64_t, sensorCount, (), (const, override));
diff --git a/tests/src/test_metric.cpp b/tests/src/test_metric.cpp
index f30b4f3..cd6a6ec 100644
--- a/tests/src/test_metric.cpp
+++ b/tests/src/test_metric.cpp
@@ -34,7 +34,8 @@
{
auto& metricMock =
result.emplace_back(std::make_shared<NiceMock<SensorMock>>());
- ON_CALL(*metricMock, metadata()).WillByDefault(Return("metadata"));
+ ON_CALL(*metricMock, metadata())
+ .WillByDefault(Return("metadata" + std::to_string(i)));
}
return result;
}
@@ -88,8 +89,7 @@
{
sut = makeSut(params);
- ASSERT_THAT(sut->getReadings(),
- ElementsAre(MetricValue({"id", "metadata", 0., 0u})));
+ ASSERT_THAT(sut->getUpdatedReadings(), ElementsAre());
}
TEST_F(TestMetric,
@@ -132,8 +132,7 @@
TEST_F(TestMetricAfterInitialization, containsEmptyReading)
{
- ASSERT_THAT(sut->getReadings(),
- ElementsAre(MetricValue({"id", "metadata", 0., 0u})));
+ ASSERT_THAT(sut->getUpdatedReadings(), ElementsAre());
}
TEST_F(TestMetricAfterInitialization, updatesMetricValuesOnSensorUpdate)
@@ -141,8 +140,8 @@
sut->sensorUpdated(*sensorMocks.front(), Milliseconds{18}, 31.2);
ASSERT_THAT(
- sut->getReadings(),
- ElementsAre(MetricValue{"id", "metadata", 31.2,
+ sut->getUpdatedReadings(),
+ ElementsAre(MetricValue{"id", "metadata0", 31.2,
std::chrono::duration_cast<Milliseconds>(
clockFake.system.timestamp())
.count()}));
@@ -356,10 +355,10 @@
const auto [expectedTimestamp, expectedReading] =
GetParam().expectedReading();
- const auto readings = sut->getReadings();
+ const auto readings = sut->getUpdatedReadings();
EXPECT_THAT(readings,
- ElementsAre(MetricValue{"id", "metadata", expectedReading,
+ ElementsAre(MetricValue{"id", "metadata0", expectedReading,
expectedTimestamp.count()}));
}
@@ -371,15 +370,15 @@
sut->sensorUpdated(*sensorMocks.front(), clockFake.steadyTimestamp(),
reading);
clockFake.advance(timestamp);
- sut->getReadings();
+ sut->getUpdatedReadings();
}
const auto [expectedTimestamp, expectedReading] =
GetParam().expectedReading();
- const auto readings = sut->getReadings();
+ const auto readings = sut->getUpdatedReadings();
EXPECT_THAT(readings,
- ElementsAre(MetricValue{"id", "metadata", expectedReading,
+ ElementsAre(MetricValue{"id", "metadata0", expectedReading,
expectedTimestamp.count()}));
}
@@ -388,3 +387,72 @@
EXPECT_THAT(sut->isTimerRequired(),
Eq(GetParam().expectedIsTimerRequired()));
}
+
+class TestMetricWithMultipleSensors : public TestMetric
+{
+ public:
+ TestMetricWithMultipleSensors()
+ {
+ sensorMocks = makeSensorMocks(7u);
+
+ sut = makeSut(params);
+ sut->initialize();
+ }
+};
+
+TEST_F(TestMetricWithMultipleSensors, readingsContainsAllReadingsInOrder)
+{
+ for (size_t i = 0; i < sensorMocks.size(); ++i)
+ {
+ sut->sensorUpdated(*sensorMocks[i], Milliseconds{i + 100}, i + 10.0);
+ sut->getUpdatedReadings();
+ }
+
+ clockFake.system.set(Milliseconds{72});
+
+ EXPECT_THAT(sut->getUpdatedReadings(),
+ ElementsAre(MetricValue{"id", "metadata0", 10.0, 72},
+ MetricValue{"id", "metadata1", 11.0, 72},
+ MetricValue{"id", "metadata2", 12.0, 72},
+ MetricValue{"id", "metadata3", 13.0, 72},
+ MetricValue{"id", "metadata4", 14.0, 72},
+ MetricValue{"id", "metadata5", 15.0, 72},
+ MetricValue{"id", "metadata6", 16.0, 72}));
+}
+
+TEST_F(TestMetricWithMultipleSensors, readingsContainOnlyAvailableSensors)
+{
+ for (auto i : {5u, 3u, 6u, 0u})
+ {
+ sut->sensorUpdated(*sensorMocks[i], Milliseconds{i + 100}, i + 10.0);
+ sut->getUpdatedReadings();
+ }
+
+ clockFake.system.set(Milliseconds{62});
+
+ EXPECT_THAT(sut->getUpdatedReadings(),
+ ElementsAre(MetricValue{"id", "metadata5", 15.0, 62},
+ MetricValue{"id", "metadata3", 13.0, 62},
+ MetricValue{"id", "metadata6", 16.0, 62},
+ MetricValue{"id", "metadata0", 10.0, 62}));
+}
+
+TEST_F(TestMetricWithMultipleSensors, readingsContainsAllReadingsOutOfOrder)
+{
+ for (auto i : {6u, 5u, 3u, 4u, 0u, 2u, 1u})
+ {
+ sut->sensorUpdated(*sensorMocks[i], Milliseconds{i + 100}, i + 10.0);
+ sut->getUpdatedReadings();
+ }
+
+ clockFake.system.set(Milliseconds{52});
+
+ EXPECT_THAT(sut->getUpdatedReadings(),
+ ElementsAre(MetricValue{"id", "metadata6", 16.0, 52},
+ MetricValue{"id", "metadata5", 15.0, 52},
+ MetricValue{"id", "metadata3", 13.0, 52},
+ MetricValue{"id", "metadata4", 14.0, 52},
+ MetricValue{"id", "metadata0", 10.0, 52},
+ MetricValue{"id", "metadata2", 12.0, 52},
+ MetricValue{"id", "metadata1", 11.0, 52}));
+}
diff --git a/tests/src/test_report.cpp b/tests/src/test_report.cpp
index a122544..c5eb4b9 100644
--- a/tests/src/test_report.cpp
+++ b/tests/src/test_report.cpp
@@ -81,8 +81,8 @@
for (size_t i = 0; i < metricParameters.size(); ++i)
{
- ON_CALL(*metricMocks[i], getReadings())
- .WillByDefault(Return(std::vector({readings[i]})));
+ ON_CALL(*metricMocks[i], getUpdatedReadings())
+ .WillByDefault(ReturnRefOfCopy(std::vector({readings[i]})));
ON_CALL(*metricMocks[i], dumpConfiguration())
.WillByDefault(Return(metricParameters[i]));
}