Skip to content

Commit 1f607ce

Browse files
committed
feat: add --verbosity and --log-file CLI options
Wire Serilog logging to CLI commands: - Add --log-file option to enable file logging from CLI - Wire --verbosity to Serilog log levels (silent=Fatal, quiet=Warning, normal=Info, verbose=Debug) - Inject ILoggerFactory via DI into all commands - Update BaseCommand to accept ILoggerFactory - Update all command constructors to pass logger factory - Fix environment detector test isolation
1 parent c3c5096 commit 1f607ce

17 files changed

Lines changed: 203 additions & 105 deletions

src/Main/CLI/CliApplicationBuilder.cs

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
// Copyright (c) Microsoft. All rights reserved.
22

33
using KernelMemory.Core.Config;
4+
using KernelMemory.Core.Logging;
45
using KernelMemory.Main.CLI.Commands;
56
using KernelMemory.Main.CLI.Infrastructure;
67
using Microsoft.Extensions.DependencyInjection;
8+
using Microsoft.Extensions.Logging;
9+
using Serilog.Events;
710
using Spectre.Console.Cli;
811

912
namespace KernelMemory.Main.CLI;
@@ -45,25 +48,36 @@ public sealed class CliApplicationBuilder
4548
/// </summary>
4649
/// <param name="args">Command line arguments (used to extract --config flag).</param>
4750
/// <returns>A configured CommandApp ready to execute commands.</returns>
51+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope",
52+
Justification = "ILoggerFactory lifetime is managed by DI container and must remain alive for duration of CLI execution")]
4853
public CommandApp Build(string[]? args = null)
4954
{
55+
var actualArgs = args ?? [];
56+
5057
// 1. Determine config path from args early (before command execution)
51-
string configPath = this.DetermineConfigPath(args ?? []);
58+
string configPath = this.DetermineConfigPath(actualArgs);
5259

5360
// 2. Load config ONCE (happens before any command runs)
5461
AppConfig config = ConfigParser.LoadFromFile(configPath);
5562

56-
// 3. Create DI container and register AppConfig as singleton
63+
// 3. Parse logging options from args and config
64+
var loggingConfig = this.BuildLoggingConfig(actualArgs, config);
65+
66+
// 4. Create logger factory using Serilog
67+
ILoggerFactory loggerFactory = SerilogFactory.CreateLoggerFactory(loggingConfig);
68+
69+
// 5. Create DI container and register services
5770
ServiceCollection services = new();
5871
services.AddSingleton(config);
72+
services.AddSingleton(loggerFactory);
5973

6074
// Also register the config path so commands can access it
6175
services.AddSingleton(new ConfigPathService(configPath));
6276

63-
// 4. Create type registrar for Spectre.Console.Cli DI integration
77+
// 6. Create type registrar for Spectre.Console.Cli DI integration
6478
TypeRegistrar registrar = new(services);
6579

66-
// 5. Build CommandApp with DI support
80+
// 7. Build CommandApp with DI support
6781
CommandApp app = new(registrar);
6882
this.Configure(app);
6983
return app;
@@ -93,6 +107,61 @@ private string DetermineConfigPath(string[] args)
93107
Constants.DefaultConfigFileName);
94108
}
95109

110+
/// <summary>
111+
/// Builds logging configuration from CLI args and app config.
112+
/// CLI args take precedence over config file settings.
113+
/// </summary>
114+
/// <param name="args">Command line arguments.</param>
115+
/// <param name="config">Application configuration.</param>
116+
/// <returns>Logging configuration for SerilogFactory.</returns>
117+
private LoggingConfig BuildLoggingConfig(string[] args, AppConfig config)
118+
{
119+
// Start with config file settings or defaults
120+
var loggingConfig = config.Logging ?? new LoggingConfig();
121+
122+
// Parse --verbosity / -v from args (takes precedence)
123+
var verbosity = this.ParseArgValue(args, "--verbosity", "-v");
124+
if (verbosity != null)
125+
{
126+
loggingConfig.Level = verbosity.ToLowerInvariant() switch
127+
{
128+
"silent" => LogEventLevel.Fatal,
129+
"quiet" => LogEventLevel.Warning,
130+
"verbose" => LogEventLevel.Debug,
131+
_ => LogEventLevel.Information // "normal" and default
132+
};
133+
}
134+
135+
// Parse --log-file from args (takes precedence)
136+
var logFile = this.ParseArgValue(args, "--log-file", null);
137+
if (logFile != null)
138+
{
139+
loggingConfig.FilePath = logFile;
140+
}
141+
142+
return loggingConfig;
143+
}
144+
145+
/// <summary>
146+
/// Parses a value for a command line argument.
147+
/// </summary>
148+
/// <param name="args">Command line arguments.</param>
149+
/// <param name="longName">Long form of argument (e.g., "--verbosity").</param>
150+
/// <param name="shortName">Short form of argument (e.g., "-v"), or null if none.</param>
151+
/// <returns>The argument value, or null if not found.</returns>
152+
private string? ParseArgValue(string[] args, string longName, string? shortName)
153+
{
154+
for (int i = 0; i < args.Length - 1; i++)
155+
{
156+
if (args[i] == longName || (shortName != null && args[i] == shortName))
157+
{
158+
return args[i + 1];
159+
}
160+
}
161+
162+
return null;
163+
}
164+
96165
/// <summary>
97166
/// Configures the CommandApp with all commands and examples.
98167
/// Made public to allow tests to reuse command configuration.

