[m96-chan]

Summary

The last two things #72 asked for. Stacked on #84 — base is feat/72-nested-structs, so review that first; GitHub will retarget this to main when #84 merges.

With this, every item in #72's proposed approach is either done or has a stated reason it cannot be.

Changes

1. Events TDH cannot describe keep their payload

TdhGetEventInformation has no schema for WPP without a matching TMF, nor for a manifest provider whose manifest is not installed here. Those events came back with an empty dict and nothing else — the payload was read, then dropped.

There are no names to report it under, so it cannot become properties. Keep the bytes instead. EtwEvent.raw_data has existed all along but was never populated, the comment in session.rs explaining that ferrisetw keeps its user_buffer private — but the record handed to TDH carries the same bytes and this module already holds it.

Only for schema-less events; a second copy of every decoded event's data would cost memory for nothing.

Also set_wpp_tmf_search_path, passing TDH a TDH_CONTEXT_WPP_TMFSEARCHPATH so a caller who has .tmf files gets WPP decoded properly instead of falling back to bytes. Unset by default, in which case TDH is passed no context at all.

2. TDH's display strings, opt-in

#72 asked whether typed values or TdhFormatProperty's strings should be the default and whether the other should be available. Typed values stay the default — the Parquet/Arrow exporters need them — and the strings become a second map, formatted_properties, behind set_property_formatting. Off by default, so the normal path pays nothing.

TdhFormatProperty is given one property's bytes, already isolated by TdhGetProperty, rather than the event payload with running offsets. That walk is what this module avoids on purpose, and skipping it costs nothing because TDH has already located the value.

Related Issue

Advances #72 to completion — see the closing section.

Test Plan

Added/Modified Tests

• 3 unit tests in tdh.rs (formatting default/toggle, TMF path round-trip, no context when unset).
• 7 tests in test_etl_reader.py over a new no_schema.etl fixture.

The existing fixtures cannot exercise the schema-less path — everything in sample.etl and nested_struct.etl decodes cleanly, which is the point of them. no_schema.etl is the capture that used to be sample.etl before #76 replaced it. It was never a good property-parsing fixture (unrelated telemetry, no Kernel-Process events at all), and that is exactly what makes it right here: 73 of its 149 events have no schema TDH can find.

Test Results

Payload recovery, on that fixture:

no_schema.etl: 149 events
  decoded (properties non-empty): 74
  no properties:                  75
  carrying raw_data:              73
  payload bytes recovered:        77058
  decoded events also holding raw_data (should be 0): 0

  event_id=0 provider_id=9b79ee91-b5fd-41c0-a243-4248e266e9d0
    properties = {}
    raw_data[64] = b'C\x00:\x00\\x00W\x00i\x00n\x00d\x00o\x00w\x00s\x00\\x00s\x00y\x00s\x00t\x00e\x00'

77 KB that used to be discarded, and plainly the real thing — UTF-16 C:\Windows\syste..., \Device\Harddisk....

Display strings, on a live DNS-Client capture. These are the properties where the string differs from str() of the typed value, i.e. what typed values cannot express:

with formatting OFF: formatted_properties non-empty on 0 / 34 events
with formatting ON:  formatted_properties non-empty on 33 / 34 events

event_id=1001 provider=Microsoft-Windows-DNS-Client
    DynamicAddress   typed=1                    formatted='dynamic '
    Address          typed=b'\x02\x00\x00\x00\xc0\xa82\x01\x00...'
                                                formatted='192.168.50.1'
event_id=3009
    QueryBlob        typed=2525199111760        formatted='0x24BF1989650'

A value map resolved (1 → dynamic), a raw SOCKADDR rendered as an IP, and pointers in hex.

One bug caught while checking this rather than after: TdhFormatProperty renders a single value, so handing it an array silently described only the first element — a 176-byte TimeZoneInformation came out as "228", the same failure mode as the array bug in #84. Arrays and structs are now excluded from the string view; properties reports them in full.

TMF plumbing:

default search path: None
after set:           'C:\Windows\Temp'
manifest events unchanged:  True (34 events)
no-schema events unchanged: True (149 events)
nonexistent path, manifest events still fine: True

Suite:

