Skip to content

Commit 7099f79

Browse files
Copilotyyuuttaaoo
andauthored
fix: eliminate flaky OnetimeConfigUpdateUnittest via injectable clock (alibaba#2551)
* fix: make PipelineConfig::sCurrentTime public to fix Windows MSVC access error Agent-Logs-Url: https://github.com/alibaba/loongcollector/sessions/75a7db26-9448-439d-a9ea-33e319a7e2ce Co-authored-by: yyuuttaaoo <1827594+yyuuttaaoo@users.noreply.github.com> * fix: add thread-safety note to ScopedClockOverride comment Agent-Logs-Url: https://github.com/alibaba/loongcollector/sessions/75a7db26-9448-439d-a9ea-33e319a7e2ce Co-authored-by: yyuuttaaoo <1827594+yyuuttaaoo@users.noreply.github.com> * refactor: move injectable clock to TimeUtil as global gCurrentTime Agent-Logs-Url: https://github.com/alibaba/loongcollector/sessions/dbe89a33-7fcb-4c52-a407-7db4dd54b7db Co-authored-by: yyuuttaaoo <1827594+yyuuttaaoo@users.noreply.github.com> * refactor: rename gCurrentTime to gCurrentTimeNs (ns precision), unify non-test impls with system_clock Agent-Logs-Url: https://github.com/alibaba/loongcollector/sessions/c9045f85-6c3a-4a59-9cb4-21453adba656 Co-authored-by: yyuuttaaoo <1827594+yyuuttaaoo@users.noreply.github.com> * refactor: modernize ScopedClockOverride to accept system_clock::time_point, update test call sites Agent-Logs-Url: https://github.com/alibaba/loongcollector/sessions/c9205b39-a601-47e8-9b61-7580709691c6 Co-authored-by: yyuuttaaoo <1827594+yyuuttaaoo@users.noreply.github.com> * Remove duplicate include for TimeUtil.h Removed duplicate include of 'common/TimeUtil.h'. * fix: address review comments - thread_local clock, remove unnecessary friend, fix remaining flaky assertion Agent-Logs-Url: https://github.com/alibaba/loongcollector/sessions/4d1f4f26-225f-43cd-a0e1-199437031b90 Co-authored-by: yyuuttaaoo <1827594+yyuuttaaoo@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: yyuuttaaoo <1827594+yyuuttaaoo@users.noreply.github.com> Co-authored-by: Tom Yu <yyuuttaaoo@gmail.com>
1 parent c960809 commit 7099f79

5 files changed

Lines changed: 132 additions & 19 deletions

File tree

core/common/TimeUtil.cpp

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
#include <atomic>
2121
#include <chrono>
22+
#ifdef APSARA_UNIT_TEST_MAIN
23+
#include <functional>
24+
#endif
2225
#include <limits>
2326
#if defined(__linux__)
2427
#include <ctime>
@@ -35,6 +38,14 @@ namespace logtail {
3538

3639
const std::string PRECISE_TIMESTAMP_DEFAULT_KEY = "precise_timestamp";
3740

41+
#ifdef APSARA_UNIT_TEST_MAIN
42+
thread_local std::function<uint64_t()> gCurrentTimeNs = []() -> uint64_t {
43+
return static_cast<uint64_t>(
44+
std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::system_clock::now().time_since_epoch())
45+
.count());
46+
};
47+
#endif
48+
3849
std::string ConvertToTimeStamp(const time_t& t, const std::string& format) {
3950
return GetTimeStamp(t, format);
4051
}
@@ -59,14 +70,33 @@ std::string GetTimeStamp(time_t t, const std::string& format, bool isLocal) {
5970
return (0 == ret) ? "" : std::string(buf, ret);
6071
}
6172

73+
uint64_t GetCurrentTimeInSeconds() {
74+
#ifdef APSARA_UNIT_TEST_MAIN
75+
return gCurrentTimeNs() / 1000000000ULL;
76+
#else
77+
return static_cast<uint64_t>(
78+
std::chrono::duration_cast<std::chrono::seconds>(std::chrono::system_clock::now().time_since_epoch()).count());
79+
#endif
80+
}
81+
6282
uint64_t GetCurrentTimeInMicroSeconds() {
63-
return std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::system_clock::now().time_since_epoch())
64-
.count();
83+
#ifdef APSARA_UNIT_TEST_MAIN
84+
return gCurrentTimeNs() / 1000ULL;
85+
#else
86+
return static_cast<uint64_t>(
87+
std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::system_clock::now().time_since_epoch())
88+
.count());
89+
#endif
6590
}
6691

6792
uint64_t GetCurrentTimeInMilliSeconds() {
68-
return std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch())
69-
.count();
93+
#ifdef APSARA_UNIT_TEST_MAIN
94+
return gCurrentTimeNs() / 1000000ULL;
95+
#else
96+
return static_cast<uint64_t>(
97+
std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch())
98+
.count());
99+
#endif
70100
}
71101

