Repository navigation
Put an ILogger abstraction over Serilog and unify the two console output paths #428
Description
Description
Refactor Fallout's console/logging output to sit behind a clean abstraction and collapse the two parallel output paths that exist today.
Problem
Serilog is already the core logger — Log.* runs a real Serilog pipeline (console / host-CI / in-memory / file sinks, configured in src/Fallout.Build/Logging.cs::Configure). But all UI/presentation output bypasses Serilog: the logo, the target-outcome table, WriteBlock banners, build success/fail lines, and the "Errors & Warnings" block call static Host.Success/Information/... (src/Fallout.Build/Host.Theming.cs) → IHostTheme.Write* → raw Console.WriteLine(Format(text, ansiCode)) (src/Fallout.Build/Theming/AnsiConsoleHostTheme.cs:106).
The seam joining the two paths is IHostTheme, which does awkward double duty: it implements the direct-write methods and inherits Serilog's ConsoleTheme so it can be cast (ConsoleTheme)Theme and handed to the console sink (Logging.cs:103).
Resulting smells:
Host.WriteErrorsAndWarnings(Host.cs:85) rebuilds the globalLog.Loggerjust to flush in-memory events — there is a literal// TODO: move to Logging.- Direct-write UI lines do not reach the file/CI sinks and are not subject to the level switch / verbosity, unlike
Log.*output — inconsistent behaviour for "the same" output. - Serilog types leak into the public API:
Logging.LevelSwitch(publicLoggingLevelSwitch), theConsoleThemebase classes on the public theme types, andLogEventLevelin the publicLogLevelPattern/DefaultLogLevelattributes (src/Fallout.Tooling/ToolTasks.Logger.cs).
Colour requirement (resolved)
Colourful console output is a hard requirement. Findings:
ILoggerloses no colour — colour is a provider/formatter concern, not anILogger-API concern.- Serilog's console sink already does rich per-token colour (ANSI/256/truecolor) but not layout (tables/boxes/progress) — which is exactly why the rich UI bypasses it today.
- A custom
ILoggerProviderwas rejected — zero colour benefit, and it would reimplement everything Serilog already gives us (level switch, in-memory capture, CI-annotation sink, file sinks + rolling cleanup, enrichers, filters). - Spectre.Console (already a pinned dependency, used by the CLI prompts) is the right tool for the rich-UI part.
Target architecture (hybrid)
- Logging — code against
Microsoft.Extensions.Logging.ILogger, keeping Serilog as the provider behind it viaSerilog.Extensions.Logging(SerilogLoggerFactory). The existing Serilog pipeline is unchanged. - Theming — keep an
IHostTheme-style interface but decouple it from Serilog (stop inheritingConsoleTheme); add an internalSerilogConsoleThemeAdapter : ConsoleThemethat wraps the theme's colour data for the sink. - Rich UI — move the logo / outcome table / banners to Spectre.Console behind a new internal
IHostOutputpresenter, eliminating the direct-Console.WriteLinebypass and fixingWriteErrorsAndWarnings.
Key decisions
- Façade-over-DI, not full constructor injection. The
IFalloutCommand+ MS.DI container introduced in #d0452cae lives only inFallout.Cli— the build engine (BuildManager.Execute<T>) is fully static (new T(), staticLog/Host.Instance, ~50 call sites). So: introduce a composition root insideBuildManager.Execute<T>(a per-runServiceCollection+ a sharedAddFalloutLoggingextension, also registered in the CLI container), but expose the resolved logger / theme / output through the existing static façades so the ~50 call sites are unchanged. - Adopt Spectre.Console now (not deferred) for the rich-UI rendering.
Additive-now vs deferred-breaking
Per the backwards-compatibility rules in AGENTS.md (#1/#2), removing the public Serilog types is a breaking change and must batch to the next yearly major on experimental.
Additive now (main, target/2026):
- ILogger seam + composition root +
AddFalloutLogging; log façade re-pointed (SerilogLog.*keeps working). - Decoupled
IHostThemecolour model +SerilogConsoleThemeAdapter(keep the oldConsoleThemebase classes in place so the cast still compiles). - Spectre
IHostOutputpresenter; fixWriteErrorsAndWarnings(noLog.Loggerrebuild). [Obsolete]/[Experimental("FALLOUT002")]markers on the Serilog leaks; addFallout.Common.LogLeveloverloads toLogLevelPattern/DefaultLogLevel; register the experimental ID indocs/experimental-apis.md.
Deferred to experimental (breaking, next yearly major):
- Remove
ConsoleThemebase classes from theIHostThemeimpls; removepublic LevelSwitch; removeLogEventLevel/ConsoleThemeStylefrom public signatures. Each gets thebreaking-changelabel +⚠️ Breaking changecallout + CHANGELOG entry under the next major.
Proposed PR sequence
- (main, additive) Packages (
Serilog.Extensions.Logging,Microsoft.Extensions.Logging.Abstractions);AddFalloutLogging+ILoggerFactoryregistration backed by the existing Serilog pipeline; composition root inBuildManager.Execute<T>; static log façade re-pointed. No behaviour change. + bridge tests. - (main, additive) Decoupled
IHostThemecolour model +SerilogConsoleThemeAdapter; rewireLogging.cs:103/Host.cs:96to use the adapter. Adapter parity tests + baseline Verify snapshots. - (main, additive)
IHostOutputSpectre presenter for logo/table/banners/outcome; fixWriteErrorsAndWarnings; SpectrePackageReferenceonFallout.Build; CI host overrides updated. Verify-snapshot review. - (main, additive)
[Obsolete]/[Experimental("FALLOUT002")]markers + newLogLevelattribute overloads; registerFALLOUT002indocs/experimental-apis.md; wire the CLI container to the same registrations. - (experimental, breaking — next major) Remove
ConsoleThemebase classes,public LevelSwitch, and allLogEventLevel/ConsoleThemeStylefrom public signatures.
Packages (central — Directory.Packages.props)
- New:
Serilog.Extensions.Logging,Microsoft.Extensions.Logging.Abstractions. - Already present (add refs to
Fallout.Build):Microsoft.Extensions.DependencyInjection,Spectre.Console.
Test strategy
- Verify snapshots are the regression contract for every output-format change (logo / table / banner) in ANSI, plain, and redirected modes.
SerilogConsoleThemeAdapterparity tests (same ANSI codes per level/token as today).ILogger→ Serilog bridge tests (correctLogEventLevelmapping).LogLevelPattern/DefaultLogLeveloverload parity tests.WriteErrorsAndWarningstest asserting noLog.Loggerreset.
Risks / open questions
- The composition root in
BuildManager.Execute<T>is new architecture, not just rewiring. - Spectre truecolor downgrade for the Fallout-yellow logo (
#F5C800,Host.cs:36) must match the current "older sinks ignore the escape bytes" intent. applyThemeToRedirectedOutput: true(Logging.cs:104,Host.cs:97) must be preserved — CI logs rely on it.- 7 CI
*.Theming.csoverrides (esp. GitHub Actions::-prefixed annotations) must not be stranded by theIHostOutputsplit. InMemorySinkis a process-wide singleton (Logging.cs:214) — confirm lifetime under per-run providers so the end-of-build summary still captures everything.- Confirm
FALLOUT002is the next free experimental diagnostic ID before allocating.
Usage Example
No change to the public logging surface in the additive phases — existing Log.Information(...) and the colored build UI keep working. Internally, components resolve logging through the abstraction instead of touching Serilog directly:
// Composition root (BuildManager.Execute<T>) — wired once, façades fed from it
services.AddFalloutLogging(build); // registers ILoggerFactory (Serilog provider), IHostTheme, IHostOutput
// Theme decoupled from Serilog; sink still gets colour via an adapter
configuration.WriteTo.Console(theme: new SerilogConsoleThemeAdapter(hostTheme), ...);
// Rich UI renders through the presenter instead of raw Console.WriteLine
hostOutput.WriteTargetOutcome(build); // Spectre table, not hand-rolled PadRight + ANSIAlternative
- Status quo (the hybrid bypass): keep two output paths — inconsistent sink coverage, the
Log.Logger-rebuild hack, and the dual-hatIHostTheme/ConsoleTheme. - Push everything through a Serilog sink: fights the log pipeline for non-log UI (tables/banners) and deepens the public Serilog coupling that we want to be able to unwind.
- Write a fully custom
ILoggerProvider: rejected — reimplements Serilog's sink fan-out / enrichment / capture for no colour benefit.
Activity
phmatray commentedon Aug 10, 2026
Implementation plan — PR 1 (ILogger seam)
Scoping this to PR 1 of the 5-PR sequence in the issue body. Nothing else (theme decoupling, Spectre presenter,
[Obsolete]/[Experimental]markers, breaking removals) is in scope here.Outcome: framework code can obtain an
ILoggerfrom a per-run DI container backed by the existing, unchanged Serilog pipeline. Zero behaviour change, zero public-API change.Design decisions
SerilogLoggerFactorydirectly, notservices.AddLogging(...).AddLogginginstalls MEL'sLoggerFilterOptions, default minimumInformation— that would silently swallowTrace/Debugand create a second level authority competing withLogging.LevelSwitch. A bareSerilogLoggerFactorykeeps the Serilog level switch the single authority.SerilogLoggerFactory(logger: null, dispose: false)— null means "resolveSerilog.Log.Loggerat write time". Required becauseHost.WriteErrorsAndWarnings(Host.cs:103) reassignsLog.Loggermid-run; capturing eagerly would strand the bridge. PR 3 removes that hack; until then the bridge must tolerate it.AddFalloutLogging+ façade accessors stayinternal. RootAssemblyInfo.csalready grantsInternalsVisibleTotoFallout.Cliand every*.Specs, so PR 4's CLI wiring and the tests reach it. Keeps PR 1 a true no-op on public surface (rule Enterprise hardening: repo nuget.config breaks corporate packageSourceMapping setups #2 / "internal foundation").Log.*call sites and the staticBuildManager.Execute<T>are untouched.Changes
Directory.Packages.props+ Serilog.Extensions.Logging,+ Microsoft.Extensions.Logging.Abstractionssrc/Fallout.Build/Fallout.Build.csprojPackageReferences:Microsoft.Extensions.DependencyInjection,Microsoft.Extensions.Logging.Abstractions,Serilog.Extensions.Loggingsrc/Fallout.Build/Logging.DependencyInjection.cs(new)internal static IServiceCollection AddFalloutLogging(this IServiceCollection, IFalloutBuild = null)— callsLogging.Configure(build), registersILoggerFactory/ILogger<>/ILoggersrc/Fallout.Build/Logging.csinternal ILoggerFactory Factory(lazy fallback over the ambientLog.Logger),internal ILogger Logger,internal IDisposable UseLoggerFactory(...)bracketsrc/Fallout.Build/Execution/BuildManager.csServiceCollectioncomposition root replaces the bareLogging.Configure(build); provider disposed infinallyso it outlivesFinish(), constructed insidetryso config failures still returnErrorExitCodetests/Fallout.Build.Specs/LoggerBridgeSpecs.cs(new)The lazy fallback in
Logging.Factorymatters:Log.*is used well outside aBuildManagerrun (the CLI commands callLogging.Configure()directly), so the façade must work with no composition root active.Test coverage
Trace→Verbose,Debug→Debug,Information→Information,Warning→Warning,Error→Error,Critical→FatalTrace/Debugare not swallowed — regression guard for decision 1Logging.Levelstill governs what reaches sinks through the bridgeHello {Name}stays a template with aNameproperty)LogEvent.ExceptionLog.Logger— decision 2AddFalloutLoggingresolvesILoggerFactory/ILogger<T>/ILogger; façade fallback + restore-on-disposeOpen questions from the issue, answered by this slice
InMemorySinklifetime — untouched here. It stays the process-wide singleton wired byLogging.Configure; the per-run provider owns no sink state, so the end-of-build summary is unaffected.applyThemeToRedirectedOutput/ logo truecolor / CI*.Theming.cs— all PR 2/3 concerns; not touched.FALLOUT002allocation — deferred to PR 4. (FALLOUT001is the only allocated ID indocs/experimental-apis.mdtoday, soFALLOUT002is free.)Verification:
dotnet build fallout.slnxclean, fulldotnet test, plus a dogfood./build.ps1 Compileto confirm console output is unchanged.Split logging registration from pipeline configuration
Put an ILogger seam over the Serilog pipeline