[PR #241] [MERGED] fix: record bare tracing instrument fields on fetch_byte_range and fork_sandbox #282

New issue

Closed

opened 2026-10-29 14:21:47 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:21:47 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/241
Author: @YXalix
Created: 9/2/2026
Status: ✅ Merged
Merged: 9/3/2026
Merged by: @LSX-s-Software

Base: main ← Head: fix/instrument-empty-span-fields

📝 Commits (1)

8134dde fix: record bare tracing instrument fields on fetch_byte_range and fork_sandbox

📊 Changes

2 files changed (+3 additions, -3 deletions)

View changed files

📝 src/orchestrator/service.rs (+2 -2)
📝 src/p2p/iroh/transport.rs (+1 -1)

📄 Description

What

Give explicit values to bare field names in two #[tracing::instrument(fields(...))] attributes:

IrohBlobsP2pTransport::fetch_byte_range (src/p2p/iroh/transport.rs): offset, len -> offset = offset, len = len
Orchestrator::fork_sandbox_inner (src/orchestrator/service.rs): count -> count = child_specs.len(), and child_specs added to skip(...) so the full Vec<SandboxForkChildSpec> (volume mounts, extra drives) is no longer Debug-dumped into every span event.

Why

In tracing-attributes, a bare field name inside #[instrument(fields(...))] does not record the local variable of the same name — it expands to ::tracing::field::Empty, a placeholder meant to be filled later via Span::record() (see the XXX(eliza) comment in tracing-attributes' attr.rs). Since nothing ever records these fields, the fmt layer prints nothing for them: P2P range reads never logged offset/len and fork never logged count, even though the attributes suggest they should.

N/A — minor observability fix found by log inspection.

Scope and non-goals

In scope: the two fields(...) entries above. These are the only bare fields project-wide that were never recorded (verified by scanning every #[instrument] attribute and *_span! macro in the workspace).
Out of scope: storage/ublk/src/dev.rs start() uses fields(dev_id) intentionally and fills it via Span::current().record("dev_id", ...) — left unchanged.
On main, fork_sandbox_inner derives count inside the body from child_specs, so the span records child_specs.len() at entry rather than a count argument.

Design and behavior changes

No behavior change; only span metadata. fork_sandbox_inner records count = child_specs.len() at span creation (function entry), before the body's existing u32::try_from(child_specs.len()) conversion — the value printed is the same child count.

Compatibility and operations

Public API or generated protocol: N/A — tracing span fields only.
Configuration or defaults: N/A.
Snapshot manifest, artifact layout, or storage format: N/A.
Upgrade and rollback: N/A.
Host requirements, permissions, ports, or dependencies: N/A.

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target
Documentation updated
Benchmarks or performance comparison completed

Commands and results:

make fmt     # pass
make clippy  # pass (exit 0)

Skipped checks and reasons: unit/integration tests not run — the change only touches #[instrument] field expressions (no runtime logic); compile-level verification via make clippy covers the attribute expansion.

Risks and reviewer notes

Zero functional risk: only tracing span field expressions changed. Reviewers can confirm by checking that both edited lines are field expressions evaluated at span creation.

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/241 **Author:** [@YXalix](https://github.com/YXalix) **Created:** 9/2/2026 **Status:** ✅ Merged **Merged:** 9/3/2026 **Merged by:** [@LSX-s-Software](https://github.com/LSX-s-Software) **Base:** `main` ← **Head:** `fix/instrument-empty-span-fields` --- ### 📝 Commits (1) - [`8134dde`](https://github.com/kvcache-ai/AgentENV/commit/8134ddec94ed4e92f8a0f8fa24de87fccab55950) fix: record bare tracing instrument fields on fetch_byte_range and fork_sandbox ### 📊 Changes **2 files changed** (+3 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/orchestrator/service.rs` (+2 -2) 📝 `src/p2p/iroh/transport.rs` (+1 -1) </details> ### 📄 Description ## What Give explicit values to bare field names in two `#[tracing::instrument(fields(...))]` attributes: - `IrohBlobsP2pTransport::fetch_byte_range` (`src/p2p/iroh/transport.rs`): `offset, len` -> `offset = offset, len = len` - `Orchestrator::fork_sandbox_inner` (`src/orchestrator/service.rs`): `count` -> `count = child_specs.len()`, and `child_specs` added to `skip(...)` so the full `Vec<SandboxForkChildSpec>` (volume mounts, extra drives) is no longer Debug-dumped into every span event. ## Why In `tracing-attributes`, a bare field name inside `#[instrument(fields(...))]` does **not** record the local variable of the same name — it expands to `::tracing::field::Empty`, a placeholder meant to be filled later via `Span::record()` (see the `XXX(eliza)` comment in tracing-attributes' `attr.rs`). Since nothing ever records these fields, the fmt layer prints nothing for them: P2P range reads never logged `offset`/`len` and fork never logged `count`, even though the attributes suggest they should. ## Related issue N/A — minor observability fix found by log inspection. ## Scope and non-goals - In scope: the two `fields(...)` entries above. These are the only bare fields project-wide that were never recorded (verified by scanning every `#[instrument]` attribute and `*_span!` macro in the workspace). - Out of scope: `storage/ublk/src/dev.rs` `start()` uses `fields(dev_id)` intentionally and fills it via `Span::current().record("dev_id", ...)` — left unchanged. - On `main`, `fork_sandbox_inner` derives `count` inside the body from `child_specs`, so the span records `child_specs.len()` at entry rather than a `count` argument. ## Design and behavior changes No behavior change; only span metadata. `fork_sandbox_inner` records `count = child_specs.len()` at span creation (function entry), before the body's existing `u32::try_from(child_specs.len())` conversion — the value printed is the same child count. ## Compatibility and operations - Public API or generated protocol: N/A — tracing span fields only. - Configuration or defaults: N/A. - Snapshot manifest, artifact layout, or storage format: N/A. - Upgrade and rollback: N/A. - Host requirements, permissions, ports, or dependencies: N/A. ## Validation - [x] `make fmt` - [x] `make clippy` - [ ] `make test-unit` - [ ] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [ ] Documentation updated - [ ] Benchmarks or performance comparison completed Commands and results: ```text make fmt # pass make clippy # pass (exit 0) ``` Skipped checks and reasons: unit/integration tests not run — the change only touches `#[instrument]` field expressions (no runtime logic); compile-level verification via `make clippy` covers the attribute expansion. ## Risks and reviewer notes Zero functional risk: only tracing span field expressions changed. Reviewers can confirm by checking that both edited lines are field expressions evaluated at span creation. ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 14:21:47 +00:00