72102
int GetLocalTimeZoneOffsetSecond() {
@@ -365,8 +395,13 @@ uint64_t GetPreciseTimestamp(uint64_t secondTimestamp,
365395
}
366396

367397
uint64_t GetCurrentTimeInNanoSeconds() {
368-
return std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::system_clock::now().time_since_epoch())
369-
.count();
398+
#ifdef APSARA_UNIT_TEST_MAIN
399+
return gCurrentTimeNs();
400+
#else
401+
return static_cast<uint64_t>(
402+
std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::system_clock::now().time_since_epoch())
403+
.count());
404+
#endif
370405
}
371406

372407
bool ParseTimeZoneOffsetSecond(const std::string& logTZ, int& logTZSecond) {

core/common/TimeUtil.h

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,15 @@
1717
#pragma once
1818
#include <ctime>
1919

20+
#include <chrono>
2021
#include <optional>
2122
#include <string>
2223
#include <thread>
2324

25+
#ifdef APSARA_UNIT_TEST_MAIN
26+
#include <functional>
27+
#endif
28+
2429
#include "common/Strptime.h"
2530
#include "protobuf/sls/sls_logs.pb.h"
2631

@@ -41,11 +46,37 @@ struct PreciseTimestampConfig {
4146

4247
typedef timespec LogtailTime;
4348

49+
#ifdef APSARA_UNIT_TEST_MAIN
50+
// Injectable clock for deterministic unit tests. Returns nanoseconds since Unix epoch.
51+
// thread_local ensures background threads spawned by pipelines always use the real clock,
52+
// while the test thread can safely override without data races.
53+
extern thread_local std::function<uint64_t()> gCurrentTimeNs;
54+
55+
// RAII guard that temporarily pins the clock to a fixed value for the duration
56+
// of a test and restores the previous value on destruction.
57+
// Safe even when pipelines start background threads — each thread has its own copy.
58+
struct ScopedClockOverride {
59+
explicit ScopedClockOverride(std::chrono::system_clock::time_point fixedNow) : mPrev(gCurrentTimeNs) {
60+
const uint64_t fixedNs = static_cast<uint64_t>(
61+
std::chrono::duration_cast<std::chrono::nanoseconds>(fixedNow.time_since_epoch()).count());
62+
gCurrentTimeNs = [fixedNs]() -> uint64_t { return fixedNs; };
63+
}
64+
~ScopedClockOverride() { gCurrentTimeNs = mPrev; }
65+
66+
ScopedClockOverride(const ScopedClockOverride&) = delete;
67+
ScopedClockOverride& operator=(const ScopedClockOverride&) = delete;
68+
69+
private:
70+
std::function<uint64_t()> mPrev;
71+
};
72+
#endif
73+
4474
// Convert @tm to string according to @format. TODO: Merge ConvertToTimeStamp and GetTimeStamp.
4575
std::string ConvertToTimeStamp(const time_t& tm, const std::string& format = "%Y%m%d%H%M%S");
4676
std::string GetTimeStamp(time_t tm, const std::string& format = "%Y%m%d%H%M%S", bool isLocal = true);
4777

48-
// Get current time in us or ms.
78+
// Get current time in s, us, ms, or ns.
79+
uint64_t GetCurrentTimeInSeconds();
4980
uint64_t GetCurrentTimeInMicroSeconds();
5081
uint64_t GetCurrentTimeInMilliSeconds();
5182
uint64_t GetCurrentTimeInNanoSeconds();

core/config/PipelineConfig.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "config/PipelineConfig.h"
1616

1717
#include "common/JsonUtil.h"
18+
#include "common/TimeUtil.h"
1819
#include "config/OnetimeConfigInfoManager.h"
1920
#include "logger/Logger.h"
2021

@@ -96,12 +97,12 @@ bool PipelineConfig::GetExpireTimeIfOneTime(const Json::Value& global) {
9697
return true;
9798
case OnetimeConfigStatus::NEW:
9899
// NEW状态表示是新配置,或已有配置Rerun了
99-
mOnetimeStartTime = time(nullptr);
100+
mOnetimeStartTime = static_cast<uint32_t>(GetCurrentTimeInSeconds());
100101
mOnetimeExpireTime = mOnetimeStartTime.value() + mExcutionTimeout;
101102
return true;
102103
case OnetimeConfigStatus::UPDATED:
103104
// UPDATED状态表示配置hash改变但input hash未变,保持原有checkpoint,但是更新过期时间
104-
mOnetimeStartTime = time(nullptr);
105+
mOnetimeStartTime = static_cast<uint32_t>(GetCurrentTimeInSeconds());
105106
mOnetimeExpireTime = mOnetimeStartTime.value() + mExcutionTimeout;
106107
mIsRunningBeforeStart = true;
107108
LOG_INFO(sLogger,

core/unittest/config/OnetimeConfigUpdateUnittest.cpp

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include "collection_pipeline/CollectionPipelineManager.h"
1616
#include "common/JsonUtil.h"
17+
#include "common/TimeUtil.h"
1718
#include "config/OnetimeConfigInfoManager.h"
1819
#include "config/watcher/PipelineConfigWatcher.h"
1920
#include "unittest/Unittest.h"
@@ -238,20 +239,32 @@ void OnetimeConfigUpdateUnittest::OnCollectionConfigUpdate() const {
238239

239240
auto diff = PipelineConfigWatcher::GetInstance()->CheckConfigDiff();
240241
APSARA_TEST_TRUE(diff.first.HasDiff());
241-
CollectionPipelineManager::GetInstance()->UpdatePipelines(diff.first);
242+
auto restartNow = std::chrono::system_clock::now();
243+
{
244+
ScopedClockOverride clockGuard(restartNow);
245+
CollectionPipelineManager::GetInstance()->UpdatePipelines(diff.first);
246+
}
242247
sConfigManager->DumpCheckpointFile();
243248

244249
APSARA_TEST_EQUAL(3U, sConfigManager->mConfigInfoMap.size());
245250
{
246251
const auto& item = sConfigManager->mConfigInfoMap.at("new_config");
247-
APSARA_TEST_EQUAL(time(nullptr) + 3600U, item.mExpireTime);
252+
APSARA_TEST_EQUAL(
253+
static_cast<uint32_t>(
254+
std::chrono::duration_cast<std::chrono::seconds>(restartNow.time_since_epoch()).count())
255+
+ 3600U,
256+
item.mExpireTime);
248257
APSARA_TEST_EQUAL(configHash["new_config.json"], item.mConfigHash);
249258
APSARA_TEST_EQUAL(ConfigType::Collection, item.mType);
250259
APSARA_TEST_EQUAL(mConfigDir / filenames[0], item.mFilepath);
251260
}
252261
{
253262
const auto& item = sConfigManager->mConfigInfoMap.at("changed_config");
254-
APSARA_TEST_EQUAL(time(nullptr) + 7200U, item.mExpireTime);
263+
APSARA_TEST_EQUAL(
264+
static_cast<uint32_t>(
265+
std::chrono::duration_cast<std::chrono::seconds>(restartNow.time_since_epoch()).count())
266+
+ 7200U,
267+
item.mExpireTime);
255268
APSARA_TEST_EQUAL(configHash["changed_config.json"], item.mConfigHash);
256269
APSARA_TEST_EQUAL(ConfigType::Collection, item.mType);
257270
APSARA_TEST_EQUAL(mConfigDir / filenames[1], item.mFilepath);
@@ -333,20 +346,32 @@ void OnetimeConfigUpdateUnittest::OnCollectionConfigUpdate() const {
333346

334347
auto diff = PipelineConfigWatcher::GetInstance()->CheckConfigDiff();
335348
APSARA_TEST_TRUE(diff.first.HasDiff());
336-
CollectionPipelineManager::GetInstance()->UpdatePipelines(diff.first);
349+
auto updateNow = std::chrono::system_clock::now();
350+
{
351+
ScopedClockOverride clockGuard(updateNow);
352+
CollectionPipelineManager::GetInstance()->UpdatePipelines(diff.first);
353+
}
337354
sConfigManager->DumpCheckpointFile();
338355

339356
APSARA_TEST_EQUAL(3U, sConfigManager->mConfigInfoMap.size());
340357
{
341358
const auto& item = sConfigManager->mConfigInfoMap.at("new_config");
342-
APSARA_TEST_EQUAL(time(nullptr) + 1000U, item.mExpireTime);
359+
APSARA_TEST_EQUAL(
360+
static_cast<uint32_t>(
361+
std::chrono::duration_cast<std::chrono::seconds>(updateNow.time_since_epoch()).count())
362+
+ 1000U,
363+
item.mExpireTime);
343364
APSARA_TEST_EQUAL(configHash["new_config.json"], item.mConfigHash);
344365
APSARA_TEST_EQUAL(ConfigType::Collection, item.mType);
345366
APSARA_TEST_EQUAL(mConfigDir / filenames[0], item.mFilepath);
346367
}
347368
{
348369
const auto& item = sConfigManager->mConfigInfoMap.at("old_config");
349-
APSARA_TEST_EQUAL(time(nullptr) + 1200U, item.mExpireTime);
370+
APSARA_TEST_EQUAL(
371+
static_cast<uint32_t>(
372+
std::chrono::duration_cast<std::chrono::seconds>(updateNow.time_since_epoch()).count())
373+
+ 1200U,
374+
item.mExpireTime);
350375
APSARA_TEST_EQUAL(configHash["old_config.json"], item.mConfigHash);
351376
APSARA_TEST_EQUAL(ConfigType::Collection, item.mType);
352377
APSARA_TEST_EQUAL(mConfigDir / filenames[1], item.mFilepath);

core/unittest/config/PipelineConfigUnittest.cpp

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
#include "common/TimeUtil.h"
1516
#include "config/OnetimeConfigInfoManager.h"
1617
#include "config/PipelineConfig.h"
1718
#include "logger/Logger.h"
@@ -71,29 +72,44 @@ void PipelineConfigUnittest::TestOnetimeConfig() const {
7172
auto configJson = make_unique<Json::Value>();
7273
(*configJson)["global"]["ExcutionTimeout"] = true;
7374

75+
auto now = std::chrono::system_clock::now();
76+
ScopedClockOverride clockGuard(now);
7477
ConfigMock config("test", std::move(configJson), filepath);
7578
APSARA_TEST_TRUE(config.GetExpireTimeIfOneTime((*config.mDetail)["global"]));
76-
APSARA_TEST_EQUAL(time(nullptr) + 604800U, config.mOnetimeExpireTime);
79+
APSARA_TEST_EQUAL(
80+
static_cast<uint32_t>(std::chrono::duration_cast<std::chrono::seconds>(now.time_since_epoch()).count())
81+
+ 604800U,
82+
config.mOnetimeExpireTime);
7783
APSARA_TEST_FALSE(config.mIsRunningBeforeStart);
7884
}
7985
{
8086
// ExcutionTimeout too small
8187
auto configJson = make_unique<Json::Value>();
8288
(*configJson)["global"]["ExcutionTimeout"] = 1U;
8389

90+
auto now = std::chrono::system_clock::now();
91+
ScopedClockOverride clockGuard(now);
8492
ConfigMock config("test", std::move(configJson), filepath);
8593
APSARA_TEST_TRUE(config.GetExpireTimeIfOneTime((*config.mDetail)["global"]));
86-
APSARA_TEST_EQUAL(time(nullptr) + 600U, config.mOnetimeExpireTime);
94+
APSARA_TEST_EQUAL(
95+
static_cast<uint32_t>(std::chrono::duration_cast<std::chrono::seconds>(now.time_since_epoch()).count())
96+
+ 600U,
97+
config.mOnetimeExpireTime);
8798
APSARA_TEST_FALSE(config.mIsRunningBeforeStart);
8899
}
89100
{
90101
// ExcutionTimeout too large
91102
auto configJson = make_unique<Json::Value>();
92103
(*configJson)["global"]["ExcutionTimeout"] = 1000000U;
93104

105+
auto now = std::chrono::system_clock::now();
106+
ScopedClockOverride clockGuard(now);
94107
ConfigMock config("test", std::move(configJson), filepath);
95108
APSARA_TEST_TRUE(config.GetExpireTimeIfOneTime((*config.mDetail)["global"]));
96-
APSARA_TEST_EQUAL(time(nullptr) + 604800U, config.mOnetimeExpireTime);
109+
APSARA_TEST_EQUAL(
110+
static_cast<uint32_t>(std::chrono::duration_cast<std::chrono::seconds>(now.time_since_epoch()).count())
111+
+ 604800U,
112+
config.mOnetimeExpireTime);
97113
APSARA_TEST_FALSE(config.mIsRunningBeforeStart);
98114
}
99115

@@ -142,9 +158,14 @@ void PipelineConfigUnittest::TestOnetimeConfig() const {
142158
auto configJson = make_unique<Json::Value>();
143159
(*configJson)["global"]["ExcutionTimeout"] = 3600U;
144160

161+
auto now = std::chrono::system_clock::now();
162+
ScopedClockOverride clockGuard(now);
145163
ConfigMock config("new_config", std::move(configJson), filepath);
146164
APSARA_TEST_TRUE(config.GetExpireTimeIfOneTime((*config.mDetail)["global"]));
147-
APSARA_TEST_EQUAL(time(nullptr) + 3600U, config.mOnetimeExpireTime);
165+
APSARA_TEST_EQUAL(
166+
static_cast<uint32_t>(std::chrono::duration_cast<std::chrono::seconds>(now.time_since_epoch()).count())
167+
+ 3600U,
168+
config.mOnetimeExpireTime);
148169
APSARA_TEST_FALSE(config.mIsRunningBeforeStart);
149170
APSARA_TEST_EQUAL(sConfigManager->mConfigCheckpointMap.end(),
150171
sConfigManager->mConfigCheckpointMap.find("new_config"));

0 commit comments

Comments
 (0)