Add millisecond precision to StreamLogHandler timestamp#483
Add millisecond precision to StreamLogHandler timestamp#483krishnapermi wants to merge 8 commits into
Conversation
Refactor timestamp formatting to include milliseconds and timezone.
kukushechkin
left a comment
There was a problem hiding this comment.
Thank you @krishnapermi for looking into this! My concern is — not making the change across all the supported platforms is breaking user expectations for a cross-platform package. Could you please look into the windows implementation? In addition, this functionality is testable, so I would recommend adding tests.
|
Good catches - thanks for the thorough review. I've pushed two more commits to address both points: Windows support ( Tests ( |
kukushechkin
left a comment
There was a problem hiding this comment.
It seems Wasm tests are failing for a legit reason, CLOCK_REALTIME is not available. There are 2 jobs:
- https://github.com/apple/swift-log/actions/runs/29158273220/job/86911285818?pr=483
- https://github.com/apple/swift-log/actions/runs/29158273220/job/86911285870?pr=483
Could you please follow the same pattern as in other places, leveraging ifdef to provide implementation compatible with Wasm?
The
timestamp()function inStreamLogHandlerusestime(nil)withstrftime("%Y-%m-%dT%H:%M:%S%z"), which only provides second-level precision. When multiple log events occur within the same second they share an identical timestamp, making it impossible to determine their ordering from the log output.Fix: On non-Windows platforms, replace
time(nil)withclock_gettime(CLOCK_REALTIME, &ts)to obtain nanosecond-resolution wall-clock time, then inject milliseconds between the seconds and timezone fields. Example output:I noticed this while looking at swift-log's
StreamLogHandlerimplementation - the hardcoded format string never included sub-second digits, and on busy systems many consecutive log lines ended up with the same timestamp.The Windows path is unchanged; adding sub-second support there needs different APIs and is a separate concern.
Fixes #212