-
Notifications
You must be signed in to change notification settings - Fork 2
runtime: make the log pipeline pluggable through the components seam #442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -8,9 +8,11 @@ | |||||
|
|
||||||
| use crate::addons::{AddOns, PrometheusAddOn}; | ||||||
| use crate::host::component::{ | ||||||
| ComponentBuilder, ComponentsBuilder, LocalStoreBuilder, ProviderPoolBuilder, RuntimeTypes, | ||||||
| ComponentBuilder, ComponentsBuilder, LocalStoreBuilder, LogPipelineBuilder, | ||||||
| ProviderPoolBuilder, RuntimeTypes, | ||||||
| }; | ||||||
| use crate::host::local_store_redb::LocalStore; | ||||||
| use crate::host::logs::LogPipeline; | ||||||
| use crate::host::provider_pool::ProviderPool; | ||||||
|
|
||||||
| /// A bundled runtime assembly: the [`RuntimeTypes`] lattice plus the component | ||||||
|
|
@@ -27,9 +29,16 @@ pub trait Runtime { | |||||
| type StoreBuilder: ComponentBuilder<Output = <Self::Types as RuntimeTypes>::Store>; | ||||||
| /// Builds the extension payload ([`RuntimeTypes::Ext`]). | ||||||
| type ExtBuilder: ComponentBuilder<Output = <Self::Types as RuntimeTypes>::Ext>; | ||||||
| /// Builds the shared [`LogPipeline`]. | ||||||
| type LogsBuilder: ComponentBuilder<Output = LogPipeline>; | ||||||
|
|
||||||
| /// The component builders that open the backends at launch. | ||||||
| fn components() -> ComponentsBuilder<Self::ChainBuilder, Self::StoreBuilder, Self::ExtBuilder>; | ||||||
| fn components() -> ComponentsBuilder< | ||||||
| Self::ChainBuilder, | ||||||
| Self::StoreBuilder, | ||||||
| Self::ExtBuilder, | ||||||
| Self::LogsBuilder, | ||||||
| >; | ||||||
|
|
||||||
| /// The cross-cutting add-ons installed before the engine boots. | ||||||
| fn add_ons() -> AddOns; | ||||||
|
|
@@ -52,6 +61,7 @@ impl Runtime for CoreRuntime { | |||||
| type ChainBuilder = ProviderPoolBuilder; | ||||||
| type StoreBuilder = LocalStoreBuilder; | ||||||
| type ExtBuilder = (); | ||||||
| type LogsBuilder = LogPipelineBuilder; | ||||||
|
|
||||||
| fn components() -> ComponentsBuilder<ProviderPoolBuilder, LocalStoreBuilder, ()> { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 (
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed still valid at HEAD. |
||||||
| ComponentsBuilder::new(ProviderPoolBuilder, LocalStoreBuilder, ()) | ||||||
|
|
||||||
There was a problem hiding this comment.
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::launchcallsR::components().build(...)directly with no hook to override the components builder first, soRuntimeBuilder::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 givingPresetBuilderan escape hatch.There was a problem hiding this comment.
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 callsself.preset.components().build(...)directly with no override hook, so all four seams stay unreachable via the preset shortcut (with_componentsatbuilder.rs:516is on the non-presetTypedBuilderpath only). The frozen PR-body framing is moot post-merge; the design gap is tracked in #509.