Skip to content

telemetry [6]: add agent configuration opt-in - #884

Draft
bfjelds (bfjelds) wants to merge 3 commits into
user/bfjelds/mjolnir/telemetry-servicing-idfrom
user/bfjelds/mjolnir/telemetry-agent-config
Draft

bfjelds (bfjelds) wants to merge 3 commits into
user/bfjelds/mjolnir/telemetry-servicing-idfrom
user/bfjelds/mjolnir/telemetry-agent-config

Conversation

@bfjelds

@bfjelds bfjelds (bfjelds) commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.
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.

🟡 Changes recommended

The new telemetry documentation link targets a missing page, which the website’s strict broken-link checks reject.

1 open issue
What changed in this PR

This PR adds default-off telemetry configuration as groundwork for the telemetry stack and preserves existing agent settings when adding a custom datastore path.

Changes:

  • Adds case-insensitive telemetry opt-in parsing and unit tests.
  • Preserves existing configuration through atomic writes while retaining root-verity restrictions.
  • Adds a build-time connection-string constant and documents telemetry configuration.
File Description
docs/​Reference/​Agent-Configuration.md Documents telemetry opt-in and build-time requirements.
crates/​trident/​src/​subsystems/​management.rs Preserves existing settings when adding DatastorePath, with regression tests.
crates/​trident/​src/​lib.rs Exposes the compiled-in telemetry connection string.
crates/​trident/​src/​agentconfig.rs Parses telemetry preferences, defaults to disabled, and adds tests.
crates/​trident/​build.rs Tracks connection-string environment changes for rebuilds.

Review effort: Balanced

Comment thread docs/Reference/Agent-Configuration.md Outdated

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.

🟢 Approval recommended

The narrowly scoped configuration changes have no unresolved blocking findings and include targeted regression tests; tests were not run in this review environment.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@bfjelds bfjelds (bfjelds) changed the title telemetry: add agent configuration opt-in telemetry [6]: add agent configuration opt-in Oct 26, 2026
@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-agent-config branch from 09320cb to 3965bad Compare October 26, 2026 01:53
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 26, 2026 01:56

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.

🟢 Approval recommended

No unresolved blocking issues remain in these scoped configuration changes; targeted tests are included but were not executed during review.

No open issues 🎉

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-agent-config branch from 3965bad to 392b1c4 Compare October 28, 2026 00:06
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 28, 2026 00:36

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.

🟡 Changes recommended

Atomic writes replace symlinked configuration files rather than updating their targets, potentially disconnecting Trident from later configuration changes.

1 open issue

Review effort: Balanced

Comment thread crates/trident/src/subsystems/management.rs Outdated
bfjelds (bfjelds) and others added 2 commits October 28, 2026 20:39
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d0e7cadc-8f44-443f-affe-6ce8e856a218

Copilot-Session: 43cb811b-f191-4d90-852f-e3373041ca11
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 43cb811b-f191-4d90-852f-e3373041ca11
@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-agent-config branch from 392b1c4 to ebc7397 Compare October 28, 2026 20:45
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 28, 2026 20:54

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.

🟡 Changes recommended

The opt-out documentation overstates its guarantee because configured phone-home tracing can still send trace entries off-host.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread crates/trident/src/agentconfig.rs Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 43cb811b-f191-4d90-852f-e3373041ca11

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.

🟢 Approval recommended

No active review issues remain for the default-off telemetry configuration change.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

This branch has not been deployed

No deployments
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