Skip to content

telemetry [16]: report gRPC client transport failures - #893

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

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

Conversation

@bfjelds

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

Copy link
Copy Markdown
Member

Description

Correlate gRPC-client commands and report transport failures without double-counting daemon errors; update telemetry docs.

Document that busy-lock and unprovisioned-host admission-control rejections intentionally emit no command_error from either daemon or client; runtime behavior is unchanged.

Rationale

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

Validation

Source Command or check Result Notes
Agent (2026-10-27) git diff --check 837f40d5648465449de4f32333d269721ddbe77c 34025733f8bcb43940fa13a7cafdc476586b384d Passed Admission-control documentation correction only; runtime trees unchanged.
Agent (2026-10-27) Exact-tree comparison; 19-PR ancestry, base, native-stack and mergeability checks Passed Each affected tree is its original plus the documentation patch. All 19 PRs remain in stack 895; the final branch contains the entire stack.
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-grpc-client-errors branch from 2acd581 to d179912 Compare October 26, 2026 01:33
@bfjelds bfjelds (bfjelds) changed the title telemetry: report gRPC client transport failures telemetry [16]: report gRPC client transport failures 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.

🟡 Changes recommended

Client telemetry incorrectly synthesizes servicing IDs that do not correlate with the daemon’s persisted servicing identity.

1 open issue
What changed in this PR

Extends the telemetry stack to correlate gRPC-client commands and report transport failures while avoiding duplicate reports of daemon errors.

Changes:

  • Wraps client dispatch in command context and selectively reports servicing transport failures.
  • Filters busy-daemon responses out of client error telemetry.
  • Updates documentation for client correlation IDs and command-error reporting.
File Description
docs/​Reference/​Telemetry.md Documents client correlation fields and error-reporting behavior.
crates/​trident/​src/​logging/​operation_context.rs Updates module documentation for client context wiring.
crates/​trident/​src/​grpc_client/​mod.rs Adds correlated dispatch and transport-error filtering.

Review effort: Balanced

Comment thread crates/trident/src/grpc_client/mod.rs
@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-grpc-client-errors branch from d179912 to 25e0afb Compare October 26, 2026 01:53
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 26, 2026 01:57
@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-grpc-client-errors branch from a1a2dec to 837f40d Compare October 26, 2026 15:10
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 27, 2026 21:23

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 telemetry documentation incorrectly promises command_error reporting for busy-lock and unprovisioned-host rejections, which neither the server nor client reports.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread docs/Reference/Telemetry.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

No active review issues are recorded for the telemetry correlation and transport-error reporting changes.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-grpc-client-errors branch from 3402573 to 1d75c6a 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.

🟢 Approval recommended

The client telemetry and transport-failure reporting changes have no active review issues.

No open issues 🎉

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-grpc-client-errors branch from 1d75c6a to bca4953 Compare October 28, 2026 20:45
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 28, 2026 20: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.

🟢 Approval recommended

No active review issues block the client telemetry correlation and transport-failure reporting changes.

No open issues 🎉

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-grpc-client-errors branch from bca4953 to 8a8be8e 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 block approval of the telemetry correlation and transport-failure 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
Document intentionally unreported busy and unprovisioned-host rejections without changing runtime behavior.

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-grpc-client-errors branch from 8a8be8e to e56fc08 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 remain for the gRPC client command-error telemetry 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