pytest        215 passed, 102 skipped     (the 102 are #77, fixed in #82)
cargo test     63 passed,   0 failed
cargo clippy --all-targets -- -D warnings  clean
cargo fmt --check / ruff / black           clean

Checklist

• Tests have been added/updated
• All tests pass
• Ready for code review
• Documentation updated (if applicable)
• CHANGELOG.md updated (if applicable)

Where #72 stands after this

#72 item	Status
Enumerate properties via TDH instead of a guess list	#76
EventValue variants for SID / FILETIME / GUID / binary / pointer	#76
Arrays	#84
Nested structures	#84
MOF (classic) as well as manifest	Works — MSNT_SystemTrace decodes fully
PropertyParamLength / PropertyParamCount	#84
Providers with no manifest installed	This PR — payload preserved
WPP fallback	This PR — TMF search path, see caveat
Typed vs TdhFormatProperty strings, other opt-in	This PR — typed default, strings opt-in
Schema cache per (provider, event id, version)	#76

The one caveat, stated plainly. The TMF search path is not verified end to end. This machine has no .tmf files and none of the WPP tooling that produces them (tracefmt, tracepdb, traceview are all absent, and public Microsoft PDBs do not carry WPP data). What is verified is that the context reaches TDH, that setting a path leaves all other decoding byte-identical, and that a path matching nothing is harmless. Actual WPP decoding needs someone with TMFs to confirm. I would rather say so than let it look verified.

Two smaller things also not done, both from #84's notes:

• No fixture for arrays of structs. I tried hard: no ambient provider emits one reliably in a short window (Kernel-Processor-Power event 181 fired once in a 5-second capture and never in ~20 shorter ones; Kernel-Pep, Kernel-Power, Kernel-General, Direct3D12 produced none), and .NET's EventSource refuses to serialise an array of structs at all — it throws TargetInvocationException and writes an EventSourceException string instead of the event. Covered by unit tests and the tracerpt comparison in feat: Decode nested structure properties #84.
• Nested structs inside the string view are skipped, as described above.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HhPcm483PrDmuQEUSALBFn

[m96-chan]

Holding this one. Two blockers before the content is even the question.

1. This PR has never been tested by CI. Not once.

$ gh run list --branch feat/72-wpp-and-formatting
(no runs)

ci.yml triggers on pull_request: branches: [main], and this targets feat/72-nested-structs. A stacked PR therefore gets no CI at all — not a partial run, none. Every green tick you might expect here is simply absent, so nothing in the diff has been compiled, linted or tested by anything but your machine.

That is worth fixing in the workflow independently of this PR, because the failure mode is silent: a stacked PR looks unremarkable in the UI while being completely unverified. Adding pull_request: without a branches filter, or listing the feature branches, would cover it.

2. Blocked behind #84, which is red. Its two nested-struct tests fail on the runner, and my read is that the fixture's provider has no manifest on Windows Server rather than that the code is wrong — details in that PR. This cannot be evaluated on its merits until the base is green.

---

On the content, from reading rather than running, because I have no Windows and this has no CI:

Recovering the payload for schema-less events is clearly right. Reading the bytes and dropping them was the worst of both worlds, and confining raw_data to events TDH cannot describe is the correct trade — populating it always would double the memory for every decoded event to no benefit. Using the old sample.etl as no_schema.etl is a nice reuse: a fixture that was bad at proving decoding works is genuinely good at proving the fallback works, and 73 of 149 events is real coverage rather than a contrived case.

Typed values staying the default is the right call, and I agree formatted_properties belongs beside them rather than replacing them. Handing TdhFormatProperty a single property's bytes that TdhGetProperty already located, instead of walking the payload with running offsets, keeps the safety property this module was built around.

The design question I would want settled before merge: set_property_formatting and set_wpp_tmf_search_path are process-global — an AtomicBool and a Mutex<Option<Vec<u16>>> in statics. That means two EtwSessions in one process cannot disagree, a library that flips formatting changes behaviour for its caller's sessions, and the setting is invisible at the point where events are consumed. TMF search path is at least arguably machine-global; formatting is a per-consumer preference and reads more naturally as a session or reader option.

Not necessarily a blocker — a global is defensible for something set once at startup — but it is a public API that will be awkward to narrow later, so it deserves a deliberate decision rather than arriving as an implementation detail. What made you choose global over per-session?

Once #84 is green and this is retargeted at main so CI actually runs, I will review the diff properly against real results.

[m96-chan]

Heads-up on CI for this one: it will not run until #84 merges.

ci.yml triggers on pull_request: branches: [main], and this PR targets feat/72-nested-structs so the workflow never fires. gh pr checks 85 reports "no checks reported". That is a property of the stacking, not something wrong with the branch — once #84 merges GitHub retargets this to main and CI runs normally.

Rebased onto the current main (now including #82 and #83) and onto the updated #84. Local run against that state:

pytest        318 passed, 0 skipped
cargo test     66 passed, 0 failed
cargo clippy --all-targets -- -D warnings   clean
cargo fmt --check / ruff / black            clean

Worth noting the skip count is now genuinely zero locally, because #82 landed and pandas is installed here; CI will show 2 skipped for the pandas-gated to_dataframe tests until the export extra is installed there, which is the open question I left on #82.