From 1157683277d71b3e02dd92ea441dc8a9364aca02 Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 19:29:24 +0000 Subject: [PATCH 1/7] docs(net): add net-http Tracing layer design (Slice 1, PR 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Outermost ADR-0031 §6 layer: one info span per logical request (method, route, status, ErrorKind, latency, attempts), routed to the ADR-0014 Telemetry plane. Timer-generic latency; query-stripped route; structural secret-safety (reads only method/path/status/kind/clock). Includes Retry per-attempt events + an ambient attempt-count record, under a documented "inner layers emit events, not spans" composition contract. Built concurrently with the CircuitBreaker PR (independent files). Co-Authored-By: Claude Opus 4.8 (1M context) --- ...026-07-04-net-http-tracing-layer-design.md | 344 ++++++++++++++++++ 1 file changed, 344 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md diff --git a/docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md b/docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md new file mode 100644 index 0000000..66bacb5 --- /dev/null +++ b/docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md @@ -0,0 +1,344 @@ +# net-http `Tracing` layer — design (Slice 1, PR 5) + +## Context + +Slice 0 landed the net-http **construction surface** (transport contract ADR-0030; +`AuthSource`/`Auth`/`Guarded` in #66; the boot-time pacing config in #72). Slice 1 +implements the resilience *layers* of +[ADR-0031](../../adr/0031-http-resilience-venue-pacing.md) — each a standalone, +composable `Service` generic over +[`net-api::Timer`](../../adr/0029-network-adapter-stack-transport-split-compile-time-composition.md), +tested over inline service doubles + `MockTimer`. Assembly (`stack()`/`build()`) is +Slice 2. + +Landed so far: **PR 1 `RateLimit`** (#76), **PR 2 `Timeout`** (#78), **PR 3 `Retry`** +(#82). This spec covers **PR 5: `Tracing`** — the outermost layer of the ADR-0031 +stack — built **concurrently with the `CircuitBreaker` PR (PR 4)**. The two are +independent files implementing the same `Layer`/`Service` contract; they meet only at +the lib.rs re-export lines (a trivial rebase) and at a documented composition contract +(§Decision 8), never in code. It reuses every seam the prior layers established: the +`Layer`/`Service` contracts, `net-api::Timer`, and the inline-double + `MockTimer` test +pattern (net-http-api **cannot** dev-depend on `net-http-mock`'s `MockClient` — that +closes a crate cycle and the two builds' `Service` impls do not unify; `rate_limit.rs`/ +`retry.rs`/`body.rs` use inline doubles for exactly this reason). + +### Governing ADRs + +- **ADR-0031 §6** — `TracingLayer` is **outermost**, on the zero-runtime `tracing` + facade: one span per logical request covering retries and pacing waits, with `Retry` + emitting per-attempt events within it. Records method / route / status / `ErrorKind` / + latency / attempt count; **never** logs auth material or bodies. Always-on but + pay-per-use — an omitted `TracingLayer` is zero code in a hand-rolled stack; only + `build()`'s default includes it. +- **ADR-0031 §1** — the default stack `Tracing → CircuitBreaker → Retry → RateLimit → + Timeout → BufferOrStream → Auth → leaf`. `Tracing` is the first (outermost) `.layer()`, + so its span spans the entire logical request including every retry and pacing wait. +- **ADR-0014** — the net stack runs in the Adapter process, outside Core's deterministic + fold, so this layer's output is the **Telemetry** plane: wall-clock machinery metrics + (latency/throughput), never seq-stamped, lossy-tolerant, never canonical state. The + layer only *instruments*; aggregation is a subscriber's job. +- **ADR-0029 §4** — `Timer` (`now()` + `sleep()`), compile-time composition, no `dyn`. + `now()` was added for exactly the elapsed-time reads this layer needs. + +## Goal + +A `Tracing` `Service` (+ its `TracingLayer` factory) that opens one +`tracing` span per logical request — recording method, route, status, `ErrorKind`, +latency, and attempt count — routed to the ADR-0014 Telemetry plane, **structurally +incapable of leaking secrets**, body-transparent, runtime-neutral (`Timer`-generic, +zero-runtime `tracing` facade, **no** `tokio`), and driven deterministically by a fake +clock in tests. Plus the per-attempt instrumentation in `Retry` that makes attempt +count observable. + +## Scope (in) + +- The `Tracing` service + `TracingLayer` factory (impl'ing `net-api::Layer`), + in `oath-adapter-net-http-api`, new file `tracing.rs`. +- **One span** (`info_span!("http.request", …)`) per `call`, attached to the inner + future via `tracing::Instrument` so downstream events nest under it; the deferred + fields recorded on completion. +- **Latency** via `Timer::now()` deltas (the layer is `Timer`-generic). +- **Route** = `req.uri().path()` with the **query string dropped**; **method** from + `req.method()`. +- **Secret-safety by construction**: the layer reads only method + path + status + + `ErrorKind` + the clock — never headers, never the body. +- **Body-transparency**: `Response = http::Response` returned untouched. +- **Retry instrumentation** in `retry.rs`: per-attempt events + the ambient + attempt-count record (Decision 6). +- `tracing = { workspace = true }` runtime dep on net-http-api; `tracing-subscriber` + dev-dep for the capturing test subscriber. +- Capturing-subscriber tests (`MockTimer`-driven, inline doubles), including the + secret-safety assertion. + +## Non-goals (deferred — each its own PR/slice) + +| Deferred | Why | Where | +| --- | --- | --- | +| A low-cardinality templated `RouteLabel` request extension (e.g. `/iserver/account/{id}/orders`) | YAGNI — query-stripped `path()` already de-leaks and IBKR's routes are largely static; a clean additive follow-up mirroring `RateScope`/`Retryable`/`RequestTimeout` when an id-bearing route's cardinality first bites | future PR | +| Metric aggregation, exporters, span→metric rollup, sampling policy | ADR-0031 §6 + ADR-0014: the layer only instruments; a subscriber aggregates. Choosing/wiring a subscriber is a process-boot concern | Adapter/Supervisor boot | +| `stack()`/`build()` assembly that makes `Tracing` outermost-by-default | Construction/wiring; also the join point with the concurrent CircuitBreaker PR | Slice 2 | +| Tokio `Timer` impl, hyper backend | Runtime-specific | Slice 2 (`net-http-hyper`) | +| `TimeoutBody`-style mid-stream span/event instrumentation | No streaming venue yet — IBKR is all-buffered (parity with the Timeout spec's deferral) | when a streaming venue lands | +| Recording attempt count as a value *threaded back* from `Retry` to the outer span | Unnecessary coupling — the ambient current-span record (Decision 6) populates the field with none | n/a | + +## Decisions + +### 1. Layer shape & construction + +```rust +pub struct TracingLayer { timer: T } +pub struct Tracing { inner: S, timer: T } +``` + +`TracingLayer::new(timer: T) -> Self` is **infallible** — nothing to validate (contrast +`RateLimitLayer::new`), no `Result`/`BuildError`. `Clone` and `Debug` are +**hand-written** (not derived), as in `Timeout`/`Retry`: `Debug` uses +`finish_non_exhaustive`; `Clone` bounds `T: Clone` (and, for `Tracing`, `S: Clone`) so +the derives don't demand `Clone`/`Debug` on the inner service. `impl +Layer for TracingLayer { type Service = Tracing; … }` clones the `timer` +into each produced service. + +### 2. The span — name, fields, cardinality + +One span per `call`: + +```rust +let span = tracing::info_span!( + "http.request", + method = %req.method(), // recorded at creation (known up front) + route = %route, // path only, query dropped (Decision 4) + status = tracing::field::Empty, // recorded on completion (Ok) + error_kind = tracing::field::Empty, // recorded on completion (Err) + latency_us = tracing::field::Empty, // recorded on completion + attempts = tracing::field::Empty, // recorded by Retry via the current span (Decision 6) +); +``` + +- **Static span name** `"http.request"` → low cardinality (safe as a metric key). The + variable data (`method`, `route`) are **fields**, not part of the name. +- **`info` level** — the always-on span (§6 "always-on but pay-per-use"); the facade + makes it zero-cost when no subscriber is attached. Per-attempt `Retry` events are + `debug` (Decision 6) so ordinary telemetry stays lean while drill-down is available. +- `tracing::field::Empty` declares a field recordable later; recording it after the + await writes to the same span (spans are cheap-to-clone `Id` handles). + +### 3. Latency via `Timer::now()` + +The layer is **`Timer`-generic** (like `RateLimit`/`CircuitBreaker`, per ADR-0031 +Consequences). `Timer::now() -> std::time::Instant`, so: + +```rust +let start = self.timer.now(); +let out = /* inner call, see Decision 5 */; +let elapsed = self.timer.now().saturating_duration_since(start); // monotonic, panic-free +span.record("latency_us", u64::try_from(elapsed.as_micros()).unwrap_or(u64::MAX)); +``` + +`saturating_duration_since` (never panics on a non-monotonic read) and +`try_from(...).unwrap_or(u64::MAX)` (no `unwrap`/`as`-truncation — honours the +workspace lints) keep it total. Micros, not millis: network latencies span sub-ms to +seconds. Because `MockTimer::advance()` drives `now()`, a test that advances the clock +across the inner call asserts an **exact** recorded latency — the whole point of the +`Timer` seam. + +### 4. Route & secret-safety — structural, not a scrub + +```rust +let route = req.uri().path(); // NEVER req.uri() (carries ?query) — §6 names query tokens as a leak +``` + +Secret-safety is a property of **what the layer reads**, not a redaction pass over what +it emits: + +- It reads **method**, **path** (query dropped), **status**, **`ErrorKind`**, and the + **clock**. It never reads `headers()` (no `Authorization`/`Cookie`/API-key), never + reads or polls the **body**, and drops the URI's query. +- It sits **outermost** — above `Auth`, which stamps credentials *innermost* per + attempt ([auth.rs](../../../crates/adapter/net/http/api/src/auth.rs)) — so at + `Tracing`'s position the request has not even been signed yet. Defence in depth: even + if it had, the layer still never touches headers. + +This is why §6 calls `Tracing` "the one place certain not to leak": the guarantee is +enforced by the read surface, and a capturing test locks it in (§Testing). + +### 5. Data flow — instrument, then record on completion + +```rust +impl Service> for Tracing +where + S: Service, Response = http::Response, Error = HttpError> + Sync, + T: Timer, + // NOTE: no `B: Send` bound (contrast Retry). Nothing of type `B` is held across an + // await — the sole await is the inner call; `record()` is synchronous — so the + // response never crosses a yield point and cannot taint the future's `Send`-ness. +{ + type Response = http::Response; + type Error = HttpError; + + #[allow(clippy::manual_async_fn)] + fn call(&self, req: http::Request) + -> impl Future> + Send + { + use tracing::Instrument; // the trait that attaches a span to a future across awaits + let route = req.uri().path().to_owned(); + let span = /* Decision 2, using req.method() + route */; + async move { + let start = self.timer.now(); + // `.instrument(span.clone())` enters the span on every poll of the inner + // future — so EVERY event emitted downstream (CircuitBreaker, Retry's + // per-attempt, …) nests under this one span via context propagation. + let out = self.inner.call(req).instrument(span.clone()).await; + let elapsed = self.timer.now().saturating_duration_since(start); + span.record("latency_us", u64::try_from(elapsed.as_micros()).unwrap_or(u64::MAX)); + match &out { + Ok(resp) => { span.record("status", resp.status().as_u16()); } + Err(e) => { span.record("error_kind", kind_label(e.kind())); } + } + out + } + } +} +``` + +- **`Instrumented: Send` iff `F: Send`**, and the `Service` contract already promises + `S::Future: Send`, so the `+ Send` return bound is preserved. +- **`S: Sync`** because the returned `Send` future borrows `&self` (`&S: Send` needs + `S: Sync`; `T: Sync` holds via `Timer: Sync`). Same bound `Timeout`/`Retry` carry. +- **`select`-style ordering is irrelevant here** — there is no race; the layer wraps and + observes, it does not preempt. +- Not `async fn`: the trait requires the returned future be `Send` (only the desugared + `impl Future + Send` form states it), matching every other layer. + +`kind_label(ErrorKind) -> &'static str` is a `const fn` with a **`_` wildcard arm** — a +cross-crate `match` on the `#[non_exhaustive]` +[`ErrorKind`](../../../crates/adapter/net/api/src/error_kind.rs) is *forced* to have one, +which is precisely why the concurrent CircuitBreaker PR adding a circuit-open +classification cannot break this layer. + +### 6. Retry instrumentation + attempt count + +ADR-0031 §6 wants attempt count recorded and `Retry` emitting per-attempt events. Rather +than thread a count back out of `Retry`'s loop (coupling the layers), use `tracing`'s +**ambient current span**. Inside [retry.rs](../../../crates/adapter/net/http/api/src/retry.rs)'s +loop: + +```rust +// per attempt (drill-down; debug so it is pay-per-use): +tracing::event!(Level::DEBUG, attempt, outcome = /* status or kind */, "http.attempt"); +// on backoff (try_from, not `as` — the workspace lints reject truncating casts): +let backoff_us = u64::try_from(delay.as_micros()).unwrap_or(u64::MAX); +tracing::event!(Level::DEBUG, attempt, backoff_us, "http.retry.backoff"); +// after the loop settles on a final outcome (`attempt` is `Retry`'s existing `u32`): +tracing::Span::current().record("attempts", attempt); +``` + +- Because the whole inner call runs inside `Tracing`'s instrumented future, + `Span::current()` inside `Retry` **is** the `"http.request"` span (per the composition + contract, Decision 8), so `record("attempts", n)` populates that span's field — an + **always-on outer-span field**, with no direct dependency between the files. +- **Graceful no-op** when unused: with no subscriber, no `Tracing` span (a hand-rolled + stack without the layer), or a span lacking the `attempts` field, `record` on a + disabled/absent span/field does nothing. The facade guarantees zero cost. So `Retry` + stays correct standalone; the field simply fills in when composed under `Tracing`. +- Per-attempt events at **`debug`** keep normal `info` telemetry to one span per request + while preserving full drill-down when a subscriber enables `debug`. + +This is the only change to a previously-merged layer, and it is additive +(`use tracing` + three event/record lines); `Retry`'s existing behaviour and tests are +untouched. + +### 7. Error handling + +- No new `HttpError` variant — `Tracing` **observes** outcomes, it never originates one. + It records `status` (Ok) xor `error_kind` (Err) and returns the inner `Result` + verbatim. +- The concurrent CircuitBreaker PR owns any new `HttpError`/`ErrorKind` variant; this + PR's `kind_label` wildcard already accommodates it (Decision 5). + +### 8. Stack interaction & the composition contract (ADR-0031 §1) + +`Tracing → CircuitBreaker → Retry → RateLimit → Timeout → BufferOrStream → Auth → leaf`. +`Tracing` is outermost, so its span covers every retry and pacing wait; it is +body-transparent, composing with `RateLimit`'s `Guarded` output without disturbing +the permit lifetime. + +**Composition contract (new, documented here):** *`Tracing` owns the single per-request +span; every inner resilience layer emits `tracing` **events**, never opens its own +span.* A rejection or an attempt is a point-in-time fact, not a nested unit of work, so +events are the right shape — and this keeps `Span::current()` at any inner depth resolved +to `"http.request"`, which is what makes Decision 6's ambient `attempts` record land. +Concretely this is a one-line note for the **concurrent CircuitBreaker PR**: emit a +`debug` event on open/half-open/reject, do not open a span. If a future layer violates +this, the degradation is graceful (a wrongly-parented event / a no-op record), never a +panic or a leak. + +### 9. Dependencies + +- **Runtime:** promote `tracing = { workspace = true }` into net-http-api's + `[dependencies]` — the crate is the **first consumer** of the workspace dep already + declared at `Cargo.toml` (the `tracing` facade is a zero-runtime dep: no executor, no + I/O), so the crate's "no `tokio`/`hyper`/`reqwest`/`serde`" purity rule (lib.rs) is + intact. +- **Dev:** add `tracing-subscriber` to `[workspace.dependencies]` and net-http-api's + `[dev-dependencies]`. Dev-only — never in the shipped surface, consistent with the + existing `tokio` + `oath-adapter-net-mock` dev-deps. `machete`/`deny` stay green. + +## Testing (capturing subscriber, `MockTimer` clock, inline doubles) + +A test `tracing_subscriber::Layer` captures span attributes (`on_new_span`), later +`record()` calls (`on_record`), and events (`on_event`) into an +`Arc>`; installed per test via `tracing::subscriber::set_default(...)` +(RAII guard). `#[tokio::test]` is **current-thread**, so the thread-local dispatcher the +`Instrument` context relies on is the test's own — the capture is deterministic. The leaf +is an **inline** `Service` double (no `MockClient` — cycle). Cases: + +- **Happy path + body transparency:** a leaf returning `200` immediately → span records + `method`, `route`, `status = 200`, `latency_us`; the response body `collect()`s to the + expected bytes (proves `Response` untouched). +- **Error path:** a leaf returning `Err(HttpError::connection(...))` → span records + `error_kind = "connection"`, **no** `status`; the call returns `Connection` verbatim. +- **Latency is real and exact:** a leaf that `await`s `timer.sleep(d)`; spawn, + `yield_now`, `advance(d)` → recorded `latency_us == d` in micros (the `Timer` seam). +- **Attempt count + nested events:** an *eligible* request through a + `RetryLayer`-wrapped leaf that fails-then-succeeds, all inside `TracingLayer` → the + `"http.request"` span records `attempts == 2`, and two `http.attempt` events are + captured within it. +- **Secret-safety (the load-bearing test):** a request carrying an `Authorization` + header **and** a `?token=SUPERSECRET` query → assert the captured spans/events contain + neither `SUPERSECRET` nor the header value **anywhere**, and that `route` has no `?`. +- **Zero-cost / graceful path:** `Retry` used **without** `TracingLayer` and with no + subscriber → its `Span::current().record("attempts", …)` is a no-op and existing + `Retry` behaviour/tests are unchanged (guards Decision 6). + +## Definition of done + +- `Tracing` + `TracingLayer` implemented as specified; `retry.rs` gains the + per-attempt events + ambient `attempts` record; `lib.rs` gains `pub mod tracing;` + + re-exports (`Tracing`, `TracingLayer`) + a module-doc bullet; all with the tests above. +- `Cargo.toml` (workspace) declares `tracing-subscriber`; net-http-api `Cargo.toml` adds + `tracing` (dep) + `tracing-subscriber` (dev-dep). +- An append-only **ADR-0034 amendment** records the Tracing refinements (Timer-generic + latency, query-stripped route, the "inner layers emit events, not spans" composition + contract, ambient attempt-count) — same pattern #76/#78 used. Exact amendment number + depends on merge order relative to the concurrent CircuitBreaker PR (open question). +- `CHANGELOG.md` `[Unreleased]` updated. +- `just ci` green — fmt, lint (**deny**), test + doctests, **`just doc`** (intra-doc + links), deny, typos, machete; no new warnings; no `unsafe`/`unwrap`/`expect`/indexing + in non-test code. +- Delivered as one issue → one branch (worktree `.claude/worktrees/net-http-tracing`) → + one PR (`Closes #N`). + +## Open questions (for the implementation plan) + +1. **ADR amendment placement/number** — record the Tracing refinements as an append-only + ADR-0034 amendment (the living 2026-07-04 list), as the Timeout/RateLimit PRs did. + Leaning yes for trail parity. The number is contended by the **concurrent + CircuitBreaker PR**; whichever merges second renumbers its own amendment — a rebase + note, not a design issue. +2. **`attempts` field type** — record as `u64` (attempt index) vs also emitting a + terminal `http.retry.exhausted` event. Leaning the plain field + per-attempt `debug` + events; a terminal event is additive later if a subscriber wants it. +3. **Capturing subscriber location** — a small reusable capture `Layer` in the test + module of `tracing.rs`, or shared with `retry.rs`'s new attempt-count test via a + `#[cfg(test)]` helper. Leaning per-file inline (parity with the inline-double + convention) unless duplication bites. From 332bec23202b4c9414010787c72826cc9044507d Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 19:47:35 +0000 Subject: [PATCH 2/7] docs(net): add Tracing layer implementation plan (Slice 1, PR 5) Three-task TDD plan: (1) the Tracing layer + capturing-subscriber test harness + secret-safety test; (2) Retry per-attempt events + ambient attempt-count integration test; (3) ADR-0034 amendment, CHANGELOG, gate, PR. Module named 'trace' to avoid shadowing the tracing crate. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-07-04-net-http-tracing-layer.md | 823 ++++++++++++++++++ 1 file changed, 823 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-04-net-http-tracing-layer.md diff --git a/docs/superpowers/plans/2026-07-04-net-http-tracing-layer.md b/docs/superpowers/plans/2026-07-04-net-http-tracing-layer.md new file mode 100644 index 0000000..3d1cebc --- /dev/null +++ b/docs/superpowers/plans/2026-07-04-net-http-tracing-layer.md @@ -0,0 +1,823 @@ +# net-http `Tracing` Layer (Slice 1, PR 5) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Build the outermost `Tracing` HTTP middleware — one `tracing` span per logical request recording method, route, status, `ErrorKind`, latency, and attempt count, routed to the ADR-0014 Telemetry plane, structurally incapable of leaking secrets — plus the per-attempt instrumentation in the already-merged `Retry` layer that makes attempt count observable. + +**Architecture:** A `Timer`-generic, runtime-neutral `Service` wrapper in `oath-adapter-net-http-api` on the zero-runtime `tracing` facade. `call` opens `info_span!("http.request", …)`, attaches it to the inner future via `tracing`'s `Instrument` trait (so every downstream event — including `Retry`'s per-attempt events — nests under it via context propagation), measures latency with `Timer::now()` deltas, and records `status` xor `error_kind` on completion. It reads **only** method, `uri().path()` (query dropped), status, `ErrorKind`, and the clock — never headers, never the body — so secret-safety is a property of the read surface, not a scrub. `Retry` records the final attempt count onto the *ambient current span* (`Span::current().record("attempts", …)`), a no-op when no `Tracing` span is active. + +**Tech Stack:** Rust (edition 2024, MSRV 1.90), `just`, the `tracing` 0.1 facade (new runtime dep — zero executor/IO), `http`/`bytes`, `net-api::{Timer, ErrorKind, HasErrorKind, Layer}`. Tests use a capturing `tracing-subscriber` `Layer` (new dev-dep) + inline service doubles + `MockTimer` (`oath-adapter-net-mock`), driven on `tokio` (dev-only). + +## Global Constraints + +Every task implicitly includes these: + +- **Edition 2024, MSRV 1.90.** No `unsafe` — the crate is `#![forbid(unsafe_code)]`. +- **No `unwrap`/`expect`/indexing/panic and no truncating `as` casts in non-test code** — return `Result`; use `u64::try_from(x).unwrap_or(u64::MAX)` / `u64::from(x)`, never `x as u64`. Test code is exempt for `unwrap`/`expect`/indexing. +- **`just lint` = clippy `-D warnings` + `pedantic`/`nursery`** — `#[must_use]` where asked, document all public items (`missing_docs`), `Debug` on all **public** types (`missing_debug_implementations` — hand-impl where a derive would demand `Debug`/`Clone` on `S`/`T`), `const fn` where `missing_const_for_fn` asks. +- **`just doc` per task** — `just check`/`lint`/`test` do **not** catch broken rustdoc intra-doc links; every task's verify step runs `just doc`. +- **`net-http-api` charter:** no async *runtime* — no `tokio`/`hyper`/`reqwest`/`serde` in non-dev deps. The `tracing` facade is a zero-runtime dep (no executor, no IO), so it is **allowed**; `tracing-subscriber` is **dev-only** (consistent with the existing `tokio` dev-dep). +- **net-http-api tests must NOT dev-depend on `oath-adapter-net-http-mock` (`MockClient`)** — it normal-depends on this crate, so the dev-dep closes a cycle that recompiles a second, non-unifying copy of `net-http-api`. Use **inline** service doubles + `oath-adapter-net-mock`'s `MockTimer`, exactly as `rate_limit.rs`/`retry.rs`/`body.rs` do. +- **DoD per PR:** `just ci` green (fmt, lint, test + doctests, doc, deny, typos, machete). Update `CHANGELOG.md` `[Unreleased]`. One issue → one branch → worktree → one PR (`Closes #`). + +## Source spec + +[docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md](../specs/2026-07-04-net-http-tracing-layer-design.md), governed by [ADR-0031 §6](../../adr/0031-http-resilience-venue-pacing.md), [ADR-0014](../../adr/0014-observability-three-planes-deterministic-boundary.md), and [ADR-0034](../../adr/0034-http-construction-surface-auth-guarded-boot-coverage.md). This is **Slice 1, PR 5** — the outermost resilience layer, built **concurrently with the CircuitBreaker PR (PR 4)**. RateLimit (#76), Timeout (#78), Retry (#82) landed PRs 1–3. + +## File Structure + +- `crates/adapter/net/http/api/src/trace.rs` — **new** (Tasks 1–2). `TracingLayer`, `Tracing`, `kind_label`, the `Layer`/`Service` impls, and all tests (the capturing subscriber + inline leaves). + - **Module is named `trace`, not `tracing`** — a module named `tracing` would shadow the `tracing` crate at the crate root. The **public types stay `Tracing`/`TracingLayer`** (re-exported), so the module name is an internal detail. (The spec says "tracing.rs"; this is the one deliberate refinement.) +- `crates/adapter/net/http/api/src/retry.rs` — **modify** (Task 2). Add per-attempt events + the ambient `attempts` record inside the existing `call` loop. +- `crates/adapter/net/http/api/src/lib.rs` — **modify** (Task 1). `pub mod trace;` + re-exports + module-doc bullet. +- `Cargo.toml` (workspace) + `crates/adapter/net/http/api/Cargo.toml` — **modify** (Task 1). Add `tracing` (dep) + `tracing-subscriber` (dev-dep). +- `docs/adr/0034-...md`, `CHANGELOG.md` — **modify** (Task 3). + +Each task is one or more commits; the tasks together are one PR/issue. + +--- + +## Setup: issue (worktree already exists) + +> The isolated worktree **already exists** at `.claude/worktrees/net-http-tracing` (branch `feat/net-http-tracing`, branched off `origin/main` = #82). All tasks run inside it. Only the GitHub issue remains. + +- [ ] **Create the issue** + +```bash +gh issue create \ + --title "feat(net): Tracing resilience layer (Slice 1, PR 5)" \ + --label enhancement \ + --body "Slice 1 PR 5 of the net-http resilience layers (spec: docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md; ADR-0031 §6, ADR-0014). + +- \`Tracing\` + \`TracingLayer\` (impl \`net-api::Layer\`): the outermost layer — one \`info\` span per logical request (method, route, status, ErrorKind, latency, attempts), attached to the inner future via \`tracing::Instrument\` so downstream events nest under it. Routed to the ADR-0014 Telemetry plane. +- Secret-safe by construction: reads only method, \`uri().path()\` (query dropped), status, \`ErrorKind\`, and the \`Timer\` clock — never headers, never the body. +- \`Retry\` gains per-attempt \`tracing\` events + an ambient \`Span::current().record(\"attempts\", n)\` (a no-op when no Tracing span is active). +- Adds the \`tracing\` facade (runtime dep, zero executor) + \`tracing-subscriber\` (dev-dep). Built concurrently with the CircuitBreaker PR (independent files)." +``` + +Note the issue number `#` for the PR body. + +--- + +## Task 1: `Tracing` layer — span, instrument, record, secret-safety + +**Files:** +- Create: `crates/adapter/net/http/api/src/trace.rs` +- Modify: `crates/adapter/net/http/api/src/lib.rs` +- Modify: `Cargo.toml` (workspace), `crates/adapter/net/http/api/Cargo.toml` + +**Interfaces:** +- Consumes: `HttpError`, `Service` (crate); `ErrorKind`, `HasErrorKind`, `Layer`, `Timer` (`oath_adapter_net_api`); the `tracing` facade. +- Produces: + - `oath_adapter_net_http_api::TracingLayer` — `impl Layer` factory; `pub const fn new(timer: T) -> Self`. + - `oath_adapter_net_http_api::Tracing` — the wrapping `Service`; for an inner `S: Service, Response = http::Response, Error = HttpError> + Sync` and `T: Timer`, it is `Service, Response = http::Response, Error = HttpError>` (body-transparent — same `B`, **no** `B: Send` bound: nothing of type `B` crosses the single await). + - The span it opens: name `"http.request"`, fields `method`, `route`, `status`, `error_kind`, `latency_us`, `attempts` (the last four declared `Empty`, recorded later — `attempts` by Task 2's `Retry`). + +- [ ] **Step 1: Add the dependencies** + +In the **workspace** `Cargo.toml`, under `[workspace.dependencies]`, immediately after the existing `tracing = "0.1"` line, add: + +```toml +tracing-subscriber = { version = "0.3", default-features = false, features = ["registry"] } +``` + +(The `registry` feature gives the span store + `LookupSpan` the capturing test layer needs; skipping `fmt`/`ansi`/`env-filter` keeps the `deny`/`machete` surface minimal. `tracing = "0.1"` is already declared and unused — this PR is its first consumer.) + +In `crates/adapter/net/http/api/Cargo.toml`, add to `[dependencies]` (after `pin-project-lite = { workspace = true }`): + +```toml +tracing = { workspace = true } +``` + +and to `[dev-dependencies]` (after `oath-adapter-net-mock = { workspace = true }`): + +```toml +tracing-subscriber = { workspace = true } +``` + +- [ ] **Step 2: Write the failing tests** + +Create `crates/adapter/net/http/api/src/trace.rs` with the module doc, the `use` block, the capturing subscriber, the inline leaves, and the four tests below. (The `TracingLayer`/`Tracing`/`kind_label` items land in Step 4; this compiles to a failure until then.) + +```rust +//! The `Tracing` resilience layer (ADR-0031 §6) — the outermost layer. +//! +//! Opens one `tracing` span per logical request and attaches it to the inner +//! future via [`Instrument`](tracing::Instrument), so every event the inner +//! stack emits — including [`Retry`](crate::Retry)'s per-attempt events — nests +//! under it. The span records method, route (path only — the query is dropped), +//! status **xor** [`ErrorKind`](oath_adapter_net_api::ErrorKind), latency, and +//! (via `Retry`) attempt count — the ADR-0014 Telemetry plane. **Secret-safe by +//! construction:** it reads only method, path, status, `ErrorKind`, and the +//! clock — never headers, never the body. **Body-transparent:** the response is +//! returned untouched. Runtime-neutral: latency via +//! [`Timer::now`](oath_adapter_net_api::Timer::now), on the zero-runtime +//! `tracing` facade. The module is named `trace` (not `tracing`) to avoid +//! shadowing the `tracing` crate; the public types are `Tracing`/`TracingLayer`. + +use crate::{HttpError, Service}; +use bytes::Bytes; +use oath_adapter_net_api::{ErrorKind, HasErrorKind, Layer, Timer}; +use std::fmt; +use std::future::Future; +use tracing::Instrument; +use tracing::field::Empty; + +/// The stable telemetry label for an [`ErrorKind`] — a low-cardinality +/// `&'static str` for the span's `error_kind` field. +/// +/// The `_` arm covers the `#[non_exhaustive]` enum, so a new variant (e.g. a +/// future `CircuitBreaker` classification added by the concurrent PR) compiles +/// without touching this layer. +const fn kind_label(kind: ErrorKind) -> &'static str { + match kind { + ErrorKind::Timeout => "timeout", + ErrorKind::Connection => "connection", + ErrorKind::Throttled => "throttled", + ErrorKind::Auth => "auth", + ErrorKind::Client => "client", + ErrorKind::Server => "server", + _ => "unknown", // ErrorKind::Unknown and any future non_exhaustive variant + } +} + +#[cfg(test)] +mod tests { + use super::{Tracing, TracingLayer}; + use crate::{HttpError, Service}; + use bytes::Bytes; + use http_body::{Body, Frame, SizeHint}; + use http_body_util::BodyExt; + use oath_adapter_net_api::{Layer, Timer}; + use oath_adapter_net_mock::MockTimer; + use std::collections::BTreeMap; + use std::future::Future; + use std::pin::Pin; + use std::sync::{Arc, Mutex}; + use std::task::{Context, Poll}; + use std::time::Duration; + use tracing::field::{Field, Visit}; + use tracing::span::{Attributes, Id, Record}; + use tracing::{Event, Subscriber}; + use tracing_subscriber::layer::{Context as LayerCtx, Layer as SubLayer}; + use tracing_subscriber::prelude::*; + use tracing_subscriber::registry::LookupSpan; + + // ---- capturing subscriber ------------------------------------------------ + // One request per test, so the single span's fields merge into one map. + + #[derive(Default)] + struct Store { + span_fields: BTreeMap, + events: Vec>, + } + impl Store { + // A flat dump of every captured string — for the secret-safety scan. + fn haystack(&self) -> String { + let mut s = String::new(); + for (k, v) in &self.span_fields { + s.push_str(k); + s.push('='); + s.push_str(v); + s.push('\n'); + } + for ev in &self.events { + for (k, v) in ev { + s.push_str(k); + s.push('='); + s.push_str(v); + s.push('\n'); + } + } + s + } + } + + // Renders field values to strings. `record_str` keeps `&str` values quote-free + // (e.g. "connection"); everything else (Display via `%`, ints, the message) + // funnels through `record_debug`, whose Debug-of-format_args is also quote-free. + struct StrVisit<'a>(&'a mut BTreeMap); + impl Visit for StrVisit<'_> { + fn record_str(&mut self, field: &Field, value: &str) { + self.0.insert(field.name().to_owned(), value.to_owned()); + } + fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { + self.0.insert(field.name().to_owned(), format!("{value:?}")); + } + } + + #[derive(Clone, Default)] + struct Capture { + store: Arc>, + } + impl LookupSpan<'a>> SubLayer for Capture { + fn on_new_span(&self, attrs: &Attributes<'_>, _id: &Id, _ctx: LayerCtx<'_, S>) { + let mut store = self.store.lock().unwrap(); + let mut fields = std::mem::take(&mut store.span_fields); + attrs.record(&mut StrVisit(&mut fields)); + store.span_fields = fields; + } + fn on_record(&self, _id: &Id, values: &Record<'_>, _ctx: LayerCtx<'_, S>) { + let mut store = self.store.lock().unwrap(); + let mut fields = std::mem::take(&mut store.span_fields); + values.record(&mut StrVisit(&mut fields)); + store.span_fields = fields; + } + fn on_event(&self, event: &Event<'_>, _ctx: LayerCtx<'_, S>) { + let mut fields = BTreeMap::new(); + event.record(&mut StrVisit(&mut fields)); + self.store.lock().unwrap().events.push(fields); + } + } + + // Install a fresh Capture as the thread-local default; return its store + the + // RAII guard. `#[tokio::test]` is current-thread, so every `.await` below runs + // on this thread and the `Instrument` context resolves to this subscriber. + fn capture() -> (Arc>, tracing::subscriber::DefaultGuard) { + let cap = Capture::default(); + let store = cap.store.clone(); + let guard = tracing::subscriber::set_default(tracing_subscriber::registry().with(cap)); + (store, guard) + } + + // ---- inline leaves (no MockClient — dev-dep cycle) ----------------------- + + #[derive(Debug)] + struct StubBody { + data: Option, + } + impl StubBody { + fn new(b: &'static [u8]) -> Self { + Self { data: Some(Bytes::from_static(b)) } + } + } + impl Body for StubBody { + type Data = Bytes; + type Error = HttpError; + fn poll_frame( + self: Pin<&mut Self>, + _: &mut Context<'_>, + ) -> Poll, HttpError>>> { + Poll::Ready(self.get_mut().data.take().map(|d| Ok(Frame::data(d)))) + } + fn is_end_stream(&self) -> bool { + self.data.is_none() + } + fn size_hint(&self) -> SizeHint { + self.data.as_ref().map_or_else( + || SizeHint::with_exact(0), + |d| SizeHint::with_exact(d.len() as u64), + ) + } + } + + // 200 immediately. + #[derive(Clone)] + struct OkLeaf; + impl Service> for OkLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + async move { Ok(http::Response::new(StubBody::new(b"ok"))) } + } + } + + // Connection error immediately. + #[derive(Clone)] + struct ErrLeaf; + impl Service> for ErrLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + async move { Err(HttpError::connection("reset")) } + } + } + + // Advances the shared clock by `elapsed` (synchronously — MockTimer uses + // interior mutability) before returning 200, giving the layer a deterministic + // nonzero latency to record without spawning. + #[derive(Clone)] + struct ClockLeaf { + timer: MockTimer, + elapsed: Duration, + } + impl Service> for ClockLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + let timer = self.timer.clone(); + let elapsed = self.elapsed; + async move { + timer.advance(elapsed); + Ok(http::Response::new(StubBody::new(b"ok"))) + } + } + } + + fn get(uri: &str) -> http::Request { + http::Request::builder() + .method("GET") + .uri(uri) + .body(Bytes::new()) + .unwrap() + } + + #[tokio::test] + async fn records_method_route_status_and_body_is_transparent() { + let (store, _guard) = capture(); + let svc = TracingLayer::new(MockTimer::new()).layer(OkLeaf); + let resp = svc.call(get("/iserver/accounts")).await.expect("ok"); + let body = resp.into_body().collect().await.unwrap().to_bytes(); + assert_eq!(body, Bytes::from_static(b"ok")); // Response passed straight through + let store = store.lock().unwrap(); + assert_eq!(store.span_fields.get("method").map(String::as_str), Some("GET")); + assert_eq!(store.span_fields.get("route").map(String::as_str), Some("/iserver/accounts")); + assert_eq!(store.span_fields.get("status").map(String::as_str), Some("200")); + } + + #[tokio::test] + async fn records_error_kind_on_failure_and_omits_status() { + let (store, _guard) = capture(); + let svc = TracingLayer::new(MockTimer::new()).layer(ErrLeaf); + let err = svc.call(get("/x")).await.unwrap_err(); + assert!(matches!(err, HttpError::Connection(_))); // returned verbatim, not swallowed + let store = store.lock().unwrap(); + assert_eq!(store.span_fields.get("error_kind").map(String::as_str), Some("connection")); + assert!(!store.span_fields.contains_key("status"), "no status on the error path"); + } + + #[tokio::test] + async fn latency_reflects_the_clock_delta_exactly() { + let (store, _guard) = capture(); + let timer = MockTimer::new(); + let svc = TracingLayer::new(timer.clone()) + .layer(ClockLeaf { timer: timer.clone(), elapsed: Duration::from_millis(50) }); + svc.call(get("/x")).await.expect("ok"); + let store = store.lock().unwrap(); + assert_eq!(store.span_fields.get("latency_us").map(String::as_str), Some("50000")); // 50ms + } + + #[tokio::test] + async fn never_leaks_authorization_header_or_query_token() { + let (store, _guard) = capture(); + let svc = TracingLayer::new(MockTimer::new()).layer(OkLeaf); + let mut req = get("/iserver/orders?oauth_token=SUPERSECRET&api_key=SUPERSECRET"); + req.headers_mut().insert( + http::header::AUTHORIZATION, + http::HeaderValue::from_static("Bearer SUPERSECRET"), + ); + svc.call(req).await.expect("ok"); + let store = store.lock().unwrap(); + let hay = store.haystack(); + assert!(!hay.contains("SUPERSECRET"), "secret leaked into telemetry:\n{hay}"); + // route carries the path only — the query (with its tokens) is dropped. + assert_eq!(store.span_fields.get("route").map(String::as_str), Some("/iserver/orders")); + } +} +``` + +Also wire `lib.rs` now (so the module resolves). Add the module-doc bullet after the `timeout` bullet (the block ending at the current line 18): + +```rust +//! - [`trace`] — the `Tracing` layer and its `TracingLayer` factory (outermost; +//! one span per request, secret-safe, routed to the ADR-0014 Telemetry plane) +``` + +Add the module declaration after `pub mod timeout;`: + +```rust +pub mod trace; +``` + +Add the re-export after `pub use timeout::{RequestTimeout, Timeout, TimeoutLayer};`: + +```rust +pub use trace::{Tracing, TracingLayer}; +``` + +- [ ] **Step 3: Run it to verify it fails** + +Run: `just check` +Expected: FAIL — `cannot find type Tracing`/`TracingLayer` in module `trace` (only `kind_label` + tests exist). + +- [ ] **Step 4: Implement the layer** + +Insert between `kind_label` and the `#[cfg(test)] mod tests` in `trace.rs`: + +```rust +/// The `Tracing` [`Layer`] factory: holds the [`Timer`] clock (for latency) and +/// produces a [`Tracing`] around any inner service. +pub struct TracingLayer { + timer: T, +} + +impl TracingLayer { + /// Build the layer with a [`Timer`] clock. Infallible — no config to check. + #[must_use] + pub const fn new(timer: T) -> Self { + Self { timer } + } +} + +impl Clone for TracingLayer { + fn clone(&self) -> Self { + Self { timer: self.timer.clone() } + } +} + +impl fmt::Debug for TracingLayer { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("TracingLayer").finish_non_exhaustive() + } +} + +impl Layer for TracingLayer { + type Service = Tracing; + + fn layer(&self, inner: S) -> Tracing { + Tracing { inner, timer: self.timer.clone() } + } +} + +/// The `Tracing` middleware: opens one span per request and records the outcome. +/// +/// Body-transparent — the inner `http::Response` is returned untouched. +pub struct Tracing { + inner: S, + timer: T, +} + +impl Clone for Tracing { + fn clone(&self) -> Self { + Self { inner: self.inner.clone(), timer: self.timer.clone() } + } +} + +impl fmt::Debug for Tracing { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Tracing").finish_non_exhaustive() + } +} + +impl Service> for Tracing +where + S: Service, Response = http::Response, Error = HttpError> + Sync, + T: Timer, + // No `B: Send`: the sole await is the inner call; `record()` is synchronous, + // so no value of type `B` ever crosses a yield point (contrast `Retry`). +{ + type Response = http::Response; + type Error = HttpError; + + // Not `async fn`: the trait requires the returned future to be `Send`. + #[allow(clippy::manual_async_fn)] + fn call( + &self, + req: http::Request, + ) -> impl Future> + Send { + async move { + // Read method + path up front — path ONLY, never the query, which can + // carry tokens (ADR-0031 §6). `route` is owned so `req` can move on. + let route = req.uri().path().to_owned(); + let span = tracing::info_span!( + "http.request", + method = %req.method(), + route = %route, + status = Empty, + error_kind = Empty, + latency_us = Empty, + attempts = Empty, + ); + let start = self.timer.now(); + // `.instrument` enters the span on every poll of the inner future, so + // every downstream event (incl. Retry's per-attempt) nests under it. + let out = self.inner.call(req).instrument(span.clone()).await; + let elapsed = self.timer.now().saturating_duration_since(start); + span.record("latency_us", u64::try_from(elapsed.as_micros()).unwrap_or(u64::MAX)); + match &out { + Ok(resp) => { + span.record("status", u64::from(resp.status().as_u16())); + } + Err(e) => { + span.record("error_kind", kind_label(e.kind())); + } + } + out + } + } +} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `just check && cargo test -p oath-adapter-net-http-api trace && just lint && just doc` +Expected: PASS, warning-free, docs build clean. + +> Known risks (from the spec's implementation notes): +> - **Module name.** `trace`, not `tracing` — a `tracing` module would shadow the crate. If you still see `info_span!`/`Instrument` failing to resolve, confirm the file is `trace.rs` and paths are the fully-qualified `tracing::…`. +> - **`tracing-subscriber` feature.** If the compiler names a missing item behind a feature (e.g. `registry`), the `registry` feature is the one that matters; add `"std"` alongside it if `Layer`/`LookupSpan` are reported missing. +> - **`S: Sync`** is required because the returned `Send` future borrows `&self`; `T: Sync` holds via `Timer`. No `B` bound. +> - **`Empty` fields** are not emitted at span creation, so `on_new_span` captures only `method`/`route`; `status`/`error_kind`/`latency_us` arrive via `on_record`. That is why the error test asserts `!contains_key("status")`. +> - If clippy's `missing_const_for_fn` rejects `const fn new` for generic `T` (it should not — construction only), drop `const`. + +- [ ] **Step 6: Commit** + +```bash +git add Cargo.toml crates/adapter/net/http/api/Cargo.toml \ + crates/adapter/net/http/api/src/trace.rs crates/adapter/net/http/api/src/lib.rs +git commit -m "feat(net): Tracing layer — per-request span, secret-safe, Timer latency" +``` + +--- + +## Task 2: `Retry` per-attempt instrumentation + attempt-count integration test + +**Files:** +- Modify: `crates/adapter/net/http/api/src/retry.rs` +- Modify: `crates/adapter/net/http/api/src/trace.rs` (test module only) + +**Interfaces:** +- Consumes: `Tracing`/`TracingLayer` (Task 1); `RetryLayer`, `RetryConfig`, `Retryable` (crate, already shipped #82); the capturing subscriber + `StubBody` (Task 1 test module). +- Produces: `Retry`'s `call` now emits a `debug` `http.attempt` event per send, a `debug` `http.retry.backoff` event per backoff, and records `attempts` onto the ambient current span — a **no-op** when no `Tracing` span (or no subscriber) is active, so `Retry`'s standalone behaviour and its existing tests are unchanged. + +- [ ] **Step 1: Write the failing integration test** + +Add to the `tests` module in `trace.rs`. First extend that module's imports (add these `use` lines alongside the existing ones): + +```rust + use std::sync::atomic::{AtomicUsize, Ordering}; +``` + +Then add the scripted leaf + the test: + +```rust + // A leaf yielding one scripted status per call; repeats the last once exhausted. + #[derive(Clone, Copy)] + enum Step { + Status(u16), + } + #[derive(Clone)] + struct ScriptLeaf { + steps: Arc>, + calls: Arc, + } + impl ScriptLeaf { + fn new(steps: Vec) -> Self { + Self { steps: Arc::new(steps), calls: Arc::new(AtomicUsize::new(0)) } + } + } + impl Service> for ScriptLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + let i = self.calls.fetch_add(1, Ordering::Relaxed); + let step = self.steps.get(i).copied().unwrap_or_else(|| *self.steps.last().unwrap()); + async move { + match step { + Step::Status(code) => { + let mut resp = http::Response::new(StubBody::new(b"body")); + *resp.status_mut() = http::StatusCode::from_u16(code).unwrap(); + Ok(resp) + } + } + } + } + } + + #[tokio::test] + async fn retry_populates_attempt_count_and_nests_per_attempt_events() { + use crate::{RetryConfig, RetryLayer, Retryable}; + use std::num::NonZeroU32; + + let (store, _guard) = capture(); + // Zero backoff → the retry loop runs inline: MockTimer `sleep(0)` is Ready, + // so no spawn/advance is needed to drain the backoff between attempts. + let cfg = RetryConfig { + max_attempts: NonZeroU32::new(3).unwrap(), + base: Duration::ZERO, + cap: Duration::ZERO, + seed: 1, + }; + let leaf = ScriptLeaf::new(vec![Step::Status(503), Step::Status(200)]); + let svc = TracingLayer::new(MockTimer::new()) + .layer(RetryLayer::new(cfg, MockTimer::new()).layer(leaf)); + let mut req = get("/iserver/orders"); + req.extensions_mut().insert(Retryable); + + let resp = svc.call(req).await.expect("503 retried → 200"); + assert_eq!(resp.status(), http::StatusCode::OK); + + let store = store.lock().unwrap(); + // The ambient record from inside Retry lands on the outer "http.request" span. + assert_eq!(store.span_fields.get("attempts").map(String::as_str), Some("2")); + // Two sends → two nested http.attempt events (message field = "http.attempt"). + let attempts = store + .events + .iter() + .filter(|e| e.get("message").map(String::as_str) == Some("http.attempt")) + .count(); + assert_eq!(attempts, 2, "one http.attempt event per send"); + } +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cargo test -p oath-adapter-net-http-api retry_populates_attempt_count -- --nocapture` +Expected: FAIL — `attempts` is absent (still `Empty`) and there are zero `http.attempt` events, because `Retry` is not yet instrumented. Assertion fails on `Some("2")` / `attempts == 2`. + +- [ ] **Step 3: Instrument `Retry`** + +In `retry.rs`, replace the body of the `loop { … }` inside `Retry::call` (currently: fetch outcome → decide `retry` → `if !retry { return outcome; }` → drop → backoff → `attempt += 1`) with the instrumented version. The new loop body: + +```rust + loop { + // Whole-request clone per attempt (see the existing note above this loop). + let outcome = self.inner.call(req.clone()).await; + // Per-attempt telemetry — nests under Tracing's span when present, + // a no-op otherwise (ADR-0031 §6). `debug`: drill-down, pay-per-use. + match &outcome { + Ok(resp) => tracing::event!( + tracing::Level::DEBUG, + attempt = u64::from(attempt), + status = u64::from(resp.status().as_u16()), + "http.attempt" + ), + Err(e) => tracing::event!( + tracing::Level::DEBUG, + attempt = u64::from(attempt), + error_kind = ?e.kind(), + "http.attempt" + ), + } + let retry = eligible + && attempt < max + && match &outcome { + Err(e) => is_transient(e.kind()), + Ok(resp) => resp.status().is_server_error(), // 5xx only; 429 is 4xx + }; + if !retry { + // Record the final attempt count onto the current span — the + // "http.request" span when composed under Tracing; a no-op + // otherwise (no active span / the field is absent). + tracing::Span::current().record("attempts", u64::from(attempt)); + return outcome; // success, non-retryable outcome, or attempts exhausted + } + drop(outcome); // release the prior response's Guarded permit before waiting + let ceil = backoff_ceiling(self.cfg.base, self.cfg.cap, attempt); + let delay = self.rng.duration_in(ceil); + tracing::event!( + tracing::Level::DEBUG, + attempt = u64::from(attempt), + backoff_us = u64::try_from(delay.as_micros()).unwrap_or(u64::MAX), + "http.retry.backoff" + ); + self.timer.sleep(delay).await; + attempt += 1; + } +``` + +No new `use` line is needed — the events use fully-qualified `tracing::event!` / `tracing::Level` / `tracing::Span`, and `tracing` is a crate dependency as of Task 1. + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo test -p oath-adapter-net-http-api && just lint && just doc` +Expected: PASS — the new integration test passes, **and every existing `retry.rs` test still passes** (they run without a subscriber, proving the ambient record + events are a graceful no-op). Warning-free, docs clean. + +> Why no separate "graceful no-op" test: the entire pre-existing `retry.rs` suite runs with no `tracing` subscriber installed, so its continued green is exactly that assertion. + +- [ ] **Step 5: Commit** + +```bash +git add crates/adapter/net/http/api/src/retry.rs crates/adapter/net/http/api/src/trace.rs +git commit -m "feat(net): Retry emits per-attempt tracing events + ambient attempt count" +``` + +--- + +## Task 3: ADR amendment, CHANGELOG, full gate, PR + +**Files:** +- Modify: `docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md` +- Modify: `CHANGELOG.md` + +- [ ] **Step 1: ADR-0034 append-only amendment** + +Append to ADR-0034's **Amendments (2026-07-04)** numbered list, after item **8** (the Retry note), a new item **9**: + +```markdown +9. **`Tracing` layer (Slice 1 PR 5).** The outermost `Tracing` layer + + `TracingLayer` factory (ADR-0031 §6) open one `info` span per logical request + and attach it to the inner future via `tracing::Instrument`, so downstream events + — including `Retry`'s per-attempt events — nest under it. The span records method, + `route` (`uri().path()` — the **query is dropped**, since it can carry tokens), + `status` **xor** `ErrorKind` (a `_`-arm label over the `#[non_exhaustive]` enum), + `latency_us` (via `Timer::now()` deltas — the layer is `Timer`-generic), and + `attempts`. Routed to the ADR-0014 Telemetry plane (machinery metrics, lossy, never + canonical). **Secret-safe by construction:** the layer reads only method, path, + status, `ErrorKind`, and the clock — never headers, never the body. Body-transparent + (`http::Response` untouched, no `B: Send` bound — nothing of type `B` crosses the + single await). **Composition contract:** `Tracing` owns the one per-request span; + inner resilience layers emit `tracing` **events**, never open their own span — which + keeps `Span::current()` at any inner depth resolved to `http.request`, so `Retry`'s + `Span::current().record("attempts", n)` populates the field (a graceful no-op when no + such span/field is active). Adds the `tracing` facade (runtime dep, zero executor) + + `tracing-subscriber` (dev-dep). The module is named `trace` to avoid shadowing the + `tracing` crate; the public types are `Tracing`/`TracingLayer`. +``` + +> **Numbering caveat:** the CircuitBreaker PR is in flight concurrently and also appends an amendment. If it merged first and took **#9**, renumber this one to **#10** during rebase (a mechanical fix, not a design change). + +- [ ] **Step 2: CHANGELOG** + +Add to `CHANGELOG.md` `[Unreleased] → Added`, at the **end of the list** (after the `Retry` resilience-layer entry): + +```markdown +- `oath-adapter-net-http-api` `Tracing` resilience layer (Slice 1 PR 5) — the outermost + `Tracing` service + `TracingLayer` factory (`net-api::Layer`): one `info` span + per logical request (method, route, status, `ErrorKind`, latency, attempts), attached to + the inner future via `tracing::Instrument` so downstream events — including `Retry`'s new + per-attempt events — nest under it. Latency via `net-api::Timer` deltas; secret-safe by + construction (reads only method, `uri().path()` with the query dropped, status, + `ErrorKind`, and the clock — never headers or bodies); body-transparent. `Retry` now emits + `debug` per-attempt/backoff events and records the final attempt count onto the ambient + span (a no-op without a `Tracing` span). Routed to the ADR-0014 Telemetry plane. Adds the + `tracing` facade (runtime dep) + `tracing-subscriber` (dev-dep). (ADR-0031 §6, ADR-0014, + ADR-0034.) +``` + +- [ ] **Step 3: Full local gate** + +Run: `just ci` +Expected: green — fmt, lint, test + doctests, doc, deny, typos, machete. `deny`/`machete` now see the new `tracing` (used by `trace.rs`/`retry.rs`) and `tracing-subscriber` (used by the test harness), so neither is flagged unused; `deny` must accept `tracing-subscriber`'s (small, `registry`-only) subtree. + +> If `deny` rejects a license/advisory in `tracing-subscriber`'s tree, the offending crate is named in its output; the most likely case is a new transitive dep needing an allow entry in `deny.toml` — add it with a one-line justification (do not broaden the policy). + +- [ ] **Step 4: Commit, push, PR** + +```bash +git add docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md CHANGELOG.md +git commit -m "docs(net): record Tracing layer amendment (ADR-0034) + changelog" +git push -u origin feat/net-http-tracing +gh pr create \ + --title "feat(net): Tracing resilience layer (Slice 1, PR 5)" \ + --body "Closes # + +Slice 1 **PR 5** of the net-http resilience layers (spec: docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md; ADR-0031 §6, ADR-0014). The outermost layer; built concurrently with the CircuitBreaker PR (independent files). + +- **\`Tracing\`** + **\`TracingLayer\`** (\`net-api::Layer\`) — one \`info\` span per logical request (method, route, status, \`ErrorKind\`, latency, attempts), attached to the inner future via \`tracing::Instrument\` so downstream events (incl. \`Retry\`'s per-attempt) nest under it. Latency via \`net-api::Timer\` deltas. +- **Secret-safe by construction** — reads only method, \`uri().path()\` (query dropped), status, \`ErrorKind\`, and the clock; never headers, never bodies. Body-transparent (\`Response\` untouched). +- **\`Retry\` instrumentation** — \`debug\` per-attempt/backoff events + an ambient \`Span::current().record(\"attempts\", n)\` (a no-op without a \`Tracing\` span). Composition contract: \`Tracing\` owns the one span; inner layers emit events, not spans. +- Routed to the ADR-0014 **Telemetry** plane (machinery metrics, lossy, never canonical). Module named \`trace\` to avoid shadowing the \`tracing\` crate. + +Adds the \`tracing\` facade (runtime dep, zero executor) + \`tracing-subscriber\` (dev-dep). \`MockTimer\`-driven tests with inline service doubles + a capturing subscriber, including the load-bearing secret-safety test. + +Next: **Slice 2** — the \`stack()\`/\`build()\` assembly that wires the default layer order. + +🤖 Generated with [Claude Code](https://claude.com/claude-code)" +``` + +Expected: PR open, GitHub Actions CI green (same `just ci` + MSRV job). + +--- + +## Self-Review + +**Spec coverage (design doc §Scope + Decisions):** +- `Tracing` + `TracingLayer` (`Layer`), infallible `const fn new`, hand-written `Clone`/`Debug` — Task 1 Step 4. ✅ +- One `info_span!("http.request", …)` with the six fields, `Empty`-then-`record` — Task 1 Steps 2/4. ✅ +- `Instrument` attaches the span so downstream events nest — Task 1 Step 4 + Task 2 integration test. ✅ +- Latency via `Timer::now()` deltas, `try_from`/`saturating_duration_since` (no `as`, no panic) — Task 1 Step 4 + `latency_reflects_the_clock_delta_exactly`. ✅ +- Route = `uri().path()`, query dropped; secret-safety structural — Task 1 Step 4 + `never_leaks_authorization_header_or_query_token`. ✅ +- Body-transparent, no `B: Send` — Task 1 `Interfaces` + Step 4 `where`-clause comment + body-transparency assertion. ✅ +- `kind_label` `_`-arm over `#[non_exhaustive]` `ErrorKind` — Task 1 Step 2. ✅ +- Retry per-attempt events + ambient `attempts` record; graceful no-op — Task 2 Steps 3/4. ✅ +- Composition contract (inner layers emit events, not spans) — recorded in the ADR amendment (Task 3 Step 1) + relied on by the integration test. ✅ +- `tracing` runtime dep + `tracing-subscriber` dev-dep (workspace + crate) — Task 1 Step 1. ✅ +- Capturing-subscriber tests, `MockTimer`, inline doubles, no `MockClient` — Tasks 1–2. ✅ +- ADR-0034 Amendment #9 + CHANGELOG — Task 3. ✅ +- Deferred (correctly absent): `RouteLabel` templating, metric aggregation/exporters, `stack()`/`build()`, tokio `Timer`, `TimeoutBody` — noted, not built. ✅ + +**Placeholder scan:** none — every code step carries complete code; every command step carries an expected result. `#` is the real issue number captured in Setup. + +**Type consistency:** +- `TracingLayer::new(timer: T) -> Self`, `.layer(inner) -> Tracing` — match the `Interfaces` block and every test call. +- `Tracing` `Service` impl: inner `Response = http::Response` → `Response = http::Response` (transparent) — matches `OkLeaf`/`ErrLeaf`/`ClockLeaf`/`ScriptLeaf` (`Response = http::Response`, so `B = StubBody`). +- Span field names `method`/`route`/`status`/`error_kind`/`latency_us`/`attempts` — identical in the `info_span!` (Task 1), the `record` calls (Task 1 layer + Task 2 `Retry`), and every test assertion. +- Integers recorded as `u64` throughout (`u64::from(status u16)`, `u64::try_from(micros).unwrap_or(MAX)`, `u64::from(attempt u32)`) — no `as`, lint-clean, and the capture visitor renders them quote-free. +- `RetryConfig { max_attempts, base, cap, seed }` in the Task 2 test matches the shipped `#82` struct (NonZeroU32 + two `Duration`s + `u64`), and `Retryable` is the shipped opt-in marker. +- `capture() -> (Arc>, DefaultGuard)` — same signature used by all five tests. +- lib.rs re-export is `pub use trace::{Tracing, TracingLayer};` (module `trace`, types `Tracing`/`TracingLayer`) — consistent across File Structure, Task 1, and the public interface. + +**Known risks to watch during impl:** listed inline in Task 1 Step 5 (module name, `tracing-subscriber` feature, `S: Sync`, `Empty` fields) and Task 3 Step 3 (`deny` on the new subtree). From 7ed8e644a6a55f4cac1cbe6dbcfc912909decf47 Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 20:03:01 +0000 Subject: [PATCH 3/7] =?UTF-8?q?feat(net):=20Tracing=20layer=20=E2=80=94=20?= =?UTF-8?q?per-request=20span,=20secret-safe,=20Timer=20latency?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Cargo.lock | 68 ++++ Cargo.toml | 3 + crates/adapter/net/http/api/Cargo.toml | 2 + crates/adapter/net/http/api/src/lib.rs | 4 + crates/adapter/net/http/api/src/trace.rs | 430 +++++++++++++++++++++++ 5 files changed, 507 insertions(+) create mode 100644 crates/adapter/net/http/api/src/trace.rs diff --git a/Cargo.lock b/Cargo.lock index e3936aa..98394b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -223,6 +223,12 @@ version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" +[[package]] +name = "lazy_static" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" + [[package]] name = "libc" version = "0.2.186" @@ -297,6 +303,8 @@ dependencies = [ "pin-project-lite", "thiserror", "tokio", + "tracing", + "tracing-subscriber", ] [[package]] @@ -698,6 +706,15 @@ dependencies = [ "zmij", ] +[[package]] +name = "sharded-slab" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f40ca3c46823713e0d4209592e8d6e826aa57e928f09752619fc696c499637f6" +dependencies = [ + "lazy_static", +] + [[package]] name = "signal-hook-registry" version = "1.4.8" @@ -774,6 +791,15 @@ dependencies = [ "syn", ] +[[package]] +name = "thread_local" +version = "1.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f60246a4944f24f6e018aa17cdeffb7818b76356965d03b07d6a9886e8962185" +dependencies = [ + "cfg-if", +] + [[package]] name = "tokio" version = "1.52.3" @@ -802,6 +828,48 @@ dependencies = [ "syn", ] +[[package]] +name = "tracing" +version = "0.1.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63e71662fa4b2a2c3a26f570f037eb95bb1f85397f3cd8076caed2f026a6d100" +dependencies = [ + "pin-project-lite", + "tracing-attributes", + "tracing-core", +] + +[[package]] +name = "tracing-attributes" +version = "0.1.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "tracing-core" +version = "0.1.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "db97caf9d906fbde555dd62fa95ddba9eecfd14cb388e4f491a66d74cd5fb79a" +dependencies = [ + "once_cell", +] + +[[package]] +name = "tracing-subscriber" +version = "0.3.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb7f578e5945fb242538965c2d0b04418d38ec25c79d160cd279bf0731c8d319" +dependencies = [ + "sharded-slab", + "thread_local", + "tracing-core", +] + [[package]] name = "unarray" version = "0.1.4" diff --git a/Cargo.toml b/Cargo.toml index 8651497..732e96c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,6 +80,9 @@ serde = { version = "1", features = ["derive"] } thiserror = "2" tokio = { version = "1", features = ["full"] } tracing = "0.1" +tracing-subscriber = { version = "0.3", default-features = false, features = [ + "registry", +] } proptest = "1" serde_json = "1" diff --git a/crates/adapter/net/http/api/Cargo.toml b/crates/adapter/net/http/api/Cargo.toml index f6006fd..1cd8119 100644 --- a/crates/adapter/net/http/api/Cargo.toml +++ b/crates/adapter/net/http/api/Cargo.toml @@ -15,10 +15,12 @@ futures-util = { workspace = true } http-body = { workspace = true } http-body-util = { workspace = true } pin-project-lite = { workspace = true } +tracing = { workspace = true } [dev-dependencies] tokio = { workspace = true } oath-adapter-net-mock = { workspace = true } +tracing-subscriber = { workspace = true } [lints] workspace = true diff --git a/crates/adapter/net/http/api/src/lib.rs b/crates/adapter/net/http/api/src/lib.rs index 203793e..bc1ca2b 100644 --- a/crates/adapter/net/http/api/src/lib.rs +++ b/crates/adapter/net/http/api/src/lib.rs @@ -16,6 +16,8 @@ //! `Retryable`/`RetryConfig` retry directive + schedule //! - [`timeout`] — the `Timeout` layer, its `TimeoutLayer` factory, and the //! `RequestTimeout` per-request override +//! - [`trace`] — the `Tracing` layer and its `TracingLayer` factory (outermost; +//! one span per request, secret-safe, routed to the ADR-0014 Telemetry plane) //! //! The resilience layers, `stack`/`build` assembly, and backends land in later //! slices. No async runtime, `hyper`, `reqwest`, or `serde` here. @@ -30,6 +32,7 @@ pub mod rate_limit; pub mod retry; pub mod service; pub mod timeout; +pub mod trace; pub use auth::{Auth, AuthSource, NoAuth, SetHeaders}; pub use body::{BufferMode, Guarded, ResponseBody}; @@ -43,3 +46,4 @@ pub use rate_limit::{RateLimit, RateLimitLayer, RateScope, Scope}; pub use retry::{Retry, RetryConfig, RetryLayer, Retryable}; pub use service::Service; pub use timeout::{RequestTimeout, Timeout, TimeoutLayer}; +pub use trace::{Tracing, TracingLayer}; diff --git a/crates/adapter/net/http/api/src/trace.rs b/crates/adapter/net/http/api/src/trace.rs new file mode 100644 index 0000000..7108980 --- /dev/null +++ b/crates/adapter/net/http/api/src/trace.rs @@ -0,0 +1,430 @@ +//! The `Tracing` resilience layer (ADR-0031 §6) — the outermost layer. +//! +//! Opens one `tracing` span per logical request and attaches it to the inner +//! future via [`Instrument`], so every event the inner +//! stack emits — including [`Retry`](crate::Retry)'s per-attempt events — nests +//! under it. The span records method, route (path only — the query is dropped), +//! status **xor** [`ErrorKind`], latency, and +//! (via `Retry`) attempt count — the ADR-0014 Telemetry plane. **Secret-safe by +//! construction:** it reads only method, path, status, `ErrorKind`, and the +//! clock — never headers, never the body. **Body-transparent:** the response is +//! returned untouched. Runtime-neutral: latency via +//! [`Timer::now`], on the zero-runtime +//! `tracing` facade. The module is named `trace` (not `tracing`) to avoid +//! shadowing the `tracing` crate; the public types are `Tracing`/`TracingLayer`. + +use crate::{HttpError, Service}; +use bytes::Bytes; +use oath_adapter_net_api::{ErrorKind, HasErrorKind, Layer, Timer}; +use std::fmt; +use std::future::Future; +use tracing::Instrument; +use tracing::field::Empty; + +/// The stable telemetry label for an [`ErrorKind`] — a low-cardinality +/// `&'static str` for the span's `error_kind` field. +/// +/// The `_` arm covers the `#[non_exhaustive]` enum, so a new variant (e.g. a +/// future `CircuitBreaker` classification added by the concurrent PR) compiles +/// without touching this layer. +const fn kind_label(kind: ErrorKind) -> &'static str { + match kind { + ErrorKind::Timeout => "timeout", + ErrorKind::Connection => "connection", + ErrorKind::Throttled => "throttled", + ErrorKind::Auth => "auth", + ErrorKind::Client => "client", + ErrorKind::Server => "server", + _ => "unknown", // ErrorKind::Unknown and any future non_exhaustive variant + } +} + +/// The `Tracing` [`Layer`] factory: holds the [`Timer`] clock (for latency) and +/// produces a [`Tracing`] around any inner service. +pub struct TracingLayer { + timer: T, +} + +impl TracingLayer { + /// Build the layer with a [`Timer`] clock. Infallible — no config to check. + #[must_use] + pub const fn new(timer: T) -> Self { + Self { timer } + } +} + +impl Clone for TracingLayer { + fn clone(&self) -> Self { + Self { + timer: self.timer.clone(), + } + } +} + +impl fmt::Debug for TracingLayer { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("TracingLayer").finish_non_exhaustive() + } +} + +impl Layer for TracingLayer { + type Service = Tracing; + + fn layer(&self, inner: S) -> Tracing { + Tracing { + inner, + timer: self.timer.clone(), + } + } +} + +/// The `Tracing` middleware: opens one span per request and records the outcome. +/// +/// Body-transparent — the inner `http::Response` is returned untouched. +pub struct Tracing { + inner: S, + timer: T, +} + +impl Clone for Tracing { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + timer: self.timer.clone(), + } + } +} + +impl fmt::Debug for Tracing { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Tracing").finish_non_exhaustive() + } +} + +impl Service> for Tracing +where + S: Service, Response = http::Response, Error = HttpError> + Sync, + T: Timer, + // No `B: Send`: the sole await is the inner call; `record()` is synchronous, + // so no value of type `B` ever crosses a yield point (contrast `Retry`). +{ + type Response = http::Response; + type Error = HttpError; + + // Not `async fn`: the trait requires the returned future to be `Send`. + #[allow(clippy::manual_async_fn)] + fn call( + &self, + req: http::Request, + ) -> impl Future> + Send { + async move { + // Read method + path up front — path ONLY, never the query, which can + // carry tokens (ADR-0031 §6). `route` is owned so `req` can move on. + let route = req.uri().path().to_owned(); + let span = tracing::info_span!( + "http.request", + method = %req.method(), + route = %route, + status = Empty, + error_kind = Empty, + latency_us = Empty, + attempts = Empty, + ); + let start = self.timer.now(); + // `.instrument` enters the span on every poll of the inner future, so + // every downstream event (incl. Retry's per-attempt) nests under it. + let out = self.inner.call(req).instrument(span.clone()).await; + let elapsed = self.timer.now().saturating_duration_since(start); + span.record( + "latency_us", + u64::try_from(elapsed.as_micros()).unwrap_or(u64::MAX), + ); + match &out { + Ok(resp) => { + span.record("status", u64::from(resp.status().as_u16())); + }, + Err(e) => { + span.record("error_kind", kind_label(e.kind())); + }, + } + out + } + } +} + +#[cfg(test)] +mod tests { + use super::TracingLayer; + use crate::{HttpError, Service}; + use bytes::Bytes; + use http_body::{Body, Frame, SizeHint}; + use http_body_util::BodyExt; + use oath_adapter_net_api::Layer; + use oath_adapter_net_mock::MockTimer; + use std::collections::BTreeMap; + use std::future::Future; + use std::pin::Pin; + use std::sync::{Arc, Mutex}; + use std::task::{Context, Poll}; + use std::time::Duration; + use tracing::field::{Field, Visit}; + use tracing::span::{Attributes, Id, Record}; + use tracing::{Event, Subscriber}; + use tracing_subscriber::layer::{Context as LayerCtx, Layer as SubLayer}; + use tracing_subscriber::prelude::*; + use tracing_subscriber::registry::LookupSpan; + + // ---- capturing subscriber ------------------------------------------------ + // One request per test, so the single span's fields merge into one map. + + #[derive(Default)] + struct Store { + span_fields: BTreeMap, + events: Vec>, + } + impl Store { + // A flat dump of every captured string — for the secret-safety scan. + fn haystack(&self) -> String { + let mut s = String::new(); + for (k, v) in &self.span_fields { + s.push_str(k); + s.push('='); + s.push_str(v); + s.push('\n'); + } + for ev in &self.events { + for (k, v) in ev { + s.push_str(k); + s.push('='); + s.push_str(v); + s.push('\n'); + } + } + s + } + } + + // Renders field values to strings. `record_str` keeps `&str` values quote-free + // (e.g. "connection"); everything else (Display via `%`, ints, the message) + // funnels through `record_debug`, whose Debug-of-format_args is also quote-free. + struct StrVisit<'a>(&'a mut BTreeMap); + impl Visit for StrVisit<'_> { + fn record_str(&mut self, field: &Field, value: &str) { + self.0.insert(field.name().to_owned(), value.to_owned()); + } + fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { + self.0.insert(field.name().to_owned(), format!("{value:?}")); + } + } + + #[derive(Clone, Default)] + struct Capture { + store: Arc>, + } + impl LookupSpan<'a>> SubLayer for Capture { + fn on_new_span(&self, attrs: &Attributes<'_>, _id: &Id, _ctx: LayerCtx<'_, S>) { + let mut store = self.store.lock().unwrap(); + let mut fields = std::mem::take(&mut store.span_fields); + attrs.record(&mut StrVisit(&mut fields)); + store.span_fields = fields; + } + fn on_record(&self, _id: &Id, values: &Record<'_>, _ctx: LayerCtx<'_, S>) { + let mut store = self.store.lock().unwrap(); + let mut fields = std::mem::take(&mut store.span_fields); + values.record(&mut StrVisit(&mut fields)); + store.span_fields = fields; + } + fn on_event(&self, event: &Event<'_>, _ctx: LayerCtx<'_, S>) { + let mut fields = BTreeMap::new(); + event.record(&mut StrVisit(&mut fields)); + self.store.lock().unwrap().events.push(fields); + } + } + + // Install a fresh Capture as the thread-local default; return its store + the + // RAII guard. `#[tokio::test]` is current-thread, so every `.await` below runs + // on this thread and the `Instrument` context resolves to this subscriber. + fn capture() -> (Arc>, tracing::subscriber::DefaultGuard) { + let cap = Capture::default(); + let store = cap.store.clone(); + let guard = tracing::subscriber::set_default(tracing_subscriber::registry().with(cap)); + (store, guard) + } + + // ---- inline leaves (no MockClient — dev-dep cycle) ----------------------- + + #[derive(Debug)] + struct StubBody { + data: Option, + } + impl StubBody { + fn new(b: &'static [u8]) -> Self { + Self { + data: Some(Bytes::from_static(b)), + } + } + } + impl Body for StubBody { + type Data = Bytes; + type Error = HttpError; + fn poll_frame( + self: Pin<&mut Self>, + _: &mut Context<'_>, + ) -> Poll, HttpError>>> { + Poll::Ready(self.get_mut().data.take().map(|d| Ok(Frame::data(d)))) + } + fn is_end_stream(&self) -> bool { + self.data.is_none() + } + fn size_hint(&self) -> SizeHint { + self.data.as_ref().map_or_else( + || SizeHint::with_exact(0), + |d| SizeHint::with_exact(d.len() as u64), + ) + } + } + + // 200 immediately. + #[derive(Clone)] + struct OkLeaf; + impl Service> for OkLeaf { + type Response = http::Response; + type Error = HttpError; + #[allow(clippy::manual_async_fn)] + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + async move { Ok(http::Response::new(StubBody::new(b"ok"))) } + } + } + + // Connection error immediately. + #[derive(Clone)] + struct ErrLeaf; + impl Service> for ErrLeaf { + type Response = http::Response; + type Error = HttpError; + #[allow(clippy::manual_async_fn)] + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + async move { Err(HttpError::connection("reset")) } + } + } + + // Advances the shared clock by `elapsed` (synchronously — MockTimer uses + // interior mutability) before returning 200, giving the layer a deterministic + // nonzero latency to record without spawning. + #[derive(Clone)] + struct ClockLeaf { + timer: MockTimer, + elapsed: Duration, + } + impl Service> for ClockLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + let timer = self.timer.clone(); + let elapsed = self.elapsed; + async move { + timer.advance(elapsed); + Ok(http::Response::new(StubBody::new(b"ok"))) + } + } + } + + fn get(uri: &str) -> http::Request { + http::Request::builder() + .method("GET") + .uri(uri) + .body(Bytes::new()) + .unwrap() + } + + #[tokio::test] + async fn records_method_route_status_and_body_is_transparent() { + let (store, _guard) = capture(); + let svc = TracingLayer::new(MockTimer::new()).layer(OkLeaf); + let resp = svc.call(get("/iserver/accounts")).await.expect("ok"); + let body = resp.into_body().collect().await.unwrap().to_bytes(); + assert_eq!(body, Bytes::from_static(b"ok")); // Response passed straight through + let store = store.lock().unwrap(); + assert_eq!( + store.span_fields.get("method").map(String::as_str), + Some("GET") + ); + assert_eq!( + store.span_fields.get("route").map(String::as_str), + Some("/iserver/accounts") + ); + assert_eq!( + store.span_fields.get("status").map(String::as_str), + Some("200") + ); + drop(store); + } + + #[tokio::test] + async fn records_error_kind_on_failure_and_omits_status() { + let (store, _guard) = capture(); + let svc = TracingLayer::new(MockTimer::new()).layer(ErrLeaf); + let err = svc.call(get("/x")).await.unwrap_err(); + assert!(matches!(err, HttpError::Connection(_))); // returned verbatim, not swallowed + let store = store.lock().unwrap(); + assert_eq!( + store.span_fields.get("error_kind").map(String::as_str), + Some("connection") + ); + assert!( + !store.span_fields.contains_key("status"), + "no status on the error path" + ); + drop(store); + } + + #[tokio::test] + async fn latency_reflects_the_clock_delta_exactly() { + let (store, _guard) = capture(); + let timer = MockTimer::new(); + let svc = TracingLayer::new(timer.clone()).layer(ClockLeaf { + timer: timer.clone(), + elapsed: Duration::from_millis(50), + }); + svc.call(get("/x")).await.expect("ok"); + let store = store.lock().unwrap(); + assert_eq!( + store.span_fields.get("latency_us").map(String::as_str), + Some("50000") + ); // 50ms + drop(store); + } + + #[tokio::test] + async fn never_leaks_authorization_header_or_query_token() { + let (store, _guard) = capture(); + let svc = TracingLayer::new(MockTimer::new()).layer(OkLeaf); + let mut req = get("/iserver/orders?oauth_token=SUPERSECRET&api_key=SUPERSECRET"); + req.headers_mut().insert( + http::header::AUTHORIZATION, + http::HeaderValue::from_static("Bearer SUPERSECRET"), + ); + svc.call(req).await.expect("ok"); + let store = store.lock().unwrap(); + let hay = store.haystack(); + assert!( + !hay.contains("SUPERSECRET"), + "secret leaked into telemetry:\n{hay}" + ); + // route carries the path only — the query (with its tokens) is dropped. + assert_eq!( + store.span_fields.get("route").map(String::as_str), + Some("/iserver/orders") + ); + drop(store); + } +} From e33027cee7cffb1664acfad5d503adae82370c7f Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 20:23:59 +0000 Subject: [PATCH 4/7] feat(net): Retry emits per-attempt tracing + ambient attempt count --- crates/adapter/net/http/api/src/retry.rs | 38 +++++++++-- crates/adapter/net/http/api/src/trace.rs | 83 ++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 5 deletions(-) diff --git a/crates/adapter/net/http/api/src/retry.rs b/crates/adapter/net/http/api/src/retry.rs index e10304f..fea42cd 100644 --- a/crates/adapter/net/http/api/src/retry.rs +++ b/crates/adapter/net/http/api/src/retry.rs @@ -235,12 +235,29 @@ where let eligible = req.extensions().get::().is_some(); let max = self.cfg.max_attempts.get(); let mut attempt: u32 = 1; + // Whole-request clone per attempt: `http::Extensions` requires + // `Clone` on insert, so `Request` is `Clone` (Bytes is a + // cheap refcount bump; the directives ride along). `Auth`/`RateLimit` + // re-run inside this call, so credentials/budget refresh for free. loop { - // Whole-request clone per attempt: `http::Extensions` requires - // `Clone` on insert, so `Request` is `Clone` (Bytes is a - // cheap refcount bump; the directives ride along). `Auth`/`RateLimit` - // re-run inside this call, so credentials/budget refresh for free. + // Whole-request clone per attempt (see the existing note above this loop). let outcome = self.inner.call(req.clone()).await; + // Per-attempt telemetry — nests under Tracing's span when present, + // a no-op otherwise (ADR-0031 §6). `debug`: drill-down, pay-per-use. + match &outcome { + Ok(resp) => tracing::event!( + tracing::Level::DEBUG, + attempt = u64::from(attempt), + status = u64::from(resp.status().as_u16()), + "http.attempt" + ), + Err(e) => tracing::event!( + tracing::Level::DEBUG, + attempt = u64::from(attempt), + error_kind = ?e.kind(), + "http.attempt" + ), + } let retry = eligible && attempt < max && match &outcome { @@ -248,11 +265,22 @@ where Ok(resp) => resp.status().is_server_error(), // 5xx only; 429 is 4xx }; if !retry { + // Record the final attempt count onto the current span — the + // "http.request" span when composed under Tracing; a no-op + // otherwise (no active span / the field is absent). + tracing::Span::current().record("attempts", u64::from(attempt)); return outcome; // success, non-retryable outcome, or attempts exhausted } drop(outcome); // release the prior response's Guarded permit before waiting let ceil = backoff_ceiling(self.cfg.base, self.cfg.cap, attempt); - self.timer.sleep(self.rng.duration_in(ceil)).await; + let delay = self.rng.duration_in(ceil); + tracing::event!( + tracing::Level::DEBUG, + attempt = u64::from(attempt), + backoff_us = u64::try_from(delay.as_micros()).unwrap_or(u64::MAX), + "http.retry.backoff" + ); + self.timer.sleep(delay).await; attempt += 1; } } diff --git a/crates/adapter/net/http/api/src/trace.rs b/crates/adapter/net/http/api/src/trace.rs index 7108980..0062fd7 100644 --- a/crates/adapter/net/http/api/src/trace.rs +++ b/crates/adapter/net/http/api/src/trace.rs @@ -164,6 +164,7 @@ mod tests { use std::collections::BTreeMap; use std::future::Future; use std::pin::Pin; + use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use std::task::{Context, Poll}; use std::time::Duration; @@ -346,6 +347,49 @@ mod tests { .unwrap() } + // A leaf yielding one scripted status per call; repeats the last once exhausted. + #[derive(Clone, Copy)] + enum Step { + Status(u16), + } + #[derive(Clone)] + struct ScriptLeaf { + steps: Arc>, + calls: Arc, + } + impl ScriptLeaf { + fn new(steps: Vec) -> Self { + Self { + steps: Arc::new(steps), + calls: Arc::new(AtomicUsize::new(0)), + } + } + } + impl Service> for ScriptLeaf { + type Response = http::Response; + type Error = HttpError; + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + let i = self.calls.fetch_add(1, Ordering::Relaxed); + let step = self + .steps + .get(i) + .copied() + .unwrap_or_else(|| *self.steps.last().unwrap()); + async move { + match step { + Step::Status(code) => { + let mut resp = http::Response::new(StubBody::new(b"body")); + *resp.status_mut() = http::StatusCode::from_u16(code).unwrap(); + Ok(resp) + }, + } + } + } + } + #[tokio::test] async fn records_method_route_status_and_body_is_transparent() { let (store, _guard) = capture(); @@ -427,4 +471,43 @@ mod tests { ); drop(store); } + + #[tokio::test] + async fn retry_populates_attempt_count_and_nests_per_attempt_events() { + use crate::{RetryConfig, RetryLayer, Retryable}; + use std::num::NonZeroU32; + + let (store, _guard) = capture(); + // Zero backoff → the retry loop runs inline: MockTimer `sleep(0)` is Ready, + // so no spawn/advance is needed to drain the backoff between attempts. + let cfg = RetryConfig { + max_attempts: NonZeroU32::new(3).unwrap(), + base: Duration::ZERO, + cap: Duration::ZERO, + seed: 1, + }; + let leaf = ScriptLeaf::new(vec![Step::Status(503), Step::Status(200)]); + let svc = TracingLayer::new(MockTimer::new()) + .layer(RetryLayer::new(cfg, MockTimer::new()).layer(leaf)); + let mut req = get("/iserver/orders"); + req.extensions_mut().insert(Retryable); + + let resp = svc.call(req).await.expect("503 retried → 200"); + assert_eq!(resp.status(), http::StatusCode::OK); + + let store = store.lock().unwrap(); + // The ambient record from inside Retry lands on the outer "http.request" span. + assert_eq!( + store.span_fields.get("attempts").map(String::as_str), + Some("2") + ); + // Two sends → two nested http.attempt events (message field = "http.attempt"). + let attempts = store + .events + .iter() + .filter(|e| e.get("message").map(String::as_str) == Some("http.attempt")) + .count(); + assert_eq!(attempts, 2, "one http.attempt event per send"); + drop(store); + } } From f4e5259495e12006cc80a0ef5997357c2bba8f42 Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 20:39:40 +0000 Subject: [PATCH 5/7] docs(net): record Tracing layer amendment (ADR-0034) + changelog --- CHANGELOG.md | 11 +++++++++++ ...tion-surface-auth-guarded-boot-coverage.md | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7d3ce2..e5b7d07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 exhaustion; body-transparent (drops a superseded response, releasing its `Guarded` permit). Adds the `Retryable` marker + `RetryConfig` schedule; jitter via an internal seeded `SplitMix64` — no new dependency. (ADR-0031 §2, ADR-0034.) +- `oath-adapter-net-http-api` `Tracing` resilience layer (Slice 1 PR 5) — the outermost + `Tracing` service + `TracingLayer` factory (`net-api::Layer`): one `info` span + per logical request (method, route, status, `ErrorKind`, latency, attempts), attached to + the inner future via `tracing::Instrument` so downstream events — including `Retry`'s new + per-attempt events — nest under it. Latency via `net-api::Timer` deltas; secret-safe by + construction (reads only method, `uri().path()` with the query dropped, status, + `ErrorKind`, and the clock — never headers or bodies); body-transparent. `Retry` now emits + `debug` per-attempt/backoff events and records the final attempt count onto the ambient + span (a no-op without a `Tracing` span). Routed to the ADR-0014 Telemetry plane. Adds the + `tracing` facade (runtime dep) + `tracing-subscriber` (dev-dep). (ADR-0031 §6, ADR-0014, + ADR-0034.) - net-http construction-surface design refinements (ADR-0034 append-only Amendments 2026-07-04, spec updated) — an absent `RateLimit` directive now **fails closed** (not "defaults to `Global`"), closing the last silent diff --git a/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md b/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md index 3f36eb9..6549467 100644 --- a/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md +++ b/docs/adr/0034-http-construction-surface-auth-guarded-boot-coverage.md @@ -219,3 +219,22 @@ carries the full reasoning. seeded `SplitMix64` (no `rand` dependency, no injected `Jitter` generic — the RNG is a pure computation); a **total-elapsed retry budget** and **`Retry-After` parsing** are deferred (each an additive follow-up). No new dependency. + +9. **`Tracing` layer (Slice 1 PR 5).** The outermost `Tracing` layer + + `TracingLayer` factory (ADR-0031 §6) open one `info` span per logical request + and attach it to the inner future via `tracing::Instrument`, so downstream events + — including `Retry`'s per-attempt events — nest under it. The span records method, + `route` (`uri().path()` — the **query is dropped**, since it can carry tokens), + `status` **xor** `ErrorKind` (a `_`-arm label over the `#[non_exhaustive]` enum), + `latency_us` (via `Timer::now()` deltas — the layer is `Timer`-generic), and + `attempts`. Routed to the ADR-0014 Telemetry plane (machinery metrics, lossy, never + canonical). **Secret-safe by construction:** the layer reads only method, path, + status, `ErrorKind`, and the clock — never headers, never the body. Body-transparent + (`http::Response` untouched, no `B: Send` bound — nothing of type `B` crosses the + single await). **Composition contract:** `Tracing` owns the one per-request span; + inner resilience layers emit `tracing` **events**, never open their own span — which + keeps `Span::current()` at any inner depth resolved to `http.request`, so `Retry`'s + `Span::current().record("attempts", n)` populates the field (a graceful no-op when no + such span/field is active). Adds the `tracing` facade (runtime dep, zero executor) + + `tracing-subscriber` (dev-dep). The module is named `trace` to avoid shadowing the + `tracing` crate; the public types are `Tracing`/`TracingLayer`. From 7f1b1f406814e9d44969a38137c05468e811c2d9 Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sat, 4 Jul 2026 20:57:41 +0000 Subject: [PATCH 6/7] test(net): guard Tracing secret-safety on the error/retry path --- crates/adapter/net/http/api/src/trace.rs | 66 ++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 5 deletions(-) diff --git a/crates/adapter/net/http/api/src/trace.rs b/crates/adapter/net/http/api/src/trace.rs index 0062fd7..6912004 100644 --- a/crates/adapter/net/http/api/src/trace.rs +++ b/crates/adapter/net/http/api/src/trace.rs @@ -118,13 +118,13 @@ where req: http::Request, ) -> impl Future> + Send { async move { - // Read method + path up front — path ONLY, never the query, which can - // carry tokens (ADR-0031 §6). `route` is owned so `req` can move on. - let route = req.uri().path().to_owned(); + // Record method + path (path ONLY — never the query, which can carry + // tokens, ADR-0031 §6). Both borrow `req` only for the macro, so `req` is + // free to move into the inner call below. let span = tracing::info_span!( "http.request", method = %req.method(), - route = %route, + route = %req.uri().path(), status = Empty, error_kind = Empty, latency_us = Empty, @@ -315,6 +315,22 @@ mod tests { } } + // Errors with a secret embedded in the error MESSAGE, so the test catches a + // regression that records `?e` (the whole HttpError) instead of `?e.kind()`. + #[derive(Clone)] + struct SecretErrLeaf; + impl Service> for SecretErrLeaf { + type Response = http::Response; + type Error = HttpError; + #[allow(clippy::manual_async_fn)] + fn call( + &self, + _req: http::Request, + ) -> impl Future> + Send { + async move { Err(HttpError::connection("secret=LEAK_TOKEN")) } + } + } + // Advances the shared clock by `elapsed` (synchronously — MockTimer uses // interior mutability) before returning 200, giving the layer a deterministic // nonzero latency to record without spawning. @@ -501,7 +517,10 @@ mod tests { store.span_fields.get("attempts").map(String::as_str), Some("2") ); - // Two sends → two nested http.attempt events (message field = "http.attempt"). + // The event count asserts two attempts were emitted (message field = + // "http.attempt"); `on_event` doesn't filter by parent span, so it can't + // prove nesting — the `attempts` span-field assertion above is what proves + // they nested under the outer span. let attempts = store .events .iter() @@ -510,4 +529,41 @@ mod tests { assert_eq!(attempts, 2, "one http.attempt event per send"); drop(store); } + + #[tokio::test] + async fn never_leaks_secret_on_the_error_or_retry_path() { + use crate::{RetryConfig, RetryLayer, Retryable}; + use std::num::NonZeroU32; + let (store, _guard) = capture(); + // Zero backoff → the retry loop runs inline. A Retryable, secret-bearing + // request through an always-Connection-erroring leaf exercises Tracing's + // error branch (error_kind) AND Retry's per-attempt/backoff events. + let cfg = RetryConfig { + max_attempts: NonZeroU32::new(2).unwrap(), + base: Duration::ZERO, + cap: Duration::ZERO, + seed: 1, + }; + let svc = TracingLayer::new(MockTimer::new()) + .layer(RetryLayer::new(cfg, MockTimer::new()).layer(SecretErrLeaf)); + let mut req = get("/iserver/orders?token=LEAK_TOKEN"); + req.headers_mut().insert( + http::header::AUTHORIZATION, + http::HeaderValue::from_static("Bearer LEAK_TOKEN"), + ); + req.extensions_mut().insert(Retryable); + let err = svc.call(req).await.unwrap_err(); + assert!(matches!(err, HttpError::Connection(_))); + let store = store.lock().unwrap(); + let hay = store.haystack(); + assert!( + !hay.contains("LEAK_TOKEN"), + "secret leaked on error/retry path:\n{hay}" + ); + assert_eq!( + store.span_fields.get("error_kind").map(String::as_str), + Some("connection") + ); + drop(store); + } } From 7adfecfaf5f1ad24a9ef481390a5e9c0844d0a21 Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Sun, 5 Jul 2026 07:43:27 +0000 Subject: [PATCH 7/7] fix(net): align Retry error_kind with trace kind_label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address CodeRabbit review on #86: - retry.rs's `http.attempt` event recorded `error_kind` via `?e.kind()` (Debug, e.g. `Connection`) while trace.rs's `http.request` span uses the stable low-cardinality `kind_label` (e.g. `connection`). Same telemetry field, two conventions — split downstream aggregation on the ADR-0014 plane. Expose `kind_label` as `pub(crate)` and reuse it in retry.rs so both surfaces emit identical values. (ErrorKind is a fieldless enum — no payload, so this was consistency-only, not a secret-safety issue.) - Design spec still referenced `tracing.rs`/`pub mod tracing;`; the shipped module is `trace`. Corrected the three stale references. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/adapter/net/http/api/src/retry.rs | 2 +- crates/adapter/net/http/api/src/trace.rs | 2 +- .../specs/2026-07-04-net-http-tracing-layer-design.md | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/adapter/net/http/api/src/retry.rs b/crates/adapter/net/http/api/src/retry.rs index fea42cd..4adf7d4 100644 --- a/crates/adapter/net/http/api/src/retry.rs +++ b/crates/adapter/net/http/api/src/retry.rs @@ -254,7 +254,7 @@ where Err(e) => tracing::event!( tracing::Level::DEBUG, attempt = u64::from(attempt), - error_kind = ?e.kind(), + error_kind = crate::trace::kind_label(e.kind()), "http.attempt" ), } diff --git a/crates/adapter/net/http/api/src/trace.rs b/crates/adapter/net/http/api/src/trace.rs index 6912004..6d399e2 100644 --- a/crates/adapter/net/http/api/src/trace.rs +++ b/crates/adapter/net/http/api/src/trace.rs @@ -27,7 +27,7 @@ use tracing::field::Empty; /// The `_` arm covers the `#[non_exhaustive]` enum, so a new variant (e.g. a /// future `CircuitBreaker` classification added by the concurrent PR) compiles /// without touching this layer. -const fn kind_label(kind: ErrorKind) -> &'static str { +pub(crate) const fn kind_label(kind: ErrorKind) -> &'static str { match kind { ErrorKind::Timeout => "timeout", ErrorKind::Connection => "connection", diff --git a/docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md b/docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md index 66bacb5..9f2e4f7 100644 --- a/docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md +++ b/docs/superpowers/specs/2026-07-04-net-http-tracing-layer-design.md @@ -53,7 +53,7 @@ count observable. ## Scope (in) - The `Tracing` service + `TracingLayer` factory (impl'ing `net-api::Layer`), - in `oath-adapter-net-http-api`, new file `tracing.rs`. + in `oath-adapter-net-http-api`, new file `trace.rs`. - **One span** (`info_span!("http.request", …)`) per `call`, attached to the inner future via `tracing::Instrument` so downstream events nest under it; the deferred fields recorded on completion. @@ -313,7 +313,7 @@ is an **inline** `Service` double (no `MockClient` — cycle). Cases: ## Definition of done - `Tracing` + `TracingLayer` implemented as specified; `retry.rs` gains the - per-attempt events + ambient `attempts` record; `lib.rs` gains `pub mod tracing;` + + per-attempt events + ambient `attempts` record; `lib.rs` gains `pub mod trace;` + re-exports (`Tracing`, `TracingLayer`) + a module-doc bullet; all with the tests above. - `Cargo.toml` (workspace) declares `tracing-subscriber`; net-http-api `Cargo.toml` adds `tracing` (dep) + `tracing-subscriber` (dev-dep). @@ -339,6 +339,6 @@ is an **inline** `Service` double (no `MockClient` — cycle). Cases: terminal `http.retry.exhausted` event. Leaning the plain field + per-attempt `debug` events; a terminal event is additive later if a subscriber wants it. 3. **Capturing subscriber location** — a small reusable capture `Layer` in the test - module of `tracing.rs`, or shared with `retry.rs`'s new attempt-count test via a + module of `trace.rs`, or shared with `retry.rs`'s new attempt-count test via a `#[cfg(test)]` helper. Leaning per-file inline (parity with the inline-double convention) unless duplication bites.