feat(harness): trace correlation via OTel baggage + iii.* span attrs - #138
Conversation
Every harness-wrapped function tags its span with iii.session.id / iii.message.id / iii.function.id and propagates the same triple as OTel baggage. iii-sdk's BaggageSpanProcessor materializes those attrs on every downstream worker span, so the Developer Console TRACES tab can group spans per user turn. * harness/src/otel.rs: with_harness_span / with_envelope_span / with_raw_span wrappers, ID extraction with header-injection / control-byte guards, and traceparent + x-iii-message-id response-header echo. * harness/src/lib.rs: every registered function now goes through a wrapper. * harness/tests/trace_correlation.rs: end-to-end coverage including the allowlist lock-step with iii-sdk's DEFAULT_ALLOWLIST. * hook-fanout: span attribute emission + quiescence-based exit. * provider-anthropic / provider-openai: capture_otel_context across tokio::spawn so HTTP child spans inherit baggage. * harness/Makefile: new `build` target compiles each local worker and symlinks the release binary into ~/.iii/workers/. `make all` now runs build + config + observability + engine + verify. * harness/web/src/bridge.ts: surface engine error messages instead of collapsing to "[object Object]".
📝 WalkthroughWalkthroughThis PR implements OpenTelemetry tracing integration for the harness with ID extraction and header injection, updates provider clients to preserve tracing context across async boundaries, adds quiescence-based collection termination to hook-fanout, improves web client error handling, and updates build orchestration with local worker symlink provisioning. ChangesOpenTelemetry Tracing Infrastructure
SDK Dependency Updates and Provider Tracing
Web Client Bridge Updates and Error Handling
Hook-Fanout Quiescence and Collection Exit
Build System, Local Worker Provisioning, and Documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
skill-check — worker0 verified, 25 skipped (no docs/).
Three for three. Nicely done. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
harness/tests/trace_correlation.rs (1)
304-312: 💤 Low valueOptional: extract
find_namedhelper to reduce duplication.The
find_namedfunction is defined identically in two test functions (lines 304-312 and 421-429). Consider extracting it to a test helper module to keep the test suite DRY.♻️ Example extraction
Add to the test module before the test functions:
#[cfg(test)] fn find_named<'a>(node: &'a Value, name: &str) -> Option<&'a Value> { if node["name"].as_str() == Some(name) { return Some(node); } node["children"] .as_array() .and_then(|cs| cs.iter().find_map(|c| find_named(c, name))) }Then remove the
#[allow(clippy::items_after_statements)]blocks and the duplicate definitions.Also applies to: 421-429
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harness/tests/trace_correlation.rs` around lines 304 - 312, Two identical helper functions named find_named are duplicated in tests; extract a single test helper and remove the duplicates. Create one shared find_named function in the test module (e.g., placed above the test functions or in a test helper submodule) and have both tests call that function instead of defining their own; remove the #[allow(clippy::items_after_statements)] duplicate definitions at the original locations (the blocks containing find_named) so the tests use the single shared find_named implementation.harness/README.md (1)
130-130: 💤 Low valueMinor: Markdown heading level skip.
The static analyzer flagged line 130 for jumping from h2 to h4. Consider changing
#### Operator observability of the wrapper itselfto### Operator observability of the wrapper itselffor proper heading hierarchy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harness/README.md` at line 130, Change the markdown heading "#### Operator observability of the wrapper itself" to use a level-3 heading so the document's hierarchy is consistent: replace "#### Operator observability of the wrapper itself" with "### Operator observability of the wrapper itself" (look for that exact heading text in README.md) to fix the h2→h4 skip flagged by the static analyzer.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@harness/ARCHITECTURE.md`:
- Around line 101-109: Update the fenced command block in ARCHITECTURE.md to
match the Makefile by changing the "make all" description to include "build"
(i.e., "make all # build + config + observability + engine + verify") and add a
language tag to the fenced block (use ```bash) so markdown lint MD040 is
satisfied; locate the block containing the listed make targets and the "make
all" line and apply these two small edits.
In `@harness/Makefile`:
- Around line 71-76: The build loop that assumes target/release/$(w) may create
broken symlinks if the crate emits a different binary name; modify the Makefile
loop (the block iterating over LOCAL_WORKERS using WORKERS_REPO and WORKERS_BIN)
to, after cargo build, locate the actual executable produced under
"$(WORKERS_REPO)/$$w/target/release" (e.g. search for an executable file
matching $$w or with a $$w-containing basename), verify that this resolved
binary path exists, remove any stale symlink at "$(WORKERS_BIN)/$$w" if present,
and only then create the symlink to the real binary; if no matching binary is
found, print an error and exit non‑zero instead of creating a dangling symlink.
In `@harness/src/lib.rs`:
- Around line 113-124: The code directly extracts inner_function_id and passes
it into crate::otel::with_harness_span without applying the safety validation
used by extract_ids_for_bridge_trigger, which can allow oversized or malicious
values into the iii.function.id span attribute; fix by replacing the direct
extraction with a call to extract_ids_for_bridge_trigger to obtain validated IDs
(including the function_id) and then invoke run_in_span_with_ids (instead of
with_harness_span) so the span is created with the validated IDs; update the
call sites referencing inner_function_id, with_harness_span, and ensure
run_in_span_with_ids receives the returned validated ids from
extract_ids_for_bridge_trigger.
---
Nitpick comments:
In `@harness/README.md`:
- Line 130: Change the markdown heading "#### Operator observability of the
wrapper itself" to use a level-3 heading so the document's hierarchy is
consistent: replace "#### Operator observability of the wrapper itself" with
"### Operator observability of the wrapper itself" (look for that exact heading
text in README.md) to fix the h2→h4 skip flagged by the static analyzer.
In `@harness/tests/trace_correlation.rs`:
- Around line 304-312: Two identical helper functions named find_named are
duplicated in tests; extract a single test helper and remove the duplicates.
Create one shared find_named function in the test module (e.g., placed above the
test functions or in a test helper submodule) and have both tests call that
function instead of defining their own; remove the
#[allow(clippy::items_after_statements)] duplicate definitions at the original
locations (the blocks containing find_named) so the tests use the single shared
find_named implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b438df6-5788-4cfd-b9a2-893d069293dc
⛔ Files ignored due to path filters (4)
harness/Cargo.lockis excluded by!**/*.lockhook-fanout/Cargo.lockis excluded by!**/*.lockprovider-anthropic/Cargo.lockis excluded by!**/*.lockprovider-openai/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.gitignoreharness/ARCHITECTURE.mdharness/Cargo.tomlharness/Makefileharness/README.mdharness/src/lib.rsharness/src/otel.rsharness/tests/trace_correlation.rsharness/web/src/App.tsxharness/web/src/bridge.test.tsharness/web/src/bridge.tshook-fanout/Cargo.tomlhook-fanout/src/config.rshook-fanout/src/handler.rsprovider-anthropic/Cargo.tomlprovider-anthropic/crates/provider-base/src/openai_compat.rsprovider-anthropic/src/lib.rsprovider-openai/Cargo.tomlprovider-openai/crates/provider-base/src/openai_compat.rs
Summary
Every harness-wrapped function tags its span with
iii.session.id/iii.message.id/iii.function.idand propagates the same triple as OTel baggage. iii-sdk'sBaggageSpanProcessormaterializes those attrs on every downstream worker span — the Developer Console TRACES tab can now group spans per user turn.harness/src/otel.rs(new, 905 lines):with_harness_span/with_envelope_span/with_raw_spanwrappers, single-walk ID extraction with header-injection + control-byte guards,traceparent+x-iii-message-idresponse-header echo. 72 unit tests covering payload shapes, security guards, baggage round-trip.harness/src/lib.rs: every registered function (harness.status/harness.call/harness.info/ui.subscribe/ui.unsubscribe/harness.fs.read_inline) goes through a wrapper.harness/tests/trace_correlation.rs(new): end-to-end coverage including cross-crate allowlist parity withiii_sdk::DEFAULT_ALLOWLIST.iii_sdk::capture_otel_context()acrosstokio::spawnso HTTP child spans inherit baggage; HTTP request build wrapped in its owniii_sdk::run_in_span.buildtarget —cargo build --releasefor each local worker, symlink into~/.iii/workers/<name>.make allnow runsbuild → config → observability → engine → verify.toBridgeErrorwalks common engine-error shapes so users see real messages instead of[object Object].=0.11.7-next.3(crates.io) for the 4 workers that need the newspan_ops/run_with_baggagehelpers; the 15 unrelated workers stay on=0.11.3.Test plan
cargo test --libinharness/(72/72 passing)cargo clippy --tests -- -D warningsclean inharness/andhook-fanout/cargo fmt --all --checkclean across the 4 modified cratesmake allfromharness/succeeds: build → config → observability → engine → verifyiii trigger --function-id harness::statusreturns 14 expected workersiii.message.idon every downstream span (not just the harness root)iii trigger --function-id engine::traces::list --payload '{"attributes":[["iii.message.id","<id>"]],"search_all_spans":true}'returns ≥1 spanOut of scope
worker_path:line thatiii worker add .writes for theharnessentry intoconfig.yaml— manual edit needed until the iii CLI is taught to prefer existing~/.iii/workers/symlinks.Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores