Skip to content

runtime: make the log pipeline pluggable through the components seam#442

Merged
mfw78 merged 1 commit into
dev/m1from
feat/m2-log-pipeline-seam
Jul 20, 2026
Merged

runtime: make the log pipeline pluggable through the components seam#442
mfw78 merged 1 commit into
dev/m1from
feat/m2-log-pipeline-seam

Conversation

@mfw78

@mfw78 mfw78 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Closes #265

What

Adds LogPipelineBuilder as a fourth ComponentBuilder slot on ComponentsBuilder<C, S, E, L = LogPipelineBuilder>, defaulting to today's in-memory backend. Runtime::components and the TypedBuilder/ComponentsStage/ReadyBuilder chain thread the new L generic. Adds ComponentsBuilder::with_logs to substitute a custom pipeline builder.

Why

Chain, store, and the lattice Ext are each pluggable through a ComponentBuilder; the log pipeline was hardcoded in ComponentsBuilder::build. This gives it the same seam so an embedder can substitute a custom log sink.

Testing

with_logs_substitutes_the_pipeline unit test asserts the bundle carries the exact pipeline yielded by a custom builder.

AI Assistance

Implemented with Claude Code.

Closes #265

@lgahdl lgahdl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generic threading itself (TypedBuilderComponentsStage<...,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,
>;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, ()> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
fn components() -> ComponentsBuilder<ProviderPoolBuilder, LocalStoreBuilder, ()> {
fn components() -> ComponentsBuilder<ProviderPoolBuilder, LocalStoreBuilder, (), LogPipelineBuilder> {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@mfw78
mfw78 force-pushed the feat/m2-supervised-primitive branch from 0ddfa13 to ac5e01b Compare July 20, 2026 08:10
Base automatically changed from feat/m2-supervised-primitive to dev/m1 July 20, 2026 12:48
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.
@mfw78
mfw78 force-pushed the feat/m2-log-pipeline-seam branch from e305556 to b798540 Compare July 20, 2026 12:50
@mfw78
mfw78 merged commit 381cd8e into dev/m1 Jul 20, 2026
5 of 6 checks passed
@mfw78
mfw78 deleted the feat/m2-log-pipeline-seam branch July 20, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants