Skip to content

Fix WSL container event timestamps - #41717

Merged
beena352 merged 6 commits into
microsoft:masterfrom
beena352:users/beenachauhan/events-timestamp-nanoseconds
Sep 30, 2026
Merged

beena352 merged 6 commits into
microsoft:masterfrom
beena352:users/beenachauhan/events-timestamp-nanoseconds

Conversation

@beena352

@beena352 beena352 commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

This updates WSL container events to keep Docker’s nanosecond timestamps and display them in Docker’s timestamp format. UTC timestamps now use Z, and timestamps keep all nine fractional digits.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Docker provides both time and timeNano for events. WSL was only keeping the seconds value, which caused the CLI to lose timestamp precision.
This change carries timeNano through event tracking and storage, then uses it when formatting event timestamps. Existing seconds-based filtering and state tracking are unchanged. Volume and process event callbacks were updated to carry the same timestamp information consistently.
The output now follows Docker’s format, including nine fractional digits and Z for UTC timestamps.

Validation Steps Performed

Copilot AI balanced review requested due to automatic review settings September 29, 2026 00:00

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

Timestamp propagation, formatting, fallback behavior, and corresponding tests are consistent and complete.

Review effort: Balanced
Findings: None

What changed in this PR

Propagates Docker nanosecond timestamps through WSLC event handling and renders accurate RFC 3339 event times.

Changes:

  • Preserve Docker timeNano alongside epoch seconds.
  • Format event output with nanosecond precision and historical local offsets.
  • Extend unit and end-to-end timestamp validation.
File Description
test/​windows/​WSLCTests.cpp Verifies recorded timestamps match Docker events.
test/​windows/​wslc/​WSLCCLIRelativeTimeUnitTests.cpp Tests nanosecond timestamp formatting.
test/​windows/​wslc/​e2e/​WSLCE2EEventsTests.cpp Validates variable UTC/local timestamp forms.
src/​windows/​wslcsession/​WSLCVolumes.h Updates volume callback timestamp type.
src/​windows/​wslcsession/​WSLCVolumes.cpp Accepts structured event timestamps.
src/​windows/​wslcsession/​WSLCSession.h Updates session event callback declarations.
src/​windows/​wslcsession/​WSLCSession.cpp Propagates structured timestamps to storage.
src/​windows/​wslcsession/​WSLCProcessControl.h Updates process event callback declaration.
src/​windows/​wslcsession/​WSLCProcessControl.cpp Accepts structured process timestamps.
src/​windows/​wslcsession/​WSLCContainer.h Updates container event interfaces.
src/​windows/​wslcsession/​WSLCContainer.cpp Preserves nanoseconds through state transitions.
src/​windows/​wslcsession/​EventStore.h Updates event recording interface.
src/​windows/​wslcsession/​EventStore.cpp Stores seconds and nanoseconds.
src/​windows/​wslcsession/​DockerEventTracker.h Updates tracker callback timestamp types.
src/​windows/​wslcsession/​DockerEventTracker.cpp Parses Docker’s timeNano field.
src/​windows/​wslc/​services/​SessionService.cpp Uses nanosecond-aware output formatting.
src/​windows/​inc/​wslc_schema.h Adds timestamp structure and JSON field.
src/​windows/​common/​timestamp.hpp Declares RFC 3339 nanosecond helpers.
src/​windows/​common/​timestamp.cpp Implements nanosecond and local-offset formatting.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@beena352
beena352 marked this pull request as ready for review September 29, 2026 16:44
@beena352
beena352 requested review from a team as code owners September 29, 2026 16:44
Comment thread test/windows/WSLCTests.cpp Outdated
Comment thread src/windows/inc/wslc_schema.h Outdated
Comment thread src/windows/wslcsession/DockerEventTracker.cpp
Copilot AI balanced review requested due to automatic review settings September 29, 2026 19:28

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

A valid far-future seconds-based filter bound can overflow during comparison with a nanosecond event time.

Review effort: Balanced
Findings: 1 High severity

Open (1)

Comment thread src/windows/wslcsession/EventStore.cpp Outdated
Copilot AI balanced review requested due to automatic review settings September 29, 2026 20:23

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Cross-component timestamp handling needs final Windows and Docker integration validation, including network-event precision.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Network event test does not verify nanosecond timestamp precision

test/​windows/​WSLCTests.cpp:7518

This positive-value check does not verify the network timestamp change: it also passes if a callback forwards Docker's seconds-valued time, or if the stored value loses its fractional nanoseconds. The container lifecycle test compares each stored timeNano with Docker's TimeNano; please make the same comparison for at least one network event (matched by network and action) so a network-only precision regression fails this test.

@beena352 beena352 mentioned this pull request Sep 30, 2026
1 of 6 tasks
Copilot AI balanced review requested due to automatic review settings September 30, 2026 06:40

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

The cross-component timestamp path needs Windows integration validation, and the remaining issues need fixes.

Review effort: Balanced
Findings: None

Previously missed (2)

In code that hasn't changed since last review

Medium severity Prevent nanosecond overflow when applying UTC offsets

src/​windows/​common/​timestamp.cpp:322

time uses signed 64-bit nanoseconds, so adding a local UTC offset here can overflow even when timeNano itself is valid (for example, a time near the upper limit with a +14:00 offset). The CLI can then print a date centuries earlier. Apply the offset to whole seconds and format the nine fractional digits separately; cover both timestamp limits in the formatter tests.

Medium severity Use timed cancellation when reading events

test/​windows/​WSLCTests.cpp:7410

ReadEvents passes a null cancellation handle to GetNext. If any of these five events is missing or skipped, the stream waits until its year-3000 until bound instead of failing the test. Keep that far-future bound to test the comparison, but pass a short, timed cancellation handle to each read and fail if it expires.

Copilot AI balanced review requested due to automatic review settings September 30, 2026 07:43

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Historical time-zone offsets containing seconds can make a formatted event timestamp identify the wrong instant.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Historical sub-minute offsets produce incorrect timestamp suffix

src/​windows/​common/​timestamp.cpp:330

Historical time zones can have offsets with seconds (for example, +00:19:32). Here the clock is shifted by the full offset, but the suffix at line 330 prints only minutes, so the timestamp describes a different instant. Shift the clock by the same whole-minute offset printed in the suffix, and use Z if that offset becomes zero. Add a test for a sub-minute offset.

…beenachauhan/events-timestamp-nanoseconds
Copilot AI balanced review requested due to automatic review settings September 30, 2026 07:51

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Historical sub-minute offsets still need correction, and the asynchronous event and COM path warrants Windows integration validation.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Comment thread src/windows/common/timestamp.cpp

@OneBlue Blue (OneBlue) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, feel free to resolve the minor comment in a followup

{
WSLCFilter filter{"container", id.c_str()};
wil::com_ptr<IWSLCEventStream> stream;
VERIFY_SUCCEEDED(m_defaultSession->GetEvents(since, 32'503'680'000, &filter, 1, &stream));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: I had to look up that 32'503'680'000 evaluates to 3000-01-01. This might be worth a comment

@beena352
beena352 merged commit 35468e9 into microsoft:master Sep 30, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants