Skip to content

Document stable telemetry privacy contract - #818

Open
RamonArjona4 wants to merge 1 commit into
mainfrom
user/ramonarjona4/telemetry-01-docs
Open

Document stable telemetry privacy contract#818
RamonArjona4 wants to merge 1 commit into
mainfrom
user/ramonarjona4/telemetry-01-docs

Conversation

@RamonArjona4

@RamonArjona4 RamonArjona4 commented Aug 12, 2026

Copy link
Copy Markdown
Member

Summary

Defines the approved normative privacy and telemetry contract before implementation: exact consent wording, data inventory and exclusions, fail-closed consent behavior, administrative policy ceiling, localization boundary, and proposed event contract.

This is PR 1 of 5. Review this PR first; implementation follows in the dependent PRs.

Stack

Order Change PR
1 Normative privacy and telemetry documentation #818
2 Consent and policy foundation #819
3 Stable config and executor integration #820
4 Rust SDK and C ABI #821
5 Node.js and .NET SDKs #822

Review only this PR's diff. Later PRs are intentionally based on the preceding branch.

Microsoft Reviewers: Open in CodeFlow

Copilot AI balanced review requested due to automatic review settings August 12, 2026 01:07
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

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.

Pull request overview

Documents the proposed stable telemetry privacy, consent, and administrative-policy contract ahead of implementation.

Changes:

  • Defines consent wording, persistence, presenter, and SDK contracts.
  • Specifies deny-only administrative policy behavior.
  • Documents telemetry event identities and privacy-review inventory.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
docs/telemetry/telemetry.md Adds stable consent and policy overview.
docs/telemetry/telemetry-policy.md Defines Windows administrative policy.
docs/telemetry/telemetry-consent-privacy-review.md Records disclosure, inventory, and controls.
docs/telemetry/telemetry-consent-design.md Specifies consent behavior and public surfaces.
Suppressed comments (2)

docs/telemetry/telemetry-consent-privacy-review.md:27

  • The “random” guarantee is not upheld by the documented relay contract: for non-provision phases, correlationVector is caller-supplied and is_relayable checks only its shape (src/core/wxc/src/main.rs:289-295 and correlation_vector.rs:404-412). A caller can encode 128 bits of arbitrary data in a canonical base and have that base emitted, potentially including identity data that this inventory says is excluded. Require provenance/authentication for relayed vectors, or describe and review this identifier as caller-controlled rather than random.
- Random app-session and sandbox-lifecycle correlation identifiers

docs/telemetry/telemetry-consent-privacy-review.md:65

  • These unresolved decisions conflict with the PR description, which calls the data inventory, exclusions, and localization boundary an approved normative contract. The document cannot serve as the stable implementation contract while those same items still await confirmation. Either record the approved decisions here or mark the relevant contract and PR claims as pending.
1. Confirm the listed included/excluded data categories.
2. Confirm service-side retention, access, regional, and deletion requirements.
3. Confirm whether additional locale or accessibility requirements apply at
   initial stable release.

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

Comment thread docs/telemetry/telemetry-consent-privacy-review.md Outdated
Comment thread docs/telemetry/telemetry-administrative-policy.md
Comment thread docs/telemetry/telemetry-consent-design.md Outdated
Comment thread docs/telemetry/telemetry-consent-design.md Outdated
Comment thread docs/telemetry/telemetry-consent-design.md Outdated
Comment thread docs/telemetry/telemetry-consent-design.md Outdated
Comment thread docs/telemetry/telemetry-consent-design.md Outdated
Comment thread docs/telemetry/telemetry-consent-privacy-review.md Outdated
Comment thread docs/telemetry/telemetry-policy.md
Comment thread docs/telemetry/telemetry.md Outdated
Comment thread docs/telemetry/telemetry.md Outdated
Comment thread docs/telemetry/telemetry.md Outdated
Comment thread docs/telemetry/telemetry-consent-design.md Outdated
Comment thread docs/telemetry/telemetry-consent-design.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

docs/telemetry/telemetry.md:327

  • This leaves the “final data inventory” explicitly pending even though this PR is described as defining the approved, exact normative inventory that follow-on implementation PRs should implement. Since material inventory changes require a new consent-resource version, implementing against an unconfirmed inventory can invalidate the supposedly approved wording. Finalize the inventory/privacy review here, or clearly narrow the PR and consent wording to a non-approved draft contract before dependent implementation proceeds.
The following items remain pending and must be confirmed before release:

- The final data inventory, including the privacy implications of
  caller-controlled lifecycle correlation identifiers

docs/telemetry/telemetry-administrative-policy.md:112

  • These Intune instructions cannot be followed with the material provided: importing a custom administrative template requires both the ADMX and its matching en-US ADML, but this document supplies only the ADMX and a list of strings/presentation elements the reader would have to author. Include a complete Mxc.adml example (or point to a shipped pair) and instruct administrators to upload both files.
Import `Mxc.admx` (above) via **Devices → Configuration → Import ADMX**, then
create a Configuration profile from the imported template and set
**AllowTelemetry**.

Comment thread docs/telemetry/telemetry.md Outdated
Copilot AI review requested due to automatic review settings August 13, 2026 03:27

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

docs/telemetry/telemetry.md:217

  • This data inventory ends mid-sentence, so readers never learn what the relayed identifier contains and the section never states the promised customer-content exclusions. Complete the bullet and explicitly enumerate the remaining metadata and exclusions.
- A lifecycle correlation identifier; for relayed state-aware phases

docs/telemetry/telemetry.md:166

  • This edit introduces a lowercase sentence start and leaves the duplicated article in “a ‘a panic occurred’ sentinel.” Rephrase the sentence so the limitation reads grammatically.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a

Comment thread docs/telemetry/telemetry-administrative-policy.md Outdated
Comment thread docs/telemetry/telemetry-administrative-policy.md Outdated
Copilot AI review requested due to automatic review settings August 13, 2026 05:04

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.

Pull request overview

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

Suppressed comments (3)

docs/telemetry/telemetry-administrative-policy.md:101

  • This is not enough to deploy the advertised policy: Group Policy and Intune require a valid matching ADML resource file, and Intune requires the ADMX and en-US ADML to be uploaded together. A prose list of resource IDs cannot be imported. Please provide the complete Mxc.adml (preferably as a checked-in ADMX/ADML pair) and update the deployment steps to instruct administrators to use both files.
The matching `Mxc.adml` needs `MXC`, `AllowTelemetry`, `AllowTelemetry_Help`,
`Off`, `Required` and `Optional` strings plus a `dropdownList` presentation for
`AllowTelemetry_Enum`.

docs/telemetry/telemetry.md:166

  • Capitalize the new sentence and remove the duplicated article.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a

docs/telemetry/telemetry.md:217

  • The normative data inventory ends mid-sentence at EOF, so it never states how caller-relayed lifecycle identifiers are constrained. Complete the bullet with the validation/reseeding behavior; otherwise the document does not preserve the stated guarantee that arbitrary customer content cannot be uploaded.
- A lifecycle correlation identifier; for relayed state-aware phases

Comment thread docs/telemetry/telemetry-consent-privacy-review.md Outdated
Comment thread docs/telemetry/telemetry-consent-design.md Outdated
Comment thread docs/telemetry/telemetry-consent-design.md Outdated
Comment thread docs/telemetry/telemetry-consent-design.md
Copilot AI review requested due to automatic review settings August 18, 2026 18:50
@RamonArjona4
RamonArjona4 force-pushed the user/ramonarjona4/telemetry-01-docs branch from 179c96d to 769d0f0 Compare August 18, 2026 18:50
@RamonArjona4
RamonArjona4 requested a review from a team August 18, 2026 18:50

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.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.

Suppressed comments (8)

docs/schema.md:129

  • The example still places telemetry inside experimental, despite this PR defining it as a stable top-level setting. Copying this example would produce the old wire shape and still require --experimental. Close the experimental object before the telemetry block.
        "telemetry": {                // Telemetry (Windows only)
            "enabled": true                // Emit TraceLogging ETW events via pure Rust tracelogging crate
        }

README.md:239

  • This reads as currently supported, but this PR does not add a top-level telemetry field; the parser still accepts only experimental.telemetry behind --experimental. Mark this as a proposed contract or defer the user-facing enablement instructions to the runtime integration PR so users are not given a rejected configuration.
Telemetry requires:
1. Top-level `"telemetry": { "enabled": true }` in the JSON config
2. Explicit per-user telemetry consent on Windows
3. An administrative policy that permits collection, when a policy is configured

