Repository navigation
Improve MXC execution telemetry and lifecycle correlation - #1575
Conversation
Emit execution Start/End events, add lifecycle session correlation and denial-capture mode attribution, and preserve invocation isolation across streaming operations and backend cleanup. Update consent disclosure, event documentation, and regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e46bdb3-c7d8-46aa-af26-56f317c36a58
There was a problem hiding this comment.
🟡 Changes recommended
Invocation context is lost across backend thread boundaries, and missing session IDs incorrectly suppress authorized streaming telemetry.
2 open issues
What changed in this PR
This PR adds execution Start/End telemetry and lifecycle-wide session correlation across MXC executors and SDK operations.
Changes:
- Replace
MXC.Executionwith Start/End events and add session IDs and bounded denial-capture modes. - Preserve attribution across streaming-handle operations and persist lifecycle correlation roots.
- Update consent disclosure to version 4, documentation, and regression coverage.
| File | Description |
|---|---|
| tests/scripts/run_telemetry_etw_smoke_test.ps1 | Validate execution pairs, attribution, and consent v4. |
| tests/scripts/run_telemetry_consent_smoke_test.ps1 | Update consent expectations to v4. |
| tests/scripts/run_telemetry_consent_release_test.ps1 | Update release consent checks to v4. |
| src/tools/wxc/src/main.rs | Initialize session attribution and emit execution Start. |
| src/tools/mxc_darwin/src/main.rs | Add the execution Start call. |
| src/tools/lxc/src/main.rs | Add the execution Start call. |
| src/mxc-sdk/tests/wxc_e2e_tests_e2e_telemetry_etw.rs | Check new event names and attribution fields. |
| src/mxc-sdk/src/core/mxc_telemetry/mod.rs | Emit Start/End events and session fields. |
| src/mxc-sdk/src/core/mxc_engine/state_aware.rs | Instrument lifecycle dispatch with session scopes and Start events. |
| src/mxc-sdk/src/core/mxc_engine/mod.rs | Retain streaming-handle attribution during operations and cleanup. |
| src/mxc-sdk/src/core/mxc_common/telemetry/session.rs | Add scoped session and learning-mode context. |
| src/mxc-sdk/src/core/mxc_common/telemetry/mod.rs | Integrate Start/End emission and invocation suppression. |
| src/mxc-sdk/src/core/mxc_common/telemetry/events.rs | Add learning-mode classification and event test support. |
| src/mxc-sdk/src/core/mxc_common/telemetry/correlation_state.rs | Persist late-enabled lifecycle roots and clear stale state. |
| src/mxc-sdk/src/core/mxc_common/telemetry/consent_prompt.rs | Update disclosure text assertions. |
| src/mxc-sdk/src/core/mxc_common/process_util.rs | Extract Windows parent-process and image-path helpers. |
| src/mxc-sdk/src/core/mxc_common/models.rs | Centralize learning-mode capability classification. |
| src/mxc-sdk/src/core/mxc_common/diagnostic.rs | Reuse process helpers for local diagnostics. |
| src/mxc-sdk/src/backends/process_container/common/appcontainer_runner.rs | Reuse the shared capability classifier. |
| src/mxc-sdk/resources/telemetry/consent/en-US.json | Introduce v4 lifecycle-correlation disclosure. |
| sdk/dotnet/Microsoft.Mxc.Sdk.Tests/MxcTelemetryTests.cs | Align .NET consent tests with v4. |
| docs/telemetry.md | Explain execution pairs and lifecycle correlation. |
| docs/development/architecture/telemetry.md | Document event contracts and correlation persistence. |
| docs/development/architecture/telemetry-consent-design.md | Document re-consent and collector migration requirements. |
Review effort: Balanced
Restore complete invocation context across IsolationSession workers and moved streaming handles. Preserve upstream fallback removal and session lifecycle support. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e46bdb3-c7d8-46aa-af26-56f317c36a58
Align release consent fixtures with resource v4, gate Windows-only attribution readers, and validate cross-process lifecycle correlation with isolated local ETW capture on scheduled IsolationSession hosts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e46bdb3-c7d8-46aa-af26-56f317c36a58
Validate the actual lifecycle scenario requests through real CLI dry-runs, with telemetry disabled and a missing-network rejection control, before publishing the test executor artifact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e46bdb3-c7d8-46aa-af26-56f317c36a58
Match actual tracerpt XML rather than numeric System task IDs, and exercise the real layout in the offline regression fixture without weakening pair or lifecycle assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e46bdb3-c7d8-46aa-af26-56f317c36a58
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Preserve lifecycle telemetry validation alongside the shared Node integration suite dispatcher. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08308f81-0ab1-4b64-9efa-f373a119e864
There was a problem hiding this comment.
🔵 Needs a closer look
Development telemetry activates despite disabled telemetry and denied consent, contradicting the linked opt-in requirements.
9 open issues
Account for ineligible builds in worker regression test Make telemetry CI assertions eligibility-aware Make critical_events available on non-Windows targets Debug build detection disables telemetry keyword tests in Windows CI Add CI coverage for Required-collection consent and policy gates Isolate dry-run telemetry state from real user data Restore saved session before authorizing panic and control events Preserve opt-in consent checks for development telemetry Trace assertions do not verify backend event session IDs
Review effort: Balanced
Compile critical_events on every platform, restore the saved session before authorizing panic and cancellation emission, make tests honor build eligibility, and keep ETW dry runs out of the real user profile. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08308f81-0ab1-4b64-9efa-f373a119e864
There was a problem hiding this comment.
🔵 Needs a closer look
The development telemetry path collects despite disabled telemetry or withdrawn consent, contradicting the linked opt-in requirements, while tests leave session attribution and consent/policy collection gates insufficiently covered.
4 open issues
5 resolved since last review
Review effort: Balanced
Dry-run and rejected start/exec/stop calls no longer leave records in the correlation store. Also update the consent v4 wording in the telemetry architecture doc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08308f81-0ab1-4b64-9efa-f373a119e864
|
Thanks, both addressed in e5d4309. 1. Correlation store writes on dry run / rejected calls. 2. |
There was a problem hiding this comment.
🟡 Changes recommended
Telemetry collection bypasses explicit opt-in and consent requirements without an approved scope change, and concurrent invocations can emit mismatched session IDs.
5 open issues
Debug build detection disables telemetry keyword tests in Windows CI Coordinate session IDs across concurrent telemetry starts · New Add CI coverage for Required-collection consent and policy gates Preserve opt-in consent checks for development telemetry Trace assertions do not verify backend event session IDs
Review effort: Balanced
Add a fresh-thread regression for out-of-band development End events, assert every backend event carries the ExecutionStart session ID, and add live ETW gating cases: development requests emit regardless of consent and policy 1/3 but not policy 0 or unsupported values; published requests still need consent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08308f81-0ab1-4b64-9efa-f373a119e864
|
Addressed the remaining Copilot review comments. Fixed in 393b6c4, e5d4309 and e8a1a78:
Design thread (#1588/#1517): the Required-only model for the development contract is intentional and was approved by Privacy. Published contracts keep the consent-gated model. Not run: the live ETW script needs Administrator and was not run locally; it runs in the Windows CI job. |
There was a problem hiding this comment.
🔵 Needs a closer look
Development telemetry bypasses linked opt-in and consent requirements without a linked approved revision, and concurrent first executions can emit inconsistent correlation IDs.
4 open issues
1 resolved since last review
Review effort: Balanced
CI dot-sources the script, so the last expected-failure wxc-exec exit code was reported as the step result even though every case passed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08308f81-0ab1-4b64-9efa-f373a119e864
There was a problem hiding this comment.
🔵 Needs a closer look
Telemetry collection bypasses consent and request opt-in gates despite linked requirements explicitly prohibiting opt-out bypass, so the conflicting design must be reconciled or the gates retained.
2 open issues
2 resolved since last review
Review effort: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08308f81-0ab1-4b64-9efa-f373a119e864
There was a problem hiding this comment.
🔵 Needs a closer look
Human assessment is needed because the ETW keyword check can pass CI despite a failed positive ProcessContainer request, leaving successful ProcessExited behavior unverified.
No open issues 🎉
4 resolved since last review
Review effort: Balanced
Gudge (MGudgin)
left a comment
There was a problem hiding this comment.
Both findings from my previous review are addressed: lifecycle correlation records are persisted only after successful non-dry-run operations, and both canonical consent passages now identify resource v4. Verified at 7e07059. Correlation-store tests: 30 passed; CLI correlation regression: 1 passed. Lifecycle engine suite: 33 passed, with 2 tests blocked during HKCU test-policy setup by sandbox restrictions. Rust source is unchanged since that verification. The updated executor build and dot-sourced ETW assertion harness passed, leaving exit code 0. Live ETW collection was not run locally.


📖 Description
This PR improves execution telemetry and correlation across a container lifecycle, and makes the development
1.1.0-alphacontract collect only four Required diagnostic events by default, without MXC consent.MXC.ExecutionwithMXC.ExecutionStartandMXC.ExecutionEnd, including streaming, PTY, and container lifecycle paths.mxc.session_idto the event families that remain. Reuse the persisted random lifecycle root across executor processes and preserve attribution across streaming handles, worker threads, and cleanup.mxc.capture_denialsmode to Start/End and reuse the existing capability classifier.1.1.0-alpharequests, emit onlyMXC.ExecutionStart,MXC.ExecutionEnd,MXC.Error, andMXC.ProcessExited, with Critical Data and Product and Service Performance metadata. The other seven event families are not emitted for this contract. Published0.9.0-alphaand1.0.0requests keep their full event set and Measures/Usage metadata.telemetry.enabledflag (the field is still accepted, includingfalse). Collection requires a build routed through the WinExt group (or the local-only test mode) and an administrative policy that is absent,1, or3. Policy0, unsupported values, wrong types, and read failures block collection. Published contracts keep request opt-in, MXC consent, and their existing policy handling.MXC_TELEMETRY_PROVIDER_GROUP_GUIDis the only routing input; a nonemptyMXC_TELEMETRY_CORE_PROVIDER_GROUP_GUIDnow fails the Windows build.MXC_TELEMETRY_TEST_KEYWORDS=1to local-only Windows debug/test-support builds. Public CI clears the routing group and validates actual keyword/privacy metadata without Microsoft routing.docs/telemetry.mdwith the Required-data description and update the architecture and CI documentation and the ETW validation scripts.Collectors must update filters and queries from
MXC.Executionto the new Start/End names. Start describes a pre-dispatch attempt, not a successful process launch; missing End is not definitive evidence of a crash.Privacy: Privacy review approved these four events as Required diagnostic data for the documented purposes. The approval does not cover additional collection. Privacy also asked how developers who integrate MXC are told that it collects Required diagnostics; that notice is not part of this PR.
Publication: No private WinExt group value is committed. The group identifier is injected through a private process-local build input. Real-group binaries, generated provider files, and VM capture artifacts are not part of this source change and must not be published as public artifacts.
🔗 References
Refs #1588. Refs #1517.
🔍 Validation
Local validation of the final change, with
MXC_TELEMETRY_TEST_KEYWORDS=1and a fake routing group:src:cargo fmt --all -- --check, andrustfmt --checkonprovider_codegen.rssrc:cargo test --locked -p mxc-sdk --lib --features test-support,isolation_session -- mxc_common::telemetry:: mxc_telemetry:: mxc_engine:: mxc_common::exec_stream::tests::exec_waiter_preserves_telemetry_context piped_exec_workers_preserve_telemetry_authorization_and_attributioncorrelation_state::requested_long_lived_record_is_refreshed..., a timing-sensitive test outside this change; it passes when run alone.src: policy and provider selectorssrc:cargo test --locked -p wxc --bin wxc-exec --features test-support,isolation_sessionsrc:cargo clippy --locked -p mxc-sdk -p wxc --all-targets --features test-support,isolation_session -- -D warningssrc:cargo build --locked -p wxc --features test-support,isolation_sessiontests\scripts\test_telemetry_etw_assertions.ps1 -WxcExePath src\target\debug\wxc-exec.exetests\scripts\run_telemetry_consent_smoke_test.ps1 -BinDir src\target\debugnode scripts\check-telemetry-policy-parity.jsManual live validation on an internal Windows x64 VM, using a release executor built with the real WinExt group (value not recorded here):
1.1.0-alpharequests withtelemetry.enabled=false, one successful ProcessContainer run and one unsupported-backend failure each. Published1.0.0requests withenabled=truewere the control.0:HKLM\SOFTWARE\Policies\Mxc\AllowTelemetry=0produced no MXC events for either contract. The Telemetry RealTime Tool showed no MXC events for that window.Not established: whether UTC accepts these events from the WinExt registration. In the Telemetry RealTime Tool the development events showed Critical category and persistence but
isRequired: false. An earlier run with a separate Core registration was rejected by UTC withIncorrectProductOrDataCategory; that registration is removed in this change and WinExt acceptance has not been re-checked in Diagnostic Data Viewer. Local ETW is not evidence of Microsoft ingestion.The scheduled IsolationSession lifecycle scenario is wired into CI but was not run on the VM. Non-Windows runtime validation and SDK integration suites were not rerun.
✅ Checklist
feature-acceptedCargo.lock, thedependency-feed-checkcheck passes (see pull request builds)📋 Issue Type
Microsoft Reviewers: Open in CodeFlow