[otel-rust] 15 instrumentation gaps in 5 files (advisory) #1203

finding-id: o(redacted)

Current state

The workspace adopts tracing across 21 crates (Cargo.toml presence), uses
tracing::info!/warn!/debug! at call-sites, but has zero #[instrument]
attributes. All spans are created manually via tracing::info_span! /
tracing::debug_span! at 6 call-sites only. Error-returning functions do not
emit structured error events by default, making error rates invisible to any
connected OTel collector.

Suggested instrumentation additions

Scored per file: (Class A count) + 0.5×(Class B count) + 0.3×(Class C count)
where A = uninstrumented error-returning function, B = async handler entry point,
C = tokio::spawn without span propagation. Top 5 files, up to 3 gaps each.

crates/graph/src/loader.rs (score: 8.0)

• Class A — loader.rs:475 — pub fn load_chain(&mut self, leaf: LinkConfig) -> Result<Graph, Error>
  The primary public API for assembling the full dependency graph; errors (missing layers, decode failures) return without any span context, making graph-load failures invisible in traces.

• Class A — loader.rs:410 — fn load_layer(&mut self, upstream: &Upstream, params: Option<Value>) -> Result<Layer, Error>
  Loads and validates a single upstream layer; I/O or format errors propagate without a span to attribute them to a specific layer URL or path.

• Class A — loader.rs:385 — fn resolve_source(&mut self, origin: &SpecOrigin) -> Result<PathBuf, Error>
  Resolves the on-disk path for a layer's source; filesystem errors return with no structured context on which origin was being resolved.

crates/check/src/stack.rs (score: 5.5)

• Class A+B — stack.rs:10 — pub(crate) async fn check_stack(...) -> Result<Vec<CheckResult>, Error>
  Top-level async entry point for all stack checks; zero tracing instrumentation means any check failure is completely unattributed in dashboards.

• Class A — stack.rs:138 — fn check_stack_packages_valid(...) -> Result<CheckResult, Error>
  Validates that every package referenced by a stack exists in the graph; a missing-package error has no span to capture which stack or package caused it.

• Class A — stack.rs:207 — fn check_project_matcher_regexes(...) -> Result<CheckResult, Error>
  Compiles and validates project-matcher regex patterns; a regex compile error returns without any structured field naming the offending pattern.

crates/check/src/naming.rs (score: 5.0)

Five GraphBasedChecker::check implementations are all async, all return Result<CheckResult, Error>, and none carry an #[instrument] attribute. The three highest-impact:

• Class A+B — naming.rs:12 — impl GraphBasedChecker for SpecNameMatchesDir — async fn check(...)
  Checks that the spec name matches its directory; per-package errors are untraced.

• Class A+B — naming.rs:52 — second GraphBasedChecker impl
  Checks output naming conventions; failures not tied to any span.

• Class A+B — naming.rs:90 — third GraphBasedChecker impl
  Additional naming rule; same gap.

crates/remote-client/src/lib.rs (score: 4.0)

make_env (line 79) and exec (line 212) are instrumented. The three remaining
async RPC entry points are not:

• Class A — lib.rs:265 — pub async fn build(...) -> Result<(), Error>
  Primary RPC entry for remote builds; transport errors and RPC failures have no span context.

• Class A — lib.rs:400 — pub async fn download(...) -> Result<NamedTempFile, Error>
  Downloads built artifacts from the remote execution service; download failures are invisible to traces.

• Class A — lib.rs:47 — pub async fn connect<S: Into<String>>(...) -> Result<Self, TransportError>
  Connection establishment; a failed connect has no span to capture the target address.

crates/minimald/src/exec.rs (score: 1.8)

• Class B — exec.rs:1080 — async fn run_in_session(...)
  Main dispatch path for all SSH exec requests in the daemon; the absence of a span means every command run inside a session is invisible to traces.

• Class B — exec.rs:1104 — async fn run_build_exec(...)
  Handles min package build SSH exec requests; build progress and errors are emitted only as log lines, not as child spans.

• Class B — exec.rs:1197 — async fn run_check_exec(...)
  Handles min check SSH exec requests; same gap as run_build_exec.

Suggested fixes

Add #[instrument] to error-returning functions so spans are created automatically
and errors are recorded as span events:

use tracing::instrument;

// Preferred for async handlers
#[instrument(skip_all, err)]
pub async fn build(&mut self, verbose: bool, ...) -> Result<(), Error> {
    // tracing records the error automatically on Err return
    ...
}

// For sync functions where key fields add context
#[instrument(skip(self), fields(origin = ?origin), err)]
fn resolve_source(&mut self, origin: &SpecOrigin) -> Result<PathBuf, Error> {
    ...
}

For run_in_session / run_build_exec / run_check_exec which return () and
so cannot use err, create an explicit span at entry:

async fn run_build_exec(session: SessionHandle, channel_id: ChannelId, ...) {
    let span = tracing::info_span!("run_build_exec", %channel_id);
    let _guard = span.enter();
    ...
}

Reproduce locally

cd <workspace-root>

# Confirm zero #[instrument] usage
grep -rn --include='*.rs' '#\[instrument' .

# List uninstrumented Result-returning functions in loader.rs
grep -n ') -> Result<' crates/graph/src/loader.rs

# List uninstrumented async check functions in stack.rs
grep -n 'async fn\|) -> Result<' crates/check/src/stack.rs

# List uninstrumented async RPC methods in remote-client
grep -n 'pub async fn' crates/remote-client/src/lib.rs

Severity

LOW — Instrumentation is advisory; no runtime correctness bug. These gaps make
error rates and latency invisible to dashboards when deployed with an OTel collector.

Generated by OTel instrumentation chore: Rust (side-repo-ops operator) · ◷