feat(asm-runner): use strata-logging from strata-common#108
Conversation
Drop the in-tree tracing-subscriber bootstrap and share the strata-common logging stack used by alpen/strata-signer. This unlocks OTLP traces/metrics, optional rolling file logs, JSON output, and the tracing-to-metrics bridge through a TOML [logging] section, with field-level defaults so existing config.toml files keep working unchanged. Init now runs inside a Tokio runtime context (required by the OTLP exporter) and finalize() flushes on both the graceful and crash shutdown paths.
The custom serde-default and matching manual Default impl were boilerplate that disagreed with the derived Default. Rely on derive(Default) so the empty-section and missing-field paths agree, and let operators populate extra_filter_directives explicitly in TOML when they want them.
Codecov Report❌ Patch coverage is
... and 8 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Commit: 865ea8b
|
Without struct-level `#[serde(default)]`, a `[logging]` section that omits `extra_filter_directives` (a `Vec`, not an `Option`) fails to deserialize with `missing field`, despite the docs promising all fields are optional. Moving the attribute to the struct piggybacks on the derived `Default` and makes every field uniformly skippable, including future non-`Option` additions. Adds deserialization tests covering both a partial `[logging]` section and a config with no `[logging]` table at all.
|
@storopoli also I'm curious to know your thoughts on using owned type as mentioned in |
Oh missed that. Yes let's make it owned it will be very useful for downstream users. |
This is done such that each portion is in charge of it's own config schema and defaults. I understand it's boilerplatey but it gives each binary what it wants to do. For example
Likewise service names, default log file prefixes, and noisy dependency filters are binary policy. The borrowed But i think your point is valid and with careful design of this ownedConfig it should be possible to do such stuff too. we can manage it with optional |
3 participants
Description
Replaces the in-tree
tracing-subscriberbootstrap inbin/asm-runnerwith the sharedstrata-loggingcrate fromstrata-common(the same one already used byalpen/strata-clientandalpen/strata-signer). The runner now picks up OTLP traces/metrics export, rolling file logs, JSON output, and the tracing-to-metrics bridge through a new optional[logging]section inconfig.toml.The init path moves below the Tokio runtime build because
strata-logging's OTLP exporter needs a runtime context, andstrata_logging::finalize()is invoked on both the graceful and crash shutdown branches so any in-flight OTLP spans get flushed.Type of Change
Notes to Reviewers
Today
LoggingInitConfig<'a>is fully borrowed (&'a str,Option<&'a str>,Option<&'a PathBuf>,&'a [&'a str]), which forces every consumer (asm-runner, strata-client, strata-signer, …) to define its own ownedLoggingConfigstruct purely to hold deserialized TOML values and then re-borrow them at the init call-site. I think it would be cleaner ifstrata-loggingshipped an owned, serde-friendly counterpart (e.g.OwnedLoggingConfigwithString/Option<String>/Vec<String>fields, derivingSerialize/Deserialize) plus anas_init_config(&self) -> LoggingInitConfig<'_>method. Consumers could then embed that type directly in their config struct and drop the per-binary boilerplate (theLoggingConfigadded in this PR would go away). Happy to follow up upstream if reviewers agree. CC: @voidashChecklist
Related Issues
None.