sdk/node/README.md:435

  • The Node SDK currently exposes telemetry only under ContainerConfig.experimental.telemetry (sdk/node/src/types.ts:326-332), not as top-level telemetry.enabled. This user-facing text therefore documents an API/config shape that this PR does not provide. Label it as planned or move it to the SDK implementation PR.
## Telemetry Consent

Telemetry is off-by-default unless the caller opts in with top-level `telemetry.enabled: true` and the applicable Windows consent/policy gates permit collection.

Telemetry consent behavior follows
[`docs/telemetry/telemetry-consent-design.md`](https://github.com/microsoft/mxc/blob/main/docs/telemetry/telemetry-consent-design.md):

sdk/dotnet/README.md:99

  • SandboxPolicy.TelemetryEnabled and a phase-level TelemetryEnabled property do not exist anywhere in the .NET SDK in this PR, so this instruction cannot compile for current consumers. Describe these names as planned, or publish this section with the .NET implementation PR.
MXC telemetry is Windows-only and remains off until both of these are true:
1. the user has explicitly granted MXC-owned consent, and
2. the caller opts this invocation in via telemetry settings.

Telemetry remains off by default unless the caller opts in with `SandboxPolicy.TelemetryEnabled = true` (or the equivalent phase-level `TelemetryEnabled` setting for state-aware requests) and applicable Windows consent/policy gates permit collection.

docs/telemetry/telemetry-administrative-policy.md:85

  • This formula omits the explicit per-run request and provider-availability gates defined in the consent contract, and build_has_telemetry_enabled is not the runtime switch. As written, it suggests consent plus policy can collect without telemetry.enabled: true. Use the same four gates as the normative contract.
collect = policy_permits AND user_consented AND build_has_telemetry_enabled

docs/schema.md:139

  • Use singular agreement because only sandboxId remains in this sentence.
> top-level fields are state-aware-only — a one-shot request carrying
> `sandboxId` is rejected with a parse error. Lifecycle correlation is internal

docs/telemetry/telemetry.md:135

  • Capitalize the new sentence and remove the duplicated article.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a

docs/host-prep.md:139

  • Use a before the adverb centrally.
from an elevated context (e.g. a scheduled task, an centrally deployed

Comment thread sdk/node/src/state-aware.ts
Comment thread schemas/dev/mxc-config.schema.0.8.0-dev.json
Comment thread tests/scripts/run_telemetry_etw_smoke_test.ps1
Comment thread tests/scripts/run_telemetry_etw_smoke_test.ps1
Copilot AI review requested due to automatic review settings August 18, 2026 19:04
@RamonArjona4
RamonArjona4 force-pushed the user/ramonarjona4/telemetry-01-docs branch from 769d0f0 to 6164f27 Compare August 18, 2026 19:04

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.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.

Suppressed comments (12)

tests/scripts/run_telemetry_etw_smoke_test.ps1:127

  • The consent seed does not match the canonical resource version defined in this PR (1). Because the contract treats an unknown prompt version as non-consent, a record containing 1.0 will fail closed once consent gating is implemented and this smoke test will capture no events.
    promptResourceVersion = '1.0'

sdk/node/src/state-aware.ts:76

  • The Rust executor still returns result.correlationVector from provision and consumes the relayed value on later phases (src/core/wxc/src/main.rs:260-320; src/core/wxc_common/src/state_aware_request.rs:88-92). Dropping it here causes every later phase to reseed independently, breaking lifecycle correlation while the runtime still advertises and emits it. Keep the relay until the Rust wire/runtime contract is removed atomically.
  return {
    sandboxId: result.sandboxId as SandboxId<C>,
    metadata: result.metadata,

schemas/dev/mxc-config.schema.0.8.0-dev.json:958

  • The schema is generated from wxc_common::wire, where MxcConfig.correlation_vector still exists (src/core/wxc_common/src/wire.rs:65-75). scripts/versioning/check-schema-codegen.js:45-73 performs an exact regeneration check, so this manual removal is guaranteed to fail CI. Remove the field from the Rust source and regenerate the schema in the same change.
    },

README.md:244

  • The new explicit-consent rule is contradicted by the retained Data Collection paragraph at line 250, which says use of the software itself operates as consent. That conflicts with this PR's normative rule that consent must be an explicit current-version grant and must never be inferred. Reconcile the legal paragraph with the new consent contract before treating this as canonical user guidance.
The configuration flag is an additional per-run opt-in; it cannot grant consent
or bypass an administrative block. Telemetry remains off unless every applicable
gate is open. MXC does not use the Windows Diagnostics & feedback setting as a
substitute for application consent.

docs/telemetry/telemetry-administrative-policy.md:85

  • This formula omits the required per-run request and provider-availability gates, and replaces them with a build-time condition not present in the four-gate consent contract. That can be read as allowing collection whenever policy and consent permit it, even when the run did not opt in.
collect = policy_permits AND user_consented AND build_has_telemetry_enabled

sdk/node/src/state-aware-types.ts:382

  • This removes a public SDK result property as part of a PR described as normative documentation only, with implementation assigned to later PRs in the stack. The same diff also changes request behavior and tests, so merging PR 1 changes the SDK before the stated implementation PRs. Keep this PR contract-only or update the stack/scope and include the corresponding Rust runtime change here.
export interface ProvisionResult<C extends StateAwareContainmentBackend> {
  sandboxId: SandboxId<C>;
  metadata?: ProvisionMetadataFor<C>;

README.md:239

  • These are presented as instructions for the currently shipped CLI, but this PR does not implement them: the Rust wire model/parser still accepts only experimental.telemetry and requires --experimental (src/core/wxc_common/src/wire.rs:460-493, config_parser.rs:1429-1436). Since the PR description says stable runtime integration follows in #820, keep current usage instructions here until that implementation lands, or clearly label this as a future contract.

This issue also appears on line 241 of the same file.

Telemetry requires:
1. Top-level `"telemetry": { "enabled": true }` in the JSON config
2. Explicit per-user telemetry consent on Windows
3. An administrative policy that permits collection, when a policy is configured

sdk/node/README.md:435

  • This documents a top-level Node.js telemetry option that does not exist in this PR: ContainerConfig still exposes telemetry only under experimental (sdk/node/src/types.ts:326-332). Readers cannot follow this guidance until the later SDK/runtime changes land, so defer it or identify it explicitly as a future API.
## Telemetry Consent

Telemetry is off-by-default unless the caller opts in with top-level `telemetry.enabled: true` and the applicable Windows consent/policy gates permit collection.

Telemetry consent behavior follows
[`docs/telemetry/telemetry-consent-design.md`](https://github.com/microsoft/mxc/blob/main/docs/telemetry/telemetry-consent-design.md):

sdk/dotnet/README.md:99

  • SandboxPolicy.TelemetryEnabled is not present anywhere in the current C# SDK, so this instructs users to use a nonexistent API. The PR stack says the .NET telemetry surface is added only in #822; defer this usage guidance to that PR or clearly mark it as proposed rather than available.
MXC telemetry is Windows-only and remains off until both of these are true:
1. the user has explicitly granted MXC-owned consent, and
2. the caller opts this invocation in via telemetry settings.

Telemetry remains off by default unless the caller opts in with `SandboxPolicy.TelemetryEnabled = true` (or the equivalent phase-level `TelemetryEnabled` setting for state-aware requests) and applicable Windows consent/policy gates permit collection.

docs/schema.md:130

  • The example still nests telemetry inside experimental, despite relabeling it stable and the new contract requiring a top-level setting. As written, the sample contradicts lines 138-139 and the other new documentation. Move it to the root when the stable parser lands, or keep the experimental label/syntax until then.
        "telemetry": {                // Telemetry (Windows only)
            "enabled": true                // Emit TraceLogging ETW events via pure Rust tracelogging crate
        }
    }

docs/telemetry/telemetry.md:135

  • Correct the sentence capitalization and duplicated article.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a

docs/host-prep.md:139

  • Use the correct article before “centrally.”
from an elevated context (e.g. a scheduled task, an centrally deployed

Copilot AI review requested due to automatic review settings August 19, 2026 06:17
@RamonArjona4
RamonArjona4 force-pushed the user/ramonarjona4/telemetry-01-docs branch from 6164f27 to 5d547a3 Compare August 19, 2026 06:17

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.

Pull request overview

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

Suppressed comments (8)

tests/scripts/run_telemetry_etw_smoke_test.ps1:128

  • This seed does not match the consent-store schema used by the dependent consent implementation: it requires schemaVersion, an integer promptResourceVersion of 1, and uses promptedMxcVersion/updatedAtEpoch. As written, deserialization fails at the missing schemaVersion, so fail-closed gating rejects the grant and the smoke test cannot deterministically emit telemetry.
$consentRecord = @{
    version               = 2
    consent               = 'granted'
    promptResourceVersion = '1.0'
    promptLocale          = 'en-US'

tests/scripts/run_telemetry_etw_smoke_test.ps1:232

  • At this stack level the ETW provider does not emit mxc.sandbox_kind, so no captured event can satisfy this new required-field set and the smoke test always fails. The emitter change is planned in the dependent runtime PR; defer this assertion to that PR or include the corresponding provider change here.
    $expectedFields = @('mxc.backend', 'mxc.sandbox_kind', 'mxc.exit_code', 'mxc.outcome', 'mxc.duration_ms')

docs/telemetry/telemetry-administrative-policy.md:85

  • This equation substitutes a build-time flag for the required per-run opt-in and omits provider availability, contradicting the four-gate contract in telemetry-consent-design.md. Use the same gate names so administrators do not infer that consent plus an enabled build is sufficient to collect.
collect = policy_permits AND user_consented AND build_has_telemetry_enabled

docs/schema.md:129

  • The example still places telemetry inside experimental, even though this PR defines it as a stable top-level setting. A user copying this example would send the legacy shape and contradict the README contract. Close the experimental object before declaring telemetry.
        "telemetry": {                // Telemetry (Windows only)
            "enabled": true                // Emit TraceLogging ETW events via pure Rust tracelogging crate
        }

tests/scripts/run_telemetry_etw_smoke_test.ps1:132

  • On Windows PowerShell 5.1, Set-Content -Encoding utf8 writes a UTF-8 BOM. The consent reader passes the file directly to serde_json::from_str, which rejects that BOM, so the seeded grant fails closed on the Windows shell this script is expected to support. Write BOM-less UTF-8 explicitly.
$consentRecord | ConvertTo-Json -Depth 4 | Set-Content -Path $consentFile -Encoding utf8

tests/scripts/run_telemetry_etw_smoke_test.ps1:120

  • The executable search also accepts release builds, but the dependent consent implementation compiles MXC_TEST_LOCALAPPDATA_OVERRIDE out of release binaries and resolves LocalAppData through the Windows known-folder API rather than this environment variable. If only a release binary exists, this test reads the real user's consent and is no longer isolated or deterministic. Skip release binaries (or seed consent through a supported public test surface).
$localAppDataOverride = Join-Path $etlDir 'localappdata'
$consentDir = Join-Path $localAppDataOverride 'mxc'
New-Item -ItemType Directory -Path $consentDir -Force | Out-Null
$consentFile = Join-Path $consentDir 'telemetry-consent.json'

docs/telemetry/telemetry.md:184

  • The data inventory omits __TlgCV__, although the current provider and the dependent runtime implementation still write that field on both public events. Since this PR is the normative privacy inventory, either disclose the random lifecycle correlation vector here and in the event tables, or remove it from the implementation before stabilization.
- State-aware lifecycle phase
- `UTCReplace_AppSessionGuid`, which asks the telemetry pipeline to supply a
  random per-session app identifier

docs/telemetry/telemetry.md:135

  • Capitalize the new sentence and remove the duplicated article.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a

@RamonArjona4
RamonArjona4 force-pushed the user/ramonarjona4/telemetry-01-docs branch from 5d547a3 to a8647b1 Compare August 19, 2026 08:20
Copilot AI review requested due to automatic review settings August 19, 2026 15:48
@RamonArjona4
RamonArjona4 force-pushed the user/ramonarjona4/telemetry-01-docs branch from a8647b1 to 814b542 Compare August 19, 2026 15:48

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.

Pull request overview

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

Suppressed comments (15)

tests/scripts/run_telemetry_etw_smoke_test.ps1:232

  • The current ETW writer emits mxc.backend, mxc.exit_code, mxc.outcome, mxc.duration_ms, mxc.failure_reason, mxc.phase, and __TlgCV__, but not mxc.sandbox_kind (src/mxc_telemetry/src/lib.rs:181-194). This new mandatory check therefore makes the smoke test fail for every event produced by this PR; add the assertion only with the event-field implementation.
    $expectedFields = @('mxc.backend', 'mxc.sandbox_kind', 'mxc.exit_code', 'mxc.outcome', 'mxc.duration_ms')

docs/telemetry/telemetry-administrative-policy.md:86

  • This formula omits two gates from the normative four-gate contract: the per-run request and provider availability. As written, the following table also says an unrestricted/consented user always collects even when the run did not opt in or the provider is unavailable.
MXC collects diagnostic data only when **every** gate is open:

collect = policy_permits AND user_consented AND build_has_telemetry_enabled

tests/scripts/run_telemetry_etw_smoke_test.ps1:127

  • This seed uses prompt resource version 1.0, while the canonical consent resource in docs/telemetry/telemetry-consent-design.md:28 defines version 1. Because unsupported prompt versions must not authorize collection, these two normative artifacts must use exactly the same representation or the seeded grant will fail closed.
    promptResourceVersion = '1.0'

tests/scripts/run_telemetry_etw_smoke_test.ps1:132

  • Windows PowerShell 5.1 writes a UTF-8 BOM for Set-Content -Encoding utf8. A strict JSON reader such as serde_json rejects that leading BOM, so this consent seed can be treated as malformed and fail closed on the Windows environment this test targets. Write UTF-8 without a BOM.
$consentRecord | ConvertTo-Json -Depth 4 | Set-Content -Path $consentFile -Encoding utf8

docs/schema.md:129

  • Despite being labeled stable, this object is still structurally inside experimental, so copying the documented example produces experimental.telemetry rather than the top-level telemetry required everywhere else in this PR. Move it outside the experimental object.
        "telemetry": {                // Telemetry (Windows only)
            "enabled": true                // Emit TraceLogging ETW events via pure Rust tracelogging crate
        }

tests/scripts/run_telemetry_etw_smoke_test.ps1:123

  • Seeding user consent does not make capture deterministic when HKLM\SOFTWARE\Policies\Mxc\AllowTelemetry can still block collection. On a managed host the test will report a product failure even though suppressing events is the required behavior. Detect a blocking/unreadable policy in preflight and skip with an explicit reason, or use a supported test-only policy isolation mechanism.
# Seed an explicit granted consent record for this test run so event capture
# does not depend on host-global consent state.

docs/telemetry/telemetry-consent-design.md:203

  • This release gate says the wording and data/control inventory still require privacy/legal approval, while the PR summary calls the contract approved and telemetry.md:193 says the wording is approved for release review. Clarify the actual approval state consistently; otherwise reviewers and later implementation PRs cannot tell whether this normative contract is finalized.
The versioned wording and data/control inventory still require Microsoft
privacy/legal approval before release.

README.md:239

  • The new explicit-consent requirement conflicts with the unchanged Data Collection statement at line 250 that “Your use of the software operates as your consent.” That language makes ordinary use a grant, while this contract requires a persisted explicit Yes; the privacy text needs to be reconciled before this can be normative.
Telemetry requires:
1. Top-level `"telemetry": { "enabled": true }` in the JSON config
2. Explicit per-user telemetry consent on Windows
3. An administrative policy that permits collection, when a policy is configured

sdk/node/README.md:251

  • This example now discards correlationVector, but the current SDK still returns it from provisionSandbox and sends only options.correlationVector on later phases (sdk/node/src/state-aware.ts:70-78,96). Following the updated example silently removes lifecycle correlation until the later implementation PR actually internalizes it.
const { sandboxId } = await provisionSandbox(
  'isolation_session',
  { network: { defaultPolicy: 'allow', allowLocalNetwork: true } },
  { experimental: true },
);

docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md:398

  • The documented result no longer includes correlationVector, but the current TypeScript API still declares and returns that field (sdk/node/src/state-aware-types.ts:380-392, state-aware.ts:70-79). Removing it from the contract before the implementation changes makes this API reference inaccurate.
interface ProvisionResult<C extends StateAwareContainmentBackend> {
  sandboxId: SandboxId<C>;
  metadata?: ProvisionMetadataFor<C>;
}

docs/schema.md:140

  • The current parser does not reject this field as unknown: wire::Config still declares correlation_vector and the state-aware parser consumes it (src/core/wxc_common/src/wire.rs:65-75). This statement should remain aligned with the current schema until the implementation PR removes the field; otherwise valid current lifecycle requests are documented as invalid.
> is rejected with a parse error. Callers cannot supply `correlationVector`;
> it is rejected as an unknown field because lifecycle correlation is internal
> to MXC and is not part of the request or response contract. See

sdk/dotnet/README.md:99

  • SandboxPolicy.TelemetryEnabled and phase-level TelemetryEnabled do not exist in the C# SDK (SandboxPolicy.cs currently exposes only version, filesystem, network, UI, and timeout). Because this PR explicitly precedes implementation, the public SDK README should describe this as a planned contract or defer the concrete API documentation until the implementing PR lands.
Telemetry remains off by default unless the caller opts in with `SandboxPolicy.TelemetryEnabled = true` (or the equivalent phase-level `TelemetryEnabled` setting for state-aware requests) and applicable Windows consent/policy gates permit collection.

docs/telemetry/telemetry-consent-design.md:106

  • This calls the persisted document schema version 2 but does not define the schema-version key or the exact keys/types for MXC version, source, and timestamp. The smoke test already has to guess these details. Since multiple executables and SDKs must share this stable consent store, provide a complete normative JSON example with required/optional fields and exact version representation.
The persisted consent record is a versioned JSON document. The current schema
version is 2 and records:

- `consent`: `granted` or `denied`
- `promptResourceVersion`
- `promptLocale`
- MXC version, source, and update timestamp for local audit/support provenance

docs/telemetry/telemetry.md:135

  • Capitalize the sentence after the period.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a

sdk/node/README.md:432

  • This public README documents top-level telemetry as currently available, but the current Node config type only exposes experimental.telemetry (sdk/node/src/types.ts:326-332) and the runtime still parses that experimental location. Rephrase this as the planned stable contract or defer it to the implementation PR so users are not directed to a config shape that is currently ignored/rejected.
Telemetry is off-by-default unless the caller opts in with top-level `telemetry.enabled: true` and the applicable Windows consent/policy gates permit collection.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7
Copilot AI review requested due to automatic review settings August 19, 2026 20:52
@RamonArjona4
RamonArjona4 force-pushed the user/ramonarjona4/telemetry-01-docs branch from 814b542 to 51a2ca4 Compare August 19, 2026 20:52

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.

Pull request overview

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

Suppressed comments (14)

tests/scripts/run_telemetry_etw_smoke_test.ps1:232

  • This assertion cannot pass on this PR: mxc_telemetry::log_execution currently emits mxc.backend, exit code, outcome, duration, failure reason, phase, and __TlgCV__, but not mxc.sandbox_kind. Defer this field assertion to the runtime-integration PR (or add the emitter change here); otherwise the checked-in smoke test is broken immediately.
    $expectedFields = @('mxc.backend', 'mxc.sandbox_kind', 'mxc.exit_code', 'mxc.outcome', 'mxc.duration_ms')

tests/scripts/run_telemetry_etw_smoke_test.ps1:130

  • The seeded record does not match the consent-store contract: the dependent consent reader expects schemaVersion (not version), a numeric promptResourceVersion, and updatedAtEpoch. As written, the missing schema version makes this record malformed, so fail-closed gating treats it as undetermined and this smoke test captures no events.
$consentRecord = @{
    version               = 2
    consent               = 'granted'
    promptResourceVersion = '1.0'
    promptLocale          = 'en-US'
    source                = 'run_telemetry_etw_smoke_test.ps1'
    updatedAtUtc          = [DateTime]::UtcNow.ToString('o')

README.md:238

  • This new explicit-consent requirement conflicts with the unchanged Data Collection notice below, which says “Your use of the software operates as your consent to these practices.” Under the normative contract, product use cannot replace an explicit current-version Yes decision. Reconcile or qualify that legacy notice so users are not given two incompatible consent rules.
2. Explicit per-user telemetry consent on Windows

docs/schema.md:140

  • This says correlationVector is rejected, but the parser's current top-level wire model accepts it for state-aware requests and the Node SDK still emits it. That removal belongs with the later runtime/SDK implementation; until then this reference must describe the contract users actually have, or clearly identify the paragraph as a future shape.
> is rejected with a parse error. Callers cannot supply `correlationVector`;
> it is rejected as an unknown field because lifecycle correlation is internal
> to MXC and is not part of the request or response contract. See

docs/schema.md:129

  • The stable telemetry block is still nested inside experimental, contradicting the newly documented top-level telemetry.enabled contract. Move it outside the experimental object so the schema example shows the intended wire shape.

This issue also appears on line 138 of the same file.

        "telemetry": {                // Telemetry (Windows only)
            "enabled": true                // Emit TraceLogging ETW events via pure Rust tracelogging crate
        }

README.md:244

  • The PR description says stable runtime integration follows in #820, but this user-facing README presents top-level telemetry, consent, and policy gating as behavior available now. On this branch the parser only accepts experimental.telemetry behind --experimental, and no consent/policy gate exists. Keep current usage documented here or clearly mark the entire section as a future contract until the implementation lands.

This issue also appears on line 238 of the same file.

Telemetry requires:
1. Top-level `"telemetry": { "enabled": true }` in the JSON config
2. Explicit per-user telemetry consent on Windows
3. An administrative policy that permits collection, when a policy is configured

The configuration flag is an additional per-run opt-in; it cannot grant consent
or bypass an administrative block. Telemetry remains off unless every applicable
gate is open. MXC does not use the Windows Diagnostics & feedback setting as a
substitute for application consent.

sdk/dotnet/README.md:164

  • SandboxPolicy.TelemetryEnabled and the phase-level TelemetryEnabled setting do not exist in the .NET SDK on this branch, so this reads as instructions for an API that consumers cannot compile against. Since the PR explicitly precedes SDK implementation, describe this as the planned surface (or defer it to #822).
Telemetry remains off by default unless the caller opts in with `SandboxPolicy.TelemetryEnabled = true` (or the equivalent phase-level `TelemetryEnabled` setting for state-aware requests) and applicable Windows consent/policy gates permit collection.

sdk/node/README.md:439

  • The Node SDK currently exposes telemetry only under experimental.telemetry and has no consent presenter/persistence API, but this section describes the planned stable top-level and presenter behavior as current. Mark this section as planned until #822 lands, or move it to that implementation PR, so the published README remains usable for the package version in this branch.
Telemetry is off-by-default unless the caller opts in with top-level `telemetry.enabled: true` and the applicable Windows consent/policy gates permit collection.

Telemetry consent behavior follows
[`docs/telemetry/telemetry-consent-design.md`](https://github.com/microsoft/mxc/blob/main/docs/telemetry/telemetry-consent-design.md):
the SDK stays UI-agnostic, renders the canonical resource verbatim through a
host presenter, persists only explicit yes/no decisions, treats dismissal and
failures as non-grants, and never lets policy or transport failures opt a user
in.

docs/telemetry/telemetry-consent-design.md:16

  • “No prompt” also describes the AlreadyGranted path below, where MXC intentionally does not invoke a presenter but an existing current-version grant still authorizes telemetry. Say “never requesting consent” (or equivalent) to distinguish absence of any grant from prompt suppression after a valid grant.
No prompt, no response, the literal string "No", dismissal, withdrawal,
malformed state, an unknown prompt version, or any read/write failure means no

docs/telemetry/telemetry.md:135

  • Capitalize the sentence after the period.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a

docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md:398

  • The current ProvisionResult still includes correlationVector, and the Node SDK still relays it through SandboxSpawnOptions. Removing it from the API reference before the implementation PR removes that contract makes this document inaccurate for the code/package on this branch. Retain the field for now or explicitly label this interface as the proposed post-integration shape.
interface ProvisionResult<C extends StateAwareContainmentBackend> {
  sandboxId: SandboxId<C>;
  metadata?: ProvisionMetadataFor<C>;
}

docs/telemetry/telemetry-administrative-policy.md:85

  • This formula omits two gates required by the consent contract: the per-run request and provider availability. Include the full four-way conjunction; otherwise this policy document says collection occurs when the normative consent design says it must remain off.
collect = policy_permits AND user_consented AND build_has_telemetry_enabled

docs/telemetry/telemetry.md:85

  • This field is described as part of the currently emitted event, but no emitter change is included here and mxc_telemetry::log_execution/log_error do not write mxc.sandbox_kind. The PR description calls this a proposed event contract, so label this field as proposed until runtime integration lands rather than documenting data that current builds do not send.
| `mxc.sandbox_kind` | string | Containment kind requested by the caller (`process`, `vm`, or a concrete backend name) |

sdk/node/README.md:252

  • This example now drops the correlationVector returned by the current provisionSandbox implementation, so users enabling the existing experimental telemetry lose cross-phase correlation. Keep relaying it until the later implementation actually removes the field; merely changing the README does not change the current SDK/wire contract.
const { sandboxId } = await provisionSandbox(
  'isolation_session',
  { network: { defaultPolicy: 'allow', allowLocalNetwork: true } },
  { experimental: true },
);
const opts = { experimental: true };

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.

4 participants