timestamp formatting bug

[sandyharvie]

We are occasionally seeing timestamps at midnight–that is, when the time is in the range [00:00:00, 00:00:01)–with the hour component as 24 instead of 0. Looking at the code in quill/include/quill/detail/backend/StringFromTime.h and quill/src/detail/backend/StringFromTime.cpp, it was not immediately obvious to me how this could happen, though the inconsistent nature of this issue suggests that the caching logic is likely the culprit. I tried to craft a test that would reproduce this behavior, but was unable to do so. Below is a sequence of timestamps for which this happened:

20230720T23:59:54.531861641
20230720T24:00:00.170651456
20230720T24:00:00.170651772
20230720T24:00:00.170683949
20230720T24:00:00.363200064
20230720T24:00:00.363200292
20230720T24:00:00.363224124
20230720T24:00:00.466231381
20230720T24:00:00.466231787
20230720T24:00:00.471552264
20230720T24:00:00.471552513
20230720T24:00:00.471553611
20230720T24:00:00.473809229
20230720T24:00:00.473809577
20230720T24:00:00.473810382
20230721T00:00:01.147892842

Our timestamp format is "%Y%m%dT%H:%M:%S.%Qns".

[sandyharvie]

We recently upgraded to 2.0.2 and never saw this on 1.6.2, so I'm wondering if 6ac18e6 might be the problematic change. It does not seem like there have been any other substantive changes to the timestamp formatting logic since 1.6.2.

[odygrd]

Thanks for reporting this! I will try to have a look and reproduce it over the next few days

[sandyharvie]

Could it have to do with using timestamp here instead of timestamp + 1 as is done below?

[sandyharvie]

Could it have to do with using timestamp here instead of timestamp + 1 as is done below?

Ah, never mind–we use quill::Timezone::LocalTime with our local time as GMT.

[odygrd]

I tried to reproduce the issue, but unfortunately, I couldn't. However, I'm feeling confident that the changes made in the pull request at https://github.com/odygrd/quill/pull/336/files will likely fix it.
Could you please give it a try and let me know if you're still experiencing the problem?

[sandyharvie]

Could you backport this fix to earlier versions? Upgrading to the latest version is not something we can necessarily do right now due to the many libfmt-related changes we'll need to make.

[odygrd]

Which version do you need patched?

[sandyharvie]

Could you do it for 2.0.2?

[odygrd]

I have made one here https://github.com/odygrd/quill/releases/tag/v2.0.3
I will keep this open until you confirm you are good
I would recommend to upgrade to latest release as there are some improvements there that are not in this release

[odygrd]

Could you backport this fix to earlier versions? Upgrading to the latest version is not something we can necessarily do right now due to the many libfmt-related changes we'll need to make.

Probably you are already aware of it but I just wanted to let you know. It is possible to build quill with an external libfmt version. You can have installed an older fmt version on your system and the logger should work.

quill/CMakeLists.txt

Line 8 in 7612b4a

option(QUILL_FMT_EXTERNAL "Use external fmt library instead of bundled" OFF)