Skip to content

Commit 5aa3218

Browse files
Philippe Matrayclaude
authored andcommitted
Put an ILogger seam over the Serilog pipeline
Framework code reaches logging through Serilog's static Log, which pins the project to one logger implementation and leaks Serilog types outward. Introduce Microsoft.Extensions.Logging.ILogger as the abstraction in front of it, keeping Serilog as the provider and the pipeline itself untouched. AddFalloutLogging registers the abstraction over the pipeline and nothing else, so any container can call it any number of times. Configuring the pipeline stays separate because Logging.Configure is not idempotent: it reassigns Serilog's Log.Logger on every call, and with no build it installs a pipeline with no file sinks, no host sink and no filter, wiping out whatever a previous caller had set up. BuildManager.Execute runs Configure explicitly, just before it builds the provider. The registration deliberately avoids services.AddLogging, which would install MEL's own filter pipeline with an Information default -- a second level authority that would drop trace and debug records before Serilog saw them, displacing Logging.LevelSwitch. ILogger<T> and ILogger are transient, not singleton. Logger<T> binds its inner logger in its constructor, so a singleton would pin every consumer to whichever pipeline was current at the first resolution -- the same failure Logging.Logger stays uncached to avoid. ILoggerFactory stays a singleton because it is left unbound and pins nothing. Transient does not rescue a component that holds a logger across a pipeline swap, so that remaining constraint is documented at the registration and on CreateSerilogLoggerFactory. BuildManager.Execute now owns a per-run composition root and feeds the resolved factory to a static facade on Logging, so the ~85 Log.* call sites and the static build engine are unchanged. The provider is declared outside the try so it survives into Finish(), but built inside it so a configuration failure still returns the same exit code as before. The seam is internal: it is framework foundation, not public surface yet. Nothing in the public API changes and no output changes. docs/dependencies.md gains rows for Serilog.Extensions.Logging, Microsoft.Extensions.Logging.Abstractions and Microsoft.Extensions.DependencyInjection. The DI row notes that Fallout.Build is consumer-facing, so every consumer now pulls the container transitively. First of the additive PRs in #428. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c3a46e6 commit 5aa3218

7 files changed

Lines changed: 485 additions & 0 deletions

File tree

Directory.Packages.props

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@
1616
<PackageVersion Include="JetBrains.Annotations" Version="2026.2.0" />
1717
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="10.0.0" />
1818
<PackageVersion Include="Microsoft.Extensions.DependencyModel" Version="10.0.0" />
19+
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="10.0.0" />
1920
<PackageVersion Include="Nerdbank.GitVersioning" Version="3.7.115" />
2021
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="8.0.0" />
2122
<PackageVersion Include="NuGet.Packaging" Version="6.14.3" />
2223
<PackageVersion Include="Octokit" Version="14.0.0" />
2324
<PackageVersion Include="Serilog" Version="4.3.0" />
25+
<PackageVersion Include="Serilog.Extensions.Logging" Version="10.0.0" />
2426
<PackageVersion Include="Serilog.Formatting.Compact" Version="3.0.0" />
2527
<PackageVersion Include="Serilog.Formatting.Compact.Reader" Version="4.0.0" />
2628
<PackageVersion Include="Serilog.Sinks.Console" Version="6.1.1" />

