Skip to content

APP-14871: Report WebRTC client connection metadata - #189

Draft
Daniel Botros (danielbotros) wants to merge 3 commits into
mainfrom
APP-14871-report-webrtc-connection-metadata
Draft

Daniel Botros (danielbotros) wants to merge 3 commits into
mainfrom
APP-14871-report-webrtc-connection-metadata

Conversation

@danielbotros

@danielbotros Daniel Botros (danielbotros) commented Sep 16, 2026

Copy link
Copy Markdown
Member

Port of goutils#583 to rust-utils. After a WebRTC dial finishes — success or failure — the client best-effort reports to the app signaling server it dialed through: the furthest dial stage reached, the gRPC failure code, the dial duration, how the dial was signaled, and the selected ICE candidate pair per side (host / stun / relay, plus the relay address).

Changes

  • New src/rpc/dial_report.rs — the reporting subsystem: StageTracker (monotonic atomic dial stage), classify_signaling_path, classify_connection / classify_candidate (reads peer.get_stats() and maps the nominated + succeeded ICE candidate pair to host/stun/relay + relay address), failure_code (extracts a tonic::Status code, else UNKNOWN), should_deliver (report-selection semantics), and send_dial_report (best-effort delivery, 5s timeout). Includes unit tests.
  • src/rpc/dial.rsmaybe_connect_via_webrtc is split into a reporting wrapper and an _inner; stage advance() calls are threaded through the dial, the spawned call-response task, and the ICE callback. Both connect_inner variants classify the signaling path and pass it down. The report is built and delivered on a detached tokio::spawn over the same already-authenticated, rpc-host-stamped signaling channel, so reporting adds no latency to the dial.
  • src/rpc/webrtc.rsnew_peer_connection_for_client takes the stage tracker and advances DTLS_CONNECTED on peer-connection Connected.

The proto types are already vendored on main (Automated Protos Update #187), so no src/gen changes are needed.

This PR is three commits: (1) cloud-signaled reporting; (2) the local-signaled case (Go's fixUpReportDialOpts); (3) refinements to failure suppression, stage tracking, and test gating.

Design notes / divergences from Go

  • Cloud-signaled dials (routed through app.viam.com / app.viam.dev) report over their own authenticated channel.
  • Locally-signaled WebRTC dials — dialing a .local.viam.cloud name or a LAN address directly, i.e. WebRTC signaled through the robot's own signaling server — report to prod app, reconstructing an authenticated connection reusing the dial's credentials and stamping the robot's rpc-host (only app implements ReportConnectionMetadata; a robot's local signaling server doesn't aggregate these metrics). A locally-signaled dial with no credentials is dropped, matching Go. Like Go, the app address is hard-coded to prod, so a staging robot dialed locally isn't routed to app.viam.dev.
  • mDNS-discovered dials are direct gRPC, not WebRTC, so they have no report (WebRTC-over-mDNS is a future feature, RSDK-14026).
  • should_deliver: on success only a READY report is delivered. Failures are reported only for cloud-signaled dials — rust-utils falls back to direct gRPC on any WebRTC failure, and for a local dial that fallback to the robot generally succeeds, so the WebRTC failure did not fail the dial and is suppressed (a cloud robot isn't directly reachable, so a cloud WebRTC failure is terminal and is reported). This mirrors Go suppressing non-READY reports when the logical dial nonetheless succeeded.
  • SIGNALING_CONNECTED is set at the start of the WebRTC dial (the channel is already eagerly connected by create_channel), so a config-RPC failure against a reachable signaler reports SIGNALING_CONNECTED rather than UNSPECIFIED, matching Go.
  • Reporting runs entirely on a detached task (with timeouts) and never adds latency to or fails the dial. It is disabled under cfg(test) and whenever VIAM_DISABLE_DIAL_REPORTING is set — the latter so integration-test / CI harnesses (which cfg(test) doesn't cover) can opt out; the echo integration tests set it.
  • The public C ABI / viam_rust_utils.h is unchanged (only internal functions changed).

Downstream SDKs

No code changes are required in the C++ or Python SDKs. Both dial through the rust-utils FFI (dialDialBuilder::connect(), the instrumented path) and download libviam_rust_utils from rust-utils/releases/latest at build time, so they pick this up automatically once it merges, a new rust-utils release is cut, and each SDK is rebuilt. Receiver-side requirement: the app signaling server must implement ReportConnectionMetadata (otherwise the client gets Unimplemented, logged at debug — harmless).

Testing

  • cargo build clean; cargo clippy clean on the new/changed code.
  • cargo test --lib — all pass (4 new dial_report tests).

🤖 Generated with Claude Code

Port of goutils#583. After a WebRTC dial finishes (success or failure),
best-effort report to the app signaling server it dialed through: the
furthest dial stage reached, the gRPC failure code, the dial duration,
how the dial was signaled, and the selected ICE candidate pair per side
(host/stun/relay + relay address).

New src/rpc/dial_report.rs holds the reporting subsystem (stage tracker,
candidate/signaling-path classification from peer-connection stats,
report selection, and delivery). maybe_connect_via_webrtc is split into
a reporting wrapper and an _inner that advances a StageTracker through
the dial; the report is delivered on a detached task over the same
already-authenticated, rpc-host-stamped signaling channel, so it never
adds latency to the dial.

The proto types are already vendored on main (Automated Protos Update
#187), so no src/gen changes are needed.

Reports are delivered only for cloud-signaled dials (app.viam.com /
app.viam.dev), since only app implements ReportConnectionMetadata.
Unlike Go, rust-utils does not signal WebRTC over mDNS and does not
reconstruct a prod-app connection for raw-IP/.local dials, so
local-signaled dials produce no report. The public C ABI is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Port the local-dial half of goutils#583's fixUpReportDialOpts. A WebRTC
dial signaled through a robot's own (non-cloud) signaling server —
dialing a .local.viam.cloud name or a LAN address directly — now reports
too: the background task reconstructs an authenticated connection to
prod app.viam.com, reusing the dial's credentials and stamping the
robot's rpc-host, and delivers the report there (only app implements
ReportConnectionMetadata; a robot's local signaling server does not
aggregate these metrics).

A locally-signaled dial with no credentials (WithoutCredentials) is
dropped, matching Go. mDNS-discovered dials are direct gRPC, not WebRTC,
so they have no report. Like Go, the app address is hard-coded to prod,
so a staging robot dialed locally does not route to app.viam.dev.

Delivery is threaded through a new ReportTarget enum: Cloud reuses the
dial's channel, Local reconstructs the app connection. Reconstruction is
bounded by a 5s timeout and stays entirely on the detached report task,
so it never affects the dial.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address three differences from the Go implementation:

- Suppress local-signaled WebRTC *failure* reports. rust-utils falls
  back to a direct gRPC connection on any WebRTC failure, and for a
  locally-signaled dial that direct connection to the robot generally
  succeeds — so the WebRTC failure did not fail the dial. should_deliver
  now reports a failure only for cloud-signaled dials (where direct gRPC
  can't reach the robot, so the failure is terminal), mirroring Go's
  suppression of non-READY reports on a successful dial. Successful
  (READY) dials still report on both paths.

- Advance SIGNALING_CONNECTED at the start of the WebRTC dial rather than
  after the config RPC. The channel is already eagerly connected by
  create_channel, so a config-RPC failure against a reachable signaler
  now reports SIGNALING_CONNECTED instead of UNSPECIFIED, matching Go.

- Gate reporting off in test/CI harnesses via a VIAM_DISABLE_DIAL_REPORTING
  env var (cfg(test) only covers the crate's own unit tests, not
  integration tests or downstream consumers). The echo integration tests
  set it so no detached report task can outlive a test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant