runtime: make the log pipeline pluggable through the components seam#442
Conversation
lgahdl
left a comment
There was a problem hiding this comment.
The generic threading itself (TypedBuilder → ComponentsStage<...,L> → ReadyBuilder<...,L>, terminating cleanly at .build() before AssembledRuntime, which stays generic-free) is done correctly and completely — same shape as the supervisor_clock seam already threaded through this same chain. Default-generic call-site compatibility is fine too (ComponentsBuilder::new(c, s, e) still resolves L = LogPipelineBuilder). Two smaller things:
| Self::StoreBuilder, | ||
| Self::ExtBuilder, | ||
| Self::LogsBuilder, | ||
| >; |
There was a problem hiding this comment.
with_logs (like the chain/store/ext seams before it) is unreachable from any preset-based caller — PresetBuilder::launch calls R::components().build(...) directly with no hook to override the components builder first, so RuntimeBuilder::new(cfg).runtime::<CoreRuntime>().launch() can never reach it. This predates the PR (not a regression you introduced), but the PR body's "this gives it the same seam" framing slightly overstates parity, since none of the four seams are actually reachable via the preset shortcut today. Worth a one-line note in the description, or a follow-up giving PresetBuilder an escape hatch.
There was a problem hiding this comment.
Confirmed still valid at HEAD. PresetBuilder::launch (builder.rs:456) still calls self.preset.components().build(...) directly with no override hook, so all four seams stay unreachable via the preset shortcut (with_components at builder.rs:516 is on the non-preset TypedBuilder path only). The frozen PR-body framing is moot post-merge; the design gap is tracked in #509.
| type ExtBuilder = (); | ||
| type LogsBuilder = LogPipelineBuilder; | ||
|
|
||
| fn components() -> ComponentsBuilder<ProviderPoolBuilder, LocalStoreBuilder, ()> { |
There was a problem hiding this comment.
The trait declaration above spells out all four type params, but this impl's signature stayed at 3 (ComponentsBuilder<ProviderPoolBuilder, LocalStoreBuilder, ()>), silently relying on the L = LogPipelineBuilder default to satisfy the trait. It compiles today only because that default happens to match Self::LogsBuilder, but it reads as if this doesn't build a log pipeline at all — and the moment LogsBuilder changes to something else, this becomes a compile error at a line that looks unrelated.
| fn components() -> ComponentsBuilder<ProviderPoolBuilder, LocalStoreBuilder, ()> { | |
| fn components() -> ComponentsBuilder<ProviderPoolBuilder, LocalStoreBuilder, (), LogPipelineBuilder> { |
There was a problem hiding this comment.
Confirmed still valid at HEAD. CoreRuntime::components (preset.rs:94) still returns the 3-param ComponentsBuilder<ProviderPoolBuilder, LocalStoreBuilder, ()>, riding the L = LogPipelineBuilder default; no later car spells out the fourth param. Tracked in #508.
0ddfa13 to
ac5e01b
Compare
ComponentsBuilder grows a fourth logs slot, defaulting to the new LogPipelineBuilder (the in-memory pipeline sized from [limits.logs]); with_logs substitutes a custom builder. The Runtime preset names the slot as LogsBuilder and the launcher cars carry the parameter through.
e305556 to
b798540
Compare
Closes #265
What
Adds
LogPipelineBuilderas a fourthComponentBuilderslot onComponentsBuilder<C, S, E, L = LogPipelineBuilder>, defaulting to today's in-memory backend.Runtime::componentsand theTypedBuilder/ComponentsStage/ReadyBuilderchain thread the newLgeneric. AddsComponentsBuilder::with_logsto substitute a custom pipeline builder.Why
Chain, store, and the lattice
Extare each pluggable through aComponentBuilder; the log pipeline was hardcoded inComponentsBuilder::build. This gives it the same seam so an embedder can substitute a custom log sink.Testing
with_logs_substitutes_the_pipelineunit test asserts the bundle carries the exact pipeline yielded by a custom builder.AI Assistance
Implemented with Claude Code.
Closes #265