src/Main/CLI/Commands/BaseCommand.cs

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,28 +15,36 @@ namespace KernelMemory.Main.CLI.Commands;
1515

1616
/// <summary>
1717
/// Base class for all CLI commands providing shared initialization logic.
18-
/// Config is injected via constructor (loaded once in CliApplicationBuilder).
18+
/// Config and LoggerFactory are injected via constructor (loaded once in CliApplicationBuilder).
1919
/// </summary>
2020
/// <typeparam name="TSettings">The command settings type, must inherit from GlobalOptions.</typeparam>
2121
public abstract class BaseCommand<TSettings> : AsyncCommand<TSettings>
2222
where TSettings : GlobalOptions
2323
{
2424
private readonly AppConfig _config;
25+
private readonly ILoggerFactory _loggerFactory;
2526

2627
/// <summary>
2728
/// Initializes a new instance of the <see cref="BaseCommand{TSettings}"/> class.
2829
/// </summary>
2930
/// <param name="config">Application configuration (injected by DI).</param>
30-
protected BaseCommand(AppConfig config)
31+
/// <param name="loggerFactory">Logger factory for creating loggers (injected by DI).</param>
32+
protected BaseCommand(AppConfig config, ILoggerFactory loggerFactory)
3133
{
3234
this._config = config ?? throw new ArgumentNullException(nameof(config));
35+
this._loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory));
3336
}
3437

3538
/// <summary>
3639
/// Gets the injected application configuration.
3740
/// </summary>
3841
protected AppConfig Config => this._config;
3942

43+
/// <summary>
44+
/// Gets the injected logger factory.
45+
/// </summary>
46+
protected ILoggerFactory LoggerFactory => this._loggerFactory;
47+
4048
/// <summary>
4149
/// Initializes command dependencies: node and formatter.
4250
/// Config is already injected via constructor (no file I/O).
@@ -121,15 +129,10 @@ protected ContentService CreateContentService(NodeConfig node, bool readonlyMode
121129

122130
// Create dependencies
123131
var cuidGenerator = new CuidGenerator();
124-
var logger = this.CreateLogger();
132+
var logger = this._loggerFactory.CreateLogger<ContentStorageService>();
125133

126-
// Create search indexes from node configuration
127-
var loggerFactory = LoggerFactory.Create(builder =>
128-
{
129-
builder.AddConsole();
130-
builder.SetMinimumLevel(LogLevel.Warning);
131-
});
132-
var searchIndexes = SearchIndexFactory.CreateIndexes(node.SearchIndexes, loggerFactory);
134+
// Create search indexes from node configuration using injected logger factory
135+
var searchIndexes = SearchIndexFactory.CreateIndexes(node.SearchIndexes, this._loggerFactory);
133136

134137
// Create storage service with search indexes
135138
var storage = new ContentStorageService(context, cuidGenerator, logger, searchIndexes);
@@ -138,23 +141,6 @@ protected ContentService CreateContentService(NodeConfig node, bool readonlyMode
138141
return new ContentService(storage, node.Id, searchIndexes);
139142
}
140143

141-
/// <summary>
142-
/// Creates a simple console logger for ContentStorageService.
143-
/// </summary>
144-
/// <returns>A logger instance.</returns>
145-
[SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope",
146-
Justification = "LoggerFactory lifetime is managed by the logger infrastructure. CLI commands are short-lived and disposing would terminate logging prematurely.")]
147-
private ILogger<ContentStorageService> CreateLogger()
148-
{
149-
var loggerFactory = LoggerFactory.Create(builder =>
150-
{
151-
builder.AddConsole();
152-
builder.SetMinimumLevel(LogLevel.Warning);
153-
});
154-
155-
return loggerFactory.CreateLogger<ContentStorageService>();
156-
}
157-
158144
/// <summary>
159145
/// Handles exceptions and returns appropriate exit code.
160146
/// </summary>

src/Main/CLI/Commands/ConfigCommand.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using KernelMemory.Main.CLI.Infrastructure;
99
using KernelMemory.Main.CLI.Models;
1010
using KernelMemory.Main.CLI.OutputFormatters;
11+
using Microsoft.Extensions.Logging;
1112
using Spectre.Console;
1213
using Spectre.Console.Cli;
1314

@@ -49,10 +50,12 @@ public class ConfigCommand : BaseCommand<ConfigCommandSettings>
4950
/// Initializes a new instance of the <see cref="ConfigCommand"/> class.
5051
/// </summary>
5152
/// <param name="config">Application configuration (injected by DI).</param>
53+
/// <param name="loggerFactory">Logger factory for creating loggers (injected by DI).</param>
5254
/// <param name="configPathService">Service providing the config file path (injected by DI).</param>
5355
public ConfigCommand(
5456
AppConfig config,
55-
ConfigPathService configPathService) : base(config)
57+
ILoggerFactory loggerFactory,
58+
ConfigPathService configPathService) : base(config, loggerFactory)
5659
{
5760
this._configPathService = configPathService ?? throw new ArgumentNullException(nameof(configPathService));
5861
}

src/Main/CLI/Commands/DeleteCommand.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.ComponentModel;
44
using KernelMemory.Core.Config;
55
using KernelMemory.Main.CLI.OutputFormatters;
6+
using Microsoft.Extensions.Logging;
67
using Spectre.Console;
78
using Spectre.Console.Cli;
89

@@ -43,7 +44,8 @@ public class DeleteCommand : BaseCommand<DeleteCommandSettings>
4344
/// Initializes a new instance of the <see cref="DeleteCommand"/> class.
4445
/// </summary>
4546
/// <param name="config">Application configuration (injected by DI).</param>
46-
public DeleteCommand(AppConfig config) : base(config)
47+
/// <param name="loggerFactory">Logger factory for creating loggers (injected by DI).</param>
48+
public DeleteCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory)
4749
{
4850
}
4951

src/Main/CLI/Commands/GetCommand.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using KernelMemory.Core.Storage.Models;
66
using KernelMemory.Main.CLI.Exceptions;
77
using KernelMemory.Main.CLI.OutputFormatters;
8+
using Microsoft.Extensions.Logging;
89
using Spectre.Console;
910
using Spectre.Console.Cli;
1011

@@ -49,7 +50,8 @@ public class GetCommand : BaseCommand<GetCommandSettings>
4950
/// Initializes a new instance of the <see cref="GetCommand"/> class.
5051
/// </summary>
5152
/// <param name="config">Application configuration (injected by DI).</param>
52-
public GetCommand(AppConfig config) : base(config)
53+
/// <param name="loggerFactory">Logger factory for creating loggers (injected by DI).</param>
54+
public GetCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory)
5355
{
5456
}
5557

src/Main/CLI/Commands/ListCommand.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using KernelMemory.Core.Storage.Models;
66
using KernelMemory.Main.CLI.Exceptions;
77
using KernelMemory.Main.CLI.OutputFormatters;
8+
using Microsoft.Extensions.Logging;
89
using Spectre.Console;
910
using Spectre.Console.Cli;
1011

@@ -56,7 +57,8 @@ public class ListCommand : BaseCommand<ListCommandSettings>
5657
/// Initializes a new instance of the <see cref="ListCommand"/> class.
5758
/// </summary>
5859
/// <param name="config">Application configuration (injected by DI).</param>
59-
public ListCommand(AppConfig config) : base(config)
60+
/// <param name="loggerFactory">Logger factory for creating loggers (injected by DI).</param>
61+
public ListCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory)
6062
{
6163
}
6264

