Skip to content

Use types from windows-sys#273

Merged
beaubelgrave merged 4 commits into
microsoft:mainfrom
utpilla:utpilla/Use-window-sys-types
May 19, 2026
Merged

Use types from windows-sys#273
beaubelgrave merged 4 commits into
microsoft:mainfrom
utpilla:utpilla/Use-window-sys-types

Conversation

@utpilla
Copy link
Copy Markdown
Contributor

@utpilla utpilla commented May 19, 2026

Changes

Replaces four hand-rolled ETW types that surface (or will surface) on the crate's public API with re-exports from windows-sys:

  • EVENT_RECORD
  • EVENT_HEADER
  • EVENT_HEADER_EXTENDED_DATA_ITEM
  • EVENT_DESCRIPTOR

Why

Precursor to the TDH decoder work tracked in #270. That decoder will take a &EVENT_RECORD from AncillaryData::record() and hand it to a TDH-based decode call. Sourcing the public types from windows-sys gives us a single type identity end to end and shrinks the one_collect crate's public API surface.

This was the open question I raised on #270, and this PR is following Option B mentioned in the issue.

Scope

Deliberately constrained to only the four public types. The rest of etw/abi.rs stays hand-rolled:

  • All extern "system" blocks (StartTraceW, ControlTraceW, EnableTraceEx2, OpenTraceW, ProcessTrace, CloseTrace, TraceQueryInformation, TraceSetInformation, plus the privilege helpers and GetActiveProcessorCount).
  • All internal-only structs (EVENT_TRACE_PROPERTIES, WNODE_HEADER, ENABLE_TRACE_PARAMETERS, EVENT_FILTER_DESCRIPTOR, EVENT_TRACE_LOGFILE, TRACE_LOGFILE_HEADER, EVENT_TRACE, EVENT_TRACE_HEADER, TRACE_PROFILE_INTERVAL, TOKEN_PRIVILEGES, EVENT_FILTER_LEVEL_KW, CLASSIC_EVENT_ID).
  • All TRACE_LEVEL_* / EVENT_CONTROL_CODE_* / EVENT_HEADER_EXT_TYPE_* / EVENT_ENABLE_PROPERTY_* constants.
  • TraceSession, TraceEnable, TraceFilter, flush_trace, wide_string.

A full migration of the FFI is a fair follow-up but out of scope here to keep this PR focused on the bare-minimum change the TDH work needs.

Safety

The trait impl contains three unsafe operations, each backed by a const _: () = assert!(...) guard above the impl block so future windows-sys layout drift fails the build instead of producing wrong bytes silently:

  • provider_guid / activity_guid transmute windows_sys::core::GUID → our Guid. Guarded by size_of + align_of equality asserts.
  • processor_index reads u16 out of a union arm. Guarded by an asserted size_of == 2 and an align_of >= align_of::<u16>() on ETW_BUFFER_CONTEXT_0.

Follow-ups (not in this PR)

@utpilla utpilla marked this pull request as draft May 19, 2026 05:31
@utpilla utpilla marked this pull request as ready for review May 19, 2026 05:45
Comment thread one_collect/Cargo.lock
"sha1",
"tracing",
"twox-hash",
"windows-sys",
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.

@brianrob, do we need to list this in third parties md? Not sure if this is technically 3rd party or not.

Copy link
Copy Markdown
Collaborator

@beaubelgrave beaubelgrave left a comment

Choose a reason for hiding this comment

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

Approved, looks good, thanks for doing this! We may require a follow up item on the third party tracking of windows-sys if it's technically 3rd party.

@beaubelgrave
Copy link
Copy Markdown
Collaborator

Windows-sys uses MIT license, we should go ahead and add it to our THIRD-PARTY-NOTICES.md before merging. If you update this PR with that fix, I'll go ahead and approve again and merge. @utpilla.

@utpilla
Copy link
Copy Markdown
Contributor Author

utpilla commented May 19, 2026

Windows-sys uses MIT license, we should go ahead and add it to our THIRD-PARTY-NOTICES.md before merging. If you update this PR with that fix, I'll go ahead and approve again and merge. @utpilla.

I have updated THIRD_PARTY_NOTICES.md.

Reference: https://github.com/microsoft/windows-rs/blob/master/license-mit

Copy link
Copy Markdown
Collaborator

@beaubelgrave beaubelgrave left a comment

Choose a reason for hiding this comment

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

Thanks!

@beaubelgrave beaubelgrave merged commit 4abefd5 into microsoft:main May 19, 2026
14 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.

2 participants