Skip to content

telemetry [15]: report daemon dispatch and request errors - #892

Draft
bfjelds (bfjelds) wants to merge 2 commits into
user/bfjelds/mjolnir/telemetry-cli-command-errorsfrom
user/bfjelds/mjolnir/telemetry-daemon-request-errors
Draft

bfjelds (bfjelds) wants to merge 2 commits into
user/bfjelds/mjolnir/telemetry-cli-command-errorsfrom
user/bfjelds/mjolnir/telemetry-daemon-request-errors

Conversation

@bfjelds

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

Copy link
Copy Markdown
Member

Description

Report servicing dispatch and malformed-request failures with structured fields; keep read-only and busy-lock rejections untelemetered.

Rationale

Extracted from #778 to make the telemetry stack easier to review. Combined changes are unchanged.

Validation

Source Command or check Result Notes
Agent cargo test --locked --manifest-path /home/bfjelds/mjolnir-sandbox/trident-telemetry-split778-20261025-1743/Cargo.toml -p trident -p osutils -p trident_api --features trident/grpc-preview,trident/dangerous-options,trident_api/documentation -- command_kind:: logging:: cli:: engine::context:: config::host::tests virt:: Passed 115 targeted unit tests; run from /home/bfjelds/mjolnir-sandbox.
Agent Git tree comparison Passed Final split tip exactly matches original #778; restacked #779/#781 trees unchanged.
Agent VM functional tests Not run Require a VM; only host-safe unit tests were run.
CI Pipeline checks Not run Not evaluated as part of this split; results above are local.

Related PRs in stack:

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

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-daemon-request-errors branch from a9086d9 to 6e0e24a Compare October 26, 2026 01:33
@bfjelds bfjelds (bfjelds) changed the title telemetry: report daemon dispatch and request errors telemetry [15]: report daemon dispatch and request errors Oct 26, 2026
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 26, 2026 01:45

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

Static review found no blocking issues in the reporting changes or preservation of existing response behavior and telemetry exclusions.

No open issues 🎉

What changed in this PR

Extends the telemetry stack’s command-error reporting to daemon dispatch and malformed servicing requests, while preserving existing gRPC responses and excluding read-only requests and busy-lock rejections.

Changes:

  • Wraps servicing dispatch in correlated command-error reporting.
  • Adds structured errors and telemetry helpers for missing or invalid request fields.
  • Centralizes busy-response messages and documents intentional telemetry exclusions.
File Description
crates/​trident/​src/​server/​tridentserver/​services/​validation.rs Documents why validation rejections remain untelemetered.
crates/​trident/​src/​server/​tridentserver/​services/​update.rs Reports missing update request fields.
crates/​trident/​src/​server/​tridentserver/​services/​streaming.rs Reports invalid image URLs with structured error details.
crates/​trident/​src/​server/​tridentserver/​services/​rollback.rs Reports missing rollback configurations.
crates/​trident/​src/​server/​tridentserver/​services/​mod.rs Adds shared malformed-request reporting helpers.
crates/​trident/​src/​server/​tridentserver/​services/​install.rs Reports missing install request fields.
crates/​trident/​src/​server/​tridentserver/​mod.rs Enables dispatch error reporting while preserving telemetry exclusions.
crates/​trident/​src/​logging/​operation_context.rs Defines shared busy-lock response messages.
crates/​trident_api/​src/​error.rs Adds structured missing-field and invalid-field errors.

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-daemon-request-errors branch from 6e0e24a to ed0fe6b Compare October 26, 2026 01:53
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 26, 2026 01:57

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

Malformed-request ID refresh can race with staging and overwrite the active operation’s telemetry correlation ID.

1 open issue

Review effort: Balanced

Comment thread crates/trident/src/server/tridentserver/services/mod.rs Outdated
@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-daemon-request-errors branch from ed0fe6b to c503ca3 Compare October 26, 2026 03:15
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 26, 2026 03:18

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 blockers remain, and installation-only attachment addresses the previously reported servicing-ID race with regression coverage.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-daemon-request-errors branch from c503ca3 to 73c1b9f Compare October 26, 2026 15:10
@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-daemon-request-errors branch from 73c1b9f to 1220898 Compare October 28, 2026 00:06
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 28, 2026 00:37

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 unconditional telemetry wrapper emits command_error for the non-servicing, read-only check_root RPC, violating the read-only exclusion.

1 open issue

Review effort: Balanced

Comment thread crates/trident/src/server/tridentserver/mod.rs Outdated
@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-daemon-request-errors branch from 1220898 to cb62f43 Compare October 28, 2026 20:45
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 28, 2026 20: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 active review issues remain in the telemetry error-reporting changes.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-daemon-request-errors branch from cb62f43 to 9b375f4 Compare October 28, 2026 21:58
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 28, 2026 22:05

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 are identified for the telemetry reporting changes.

No open issues 🎉

Review effort: Balanced

bfjelds (bfjelds) and others added 2 commits October 28, 2026 22:54
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-daemon-request-errors branch from 9b375f4 to 9dce2ee Compare October 28, 2026 22:58
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 28, 2026 23:03

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 block approval of the telemetry error-reporting changes.

No open issues 🎉

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