From 28b7b010f18a223afe51538ffc09f1aabdb29cac Mon Sep 17 00:00:00 2001
From: Henning Baldersheim <balder@yahoo-inc.com>
Date: Tue, 28 Feb 2023 12:05:51 +0000
Subject: Use vespalib::steady_time for getMilliSecTime to ensure no wraps
 around and safer code.

---
 metrics/src/vespa/metrics/metricmanager.cpp | 29 +++++++++++++++--------------
 metrics/src/vespa/metrics/metricmanager.h   |  2 +-
 metrics/src/vespa/metrics/updatehook.h      |  3 +++
 3 files changed, 19 insertions(+), 15 deletions(-)

(limited to 'metrics/src')

diff --git a/metrics/src/vespa/metrics/metricmanager.cpp b/metrics/src/vespa/metrics/metricmanager.cpp
index da7b8f71fdf..9135dd86a16 100644
--- a/metrics/src/vespa/metrics/metricmanager.cpp
+++ b/metrics/src/vespa/metrics/metricmanager.cpp
@@ -25,6 +25,7 @@ using Config = MetricsmanagerConfig;
 using vespalib::IllegalStateException;
 using vespalib::IllegalArgumentException;
 using vespalib::make_string_short::fmt;
+using vespalib::count_ms;
 
 MetricManager::ConsumerSpec::ConsumerSpec() = default;
 MetricManager::ConsumerSpec::~ConsumerSpec() = default;
@@ -654,7 +655,7 @@ MetricManager::updatePeriodicMetrics(const MetricLockGuard & guard, time_t updat
 {
     assertMetricLockLocked(guard);
     time_t nextUpdateTime = std::numeric_limits<time_t>::max();
-    time_t preTime = _timer->getTimeInMilliSecs();
+    time_point preTime = _timer->getTimeInMilliSecs();
     for (auto hook : _periodicUpdateHooks) {
         if (hook->_nextCall <= updateTime) {
             hook->updateMetrics(guard);
@@ -668,13 +669,13 @@ MetricManager::updatePeriodicMetrics(const MetricLockGuard & guard, time_t updat
             } else {
                 hook->_nextCall += hook->_period;
             }
-            time_t postTime = _timer->getTimeInMilliSecs();
-            _periodicHookLatency.addValue(postTime - preTime);
+            time_point postTime = _timer->getTimeInMilliSecs();
+            _periodicHookLatency.addValue(count_ms(postTime - preTime));
             preTime = postTime;
         } else if (outOfSchedule) {
             hook->updateMetrics(guard);
-            time_t postTime = _timer->getTimeInMilliSecs();
-            _periodicHookLatency.addValue(postTime - preTime);
+            time_point postTime = _timer->getTimeInMilliSecs();
+            _periodicHookLatency.addValue(count_ms(postTime - preTime));
             preTime = postTime;
         }
         nextUpdateTime = std::min(nextUpdateTime, hook->_nextCall);
@@ -687,11 +688,11 @@ void
 MetricManager::updateSnapshotMetrics(const MetricLockGuard & guard)
 {
     assertMetricLockLocked(guard);
-    time_t preTime = _timer->getTimeInMilliSecs();
+    time_point preTime = _timer->getTimeInMilliSecs();
     for (const auto & hook : _snapshotUpdateHooks) {
         hook->updateMetrics(guard);
-        time_t postTime = _timer->getTimeInMilliSecs();
-        _snapshotHookLatency.addValue(postTime - preTime);
+        time_point postTime = _timer->getTimeInMilliSecs();
+        _snapshotHookLatency.addValue(count_ms(postTime - preTime));
         preTime = postTime;
     }
 }
@@ -708,7 +709,7 @@ MetricManager::forceEventLogging()
 void
 MetricManager::reset(time_t currentTime)
 {
-    time_t preTime = _timer->getTimeInMilliSecs();
+    time_point preTime = _timer->getTimeInMilliSecs();
     // Resetting implies visiting metrics, which needs to grab metric lock
     // to avoid conflict with adding/removal of metrics
     std::lock_guard waiterLock(_waiter);
@@ -717,8 +718,8 @@ MetricManager::reset(time_t currentTime)
         snapshot->reset(currentTime);
     }
     _totalMetrics->reset(currentTime);
-    time_t postTime = _timer->getTimeInMilliSecs();
-    _resetLatency.addValue(postTime - preTime);
+    time_point postTime = _timer->getTimeInMilliSecs();
+    _resetLatency.addValue(count_ms(postTime - preTime));
 }
 
 void
@@ -806,7 +807,7 @@ MetricManager::takeSnapshots(const MetricLockGuard & guard, time_t timeToProcess
              static_cast<uint64_t>(_snapshots[0]->getToTime()));
         return;
     }
-    time_t preTime = _timer->getTimeInMilliSecs();
+    time_point preTime = _timer->getTimeInMilliSecs();
     LOG(debug, "Updating %s snapshot and total metrics at time %" PRIu64 ".",
         _snapshots[0]->getName().c_str(), static_cast<uint64_t>(timeToProcess));
     MetricSnapshot& firstTarget(_snapshots[0]->getNextTarget());
@@ -838,8 +839,8 @@ MetricManager::takeSnapshots(const MetricLockGuard & guard, time_t timeToProcess
                 _snapshots[i]->getName().c_str(), static_cast<uint64_t>(timeToProcess));
         }
     }
-    time_t postTime = _timer->getTimeInMilliSecs();
-    _snapshotLatency.addValue(postTime - preTime);
+    time_point postTime = _timer->getTimeInMilliSecs();
+    _snapshotLatency.addValue(count_ms(postTime - preTime));
 }
 
 MemoryConsumption::UP
diff --git a/metrics/src/vespa/metrics/metricmanager.h b/metrics/src/vespa/metrics/metricmanager.h
index 2c595186ea4..3c81d9abd6a 100644
--- a/metrics/src/vespa/metrics/metricmanager.h
+++ b/metrics/src/vespa/metrics/metricmanager.h
@@ -68,7 +68,7 @@ public:
     struct Timer {
         virtual ~Timer() = default;
         virtual time_t getTime() const;
-        virtual time_t getTimeInMilliSecs() const { return getTime() * 1000; }
+        time_point getTimeInMilliSecs() const { return time_point(vespalib::from_s(getTime())); }
     };
 
     /**
diff --git a/metrics/src/vespa/metrics/updatehook.h b/metrics/src/vespa/metrics/updatehook.h
index 9fa0d52027e..f355197bbb7 100644
--- a/metrics/src/vespa/metrics/updatehook.h
+++ b/metrics/src/vespa/metrics/updatehook.h
@@ -1,10 +1,13 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
+#include <vespa/vespalib/util/time.h>
 #include <mutex>
 
 namespace metrics {
 
+using time_point = vespalib::steady_time;
+
 class MetricLockGuard {
 public:
     MetricLockGuard(std::mutex & mutex);
-- 
cgit v1.2.3