Skip to content

Add millisecond precision to StreamLogHandler timestamp - #483

Open
krishnapermi wants to merge 13 commits into
apple:mainfrom
krishnapermi:fix-timestamp-millisecond-precision
Open

krishnapermi wants to merge 13 commits into
apple:mainfrom
krishnapermi:fix-timestamp-millisecond-precision

Conversation

@krishnapermi

Copy link
Copy Markdown

The timestamp() function in StreamLogHandler uses time(nil) with strftime("%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) with clock_gettime(CLOCK_REALTIME, &ts) to obtain nanosecond-resolution wall-clock time, then inject milliseconds between the seconds and timezone fields. Example output:

2021-11-06T18:47:14.123+0300 info myapp: message

I noticed this while looking at swift-log's StreamLogHandler implementation - 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

Refactor timestamp formatting to include milliseconds and timezone.

@kukushechkin kukushechkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@krishnapermi

Copy link
Copy Markdown
Author

Good catches - thanks for the thorough review.

I've pushed two more commits to address both points:

Windows support (0c8dc77): The Windows branch now uses _ftime64_s with __timeb64 to get millisecond-precision wall-clock time, the same way the existing code already uses _localtime64_s for local time conversion. I refactored the formatting code (buffer-to-string conversion, zero-padding, the final return) to sit after the #endif so it's shared between platforms rather than duplicated.

Tests (24686d1): Added StreamLogHandlerTimestampTests.swift in Tests/LoggingTests/. The tests create a StreamLogHandler backed by a simple in-memory CapturingStream, emit log lines, and verify that the timestamp field contains a 3-digit millisecond component in the right position and within [0, 999]. There's also a structural check confirming the overall ISO 8601 shape.

Comment thread Sources/Logging/Handlers/StreamLogHandler.swift Outdated
@kukushechkin kukushechkin added the 🆕 semver/minor Adds new public API. label Jul 6, 2026

@kukushechkin kukushechkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems Wasm tests are failing for a legit reason, CLOCK_REALTIME is not available. There are 2 jobs:

Could you please follow the same pattern as in other places, leveraging ifdef to provide implementation compatible with Wasm?

@krishnapermi

Copy link
Copy Markdown
Author

Fixed in the latest commit by adding a #elseif canImport(WASILibc) branch before #else, following the same pattern used elsewhere in the codebase (e.g. the StdioOutputStream stdin/stdout/stderr setup).

WASI defines CLOCK_REALTIME as a pointer-to-struct macro, which Swift cannot import. The Wasm branch falls back to time(nil), giving second-level precision (same as the original implementation had before this PR). The ms field is set to 0 so the timestamp format stays consistent across all platforms.

Comment thread Sources/Logging/Handlers/StreamLogHandler.swift Outdated
@FranzBusch

Copy link
Copy Markdown
Member

@weissi WDYT? Do you consider this a semantically breaking change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

a hardcoded timestamp does not contain milli- or nanoseconds in any format

3 participants