src/Main/CLI/Commands/NodesCommand.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
using KernelMemory.Core.Config;
44
using KernelMemory.Main.CLI.OutputFormatters;
5+
using Microsoft.Extensions.Logging;
56
using Spectre.Console.Cli;
67

78
namespace KernelMemory.Main.CLI.Commands;
@@ -22,7 +23,8 @@ public class NodesCommand : BaseCommand<NodesCommandSettings>
2223
/// Initializes a new instance of the <see cref="NodesCommand"/> class.
2324
/// </summary>
2425
/// <param name="config">Application configuration (injected by DI).</param>
25-
public NodesCommand(AppConfig config) : base(config)
26+
/// <param name="loggerFactory">Logger factory for creating loggers (injected by DI).</param>
27+
public NodesCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory)
2628
{
2729
}
2830

src/Main/CLI/Commands/SearchCommand.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using KernelMemory.Core.Search.Models;
99
using KernelMemory.Main.CLI.Exceptions;
1010
using KernelMemory.Main.CLI.OutputFormatters;
11+
using Microsoft.Extensions.Logging;
1112
using Spectre.Console;
1213
using Spectre.Console.Cli;
1314

@@ -144,7 +145,8 @@ public class SearchCommand : BaseCommand<SearchCommandSettings>
144145
/// Initializes a new instance of the <see cref="SearchCommand"/> class.
145146
/// </summary>
146147
/// <param name="config">Application configuration (injected by DI).</param>
147-
public SearchCommand(AppConfig config) : base(config)
148+
/// <param name="loggerFactory">Logger factory for creating loggers (injected by DI).</param>
149+
public SearchCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory)
148150
{
149151
}
150152

src/Main/CLI/Commands/UpsertCommand.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using KernelMemory.Core.Config;
55
using KernelMemory.Core.Storage.Models;
66
using KernelMemory.Main.CLI.OutputFormatters;
7+
using Microsoft.Extensions.Logging;
78
using Spectre.Console;
89
using Spectre.Console.Cli;
910

@@ -65,7 +66,8 @@ public class UpsertCommand : BaseCommand<UpsertCommandSettings>
6566
/// Initializes a new instance of the <see cref="UpsertCommand"/> class.
6667
/// </summary>
6768
/// <param name="config">Application configuration (injected by DI).</param>
68-
public UpsertCommand(AppConfig config) : base(config)
69+
/// <param name="loggerFactory">Logger factory for creating loggers (injected by DI).</param>
70+
public UpsertCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory)
6971
{
7072
}
7173

src/Main/CLI/GlobalOptions.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright (c) Microsoft. All rights reserved.
22
using System.ComponentModel;
3+
using Serilog.Events;
34
using Spectre.Console;
45
using Spectre.Console.Cli;
56

@@ -24,10 +25,14 @@ public class GlobalOptions : CommandSettings
2425
public string Format { get; init; } = "human";
2526

2627
[CommandOption("-v|--verbosity")]
27-
[Description("Verbosity: silent, quiet, normal, verbose")]
28+
[Description("Log verbosity: silent, quiet, normal, verbose")]
2829
[DefaultValue("normal")]
2930
public string Verbosity { get; init; } = "normal";
3031

32+
[CommandOption("--log-file")]
33+
[Description("Path to log file (enables file logging)")]
34+
public string? LogFile { get; init; }
35+
3136
[CommandOption("--no-color")]
3237
[Description("Disable colored output")]
3338
public bool NoColor { get; init; }
@@ -51,4 +56,20 @@ public override ValidationResult Validate()
5156

5257
return ValidationResult.Success();
5358
}
59+
60+
/// <summary>
61+
/// Maps the verbosity string to a Serilog LogEventLevel.
62+
/// silent = Fatal only, quiet = Warning+, normal = Information+, verbose = Debug+
63+
/// </summary>
64+
/// <returns>The corresponding Serilog LogEventLevel.</returns>
65+
public LogEventLevel GetLogLevel()
66+
{
67+
return this.Verbosity.ToLowerInvariant() switch
68+
{
69+
"silent" => LogEventLevel.Fatal,
70+
"quiet" => LogEventLevel.Warning,
71+
"verbose" => LogEventLevel.Debug,
72+
_ => LogEventLevel.Information // "normal" and default
73+
};
74+
}
5475
}

0 commit comments

Comments
 (0)