refactor(logs): one log-capture sink, beside the pipeline it reads - #380
Merged
Merged
Conversation
The `io::Write` plus `MakeWriter` pair that reads back what `tracing` emitted was copied into five crates. `LogCapture` and `capture_logs` now live in `nexum-runtime-logs` behind a `testing` feature, beside the pipeline whose output they read, and `nexum-runtime-testing` re-exports them so a composed consumer reaches them under one name. Four copies are gone: the logs crate's own `Console`, the wasm fault funnel's `Sink`, the facade harness's `LogSink`, and the supervisor event loop's `LogSink`. `nexum-tasks` keeps its local copy deliberately: it is layer 0 with a 42-package test graph, and the logs crate normal-depends on wasmtime-wasi, so sharing there would cost 216 packages to save 25 lines. Every existing assertion is unchanged; the capture level each call site used is now an argument rather than a constant baked into its private copy. The shared sink carries three tests of its own: the level ceiling, the absence of ANSI escapes, and the scope of the install guard. `tracing-subscriber` moves from a dev-dependency to an optional dependency on the logs crate, and drops out of the wasm and supervisor dev-dependencies along with the wasm crate's `parking_lot`. Refs #364 AI Assistance: claude-opus-5 used for implementation, verification, and this commit message.
…e install `nexum-runtime-testing` re-exported `LogCapture` and `capture_logs` with no consumer. Reaching the sink through that crate costs alloy, tower and wasmtime in a test build, which is the reach #377 removed for `capture_metrics`; every call site in this change takes `nexum-runtime-logs` directly, so the re-export is a cheap-looking path to an expensive dependency and nothing else. Its `testing` feature on the normal dependency and the description edit go with it. `LogCapture::install` returns a `DefaultGuard`. `tracing::subscriber::set_default` is `#[must_use]`, but that does not carry through the wrapper, so `sink.install(Level::INFO);` as a statement dropped the guard immediately and captured nothing while the test still compiled. The attribute now sits on `install`. The comment above the `install` call in `harness.rs` restated the method's own rustdoc and the `expect` string two lines below it. AI Assistance: claude-opus-5 used for red-team review of the branch and these fixes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
One shared
LogCaptureincrates/nexum-runtime-logs/src/capture.rs, behind atestingfeature, replacing four of the five hand-rolled copies of the sameio::WriteplusMakeWriterpair.nexum-runtime-wasm,nexum-runtimeandnexum-runtime-supervisornow dev-depend onnexum-runtime-logswithfeatures = ["testing"]and drop their owntracing-subscriberdev-dependency. The logs crate uses a self dev-dependency to enable the feature for its own test targets, which is how a feature-gated helper serves the crate that hosts it.nexum-taskskeeps its own, deliberately, with a comment saying why.Why
Closes #364
Five copies of the same twenty-five-line sink, arriving one per issue that needed to assert on a log line. Asserting on log lines is how this repository proves an operator can see something, so the shape was reproducing faster than it was being noticed.
Two corrections to the issue
There were five, not six. The issue counted a sixth from #373, which was closed unmerged.
The home was no longer an open question. The issue frames it as a choice between a
publish = falseleaf and a feature-gatednexum-primitives. #377 answered it with a merged precedent: a harness belongs with the subsystem it observes.capture_metricswent tonexum-runtime-metricsbehind atestingfeature, takingcargo nextest run -p nexum-runtime-httpfrom 364 packages back to 214. This follows that shape, and the capture sits beside the pipeline whose output it reads.The consumer the principle does not fit
nexum-tasksdepends on tokio, futures and tracing and nothing else.nexum-runtime-logsdepends onnexum-primitives,nexum-runtime-apiandnexum-runtime-config.So making the leaf take the shared sink would drag api and config into a layer-0 crate's test build, which is exactly the harm #377 was merged to remove, and strictly worse than twenty-five duplicated lines. It keeps its own copy and the comment states the reason, so the next reader sees a decision rather than an oversight.
Five copies become two: one shared, one deliberate.
tracing-subscriber stays out of every non-dev graph
tracing-subscriberisoptional = truein the logs crate, reached only through thetestingfeature.nexum-runtime-testingis an optional non-dev dependency of the supervisor and the runtime behindtest-utils, so a subscriber implementation placed carelessly would reach an embedder that enables it. It does not.nexum-runtimekeeps its owntracing-subscriberfor a separate reason the diff records: theembedexample installs a subscriber, as an embedder does.This is a move
No existing test changed what it asserts. The five sinks were not identical, and the shared type serves the four that wanted the same thing rather than lowering any of them to a common denominator.
Testing
Full content job (
content-lint,zero-leak,msrv-lint,workspace-deps-lint,crate-lints-lint,cargo machete), workspace clippy and rustdoc under-D warnings, and the workspace test suite, all clean.Cargo.lockmoves only the dependency edges the manifests changed.AI Assistance
Implementation: claude-opus-5. Red-team review: claude-opus-5. Verification: claude-opus-5. PR description: claude-opus-5.