docs/dependencies.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Central package versions are pinned in `Directory.Packages.props`; this page lin
1515
| `Microsoft.Build` (+ `.Framework`, `.Tasks.Core`, `.Utilities.Core`) | MSBuild engine — read/evaluate `.csproj`/`.props` files | `Fallout.ProjectModel`, `Fallout.MSBuildTasks` |
1616
| `Microsoft.Build.Locator` | Locate an installed MSBuild at runtime | `Fallout.ProjectModel` |
1717
| `Microsoft.CodeAnalysis.*` (CSharp, Workspaces, MSBuild, Analyzers) | Roslyn — C# parsing/compilation/analysis | `Fallout.SourceGenerators`, `Fallout.Cli` (Cake rewriter) |
18+
| `Microsoft.Extensions.DependencyInjection` | DI container for the per-run composition root that wires the logging seam ([#428](https://github.com/Fallout-build/Fallout/issues/428)) | `Fallout.Build`. Note the footprint: `Fallout.Build` is consumer-facing, so every consumer now pulls the container transitively. Deliberate, not accidental. It currently serves one internal composition root, and the plugin foundation ([milestone #6](https://github.com/Fallout-build/Fallout/milestone/6)) is what will use it more widely. |
1819
| `Microsoft.Extensions.DependencyModel` | Parse `.deps.json` runtime metadata | `Fallout.Build` |
1920
| `Microsoft.SourceLink.GitHub` | Source-link symbols into published nupkgs so debuggers can step into Fallout | All packable libs |
2021
| `Nerdbank.GitVersioning` | Build-time semver derived from git history | All packable libs |
@@ -27,6 +28,8 @@ Central package versions are pinned in `Directory.Packages.props`; this page lin
2728
|---|---|
2829
| `Serilog` + `Sinks.Console` + `Sinks.File` | The logging framework. All `Log.Information/Warning/Error` calls route through this. |
2930
| `Serilog.Formatting.Compact` (+ `.Reader`) | Structured JSON log format for machine-readable logs |
31+
| `Serilog.Extensions.Logging` | Serilog provider for `Microsoft.Extensions.Logging`. Backs the `ILogger` seam in `Fallout.Build`, so framework code logs against the abstraction while Serilog stays the provider. |
32+
| `Microsoft.Extensions.Logging.Abstractions` | The `ILogger` / `ILoggerFactory` abstraction itself. Abstractions only, no implementation and no filter pipeline. |
3033

3134
## Azure
3235

src/Fallout.Build/Execution/BuildManager.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
using Fallout.Common.Tooling;
77
using Fallout.Common.Utilities;
88
using Fallout.Common.Utilities.Collections;
9+
using Microsoft.Extensions.DependencyInjection;
910
using Microsoft.Extensions.DependencyModel;
11+
using Microsoft.Extensions.Logging;
1012
using Serilog;
1113

1214
#pragma warning disable CA2255
@@ -44,9 +46,20 @@ public static int Execute<T>(Expression<Func<T, Target>>[] defaultTargetExpressi
4446
using var context = BuildContext.Activate();
4547
var build = new T();
4648

49+
// The composition root for the run. Declared out here so it survives into `finally` —
50+
// Finish() still writes the outcome summary — but built inside the `try`, so a failure while
51+
// configuring logging is reported the same way it was before there was a container.
52+
ServiceProvider services = null;
53+
IDisposable loggerFactoryScope = null;
54+
4755
try
4856
{
57+
// Configure installs the pipeline; AddFalloutLogging only registers the abstraction over
58+
// it. Order matters here rather than inside the registration: a logger binds the ambient
59+
// pipeline when it is created, so the pipeline has to exist before anything resolves one.
4960
Logging.Configure(build);
61+
services = new ServiceCollection().AddFalloutLogging().BuildServiceProvider();
62+
loggerFactoryScope = Logging.UseLoggerFactory(services.GetRequiredService<ILoggerFactory>());
5063

5164
build.ExecutableTargets = ExecutableTargetFactory.CreateAll(build, defaultTargetExpressions);
5265
build.ExecuteExtension<IOnBuildCreated>(x => x.OnBuildCreated(build.ExecutableTargets));
@@ -91,6 +104,8 @@ public static int Execute<T>(Expression<Func<T, Target>>[] defaultTargetExpressi
91104
{
92105
Finish();
93106
Log.CloseAndFlush();
107+
loggerFactoryScope?.Dispose();
108+
services?.Dispose();
94109
// Per-run teardown (handler unsubscription + state reset) is owned by the BuildContext,
95110
// run when `context` is disposed at method exit.
96111
}

src/Fallout.Build/Fallout.Build.csproj

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
</ItemGroup>
1919

2020
<ItemGroup>
21+
<PackageReference Include="Microsoft.Extensions.DependencyInjection" />
2122
<PackageReference Include="Microsoft.Extensions.DependencyModel" />
23+
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
24+
<PackageReference Include="Serilog.Extensions.Logging" />
2225
<PackageReference Include="Serilog.Formatting.Compact" />
2326
<PackageReference Include="Serilog.Formatting.Compact.Reader" />
2427
<PackageReference Include="Serilog.Sinks.Console" />
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
using System;
2+
using System.Linq;
3+
using Microsoft.Extensions.DependencyInjection;
4+
using Microsoft.Extensions.DependencyInjection.Extensions;
5+
using Microsoft.Extensions.Logging;
6+
7+
namespace Fallout.Common.Execution;
8+
9+
/// <summary>
10+
/// Composition root for the logging seam. Serilog stays the provider. This only puts
11+
/// <see cref="ILogger"/> in front of it, so framework code can stop referencing Serilog directly.
12+
/// </summary>
13+
/// <remarks>
14+
/// Internal on purpose: the abstraction is the framework's own foundation, not public surface yet.
15+
/// The root <c>AssemblyInfo.cs</c> grants <c>InternalsVisibleTo</c> to <c>Fallout.Cli</c> and the
16+
/// spec assemblies, which is everything that needs to wire a container today.
17+
/// </remarks>
18+
internal static class LoggingServiceCollectionExtensions
19+
{
20+
/// <summary>
21+
/// Registers the <see cref="ILogger"/> abstraction over the Serilog pipeline.
22+
/// </summary>
23+
/// <remarks>
24+
/// Registration only. This method never touches the pipeline, so any container can call it, any
25+
/// number of times. Installing the pipeline is <see cref="Logging.Configure"/>, which the caller
26+
/// runs explicitly just before it builds the provider.
27+
///
28+
/// The two are kept apart because <see cref="Logging.Configure"/> is not idempotent. It
29+
/// reassigns Serilog's <c>Log.Logger</c> on every call. Called with no build, it installs a
30+
/// pipeline with no file sinks, no host sink and no filter. A second container calling this
31+
/// method would then wipe out the pipeline the first one had set up.
32+
///
33+
/// Deliberately not <c>services.AddLogging(...)</c>. That installs Microsoft.Extensions.Logging's
34+
/// own filter pipeline, whose default minimum is
35+
/// <see cref="Microsoft.Extensions.Logging.LogLevel.Information"/> — qualified because the
36+
/// unqualified name binds to Fallout's own <see cref="LogLevel"/> from the enclosing namespace.
37+
/// It would be a second authority on levels, dropping trace and debug records before Serilog
38+
/// ever saw them.
39+
/// Registering the Serilog factory directly leaves <see cref="Logging.LevelSwitch"/> as the only
40+
/// thing that decides what gets logged.
41+
/// </remarks>
42+
public static IServiceCollection AddFalloutLogging(this IServiceCollection services)
43+
{
44+
// Safe as a singleton because the factory is left unbound: it reads the ambient pipeline
45+
// every time it creates a logger. See Logging.CreateSerilogLoggerFactory.
46+
services.TryAddSingleton(_ => Logging.CreateSerilogLoggerFactory());
47+
48+
// Transient, not singleton. Logger<T> resolves its inner logger in its constructor, and the
49+
// non-generic lambda would run once per container. As singletons, both would pin every
50+
// consumer to whichever pipeline was current at the first resolution. Log.Logger does not
51+
// stay put during a run: Configure installs it late, and Host.WriteErrorsAndWarnings swaps
52+
// it again to render the end-of-build summary.
53+
//
54+
// Transient means each resolution binds to the pipeline that is current right now. It does
55+
// not rescue a component that resolves a logger once and holds it across a swap. Any
56+
// component that outlives a swap must read Logging.Logger at the point of writing instead.
57+
services.TryAddTransient(typeof(ILogger<>), typeof(Logger<>));
58+
services.TryAddTransient(sp => sp.GetRequiredService<ILoggerFactory>().CreateLogger(Logging.DefaultCategoryName));
59+
60+
return services;
61+
}
62+
}

src/Fallout.Build/Logging.cs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,27 @@
1010
using Serilog;
1111
using Serilog.Core;
1212
using Serilog.Events;
13+
using Serilog.Extensions.Logging;
1314
using Serilog.Formatting.Compact;
1415
using Serilog.Sinks.SystemConsole.Themes;
1516

17+
// Both Serilog and Microsoft.Extensions.Logging declare an ILogger. This file is the seam between
18+
// them, so the unqualified name is bound to the abstraction the framework codes against; Serilog's
19+
// own pipeline is reached through the static Log class below.
20+
using ILogger = Microsoft.Extensions.Logging.ILogger;
21+
using ILoggerFactory = Microsoft.Extensions.Logging.ILoggerFactory;
22+
1623
namespace Fallout.Common.Execution;
1724

1825
public static class Logging
1926
{
2027
public static readonly LoggingLevelSwitch LevelSwitch = new();
2128

29+
/// <summary>Category for framework log records written without a category of their own.</summary>
30+
internal const string DefaultCategoryName = "Fallout";
31+
32+
private static ILoggerFactory loggerFactory;
33+
2234
internal static bool SupportsAnsiOutput =>
2335
Environment.GetEnvironmentVariable("TERM") is { } term && term.StartsWithOrdinalIgnoreCase("xterm");
2436

@@ -40,6 +52,61 @@ public static LogLevel Level
4052
set => LevelSwitch.MinimumLevel = value.ToLogEventLevel();
4153
}
4254

55+
/// <summary>
56+
/// Logger factory for the current build run, backed by the Serilog pipeline that
57+
/// <see cref="Configure"/> installs. <c>BuildManager</c> feeds this from its composition root
58+
/// (see <c>AddFalloutLogging</c>). Outside a run there is no container — the CLI commands call
59+
/// <see cref="Configure"/> directly — so this falls back to a factory over the ambient Serilog
60+
/// pipeline, and the seam is usable either way.
61+
/// </summary>
62+
internal static ILoggerFactory Factory => loggerFactory ??= CreateSerilogLoggerFactory();
63+
64+
/// <summary>
65+
/// Logger for framework code that has no category of its own. Deliberately not cached — each
66+
/// access creates a logger against the pipeline that is current right now, which is what keeps
67+
/// the façade correct across the reassignments described on
68+
/// <see cref="CreateSerilogLoggerFactory"/>.
69+
/// </summary>
70+
internal static ILogger Logger => Factory.CreateLogger(DefaultCategoryName);
71+
72+
/// <summary>
73+
/// Points <see cref="Factory"/> at <paramref name="factory"/> until the returned bracket is
74+
/// disposed. Ownership stays with the caller: disposing the bracket restores the previous
75+
/// factory, it does not dispose <paramref name="factory"/>.
76+
/// </summary>
77+
internal static IDisposable UseLoggerFactory(ILoggerFactory factory)
78+
{
79+
return DelegateDisposable.SetAndRestore(() => loggerFactory, factory.NotNull());
80+
}
81+
82+
/// <summary>
83+
/// Bridges <see cref="ILogger"/> onto Serilog.
84+
/// </summary>
85+
/// <remarks>
86+
/// Passing no logger leaves the factory itself unbound, so each logger it hands out reads the
87+
/// ambient <see cref="Log.Logger"/> as it is created. That matters because the pipeline is not
88+
/// stable for the lifetime of the process: <see cref="Configure"/> installs it late and
89+
/// replaces it on re-entry, and <c>Host.WriteErrorsAndWarnings</c> swaps it again to render the
90+
/// end-of-build summary. Pinning a logger into the factory would strand every consumer on
91+
/// whichever pipeline happened to exist first.
92+
///
93+
/// Binding still happens per logger rather than per write, because the category name is
94+
/// attached as Serilog's <c>SourceContext</c> at construction. Three consequences the callers
95+
/// depend on. <see cref="Configure"/> runs before the container is built, so a resolved logger
96+
/// can never bind a pipeline that is already gone. The container registrations for
97+
/// <c>ILogger&lt;T&gt;</c> and <see cref="ILogger"/> are transient, so each
98+
/// resolution binds the pipeline that is current right now. And <see cref="Logger"/> is not
99+
/// cached, for the same reason. A component that resolves a logger once and holds it across a
100+
/// reassignment still writes into the old pipeline, so anything living that long must read
101+
/// <see cref="Logger"/> at the point of writing.
102+
///
103+
/// Serilog owns the pipeline's lifetime (<c>Log.CloseAndFlush</c>), hence <c>dispose: false</c>.
104+
/// </remarks>
105+
internal static ILoggerFactory CreateSerilogLoggerFactory()
106+
{
107+
return new SerilogLoggerFactory(logger: null, dispose: false);
108+
}
109+
43110
public static void Configure(IFalloutBuild build = null)
44111
{
45112
if (build != null)

0 commit comments

Comments
 (0)