Skip to content

telemetry: implement deferred crash/cancellation/state-aware lifecycle instrumentation - #624

Merged
Gudge (MGudgin) merged 4 commits into
mainfrom
user/ramonarjona4/telemetry-deferred
Jul 16, 2026
Merged

telemetry: implement deferred crash/cancellation/state-aware lifecycle instrumentation#624
Gudge (MGudgin) merged 4 commits into
mainfrom
user/ramonarjona4/telemetry-deferred

Conversation

@RamonArjona4

@RamonArjona4RamonArjona4 commented Jul 8, 2026

Copy link
Copy Markdown
Member

📖 Description

Follow-up work implementing the telemetry issues deferred from the original
TraceLogging ETW PR (#493). Extends the experimental telemetry surface with
crash/panic, cancellation, and state-aware lifecycle instrumentation, and
tightens the non-zero-exit diagnostic contract, all behind the existing
--experimental + experimental.telemetry.enabled gate.

The change keeps the established PII invariant: free-form error text is never
emitted — only a bounded FailureReason/error_type enum and numeric exit
codes. On non-Windows platforms all telemetry functions compile as no-ops.

Includes review-driven hardening from adversarial + security + Copilot-style
reviews: the two telemetry-config parser paths were unified on the typed
experimental.telemetry field, a global-free crash-emit test seam was
extracted (making panic/cancellation mapping testable across any
backend/phase), and the state-aware parser now rejects a malformed
(non-object) experimental block consistently with the one-shot path.

Limitation (documented in docs/telemetry/telemetry.md): on backends that
recover panics via catch_unwind (the LXC runner), the panic hook records the
101 sentinel and claims the exactly-once terminal-emit slot, so the recovered
completion event is suppressed. There, mxc.exit_code = 101 is a
"panic occurred" marker rather than the observed process exit code.

🔗 References

Resolves#561
Resolves#562
Resolves#563
Resolves#564
Resolves#565

Related (deferred follow-up, not resolved here):

🔍 Validation

Automated (from src/, toolchain 1.93):

  • cargo fmt --all -- --check — clean
  • cargo clippy --workspace --all-targets -- -D warnings — no warnings
  • cargo test --workspace — all pass (adds panic/cancellation/state-aware
    telemetry unit tests, pure crash/state-aware event-planner tests, and
    state-aware parser tests including malformed/non-object experimental
    rejection)

CI gates run locally (all green):

  • check-version-sync, check-schema-versions, check-rust-toolchain-sync
  • check-schema-codegen + check-sdk-types-codegen (no wire-model drift)
  • validate-configs (173 configs validated against the dev schema)

Reviews: 7-axis adversarial review (findings fixed), security review (clean),
and an emulated GitHub Copilot PR review (three findings fixed).

✅ Checklist

  • Signed the Contributor License Agreement
  • Linked to an issue
  • Updated documentation (if applicable)
  • Updated Copilot instructions (if build, architecture, or conventions changed) — n/a, no build/architecture/convention changes
  • If this PR changes Cargo.lock, the dependency-feed-check check passes — n/a, Cargo.lock is unchanged

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

CopilotAI review requested due to automatic review settings July 8, 2026 19:50

CopilotAI 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.

Pull request overview

Implements the remaining deferred experimental telemetry work across executors and the state-aware lifecycle, adding crash/panic and cancellation instrumentation while tightening the “non-zero exit must surface a diagnostic” contract.

Changes:

  • Add lock-free “provider active” tracking and a new mxc.phase field to telemetry events.
  • Add a shared crash/cancellation planning + exactly-once terminal emission guard, plus state-aware lifecycle telemetry emission with per-phase attribution.
  • Factor and reuse emit_backend_error_envelope so all executors print a machine-readable diagnostic on infrastructure failures before exiting non-zero; update docs accordingly.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
FileDescription
src/mxc_telemetry/src/lib.rsAdds lock-free is_active() and threads phase through ETW event emission.
src/core/wxc/src/main.rsInitializes/attributes telemetry on the state-aware path, installs panic hook, emits cancellation telemetry from console handler, and uses shared backend-error envelope emission.
src/core/wxc_common/src/telemetry/mod.rsAdds panic/cancellation/state-aware telemetry plumbing, context attribution, and exactly-once terminal emission guard.
src/core/wxc_common/src/telemetry/events.rsExtends event model with phase and updates error logging signature accordingly.
src/core/wxc_common/src/script_runner.rsIntroduces reusable emit_backend_error_envelope helper and basic unit coverage.
src/core/wxc_common/src/config_parser.rsRejects malformed non-object experimental in state-aware parsing; populates typed experimental.telemetry from raw state-aware block.
src/core/mxc_darwin/src/main.rsWires telemetry init/emit + panic hook and backend-error envelope emission for parity (no-op telemetry on macOS).
src/core/lxc/src/main.rsWires panic hook and backend-error envelope emission for parity.
docs/telemetry/telemetry.mdDocuments state-aware phase telemetry plus crash/cancellation behavior and limitations.
docs/schema.mdReconciles supported-version table with canonical schema version bounds.

…e instrumentation
Follow-up implementing the telemetry issues deferred from the original
TraceLogging ETW PR (#493): crash/panic telemetry via a global panic hook
(#561), state-aware lifecycle instrumentation with an `mxc.phase` field
(#562), cancellation telemetry from the console control handler (#563),
non-zero-exit diagnostic parity across the executors (#564), and
reconciliation of the `docs/schema.md` supported-version table (#565).
All behind the existing `--experimental` + `experimental.telemetry.enabled`
gate. The PII invariant is preserved: free-form error text is never emitted
(bounded `FailureReason`/`error_type` and numeric exit codes only), and on
non-Windows platforms all telemetry functions compile as no-ops.
Also unifies the state-aware telemetry config on the typed
`experimental.telemetry` field, extracts a global-free crash-emit test seam
(making panic/cancellation mapping testable across any backend/phase), and
rejects a malformed (non-object) `experimental` block consistently with the
one-shot path.
Resolves#561Resolves#562Resolves#563Resolves#564Resolves#565
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@RamonArjona4
RamonArjona4force-pushed the user/ramonarjona4/telemetry-deferred branch from 67b22aa to c6a773aCompareJuly 8, 2026 20:29
@RamonArjona4
RamonArjona4 marked this pull request as ready for review July 8, 2026 22:28
@RamonArjona4
RamonArjona4 requested a review from a team as a code ownerJuly 8, 2026 22:28
Comment threadsrc/core/wxc_common/src/telemetry/mod.rs
Comment threadsrc/core/wxc_common/src/telemetry/mod.rs
Comment threadsrc/core/wxc_common/src/telemetry/mod.rs
Comment threadsrc/core/wxc/src/main.rs Outdated
Comment threadsrc/core/wxc_common/src/telemetry/mod.rs Outdated
…relation
Addresses the PR #624 review round. Squashed follow-up to the initial
deferred-telemetry commit; folds in the review fixes, the correlation
rework, and the adversarial-review hardening.
Telemetry (experimental, gated on --experimental + experimental.telemetry):
- Crash and cancellation instrumentation with a panic-free, message-free
panic hook chained onto the previous hook, wired symmetrically across all
three executors (wxc-exec, lxc-exec, mxc-exec-mac). macOS has no ETW sink
today, so its emits are structural no-ops kept for parity.
- Exactly-once terminal-emit slot made resettable and testable
(#[cfg(test)] reset seam), so the panic hook / control handler vs
emit_completion race is covered rather than only the inactive no-op path.
- Capture-sink / force-active seam over the mxc_telemetry log_execution /
log_error calls so emit_panic / emit_cancellation glue (process globals,
is_active/already_emitted guards, backend/phase/exit-code) is unit-tested.
- Cancellation emit-before-cleanup ordering locked in with a handler-level
test (the OS-budget guarantee from the PR description).
- Full phase x outcome matrix for plan_state_aware across
{provision, start, exec, stop, deprovision} x {success, non-zero guest
exit, MxcError}.
Cross-phase correlation (Microsoft Correlation Vector, MS-CV):
- Replaces the initial per-lifecycle sandbox_id with a full MS-CV
(correlation_vector.rs). Seeded at provision, returned in its result, and
relayed verbatim by the client into each later phase (emitted under
__TlgCV__). The executor is the trust boundary: on each non-provision
phase it validates the relayed value and spins a fresh child element off a
mutable base, passes an already-frozen vector through unchanged, and
reseeds a brand-new base if the relayed value is absent or malformed.
- New optional correlationVector wire field (wire.rs), regenerated dev
schema and generated SDK wire types, SDK + state-aware plumbing, and
telemetry/schema/state-aware docs.
- Hardened MS-CV operators/planner per adversarial review: tightened the
SPIN_ENTROPY_BYTES compile-time guard (< 4 so the coarse counter still
contributes to the 32-bit spun element), documented the deterministic
os_random_bytes test seam, and corrected the frozen-vector pass-through
wording across wire.rs / schema.md / SDK docs.
Backend error surfacing (issue #564):
- Shared emit_backend_error_envelope in wxc_common::script_runner so MXC
never exits non-zero on an infrastructure failure without first printing a
machine-readable {"error":{"code":"backend_error",...}} envelope on
stderr. Wired into wxc-exec / lxc-exec / mxc-exec-mac and the state-aware
finalize path; covered by a macOS seatbelt integration test (blockedHosts
rejection) plus unit tests.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 404ca98b-224b-485e-bfe6-c092c5c500d0
@RamonArjona4

Copy link
Copy Markdown
MemberAuthor

Review round addressed — pushed as a single squashed update (9a7c722e)

All five outstanding review threads have been addressed. To keep the diff reviewable, the follow-up work was squashed into one commit (9a7c722e) on top of the commit you reviewed (c6a773ae) — it is content-identical to the incremental history.

ThreadAskHow it was addressed
telemetry/mod.rs:493"How do I correlate telemetry across phases for a given lifecycle?"Added a per-lifecycle correlation field, then reworked it into a full Microsoft Correlation Vector (MS-CV) (correlation_vector.rs). Seeded at provision, relayed verbatim by the client into each later phase (emitted under __TlgCV__); the executor validates and spins/reseeds per phase. See the new "Correlating a lifecycle" section in docs/telemetry/telemetry.md.
telemetry/mod.rs:57Exactly-once terminal-emit slot is untestable — HAS_EMITTED is a bare process-global with no reset.Made the emit slot resettable + testable: added a #[cfg(test)] reset seam clearing HAS_EMITTED/PROCESS_BACKEND/PROCESS_PHASE, plus a test that claims the slot then asserts a second terminal emit is suppressed.
telemetry/mod.rs:353The emit glue (emit_panic/emit_cancellation reading globals, guarding on is_active/already_emitted, writing to the hardcoded sink) has no capture point, so none of it is asserted.Introduced a capture-sink / force-active seam over the mxc_telemetry::log_execution/log_error calls. Tests now activate emission, invoke emit_panic/emit_cancellation, and assert the captured MXC.Execution + MXC.Error carry the right backend/phase/exit-code (101 / 130).
wxc/main.rs:514Cancellation "emit before cleanup" ordering is a real guarantee but nothing locks it in.Added a handler-level test asserting emit-before-cleanup ordering (built on the sink seam above), so moving emit_cancellation() below the drain/cleanup now fails a test.
telemetry/mod.rs:692Phase coverage incomplete — stop/deprovision untested, no success/guest-exit/dispatch-error matrix.Added a table-driven plan_state_aware matrix over {provision, start, exec, stop, deprovision} × {success, non-zero guest exit, MxcError}.

Also in this update

  • MS-CV hardening from an internal adversarial-review pass: tightened the SPIN_ENTROPY_BYTES compile-time guard (< 4, so the coarse-time counter still contributes to the 32-bit spun element), documented the deterministic os_random_bytes test seam, and corrected the frozen-vector pass-through wording across wire.rs / schema.md / SDK docs. Both codegen CI gates (dev schema + generated SDK wire types) pass.
  • Backend error surfacing (Ensure MXC never exits non-zero without first printing an error message #564): shared emit_backend_error_envelope in wxc_common::script_runner so MXC never exits non-zero on an infrastructure failure without first printing a machine-readable {"error":{"code":"backend_error",…}} envelope on stderr — wired into all three executors and the state-aware finalize path, covered by a macOS seatbelt integration test plus unit tests. Flagging this explicitly since it is adjacent-but-distinct from the telemetry work; happy to split it into its own PR if you'd prefer.

cargo fmt/clippy clean; wxc_common + wxc tests green (446 + 25).

Resolves 4 merge conflicts after main advanced 17 commits (notably the
mxc_engine refactor that centralized one-shot + state-aware dispatch):
- wxc/src/main.rs: keep the state-aware telemetry wrapper in
run_state_aware_main but dispatch via mxc_engine::run_state_aware,
dropping the now-duplicated local dispatch_state_aware_request; keep
the tested StateAwareExit / finalize_state_aware_outcome helpers.
- mxc_darwin/src/main.rs: keep the Seatbelt telemetry init + panic hook,
drop the orphaned Runner::new(SeatbeltScriptRunner) (dispatch is now
mxc_engine::run).
- docs/schema.md: adopt canonical min schema version 0.6.0-alpha
(schemas/schema-version.json) over the branch's stale 0.4.
- state-aware overview doc: adopt sdk/node/ paths, correct docs/schema.md.
Verified: cargo build -p wxc, cargo clippy --workspace --all-targets
-D warnings, cargo fmt --check, cargo test -p wxc -p wxc_common (445
passed), and check-schema-versions / check-schema-codegen /
check-sdk-types-codegen / validate-configs all pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fddfe62a-90da-425e-868f-fae6958d68c6
@RamonArjona4RamonArjona4 self-assigned this Jul 16, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fddfe62a-90da-425e-868f-fae6958d68c6
@MGudgin
Gudge (MGudgin) merged commit ac4682f into mainJul 16, 2026
22 checks passed
@MGudgin
Gudge (MGudgin) deleted the user/ramonarjona4/telemetry-deferred branch July 16, 2026 20:33
Darren Hoehna (dhoehna) added a commit to dhoehna/mxc that referenced this pull request Jul 17, 2026
Resolve conflict in wxc_common/src/state_aware_dispatch.rs: register both the `lxc` (this PR) and `wsb` (upstream microsoft#578) state-aware backend prefixes in backend_from_prefix, and keep both resolve_backend unit tests. Added `correlation_vector: None` to the lxc test to match the ParsedStateAwareRequest field introduced upstream (microsoft#624).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants

@RamonArjona4@MGudgin