diff --git a/docs b/docs index 09132cdc0..03362c93d 160000 --- a/docs +++ b/docs @@ -1 +1 @@ -Subproject commit 09132cdc016d3316795e03e19709f9f6b25a273e +Subproject commit 03362c93d3f478ae345db7976db07b5e1043348c diff --git a/src/Core/Config/AppConfig.cs b/src/Core/Config/AppConfig.cs index 574b9aa36..98f95d0d6 100644 --- a/src/Core/Config/AppConfig.cs +++ b/src/Core/Config/AppConfig.cs @@ -2,6 +2,7 @@ using System.Text.Json.Serialization; using KernelMemory.Core.Config.Cache; using KernelMemory.Core.Config.Validation; +using KernelMemory.Core.Logging; namespace KernelMemory.Core.Config; @@ -36,6 +37,13 @@ public sealed class AppConfig : IValidatable [JsonPropertyName("search")] public SearchConfig? Search { get; set; } + /// + /// Logging configuration settings + /// Controls log level, file output, and format + /// + [JsonPropertyName("logging")] + public LoggingConfig? Logging { get; set; } + /// /// Validates the entire configuration tree /// diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index 2a3b66727..0518136d4 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -10,6 +10,14 @@ + + + + + + + + diff --git a/src/Core/Logging/ActivityEnricher.cs b/src/Core/Logging/ActivityEnricher.cs new file mode 100644 index 000000000..8efeadde2 --- /dev/null +++ b/src/Core/Logging/ActivityEnricher.cs @@ -0,0 +1,52 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Diagnostics; +using Serilog.Core; +using Serilog.Events; + +namespace KernelMemory.Core.Logging; + +/// +/// Serilog enricher that adds correlation IDs from System.Diagnostics.Activity. +/// Provides TraceId and SpanId properties for distributed tracing and log correlation. +/// +public sealed class ActivityEnricher : ILogEventEnricher +{ + /// + /// Property name for the trace ID (from Activity.TraceId). + /// + public const string TraceIdPropertyName = "TraceId"; + + /// + /// Property name for the span ID (from Activity.SpanId). + /// + public const string SpanIdPropertyName = "SpanId"; + + /// + /// Enriches the log event with Activity correlation IDs if available. + /// + /// The log event to enrich. + /// Factory for creating log event properties. + public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory) + { + var activity = Activity.Current; + if (activity == null) + { + return; + } + + // Add TraceId for correlating logs across the entire operation + var traceId = activity.TraceId.ToString(); + if (!string.IsNullOrEmpty(traceId) && traceId != LoggingConstants.EmptyTraceId) + { + logEvent.AddPropertyIfAbsent(propertyFactory.CreateProperty(TraceIdPropertyName, traceId)); + } + + // Add SpanId for correlating logs within a specific span + var spanId = activity.SpanId.ToString(); + if (!string.IsNullOrEmpty(spanId) && spanId != LoggingConstants.EmptySpanId) + { + logEvent.AddPropertyIfAbsent(propertyFactory.CreateProperty(SpanIdPropertyName, spanId)); + } + } +} diff --git a/src/Core/Logging/EnvironmentDetector.cs b/src/Core/Logging/EnvironmentDetector.cs new file mode 100644 index 000000000..c48e24595 --- /dev/null +++ b/src/Core/Logging/EnvironmentDetector.cs @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace KernelMemory.Core.Logging; + +/// +/// Detects the current runtime environment from environment variables. +/// Environment detection is critical for security decisions like sensitive data scrubbing. +/// +public static class EnvironmentDetector +{ + /// + /// Gets the current environment name. + /// Checks DOTNET_ENVIRONMENT first, falls back to ASPNETCORE_ENVIRONMENT, + /// then defaults to Development if neither is set. + /// + /// The environment name (e.g., "Development", "Production", "Staging"). + public static string GetEnvironment() + { + // Check DOTNET_ENVIRONMENT first (takes precedence) + var dotNetEnv = Environment.GetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable); + if (!string.IsNullOrWhiteSpace(dotNetEnv)) + { + return dotNetEnv; + } + + // Fall back to ASPNETCORE_ENVIRONMENT + var aspNetEnv = Environment.GetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable); + if (!string.IsNullOrWhiteSpace(aspNetEnv)) + { + return aspNetEnv; + } + + // Default to Development for safety (full logging) + return LoggingConstants.DefaultEnvironment; + } + + /// + /// Checks if the current environment is Production. + /// In Production, sensitive data is scrubbed from logs. + /// + /// True if running in Production environment. + public static bool IsProduction() + { + return string.Equals( + GetEnvironment(), + LoggingConstants.ProductionEnvironment, + StringComparison.OrdinalIgnoreCase); + } + + /// + /// Checks if the current environment is Development. + /// In Development, full logging is enabled for debugging. + /// + /// True if running in Development environment. + public static bool IsDevelopment() + { + return string.Equals( + GetEnvironment(), + LoggingConstants.DefaultEnvironment, + StringComparison.OrdinalIgnoreCase); + } +} diff --git a/src/Core/Logging/LoggerExtensions.cs b/src/Core/Logging/LoggerExtensions.cs new file mode 100644 index 000000000..dd62a92a1 --- /dev/null +++ b/src/Core/Logging/LoggerExtensions.cs @@ -0,0 +1,69 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Runtime.CompilerServices; +using Microsoft.Extensions.Logging; + +namespace KernelMemory.Core.Logging; + +/// +/// Extension methods for ILogger providing CallerMemberName-based method logging. +/// These helpers automatically capture the calling method name for entry/exit logging. +/// +public static class LoggerExtensions +{ + /// + /// Logs method entry at Debug level with automatic method name capture. + /// Use this at the beginning of public methods for diagnostics. + /// + /// The logger instance. + /// Optional first parameter to log. + /// Optional second parameter to log. + /// Optional third parameter to log. + /// Automatically captured method name (do not pass explicitly). + public static void LogMethodEntry( + this ILogger logger, + object? param1 = null, + object? param2 = null, + object? param3 = null, + [CallerMemberName] string methodName = "") + { + // Skip logging if Debug level is disabled for performance + if (!logger.IsEnabled(LogLevel.Debug)) + { + return; + } + + // Log with structured parameters for queryability + logger.LogDebug( + "{MethodName} called with {Param1}, {Param2}, {Param3}", + methodName, + param1, + param2, + param3); + } + + /// + /// Logs method exit at Debug level with automatic method name capture. + /// Use this before returning from public methods for diagnostics. + /// + /// The logger instance. + /// Optional result value to log. + /// Automatically captured method name (do not pass explicitly). + public static void LogMethodExit( + this ILogger logger, + object? result = null, + [CallerMemberName] string methodName = "") + { + // Skip logging if Debug level is disabled for performance + if (!logger.IsEnabled(LogLevel.Debug)) + { + return; + } + + // Log with structured parameters for queryability + logger.LogDebug( + "{MethodName} completed with {Result}", + methodName, + result); + } +} diff --git a/src/Core/Logging/LoggingConfig.cs b/src/Core/Logging/LoggingConfig.cs new file mode 100644 index 000000000..d3d0d69a9 --- /dev/null +++ b/src/Core/Logging/LoggingConfig.cs @@ -0,0 +1,45 @@ +// Copyright (c) Microsoft. All rights reserved. + +using Serilog.Events; + +namespace KernelMemory.Core.Logging; + +/// +/// Configuration model for the logging system. +/// Stores log level and file path settings that can be loaded from config files or CLI flags. +/// +public sealed class LoggingConfig +{ + /// + /// Gets or sets the minimum log level for log output. + /// Defaults to Information which provides useful diagnostics. + /// + public LogEventLevel Level { get; set; } = LogEventLevel.Information; + + /// + /// Gets or sets the file path for file logging. + /// When null or empty, file logging is disabled. + /// Supports both relative paths (from cwd) and absolute paths. + /// + public string? FilePath { get; set; } + + /// + /// Gets or sets whether to use JSON format for log output. + /// When false, uses human-readable format (default). + /// JSON format is better for log aggregation systems. + /// + public bool UseJsonFormat { get; set; } + + /// + /// Gets or sets whether to use async logging. + /// When false (default), uses synchronous logging suitable for CLI. + /// Enable for long-running services to improve performance. + /// + public bool UseAsyncLogging { get; set; } + + /// + /// Gets a value indicating whether file logging is enabled. + /// Returns true when FilePath is set to a non-empty value. + /// + public bool IsFileLoggingEnabled => !string.IsNullOrWhiteSpace(this.FilePath); +} diff --git a/src/Core/Logging/LoggingConstants.cs b/src/Core/Logging/LoggingConstants.cs new file mode 100644 index 000000000..52f2768ba --- /dev/null +++ b/src/Core/Logging/LoggingConstants.cs @@ -0,0 +1,92 @@ +// Copyright (c) Microsoft. All rights reserved. + +using Serilog.Events; + +namespace KernelMemory.Core.Logging; + +/// +/// Centralized constants for the logging system. +/// All magic values related to logging are defined here for maintainability. +/// +public static class LoggingConstants +{ + /// + /// Default maximum file size before rotation (100MB). + /// Balances history retention with disk usage. + /// + public const long DefaultFileSizeLimitBytes = 100 * 1024 * 1024; + + /// + /// Default number of log files to retain (30 files). + /// Approximately 1 month of daily logs or ~3GB max storage. + /// + public const int DefaultRetainedFileCountLimit = 30; + + /// + /// Default minimum log level for file output. + /// Information level provides useful diagnostics without excessive verbosity. + /// + public const LogEventLevel DefaultFileLogLevel = LogEventLevel.Information; + + /// + /// Default minimum log level for console/stderr output. + /// Only warnings and errors appear on stderr by default. + /// + public const LogEventLevel DefaultConsoleLogLevel = LogEventLevel.Warning; + + /// + /// Environment variable for .NET runtime environment detection. + /// Takes precedence over ASPNETCORE_ENVIRONMENT. + /// + public const string DotNetEnvironmentVariable = "DOTNET_ENVIRONMENT"; + + /// + /// Fallback environment variable for ASP.NET Core applications. + /// Used when DOTNET_ENVIRONMENT is not set. + /// + public const string AspNetCoreEnvironmentVariable = "ASPNETCORE_ENVIRONMENT"; + + /// + /// Default environment when no environment variable is set. + /// Defaults to Development for developer safety (full logging enabled). + /// + public const string DefaultEnvironment = "Development"; + + /// + /// Production environment name for comparison. + /// Sensitive data is scrubbed only in Production. + /// + public const string ProductionEnvironment = "Production"; + + /// + /// Placeholder text for redacted sensitive data. + /// Used to indicate data was intentionally removed from logs. + /// + public const string RedactedPlaceholder = "[REDACTED]"; + + /// + /// Human-readable output template for log messages. + /// Includes timestamp, level, source context, message, and optional exception. + /// + public const string HumanReadableOutputTemplate = + "{Timestamp:yyyy-MM-dd HH:mm:ss.fff} [{Level:u3}] {SourceContext}: {Message:lj}{NewLine}{Exception}"; + + /// + /// Compact output template for console (stderr) output. + /// Shorter format suitable for CLI error reporting. + /// + public const string ConsoleOutputTemplate = + "{Timestamp:HH:mm:ss} [{Level:u3}] {Message:lj}{NewLine}{Exception}"; + + /// + /// Empty trace ID value (32 zeros) used when no Activity is present. + /// Indicates no distributed tracing context is available. + /// + public const string EmptyTraceId = "00000000000000000000000000000000"; + + /// + /// Empty span ID value (16 zeros) used when no Activity is present. + /// Indicates no distributed tracing context is available. + /// + public const string EmptySpanId = "0000000000000000"; +} diff --git a/src/Core/Logging/SensitiveDataScrubbingPolicy.cs b/src/Core/Logging/SensitiveDataScrubbingPolicy.cs new file mode 100644 index 000000000..7861479ed --- /dev/null +++ b/src/Core/Logging/SensitiveDataScrubbingPolicy.cs @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft. All rights reserved. + +using Serilog.Core; +using Serilog.Events; + +namespace KernelMemory.Core.Logging; + +/// +/// Serilog destructuring policy that scrubs sensitive data in Production environment. +/// In Production, all string values are replaced with a redacted placeholder. +/// In Development/Staging, data is passed through unchanged for debugging. +/// +public sealed class SensitiveDataScrubbingPolicy : IDestructuringPolicy +{ + /// + /// Attempts to destructure a value, scrubbing strings in Production. + /// + /// The value to potentially destructure. + /// Factory for creating log event property values. + /// The destructured value if handled. + /// True if this policy handled the value, false to let Serilog process normally. + public bool TryDestructure( + object value, + ILogEventPropertyValueFactory propertyValueFactory, + [System.Diagnostics.CodeAnalysis.NotNullWhen(true)] out LogEventPropertyValue? result) + { + // Handle null - let Serilog process normally + if (value is null) + { + result = null; + return false; + } + + // Only scrub in Production environment + if (!EnvironmentDetector.IsProduction()) + { + result = null; + return false; + } + + // Only scrub string values - other types pass through + // Strings are most likely to contain sensitive data like: + // - API keys, tokens, passwords + // - User content, PII + // - File contents, queries + if (value is string) + { + result = new ScalarValue(LoggingConstants.RedactedPlaceholder); + return true; + } + + // Non-string types pass through unchanged + // Integers, booleans, DateTimes, Guids are generally safe + result = null; + return false; + } +} diff --git a/src/Core/Logging/SerilogFactory.cs b/src/Core/Logging/SerilogFactory.cs new file mode 100644 index 000000000..11244ddbb --- /dev/null +++ b/src/Core/Logging/SerilogFactory.cs @@ -0,0 +1,129 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Globalization; +using Microsoft.Extensions.Logging; +using Serilog; +using Serilog.Events; +using Serilog.Extensions.Logging; +using Serilog.Formatting.Compact; + +namespace KernelMemory.Core.Logging; + +/// +/// Factory for creating Serilog-based ILoggerFactory instances. +/// Configures Serilog with console, file, and optional async sinks +/// based on the provided LoggingConfig. +/// +public static class SerilogFactory +{ + /// + /// Creates an ILoggerFactory configured with Serilog based on the provided config. + /// The factory integrates with Microsoft.Extensions.Logging for DI compatibility. + /// + /// Logging configuration specifying level, file path, and options. + /// A configured ILoggerFactory that creates Serilog-backed loggers. + public static ILoggerFactory CreateLoggerFactory(LoggingConfig config) + { + var loggerConfig = new LoggerConfiguration() + .MinimumLevel.Is(config.Level) + .Enrich.FromLogContext() + .Enrich.WithProperty("Application", "KernelMemory"); + + // Add correlation IDs from System.Diagnostics.Activity + loggerConfig = loggerConfig.Enrich.With(); + + // Add sensitive data scrubbing policy + loggerConfig = loggerConfig.Destructure.With(); + + // Configure console output (stderr) for warnings and errors + loggerConfig = loggerConfig.WriteTo.Console( + restrictedToMinimumLevel: LoggingConstants.DefaultConsoleLogLevel, + outputTemplate: LoggingConstants.ConsoleOutputTemplate, + formatProvider: CultureInfo.InvariantCulture, + standardErrorFromLevel: LogEventLevel.Verbose); + + // Configure file output if path is specified + if (config.IsFileLoggingEnabled) + { + loggerConfig = ConfigureFileLogging(loggerConfig, config); + } + + var serilogLogger = loggerConfig.CreateLogger(); + + // Create Microsoft.Extensions.Logging factory backed by Serilog + return new SerilogLoggerFactory(serilogLogger, dispose: true); + } + + /// + /// Configures file logging with rotation settings. + /// Supports both sync and async modes, and human-readable or JSON formats. + /// + /// The logger configuration to extend. + /// Logging configuration with file settings. + /// The extended logger configuration. + private static LoggerConfiguration ConfigureFileLogging( + LoggerConfiguration loggerConfig, + LoggingConfig config) + { + // Ensure directory exists for the log file + var filePath = config.FilePath!; + var directory = Path.GetDirectoryName(filePath); + if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory)) + { + Directory.CreateDirectory(directory); + } + + if (config.UseAsyncLogging) + { + // Async logging for better performance in services + if (config.UseJsonFormat) + { + loggerConfig = loggerConfig.WriteTo.Async(a => a.File( + new CompactJsonFormatter(), + filePath, + fileSizeLimitBytes: LoggingConstants.DefaultFileSizeLimitBytes, + rollingInterval: RollingInterval.Day, + rollOnFileSizeLimit: true, + retainedFileCountLimit: LoggingConstants.DefaultRetainedFileCountLimit)); + } + else + { + loggerConfig = loggerConfig.WriteTo.Async(a => a.File( + filePath, + outputTemplate: LoggingConstants.HumanReadableOutputTemplate, + formatProvider: CultureInfo.InvariantCulture, + fileSizeLimitBytes: LoggingConstants.DefaultFileSizeLimitBytes, + rollingInterval: RollingInterval.Day, + rollOnFileSizeLimit: true, + retainedFileCountLimit: LoggingConstants.DefaultRetainedFileCountLimit)); + } + } + else + { + // Sync logging for CLI (short-lived commands) + if (config.UseJsonFormat) + { + loggerConfig = loggerConfig.WriteTo.File( + new CompactJsonFormatter(), + filePath, + fileSizeLimitBytes: LoggingConstants.DefaultFileSizeLimitBytes, + rollingInterval: RollingInterval.Day, + rollOnFileSizeLimit: true, + retainedFileCountLimit: LoggingConstants.DefaultRetainedFileCountLimit); + } + else + { + loggerConfig = loggerConfig.WriteTo.File( + filePath, + outputTemplate: LoggingConstants.HumanReadableOutputTemplate, + formatProvider: CultureInfo.InvariantCulture, + fileSizeLimitBytes: LoggingConstants.DefaultFileSizeLimitBytes, + rollingInterval: RollingInterval.Day, + rollOnFileSizeLimit: true, + retainedFileCountLimit: LoggingConstants.DefaultRetainedFileCountLimit); + } + } + + return loggerConfig; + } +} diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index 1fcfc1abc..9a36b4a1e 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -32,6 +32,14 @@ + + + + + + + + diff --git a/src/Main/CLI/CliApplicationBuilder.cs b/src/Main/CLI/CliApplicationBuilder.cs index bfa8eded8..ef2c68e03 100644 --- a/src/Main/CLI/CliApplicationBuilder.cs +++ b/src/Main/CLI/CliApplicationBuilder.cs @@ -1,9 +1,12 @@ // Copyright (c) Microsoft. All rights reserved. using KernelMemory.Core.Config; +using KernelMemory.Core.Logging; using KernelMemory.Main.CLI.Commands; using KernelMemory.Main.CLI.Infrastructure; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Serilog.Events; using Spectre.Console.Cli; namespace KernelMemory.Main.CLI; @@ -45,25 +48,42 @@ public sealed class CliApplicationBuilder /// /// Command line arguments (used to extract --config flag). /// A configured CommandApp ready to execute commands. + /// + /// The ILoggerFactory is intentionally not disposed here. CLI commands are short-lived + /// and process exit will flush Serilog sinks. Explicit disposal would require changes + /// to the Spectre.Console.Cli framework integration. The short-lived nature of CLI + /// commands makes this acceptable - logs are flushed when the process terminates. + /// + [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", + Justification = "ILoggerFactory lifetime is managed by DI container and must remain alive for duration of CLI execution. Process exit flushes Serilog sinks.")] public CommandApp Build(string[]? args = null) { + var actualArgs = args ?? []; + // 1. Determine config path from args early (before command execution) - string configPath = this.DetermineConfigPath(args ?? []); + string configPath = this.DetermineConfigPath(actualArgs); // 2. Load config ONCE (happens before any command runs) AppConfig config = ConfigParser.LoadFromFile(configPath); - // 3. Create DI container and register AppConfig as singleton + // 3. Parse logging options from args and config + var loggingConfig = this.BuildLoggingConfig(actualArgs, config); + + // 4. Create logger factory using Serilog + ILoggerFactory loggerFactory = SerilogFactory.CreateLoggerFactory(loggingConfig); + + // 5. Create DI container and register services ServiceCollection services = new(); services.AddSingleton(config); + services.AddSingleton(loggerFactory); // Also register the config path so commands can access it services.AddSingleton(new ConfigPathService(configPath)); - // 4. Create type registrar for Spectre.Console.Cli DI integration + // 6. Create type registrar for Spectre.Console.Cli DI integration TypeRegistrar registrar = new(services); - // 5. Build CommandApp with DI support + // 7. Build CommandApp with DI support CommandApp app = new(registrar); this.Configure(app); return app; @@ -93,6 +113,68 @@ private string DetermineConfigPath(string[] args) Constants.DefaultConfigFileName); } + /// + /// Builds logging configuration from CLI args and app config. + /// CLI args take precedence over config file settings. + /// + /// Command line arguments. + /// Application configuration. + /// Logging configuration for SerilogFactory. + private LoggingConfig BuildLoggingConfig(string[] args, AppConfig config) + { + // Start with config file settings or defaults + var loggingConfig = config.Logging ?? new LoggingConfig(); + + // Parse --verbosity / -v from args (takes precedence) + var verbosity = this.ParseArgValue(args, "--verbosity", "-v"); + if (verbosity != null) + { + loggingConfig.Level = verbosity.ToLowerInvariant() switch + { + "silent" => LogEventLevel.Fatal, + "quiet" => LogEventLevel.Warning, + "verbose" => LogEventLevel.Debug, + _ => LogEventLevel.Information // "normal" and default + }; + } + + // Parse --log-file from args (takes precedence) + var logFile = this.ParseArgValue(args, "--log-file", null); + if (logFile != null) + { + loggingConfig.FilePath = logFile; + } + + return loggingConfig; + } + + /// + /// Parses a value for a command line argument. + /// + /// Command line arguments. + /// Long form of argument (e.g., "--verbosity"). + /// Short form of argument (e.g., "-v"), or null if none. + /// The argument value, or null if not found or if no value follows the argument. + private string? ParseArgValue(string[] args, string longName, string? shortName) + { + for (int i = 0; i < args.Length; i++) + { + if (args[i] == longName || (shortName != null && args[i] == shortName)) + { + // Check if there's a value following this argument + if (i + 1 < args.Length) + { + return args[i + 1]; + } + + // Argument found but no value provided + return null; + } + } + + return null; + } + /// /// Configures the CommandApp with all commands and examples. /// Made public to allow tests to reuse command configuration. diff --git a/src/Main/CLI/Commands/BaseCommand.cs b/src/Main/CLI/Commands/BaseCommand.cs index d7bf1aad1..e2c393215 100644 --- a/src/Main/CLI/Commands/BaseCommand.cs +++ b/src/Main/CLI/Commands/BaseCommand.cs @@ -15,21 +15,24 @@ namespace KernelMemory.Main.CLI.Commands; /// /// Base class for all CLI commands providing shared initialization logic. -/// Config is injected via constructor (loaded once in CliApplicationBuilder). +/// Config and LoggerFactory are injected via constructor (loaded once in CliApplicationBuilder). /// /// The command settings type, must inherit from GlobalOptions. public abstract class BaseCommand : AsyncCommand where TSettings : GlobalOptions { private readonly AppConfig _config; + private readonly ILoggerFactory _loggerFactory; /// /// Initializes a new instance of the class. /// /// Application configuration (injected by DI). - protected BaseCommand(AppConfig config) + /// Logger factory for creating loggers (injected by DI). + protected BaseCommand(AppConfig config, ILoggerFactory loggerFactory) { this._config = config ?? throw new ArgumentNullException(nameof(config)); + this._loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory)); } /// @@ -37,6 +40,11 @@ protected BaseCommand(AppConfig config) /// protected AppConfig Config => this._config; + /// + /// Gets the injected logger factory. + /// + protected ILoggerFactory LoggerFactory => this._loggerFactory; + /// /// Initializes command dependencies: node and formatter. /// Config is already injected via constructor (no file I/O). @@ -121,15 +129,10 @@ protected ContentService CreateContentService(NodeConfig node, bool readonlyMode // Create dependencies var cuidGenerator = new CuidGenerator(); - var logger = this.CreateLogger(); + var logger = this._loggerFactory.CreateLogger(); - // Create search indexes from node configuration - var loggerFactory = LoggerFactory.Create(builder => - { - builder.AddConsole(); - builder.SetMinimumLevel(LogLevel.Warning); - }); - var searchIndexes = SearchIndexFactory.CreateIndexes(node.SearchIndexes, loggerFactory); + // Create search indexes from node configuration using injected logger factory + var searchIndexes = SearchIndexFactory.CreateIndexes(node.SearchIndexes, this._loggerFactory); // Create storage service with search indexes var storage = new ContentStorageService(context, cuidGenerator, logger, searchIndexes); @@ -138,23 +141,6 @@ protected ContentService CreateContentService(NodeConfig node, bool readonlyMode return new ContentService(storage, node.Id, searchIndexes); } - /// - /// Creates a simple console logger for ContentStorageService. - /// - /// A logger instance. - [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", - Justification = "LoggerFactory lifetime is managed by the logger infrastructure. CLI commands are short-lived and disposing would terminate logging prematurely.")] - private ILogger CreateLogger() - { - var loggerFactory = LoggerFactory.Create(builder => - { - builder.AddConsole(); - builder.SetMinimumLevel(LogLevel.Warning); - }); - - return loggerFactory.CreateLogger(); - } - /// /// Handles exceptions and returns appropriate exit code. /// diff --git a/src/Main/CLI/Commands/ConfigCommand.cs b/src/Main/CLI/Commands/ConfigCommand.cs index dddd2366a..f651705e2 100644 --- a/src/Main/CLI/Commands/ConfigCommand.cs +++ b/src/Main/CLI/Commands/ConfigCommand.cs @@ -8,6 +8,7 @@ using KernelMemory.Main.CLI.Infrastructure; using KernelMemory.Main.CLI.Models; using KernelMemory.Main.CLI.OutputFormatters; +using Microsoft.Extensions.Logging; using Spectre.Console; using Spectre.Console.Cli; @@ -49,10 +50,12 @@ public class ConfigCommand : BaseCommand /// Initializes a new instance of the class. /// /// Application configuration (injected by DI). + /// Logger factory for creating loggers (injected by DI). /// Service providing the config file path (injected by DI). public ConfigCommand( AppConfig config, - ConfigPathService configPathService) : base(config) + ILoggerFactory loggerFactory, + ConfigPathService configPathService) : base(config, loggerFactory) { this._configPathService = configPathService ?? throw new ArgumentNullException(nameof(configPathService)); } diff --git a/src/Main/CLI/Commands/DeleteCommand.cs b/src/Main/CLI/Commands/DeleteCommand.cs index 7e764fc79..35ac6ac0b 100644 --- a/src/Main/CLI/Commands/DeleteCommand.cs +++ b/src/Main/CLI/Commands/DeleteCommand.cs @@ -3,6 +3,7 @@ using System.ComponentModel; using KernelMemory.Core.Config; using KernelMemory.Main.CLI.OutputFormatters; +using Microsoft.Extensions.Logging; using Spectre.Console; using Spectre.Console.Cli; @@ -43,7 +44,8 @@ public class DeleteCommand : BaseCommand /// Initializes a new instance of the class. /// /// Application configuration (injected by DI). - public DeleteCommand(AppConfig config) : base(config) + /// Logger factory for creating loggers (injected by DI). + public DeleteCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory) { } diff --git a/src/Main/CLI/Commands/GetCommand.cs b/src/Main/CLI/Commands/GetCommand.cs index 092cd81cd..88fa1a997 100644 --- a/src/Main/CLI/Commands/GetCommand.cs +++ b/src/Main/CLI/Commands/GetCommand.cs @@ -5,6 +5,7 @@ using KernelMemory.Core.Storage.Models; using KernelMemory.Main.CLI.Exceptions; using KernelMemory.Main.CLI.OutputFormatters; +using Microsoft.Extensions.Logging; using Spectre.Console; using Spectre.Console.Cli; @@ -49,7 +50,8 @@ public class GetCommand : BaseCommand /// Initializes a new instance of the class. /// /// Application configuration (injected by DI). - public GetCommand(AppConfig config) : base(config) + /// Logger factory for creating loggers (injected by DI). + public GetCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory) { } diff --git a/src/Main/CLI/Commands/ListCommand.cs b/src/Main/CLI/Commands/ListCommand.cs index 4f9d57970..fd09d038f 100644 --- a/src/Main/CLI/Commands/ListCommand.cs +++ b/src/Main/CLI/Commands/ListCommand.cs @@ -5,6 +5,7 @@ using KernelMemory.Core.Storage.Models; using KernelMemory.Main.CLI.Exceptions; using KernelMemory.Main.CLI.OutputFormatters; +using Microsoft.Extensions.Logging; using Spectre.Console; using Spectre.Console.Cli; @@ -56,7 +57,8 @@ public class ListCommand : BaseCommand /// Initializes a new instance of the class. /// /// Application configuration (injected by DI). - public ListCommand(AppConfig config) : base(config) + /// Logger factory for creating loggers (injected by DI). + public ListCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory) { } diff --git a/src/Main/CLI/Commands/NodesCommand.cs b/src/Main/CLI/Commands/NodesCommand.cs index 4d858f45f..ec6abba7b 100644 --- a/src/Main/CLI/Commands/NodesCommand.cs +++ b/src/Main/CLI/Commands/NodesCommand.cs @@ -2,6 +2,7 @@ using KernelMemory.Core.Config; using KernelMemory.Main.CLI.OutputFormatters; +using Microsoft.Extensions.Logging; using Spectre.Console.Cli; namespace KernelMemory.Main.CLI.Commands; @@ -22,7 +23,8 @@ public class NodesCommand : BaseCommand /// Initializes a new instance of the class. /// /// Application configuration (injected by DI). - public NodesCommand(AppConfig config) : base(config) + /// Logger factory for creating loggers (injected by DI). + public NodesCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory) { } diff --git a/src/Main/CLI/Commands/SearchCommand.cs b/src/Main/CLI/Commands/SearchCommand.cs index 999ef7034..0939db150 100644 --- a/src/Main/CLI/Commands/SearchCommand.cs +++ b/src/Main/CLI/Commands/SearchCommand.cs @@ -8,6 +8,7 @@ using KernelMemory.Core.Search.Models; using KernelMemory.Main.CLI.Exceptions; using KernelMemory.Main.CLI.OutputFormatters; +using Microsoft.Extensions.Logging; using Spectre.Console; using Spectre.Console.Cli; @@ -144,7 +145,8 @@ public class SearchCommand : BaseCommand /// Initializes a new instance of the class. /// /// Application configuration (injected by DI). - public SearchCommand(AppConfig config) : base(config) + /// Logger factory for creating loggers (injected by DI). + public SearchCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory) { } diff --git a/src/Main/CLI/Commands/UpsertCommand.cs b/src/Main/CLI/Commands/UpsertCommand.cs index c83a1b2e7..2d50cbb3f 100644 --- a/src/Main/CLI/Commands/UpsertCommand.cs +++ b/src/Main/CLI/Commands/UpsertCommand.cs @@ -4,6 +4,7 @@ using KernelMemory.Core.Config; using KernelMemory.Core.Storage.Models; using KernelMemory.Main.CLI.OutputFormatters; +using Microsoft.Extensions.Logging; using Spectre.Console; using Spectre.Console.Cli; @@ -65,7 +66,8 @@ public class UpsertCommand : BaseCommand /// Initializes a new instance of the class. /// /// Application configuration (injected by DI). - public UpsertCommand(AppConfig config) : base(config) + /// Logger factory for creating loggers (injected by DI). + public UpsertCommand(AppConfig config, ILoggerFactory loggerFactory) : base(config, loggerFactory) { } diff --git a/src/Main/CLI/GlobalOptions.cs b/src/Main/CLI/GlobalOptions.cs index fd92b8836..6fb3a28e8 100644 --- a/src/Main/CLI/GlobalOptions.cs +++ b/src/Main/CLI/GlobalOptions.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. using System.ComponentModel; +using Serilog.Events; using Spectre.Console; using Spectre.Console.Cli; @@ -24,10 +25,14 @@ public class GlobalOptions : CommandSettings public string Format { get; init; } = "human"; [CommandOption("-v|--verbosity")] - [Description("Verbosity: silent, quiet, normal, verbose")] + [Description("Log verbosity: silent, quiet, normal, verbose")] [DefaultValue("normal")] public string Verbosity { get; init; } = "normal"; + [CommandOption("--log-file")] + [Description("Path to log file (enables file logging)")] + public string? LogFile { get; init; } + [CommandOption("--no-color")] [Description("Disable colored output")] public bool NoColor { get; init; } @@ -51,4 +56,20 @@ public override ValidationResult Validate() return ValidationResult.Success(); } + + /// + /// Maps the verbosity string to a Serilog LogEventLevel. + /// silent = Fatal only, quiet = Warning+, normal = Information+, verbose = Debug+ + /// + /// The corresponding Serilog LogEventLevel. + public LogEventLevel GetLogLevel() + { + return this.Verbosity.ToLowerInvariant() switch + { + "silent" => LogEventLevel.Fatal, + "quiet" => LogEventLevel.Warning, + "verbose" => LogEventLevel.Debug, + _ => LogEventLevel.Information // "normal" and default + }; + } } diff --git a/tests/Core.Tests/Core.Tests.csproj b/tests/Core.Tests/Core.Tests.csproj index 6f3158803..6210827f3 100644 --- a/tests/Core.Tests/Core.Tests.csproj +++ b/tests/Core.Tests/Core.Tests.csproj @@ -23,6 +23,11 @@ runtime; build; native; contentfiles; analyzers; buildtransitive all + + + + + diff --git a/tests/Core.Tests/Logging/ActivityEnricherTests.cs b/tests/Core.Tests/Logging/ActivityEnricherTests.cs new file mode 100644 index 000000000..eff2737c9 --- /dev/null +++ b/tests/Core.Tests/Logging/ActivityEnricherTests.cs @@ -0,0 +1,179 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Diagnostics; +using KernelMemory.Core.Logging; +using Serilog.Events; + +namespace KernelMemory.Core.Tests.Logging; + +/// +/// Tests for ActivityEnricher - validates correlation ID enrichment from System.Diagnostics.Activity. +/// Activity correlation is critical for distributed tracing and log correlation. +/// +public sealed class ActivityEnricherTests : IDisposable +{ + private Activity? _activity; + + /// + /// Cleans up activity after each test. + /// + public void Dispose() + { + this._activity?.Dispose(); + GC.SuppressFinalize(this); + } + + /// + /// Verifies TraceId property name constant is defined. + /// + [Fact] + public void TraceIdPropertyName_ShouldBeDefined() + { + // Assert + Assert.Equal("TraceId", ActivityEnricher.TraceIdPropertyName); + } + + /// + /// Verifies SpanId property name constant is defined. + /// + [Fact] + public void SpanIdPropertyName_ShouldBeDefined() + { + // Assert + Assert.Equal("SpanId", ActivityEnricher.SpanIdPropertyName); + } + + /// + /// Verifies Enrich does nothing when Activity.Current is null. + /// + [Fact] + public void Enrich_WhenNoActivity_ShouldNotAddProperties() + { + // Arrange + Activity.Current = null; + var enricher = new ActivityEnricher(); + var logEvent = CreateLogEvent(); + var propertyFactory = new TestPropertyFactory(); + + // Act + enricher.Enrich(logEvent, propertyFactory); + + // Assert - no properties should be added + Assert.Empty(logEvent.Properties); + } + + /// + /// Verifies Enrich adds TraceId and SpanId when Activity is present. + /// + [Fact] + public void Enrich_WhenActivityPresent_ShouldAddTraceAndSpanIds() + { + // Arrange + this._activity = new Activity("TestOperation"); + this._activity.Start(); + var enricher = new ActivityEnricher(); + var logEvent = CreateLogEvent(); + var propertyFactory = new TestPropertyFactory(); + + // Act + enricher.Enrich(logEvent, propertyFactory); + + // Assert - both TraceId and SpanId should be added + Assert.True(logEvent.Properties.ContainsKey(ActivityEnricher.TraceIdPropertyName)); + Assert.True(logEvent.Properties.ContainsKey(ActivityEnricher.SpanIdPropertyName)); + } + + /// + /// Verifies TraceId property has correct value from Activity. + /// + [Fact] + public void Enrich_ShouldSetCorrectTraceId() + { + // Arrange + this._activity = new Activity("TestOperation"); + this._activity.Start(); + var enricher = new ActivityEnricher(); + var logEvent = CreateLogEvent(); + var propertyFactory = new TestPropertyFactory(); + + // Act + enricher.Enrich(logEvent, propertyFactory); + + // Assert + var traceIdProperty = logEvent.Properties[ActivityEnricher.TraceIdPropertyName] as ScalarValue; + Assert.NotNull(traceIdProperty); + Assert.Equal(this._activity.TraceId.ToString(), traceIdProperty.Value); + } + + /// + /// Verifies SpanId property has correct value from Activity. + /// + [Fact] + public void Enrich_ShouldSetCorrectSpanId() + { + // Arrange + this._activity = new Activity("TestOperation"); + this._activity.Start(); + var enricher = new ActivityEnricher(); + var logEvent = CreateLogEvent(); + var propertyFactory = new TestPropertyFactory(); + + // Act + enricher.Enrich(logEvent, propertyFactory); + + // Assert + var spanIdProperty = logEvent.Properties[ActivityEnricher.SpanIdPropertyName] as ScalarValue; + Assert.NotNull(spanIdProperty); + Assert.Equal(this._activity.SpanId.ToString(), spanIdProperty.Value); + } + + /// + /// Verifies existing properties are not overwritten. + /// + [Fact] + public void Enrich_WhenPropertyExists_ShouldNotOverwrite() + { + // Arrange + this._activity = new Activity("TestOperation"); + this._activity.Start(); + var enricher = new ActivityEnricher(); + var logEvent = CreateLogEvent(); + var propertyFactory = new TestPropertyFactory(); + + // Pre-add a TraceId property + const string existingTraceId = "existing-trace-id"; + logEvent.AddPropertyIfAbsent(new LogEventProperty(ActivityEnricher.TraceIdPropertyName, new ScalarValue(existingTraceId))); + + // Act + enricher.Enrich(logEvent, propertyFactory); + + // Assert - existing property should be preserved + var traceIdProperty = logEvent.Properties[ActivityEnricher.TraceIdPropertyName] as ScalarValue; + Assert.NotNull(traceIdProperty); + Assert.Equal(existingTraceId, traceIdProperty.Value); + } + + /// + /// Creates a test log event for enrichment testing. + /// + private static LogEvent CreateLogEvent() + { + return new LogEvent( + DateTimeOffset.UtcNow, + LogEventLevel.Information, + null, + new MessageTemplate("Test message", []), + []); + } + + /// + /// Test implementation of ILogEventPropertyFactory for unit testing. + /// + private sealed class TestPropertyFactory : Serilog.Core.ILogEventPropertyFactory + { + public LogEventProperty CreateProperty(string name, object? value, bool destructureObjects = false) + { + return new LogEventProperty(name, new ScalarValue(value)); + } + } +} diff --git a/tests/Core.Tests/Logging/EnvironmentDetectorTests.cs b/tests/Core.Tests/Logging/EnvironmentDetectorTests.cs new file mode 100644 index 000000000..47ca8999b --- /dev/null +++ b/tests/Core.Tests/Logging/EnvironmentDetectorTests.cs @@ -0,0 +1,296 @@ +// Copyright (c) Microsoft. All rights reserved. + +using KernelMemory.Core.Logging; + +namespace KernelMemory.Core.Tests.Logging; + +/// +/// Tests for EnvironmentDetector - validates environment detection logic. +/// Environment detection is critical for security (sensitive data scrubbing). +/// +public sealed class EnvironmentDetectorTests : IDisposable +{ + private readonly string? _originalDotNetEnv; + private readonly string? _originalAspNetEnv; + + /// + /// Initializes a new instance capturing original environment variables. + /// + public EnvironmentDetectorTests() + { + // Capture original values to restore after tests + this._originalDotNetEnv = Environment.GetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable); + this._originalAspNetEnv = Environment.GetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable); + } + + /// + /// Restores original environment variables after test. + /// + public void Dispose() + { + // Restore original environment variables + if (this._originalDotNetEnv != null) + { + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, this._originalDotNetEnv); + } + else + { + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, null); + } + + if (this._originalAspNetEnv != null) + { + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, this._originalAspNetEnv); + } + else + { + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + } + + GC.SuppressFinalize(this); + } + + /// + /// Verifies GetEnvironment returns DOTNET_ENVIRONMENT when set. + /// DOTNET_ENVIRONMENT takes precedence over all other sources. + /// + [Fact] + public void GetEnvironment_WhenDotNetEnvSet_ShouldReturnDotNetEnv() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Production"); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, "Staging"); + + // Act + var result = EnvironmentDetector.GetEnvironment(); + + // Assert - DOTNET_ENVIRONMENT takes precedence + Assert.Equal("Production", result); + } + + /// + /// Verifies GetEnvironment falls back to ASPNETCORE_ENVIRONMENT. + /// When DOTNET_ENVIRONMENT is not set, use ASPNETCORE_ENVIRONMENT. + /// + [Fact] + public void GetEnvironment_WhenOnlyAspNetEnvSet_ShouldReturnAspNetEnv() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, null); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, "Staging"); + + // Act + var result = EnvironmentDetector.GetEnvironment(); + + // Assert + Assert.Equal("Staging", result); + } + + /// + /// Verifies GetEnvironment returns Development by default. + /// When no environment variables are set, default to Development for safety. + /// + [Fact] + public void GetEnvironment_WhenNothingSet_ShouldReturnDevelopment() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, null); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + + // Act + var result = EnvironmentDetector.GetEnvironment(); + + // Assert + Assert.Equal("Development", result); + } + + /// + /// Verifies IsProduction returns true when Production environment is set. + /// + [Fact] + public void IsProduction_WhenProductionSet_ShouldReturnTrue() + { + // Arrange - clear both env vars to ensure isolation + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Production"); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + + // Act + var result = EnvironmentDetector.IsProduction(); + + // Assert + Assert.True(result); + } + + /// + /// Verifies IsProduction is case-insensitive. + /// Environment values should be compared case-insensitively. + /// + [Fact] + public void IsProduction_WhenProductionLowercase_ShouldReturnTrue() + { + // Arrange - clear both env vars to ensure isolation + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "production"); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + + // Act + var result = EnvironmentDetector.IsProduction(); + + // Assert + Assert.True(result); + } + + /// + /// Verifies IsProduction is case-insensitive (uppercase). + /// + [Fact] + public void IsProduction_WhenProductionUppercase_ShouldReturnTrue() + { + // Arrange - clear both env vars to ensure isolation + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "PRODUCTION"); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + + // Act + var result = EnvironmentDetector.IsProduction(); + + // Assert + Assert.True(result); + } + + /// + /// Verifies IsProduction returns false for Development. + /// + [Fact] + public void IsProduction_WhenDevelopment_ShouldReturnFalse() + { + // Arrange - set DOTNET_ENVIRONMENT to Development (takes precedence over ASPNETCORE_ENVIRONMENT) + // Set both to Development to ensure no Production leaks from other tests + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Development"); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, string.Empty); + + // Act + var result = EnvironmentDetector.IsProduction(); + + // Assert - Development environment should not be Production + Assert.False(result); + } + + /// + /// Verifies IsProduction returns false for Staging. + /// + [Fact] + public void IsProduction_WhenStaging_ShouldReturnFalse() + { + // Arrange - clear both env vars to ensure isolation + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Staging"); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + + // Act + var result = EnvironmentDetector.IsProduction(); + + // Assert + Assert.False(result); + } + + /// + /// Verifies IsProduction returns false when no environment is set. + /// Default to Development which is not production. + /// + [Fact] + public void IsProduction_WhenNotSet_ShouldReturnFalse() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, null); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + + // Act + var result = EnvironmentDetector.IsProduction(); + + // Assert + Assert.False(result); + } + + /// + /// Verifies IsDevelopment returns true for Development environment. + /// + [Fact] + public void IsDevelopment_WhenDevelopmentSet_ShouldReturnTrue() + { + // Arrange - clear both env vars to ensure isolation + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Development"); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + + // Act + var result = EnvironmentDetector.IsDevelopment(); + + // Assert + Assert.True(result); + } + + /// + /// Verifies IsDevelopment returns true when no environment is set. + /// + [Fact] + public void IsDevelopment_WhenNotSet_ShouldReturnTrue() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, null); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + + // Act + var result = EnvironmentDetector.IsDevelopment(); + + // Assert + Assert.True(result); + } + + /// + /// Verifies IsDevelopment returns false for Production. + /// + [Fact] + public void IsDevelopment_WhenProduction_ShouldReturnFalse() + { + // Arrange - clear both env vars to ensure isolation + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Production"); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + + // Act + var result = EnvironmentDetector.IsDevelopment(); + + // Assert + Assert.False(result); + } + + /// + /// Verifies empty environment variable is treated as not set. + /// + [Fact] + public void GetEnvironment_WhenDotNetEnvIsEmpty_ShouldFallbackToAspNet() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, string.Empty); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, "Staging"); + + // Act + var result = EnvironmentDetector.GetEnvironment(); + + // Assert + Assert.Equal("Staging", result); + } + + /// + /// Verifies whitespace environment variable is treated as not set. + /// + [Fact] + public void GetEnvironment_WhenDotNetEnvIsWhitespace_ShouldFallbackToAspNet() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, " "); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, "Staging"); + + // Act + var result = EnvironmentDetector.GetEnvironment(); + + // Assert + Assert.Equal("Staging", result); + } +} diff --git a/tests/Core.Tests/Logging/LoggerExtensionsTests.cs b/tests/Core.Tests/Logging/LoggerExtensionsTests.cs new file mode 100644 index 000000000..90c404c89 --- /dev/null +++ b/tests/Core.Tests/Logging/LoggerExtensionsTests.cs @@ -0,0 +1,255 @@ +// Copyright (c) Microsoft. All rights reserved. + +using KernelMemory.Core.Logging; +using Microsoft.Extensions.Logging; +using Moq; + +namespace KernelMemory.Core.Tests.Logging; + +/// +/// Tests for LoggerExtensions - validates CallerMemberName-based method logging helpers. +/// These extensions enable automatic method name capture for entry/exit logging. +/// +public sealed class LoggerExtensionsTests +{ + private readonly Mock _mockLogger; + + /// + /// Initializes test with mock logger. + /// + public LoggerExtensionsTests() + { + this._mockLogger = new Mock(); + // Enable all log levels for testing + this._mockLogger.Setup(x => x.IsEnabled(It.IsAny())).Returns(true); + } + + /// + /// Verifies LogMethodEntry logs at Debug level. + /// Method entry should be Debug level to avoid flooding logs. + /// + [Fact] + public void LogMethodEntry_ShouldLogAtDebugLevel() + { + // Arrange & Act + this._mockLogger.Object.LogMethodEntry(); + + // Assert - verify Log was called at Debug level + this._mockLogger.Verify( + x => x.Log( + LogLevel.Debug, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>()), + Times.Once); + } + + /// + /// Verifies LogMethodEntry captures calling method name automatically. + /// CallerMemberName attribute should capture the test method name. + /// + [Fact] + public void LogMethodEntry_ShouldCaptureMethodName() + { + // Arrange + string? capturedState = null; + this._mockLogger.Setup(x => x.Log( + LogLevel.Debug, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>())) + .Callback((LogLevel level, EventId eventId, object state, Exception? ex, Delegate formatter) => + { + capturedState = state?.ToString(); + }); + + // Act + this._mockLogger.Object.LogMethodEntry(); + + // Assert - method name should be captured (this test method name) + Assert.NotNull(capturedState); + Assert.Contains("LogMethodEntry_ShouldCaptureMethodName", capturedState); + } + + /// + /// Verifies LogMethodEntry includes parameters in log output. + /// Parameters help with debugging method calls. + /// + [Fact] + public void LogMethodEntry_WithParameters_ShouldIncludeParametersInLog() + { + // Arrange + string? capturedState = null; + this._mockLogger.Setup(x => x.Log( + LogLevel.Debug, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>())) + .Callback((LogLevel level, EventId eventId, object state, Exception? ex, Delegate formatter) => + { + capturedState = state?.ToString(); + }); + + // Act + this._mockLogger.Object.LogMethodEntry("param1", 42, true); + + // Assert - parameters should be in the log + Assert.NotNull(capturedState); + Assert.Contains("param1", capturedState); + } + + /// + /// Verifies LogMethodExit logs at Debug level. + /// + [Fact] + public void LogMethodExit_ShouldLogAtDebugLevel() + { + // Arrange & Act + this._mockLogger.Object.LogMethodExit(); + + // Assert + this._mockLogger.Verify( + x => x.Log( + LogLevel.Debug, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>()), + Times.Once); + } + + /// + /// Verifies LogMethodExit captures method name. + /// + [Fact] + public void LogMethodExit_ShouldCaptureMethodName() + { + // Arrange + string? capturedState = null; + this._mockLogger.Setup(x => x.Log( + LogLevel.Debug, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>())) + .Callback((LogLevel level, EventId eventId, object state, Exception? ex, Delegate formatter) => + { + capturedState = state?.ToString(); + }); + + // Act + this._mockLogger.Object.LogMethodExit(); + + // Assert + Assert.NotNull(capturedState); + Assert.Contains("LogMethodExit_ShouldCaptureMethodName", capturedState); + } + + /// + /// Verifies LogMethodExit includes result in log output. + /// Result logging helps with debugging return values. + /// + [Fact] + public void LogMethodExit_WithResult_ShouldIncludeResultInLog() + { + // Arrange + string? capturedState = null; + this._mockLogger.Setup(x => x.Log( + LogLevel.Debug, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>())) + .Callback((LogLevel level, EventId eventId, object state, Exception? ex, Delegate formatter) => + { + capturedState = state?.ToString(); + }); + + // Act + this._mockLogger.Object.LogMethodExit("success result"); + + // Assert + Assert.NotNull(capturedState); + Assert.Contains("success result", capturedState); + } + + /// + /// Verifies LogMethodEntry with null parameters handles gracefully. + /// + [Fact] + public void LogMethodEntry_WithNullParameters_ShouldNotThrow() + { + // Arrange & Act + var exception = Record.Exception(() => + this._mockLogger.Object.LogMethodEntry(null, null, null)); + + // Assert + Assert.Null(exception); + } + + /// + /// Verifies LogMethodExit with null result handles gracefully. + /// + [Fact] + public void LogMethodExit_WithNullResult_ShouldNotThrow() + { + // Arrange & Act + var exception = Record.Exception(() => + this._mockLogger.Object.LogMethodExit(null)); + + // Assert + Assert.Null(exception); + } + + /// + /// Verifies LogMethodEntry respects log level filtering. + /// When Debug is disabled, entry logging should be skipped. + /// + [Fact] + public void LogMethodEntry_WhenDebugDisabled_ShouldNotLog() + { + // Arrange + var mockLogger = new Mock(); + mockLogger.Setup(x => x.IsEnabled(LogLevel.Debug)).Returns(false); + + // Act + mockLogger.Object.LogMethodEntry(); + + // Assert - Log should not be called when level is disabled + mockLogger.Verify( + x => x.Log( + LogLevel.Debug, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>()), + Times.Never); + } + + /// + /// Verifies LogMethodExit respects log level filtering. + /// + [Fact] + public void LogMethodExit_WhenDebugDisabled_ShouldNotLog() + { + // Arrange + var mockLogger = new Mock(); + mockLogger.Setup(x => x.IsEnabled(LogLevel.Debug)).Returns(false); + + // Act + mockLogger.Object.LogMethodExit(); + + // Assert + mockLogger.Verify( + x => x.Log( + LogLevel.Debug, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>()), + Times.Never); + } +} diff --git a/tests/Core.Tests/Logging/LoggingConfigTests.cs b/tests/Core.Tests/Logging/LoggingConfigTests.cs new file mode 100644 index 000000000..33c3e0c0e --- /dev/null +++ b/tests/Core.Tests/Logging/LoggingConfigTests.cs @@ -0,0 +1,268 @@ +// Copyright (c) Microsoft. All rights reserved. + +using KernelMemory.Core.Logging; +using Serilog.Events; + +namespace KernelMemory.Core.Tests.Logging; + +/// +/// Tests for LoggingConfig - validates logging configuration model behavior. +/// LoggingConfig stores level and file path settings for the logging system. +/// +public sealed class LoggingConfigTests +{ + /// + /// Verifies default log level is Information when not specified. + /// Information level provides good diagnostics without excessive verbosity. + /// + [Fact] + public void DefaultLevel_ShouldBeInformation() + { + // Arrange & Act + var config = new LoggingConfig(); + + // Assert + Assert.Equal(LogEventLevel.Information, config.Level); + } + + /// + /// Verifies file path is null by default (file logging disabled). + /// File logging should only be enabled when explicitly configured. + /// + [Fact] + public void DefaultFilePath_ShouldBeNull() + { + // Arrange & Act + var config = new LoggingConfig(); + + // Assert + Assert.Null(config.FilePath); + } + + /// + /// Verifies log level can be set to Verbose for detailed debugging. + /// + [Fact] + public void Level_CanBeSetToVerbose() + { + // Arrange + var config = new LoggingConfig(); + + // Act + config.Level = LogEventLevel.Verbose; + + // Assert + Assert.Equal(LogEventLevel.Verbose, config.Level); + } + + /// + /// Verifies log level can be set to Debug. + /// + [Fact] + public void Level_CanBeSetToDebug() + { + // Arrange + var config = new LoggingConfig(); + + // Act + config.Level = LogEventLevel.Debug; + + // Assert + Assert.Equal(LogEventLevel.Debug, config.Level); + } + + /// + /// Verifies log level can be set to Warning. + /// + [Fact] + public void Level_CanBeSetToWarning() + { + // Arrange + var config = new LoggingConfig(); + + // Act + config.Level = LogEventLevel.Warning; + + // Assert + Assert.Equal(LogEventLevel.Warning, config.Level); + } + + /// + /// Verifies log level can be set to Error. + /// + [Fact] + public void Level_CanBeSetToError() + { + // Arrange + var config = new LoggingConfig(); + + // Act + config.Level = LogEventLevel.Error; + + // Assert + Assert.Equal(LogEventLevel.Error, config.Level); + } + + /// + /// Verifies log level can be set to Fatal. + /// + [Fact] + public void Level_CanBeSetToFatal() + { + // Arrange + var config = new LoggingConfig(); + + // Act + config.Level = LogEventLevel.Fatal; + + // Assert + Assert.Equal(LogEventLevel.Fatal, config.Level); + } + + /// + /// Verifies file path can be set to a relative path. + /// Relative paths are resolved relative to current working directory. + /// + [Fact] + public void FilePath_CanBeSetToRelativePath() + { + // Arrange + var config = new LoggingConfig(); + + // Act + config.FilePath = "logs/app.log"; + + // Assert + Assert.Equal("logs/app.log", config.FilePath); + } + + /// + /// Verifies file path can be set to an absolute path. + /// + [Fact] + public void FilePath_CanBeSetToAbsolutePath() + { + // Arrange + var config = new LoggingConfig(); + var absolutePath = Path.Combine(Path.GetTempPath(), "km", "app.log"); + + // Act + config.FilePath = absolutePath; + + // Assert + Assert.Equal(absolutePath, config.FilePath); + } + + /// + /// Verifies UseJsonFormat is false by default (human-readable). + /// Human-readable format is better for development and CLI usage. + /// + [Fact] + public void DefaultUseJsonFormat_ShouldBeFalse() + { + // Arrange & Act + var config = new LoggingConfig(); + + // Assert + Assert.False(config.UseJsonFormat); + } + + /// + /// Verifies UseJsonFormat can be enabled for structured logging. + /// + [Fact] + public void UseJsonFormat_CanBeEnabled() + { + // Arrange + var config = new LoggingConfig(); + + // Act + config.UseJsonFormat = true; + + // Assert + Assert.True(config.UseJsonFormat); + } + + /// + /// Verifies UseAsyncLogging is false by default (sync for CLI). + /// Sync logging is better for short-lived CLI commands. + /// + [Fact] + public void DefaultUseAsyncLogging_ShouldBeFalse() + { + // Arrange & Act + var config = new LoggingConfig(); + + // Assert + Assert.False(config.UseAsyncLogging); + } + + /// + /// Verifies UseAsyncLogging can be enabled for services. + /// Async logging improves performance for long-running services. + /// + [Fact] + public void UseAsyncLogging_CanBeEnabled() + { + // Arrange + var config = new LoggingConfig(); + + // Act + config.UseAsyncLogging = true; + + // Assert + Assert.True(config.UseAsyncLogging); + } + + /// + /// Verifies IsFileLoggingEnabled returns false when FilePath is null. + /// + [Fact] + public void IsFileLoggingEnabled_WhenFilePathIsNull_ShouldReturnFalse() + { + // Arrange + var config = new LoggingConfig { FilePath = null }; + + // Act & Assert + Assert.False(config.IsFileLoggingEnabled); + } + + /// + /// Verifies IsFileLoggingEnabled returns false when FilePath is empty. + /// + [Fact] + public void IsFileLoggingEnabled_WhenFilePathIsEmpty_ShouldReturnFalse() + { + // Arrange + var config = new LoggingConfig { FilePath = string.Empty }; + + // Act & Assert + Assert.False(config.IsFileLoggingEnabled); + } + + /// + /// Verifies IsFileLoggingEnabled returns false when FilePath is whitespace. + /// + [Fact] + public void IsFileLoggingEnabled_WhenFilePathIsWhitespace_ShouldReturnFalse() + { + // Arrange + var config = new LoggingConfig { FilePath = " " }; + + // Act & Assert + Assert.False(config.IsFileLoggingEnabled); + } + + /// + /// Verifies IsFileLoggingEnabled returns true when FilePath is set. + /// + [Fact] + public void IsFileLoggingEnabled_WhenFilePathIsSet_ShouldReturnTrue() + { + // Arrange + var config = new LoggingConfig { FilePath = "logs/app.log" }; + + // Act & Assert + Assert.True(config.IsFileLoggingEnabled); + } +} diff --git a/tests/Core.Tests/Logging/LoggingConstantsTests.cs b/tests/Core.Tests/Logging/LoggingConstantsTests.cs new file mode 100644 index 000000000..0f502a210 --- /dev/null +++ b/tests/Core.Tests/Logging/LoggingConstantsTests.cs @@ -0,0 +1,122 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace KernelMemory.Core.Tests.Logging; + +/// +/// Tests for LoggingConstants - validates that all logging magic values are defined +/// and have appropriate values for the application. +/// +public sealed class LoggingConstantsTests +{ + /// + /// Verifies default log file size limit is reasonable (100MB). + /// The limit prevents excessive disk usage while allowing enough history. + /// + [Fact] + public void DefaultFileSizeLimitBytes_ShouldBe100MB() + { + // Arrange & Act + const long expectedBytes = 100 * 1024 * 1024; + + // Assert + Assert.Equal(expectedBytes, Core.Logging.LoggingConstants.DefaultFileSizeLimitBytes); + } + + /// + /// Verifies retained file count is 30 (approximately 1 month of daily logs). + /// This balances disk usage with diagnostic history requirements. + /// + [Fact] + public void DefaultRetainedFileCountLimit_ShouldBe30() + { + // Assert + Assert.Equal(30, Core.Logging.LoggingConstants.DefaultRetainedFileCountLimit); + } + + /// + /// Verifies default log level for file output is Information. + /// This provides useful diagnostics without excessive verbosity. + /// + [Fact] + public void DefaultFileLogLevel_ShouldBeInformation() + { + // Assert + Assert.Equal(Serilog.Events.LogEventLevel.Information, Core.Logging.LoggingConstants.DefaultFileLogLevel); + } + + /// + /// Verifies default log level for console stderr is Warning. + /// Only warnings and errors should appear on stderr by default. + /// + [Fact] + public void DefaultConsoleLogLevel_ShouldBeWarning() + { + // Assert + Assert.Equal(Serilog.Events.LogEventLevel.Warning, Core.Logging.LoggingConstants.DefaultConsoleLogLevel); + } + + /// + /// Verifies environment variable name for .NET environment detection. + /// + [Fact] + public void DotNetEnvironmentVariable_ShouldBeDefined() + { + // Assert + Assert.Equal("DOTNET_ENVIRONMENT", Core.Logging.LoggingConstants.DotNetEnvironmentVariable); + } + + /// + /// Verifies fallback environment variable name for ASP.NET Core. + /// + [Fact] + public void AspNetCoreEnvironmentVariable_ShouldBeDefined() + { + // Assert + Assert.Equal("ASPNETCORE_ENVIRONMENT", Core.Logging.LoggingConstants.AspNetCoreEnvironmentVariable); + } + + /// + /// Verifies default environment is Development when none is set. + /// + [Fact] + public void DefaultEnvironment_ShouldBeDevelopment() + { + // Assert + Assert.Equal("Development", Core.Logging.LoggingConstants.DefaultEnvironment); + } + + /// + /// Verifies Production environment name constant. + /// + [Fact] + public void ProductionEnvironment_ShouldBeDefined() + { + // Assert + Assert.Equal("Production", Core.Logging.LoggingConstants.ProductionEnvironment); + } + + /// + /// Verifies the redacted placeholder text for sensitive data scrubbing. + /// + [Fact] + public void RedactedPlaceholder_ShouldBeDefined() + { + // Assert + Assert.Equal("[REDACTED]", Core.Logging.LoggingConstants.RedactedPlaceholder); + } + + /// + /// Verifies the output template for human-readable logs. + /// + [Fact] + public void HumanReadableOutputTemplate_ShouldContainTimestampAndLevel() + { + // Arrange & Act + const string template = Core.Logging.LoggingConstants.HumanReadableOutputTemplate; + + // Assert - template should contain key elements + Assert.Contains("{Timestamp", template); + Assert.Contains("{Level", template); + Assert.Contains("{Message", template); + } +} diff --git a/tests/Core.Tests/Logging/SensitiveDataScrubbingPolicyTests.cs b/tests/Core.Tests/Logging/SensitiveDataScrubbingPolicyTests.cs new file mode 100644 index 000000000..b8d9d6ca2 --- /dev/null +++ b/tests/Core.Tests/Logging/SensitiveDataScrubbingPolicyTests.cs @@ -0,0 +1,245 @@ +// Copyright (c) Microsoft. All rights reserved. + +using KernelMemory.Core.Logging; +using Serilog.Core; +using Serilog.Events; + +namespace KernelMemory.Core.Tests.Logging; + +/// +/// Tests for SensitiveDataScrubbingPolicy - validates sensitive data protection. +/// In Production environment, all string parameters must be scrubbed to prevent data leakage. +/// In Development/Staging, full logging is allowed for debugging. +/// +public sealed class SensitiveDataScrubbingPolicyTests : IDisposable +{ + private readonly string? _originalDotNetEnv; + private readonly SensitiveDataScrubbingPolicy _policy; + + /// + /// Initializes test capturing original environment. + /// + public SensitiveDataScrubbingPolicyTests() + { + this._originalDotNetEnv = Environment.GetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable); + this._policy = new SensitiveDataScrubbingPolicy(); + } + + /// + /// Restores original environment after test. + /// + public void Dispose() + { + if (this._originalDotNetEnv != null) + { + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, this._originalDotNetEnv); + } + else + { + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, null); + } + + GC.SuppressFinalize(this); + } + + /// + /// Verifies strings are scrubbed in Production environment. + /// This is critical for preventing sensitive data in production logs. + /// + [Fact] + public void TryDestructure_WhenProductionAndString_ShouldScrub() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Production"); + const string sensitiveValue = "secret-api-key-12345"; + + // Act + var handled = this._policy.TryDestructure(sensitiveValue, new TestPropertyValueFactory(), out var result); + + // Assert + Assert.True(handled); + Assert.NotNull(result); + Assert.IsType(result); + Assert.Equal(LoggingConstants.RedactedPlaceholder, ((ScalarValue)result).Value); + } + + /// + /// Verifies strings are NOT scrubbed in Development environment. + /// Development needs full logging for debugging. + /// + [Fact] + public void TryDestructure_WhenDevelopmentAndString_ShouldNotScrub() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Development"); + const string value = "test-value"; + + // Act + var handled = this._policy.TryDestructure(value, new TestPropertyValueFactory(), out var result); + + // Assert - not handled means Serilog will process normally + Assert.False(handled); + Assert.Null(result); + } + + /// + /// Verifies strings are NOT scrubbed when no environment is set. + /// Default to Development behavior (full logging). + /// + [Fact] + public void TryDestructure_WhenNoEnvironmentAndString_ShouldNotScrub() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, null); + Environment.SetEnvironmentVariable(LoggingConstants.AspNetCoreEnvironmentVariable, null); + const string value = "test-value"; + + // Act + var handled = this._policy.TryDestructure(value, new TestPropertyValueFactory(), out var result); + + // Assert + Assert.False(handled); + Assert.Null(result); + } + + /// + /// Verifies non-string types are NOT scrubbed even in Production. + /// Only strings are considered potentially sensitive. + /// + [Fact] + public void TryDestructure_WhenProductionAndInteger_ShouldNotScrub() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Production"); + + // Act + var handled = this._policy.TryDestructure(42, new TestPropertyValueFactory(), out var result); + + // Assert + Assert.False(handled); + Assert.Null(result); + } + + /// + /// Verifies DateTime is NOT scrubbed. + /// Timestamps are not sensitive data. + /// + [Fact] + public void TryDestructure_WhenProductionAndDateTime_ShouldNotScrub() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Production"); + var dateTime = DateTimeOffset.UtcNow; + + // Act + var handled = this._policy.TryDestructure(dateTime, new TestPropertyValueFactory(), out var result); + + // Assert + Assert.False(handled); + Assert.Null(result); + } + + /// + /// Verifies booleans are NOT scrubbed. + /// Boolean values are not sensitive. + /// + [Fact] + public void TryDestructure_WhenProductionAndBoolean_ShouldNotScrub() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Production"); + + // Act + var handled = this._policy.TryDestructure(true, new TestPropertyValueFactory(), out var result); + + // Assert + Assert.False(handled); + Assert.Null(result); + } + + /// + /// Verifies Guids are NOT scrubbed. + /// IDs are typically logged for correlation and not considered sensitive. + /// + [Fact] + public void TryDestructure_WhenProductionAndGuid_ShouldNotScrub() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Production"); + var guid = Guid.NewGuid(); + + // Act + var handled = this._policy.TryDestructure(guid, new TestPropertyValueFactory(), out var result); + + // Assert + Assert.False(handled); + Assert.Null(result); + } + + /// + /// Verifies empty strings are still scrubbed in Production. + /// Even empty strings should be hidden to prevent information leakage. + /// + [Fact] + public void TryDestructure_WhenProductionAndEmptyString_ShouldScrub() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Production"); + + // Act + var handled = this._policy.TryDestructure(string.Empty, new TestPropertyValueFactory(), out var result); + + // Assert + Assert.True(handled); + Assert.NotNull(result); + Assert.Equal(LoggingConstants.RedactedPlaceholder, ((ScalarValue)result).Value); + } + + /// + /// Verifies null values are NOT scrubbed. + /// Null doesn't contain data to protect. + /// + [Fact] + public void TryDestructure_WhenProductionAndNull_ShouldNotScrub() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Production"); + + // Act + var handled = this._policy.TryDestructure(null!, new TestPropertyValueFactory(), out var result); + + // Assert + Assert.False(handled); + Assert.Null(result); + } + + /// + /// Verifies strings are NOT scrubbed in Staging environment. + /// Staging is not Production, so full logging is allowed. + /// + [Fact] + public void TryDestructure_WhenStagingAndString_ShouldNotScrub() + { + // Arrange + Environment.SetEnvironmentVariable(LoggingConstants.DotNetEnvironmentVariable, "Staging"); + const string value = "test-value"; + + // Act + var handled = this._policy.TryDestructure(value, new TestPropertyValueFactory(), out var result); + + // Assert + Assert.False(handled); + Assert.Null(result); + } + + /// + /// Test implementation of ILogEventPropertyValueFactory for unit testing. + /// + private sealed class TestPropertyValueFactory : ILogEventPropertyValueFactory + { + public LogEventPropertyValue CreatePropertyValue(object? value, bool destructureObjects = false) + { + return new ScalarValue(value); + } + } +} diff --git a/tests/Core.Tests/Logging/SerilogFactoryTests.cs b/tests/Core.Tests/Logging/SerilogFactoryTests.cs new file mode 100644 index 000000000..c772058bb --- /dev/null +++ b/tests/Core.Tests/Logging/SerilogFactoryTests.cs @@ -0,0 +1,323 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Diagnostics.CodeAnalysis; +using KernelMemory.Core.Logging; +using Microsoft.Extensions.Logging; +using Serilog.Events; + +namespace KernelMemory.Core.Tests.Logging; + +/// +/// Tests for SerilogFactory - validates logger creation with various configurations. +/// SerilogFactory creates properly configured Serilog loggers with Microsoft.Extensions.Logging integration. +/// +public sealed class SerilogFactoryTests : IDisposable +{ + private readonly string _tempDir; + + /// + /// Initializes test with temp directory for file logging tests. + /// + public SerilogFactoryTests() + { + this._tempDir = Path.Combine(Path.GetTempPath(), $"km-test-{Guid.NewGuid()}"); + Directory.CreateDirectory(this._tempDir); + } + + /// + /// Cleans up temp directory after tests. + /// + [SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Cleanup errors should not fail tests")] + public void Dispose() + { + try + { + if (Directory.Exists(this._tempDir)) + { + Directory.Delete(this._tempDir, recursive: true); + } + } + catch (IOException) + { + // Ignore cleanup errors in tests - files may be locked + } + + GC.SuppressFinalize(this); + } + + /// + /// Verifies CreateLoggerFactory returns non-null factory. + /// + [Fact] + public void CreateLoggerFactory_WithDefaultConfig_ShouldReturnFactory() + { + // Arrange + var config = new LoggingConfig(); + + // Act + using var factory = SerilogFactory.CreateLoggerFactory(config); + + // Assert + Assert.NotNull(factory); + } + + /// + /// Verifies factory can create typed loggers. + /// + [Fact] + public void CreateLoggerFactory_ShouldCreateTypedLogger() + { + // Arrange + var config = new LoggingConfig(); + + // Act + using var factory = SerilogFactory.CreateLoggerFactory(config); + var logger = factory.CreateLogger(); + + // Assert + Assert.NotNull(logger); + } + + /// + /// Verifies logger respects minimum level from config. + /// + [Fact] + public void CreateLoggerFactory_WithWarningLevel_ShouldFilterDebugLogs() + { + // Arrange + var config = new LoggingConfig + { + Level = LogEventLevel.Warning + }; + + // Act + using var factory = SerilogFactory.CreateLoggerFactory(config); + var logger = factory.CreateLogger(); + + // Assert - Debug should be disabled when minimum level is Warning + Assert.False(logger.IsEnabled(LogLevel.Debug)); + Assert.True(logger.IsEnabled(LogLevel.Warning)); + Assert.True(logger.IsEnabled(LogLevel.Error)); + } + + /// + /// Verifies logger enables all levels when set to Verbose. + /// + [Fact] + public void CreateLoggerFactory_WithVerboseLevel_ShouldEnableAllLevels() + { + // Arrange + var config = new LoggingConfig + { + Level = LogEventLevel.Verbose + }; + + // Act + using var factory = SerilogFactory.CreateLoggerFactory(config); + var logger = factory.CreateLogger(); + + // Assert + Assert.True(logger.IsEnabled(LogLevel.Trace)); + Assert.True(logger.IsEnabled(LogLevel.Debug)); + Assert.True(logger.IsEnabled(LogLevel.Information)); + } + + /// + /// Verifies file logging creates log file when configured. + /// + [Fact] + public void CreateLoggerFactory_WithFilePath_ShouldCreateLogFile() + { + // Arrange + var logPath = Path.Combine(this._tempDir, "test.log"); + var config = new LoggingConfig + { + FilePath = logPath + }; + + // Act + using var factory = SerilogFactory.CreateLoggerFactory(config); + var logger = factory.CreateLogger(); + logger.LogInformation("Test message for file creation"); + + // Force flush by disposing + factory.Dispose(); + + // Assert - file should exist (Serilog may add date suffix) + var logFiles = Directory.GetFiles(this._tempDir, "test*.log"); + Assert.NotEmpty(logFiles); + } + + /// + /// Verifies file logging writes messages to file. + /// + [Fact] + [SuppressMessage("Usage", "CA2254:Template should be a static expression", Justification = "Test validates unique message content")] + public void CreateLoggerFactory_WithFilePath_ShouldWriteToFile() + { + // Arrange + var logPath = Path.Combine(this._tempDir, "messages.log"); + var config = new LoggingConfig + { + FilePath = logPath, + Level = LogEventLevel.Information + }; + var testMessage = $"Test message {Guid.NewGuid()}"; + + // Act + using (var factory = SerilogFactory.CreateLoggerFactory(config)) + { + var logger = factory.CreateLogger(); + logger.LogInformation(testMessage); + } + + // Assert + var logFiles = Directory.GetFiles(this._tempDir, "messages*.log"); + Assert.NotEmpty(logFiles); + + var content = File.ReadAllText(logFiles[0]); + Assert.Contains(testMessage, content); + } + + /// + /// Verifies JSON format writes structured log entries. + /// + [Fact] + public void CreateLoggerFactory_WithJsonFormat_ShouldWriteJsonToFile() + { + // Arrange + var logPath = Path.Combine(this._tempDir, "json.log"); + var config = new LoggingConfig + { + FilePath = logPath, + UseJsonFormat = true, + Level = LogEventLevel.Information + }; + + // Act + using (var factory = SerilogFactory.CreateLoggerFactory(config)) + { + var logger = factory.CreateLogger(); + logger.LogInformation("JSON test message"); + } + + // Assert + var logFiles = Directory.GetFiles(this._tempDir, "json*.log"); + Assert.NotEmpty(logFiles); + + var content = File.ReadAllText(logFiles[0]); + // JSON format should contain these markers + Assert.Contains("{", content); + Assert.Contains("\"", content); + } + + /// + /// Verifies logger factory can be disposed multiple times without error. + /// + [Fact] + public void CreateLoggerFactory_DisposeTwice_ShouldNotThrow() + { + // Arrange + var config = new LoggingConfig(); + var factory = SerilogFactory.CreateLoggerFactory(config); + + // Act + factory.Dispose(); + var exception = Record.Exception(() => factory.Dispose()); + + // Assert + Assert.Null(exception); + } + + /// + /// Verifies CreateLogger extension method creates typed logger. + /// + [Fact] + public void CreateLogger_WithType_ShouldReturnTypedLogger() + { + // Arrange + var config = new LoggingConfig(); + + // Act + using var factory = SerilogFactory.CreateLoggerFactory(config); + var logger = factory.CreateLogger(); + + // Assert + Assert.NotNull(logger); + } + + /// + /// Verifies file logging creates directory if it doesn't exist. + /// + [Fact] + public void CreateLoggerFactory_WithNestedFilePath_ShouldCreateDirectory() + { + // Arrange + var logPath = Path.Combine(this._tempDir, "nested", "dir", "test.log"); + var config = new LoggingConfig + { + FilePath = logPath + }; + + // Act + using var factory = SerilogFactory.CreateLoggerFactory(config); + var logger = factory.CreateLogger(); + logger.LogInformation("Test"); + + // Force flush + factory.Dispose(); + + // Assert - nested directory should be created + Assert.True(Directory.Exists(Path.GetDirectoryName(logPath))); + } + + /// + /// Verifies async logging option is respected. + /// + [Fact] + public void CreateLoggerFactory_WithAsyncLogging_ShouldNotThrow() + { + // Arrange + var logPath = Path.Combine(this._tempDir, "async.log"); + var config = new LoggingConfig + { + FilePath = logPath, + UseAsyncLogging = true + }; + + // Act + var exception = Record.Exception(() => + { + using var factory = SerilogFactory.CreateLoggerFactory(config); + var logger = factory.CreateLogger(); + logger.LogInformation("Async test"); + }); + + // Assert + Assert.Null(exception); + } + + /// + /// Verifies error level config filters out lower levels. + /// + [Fact] + public void CreateLoggerFactory_WithErrorLevel_ShouldFilterLowerLevels() + { + // Arrange + var config = new LoggingConfig + { + Level = LogEventLevel.Error + }; + + // Act + using var factory = SerilogFactory.CreateLoggerFactory(config); + var logger = factory.CreateLogger(); + + // Assert + Assert.False(logger.IsEnabled(LogLevel.Debug)); + Assert.False(logger.IsEnabled(LogLevel.Information)); + Assert.False(logger.IsEnabled(LogLevel.Warning)); + Assert.True(logger.IsEnabled(LogLevel.Error)); + Assert.True(logger.IsEnabled(LogLevel.Critical)); + } +} diff --git a/tests/Core.Tests/Logging/TestLoggerFactory.cs b/tests/Core.Tests/Logging/TestLoggerFactory.cs new file mode 100644 index 000000000..5db62384a --- /dev/null +++ b/tests/Core.Tests/Logging/TestLoggerFactory.cs @@ -0,0 +1,64 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using KernelMemory.Core.Logging; +using Microsoft.Extensions.Logging; +using Serilog; +using Serilog.Extensions.Logging; +using Xunit.Abstractions; + +namespace KernelMemory.Core.Tests.Logging; + +/// +/// Factory for creating loggers that output to XUnit test output. +/// Enables visibility of log messages in test output for debugging failures. +/// This class is in the tests project because it depends on XUnit. +/// +public static class TestLoggerFactory +{ + /// + /// Creates a typed logger that writes to XUnit test output. + /// Use in tests to capture log output for debugging. + /// + /// The type to create the logger for (provides SourceContext). + /// XUnit test output helper from the test constructor. + /// A configured ILogger of T that writes to test output. + /// Thrown when output is null. + [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Factory ownership transfers to caller via returned logger")] + public static ILogger Create(ITestOutputHelper output) + { + ArgumentNullException.ThrowIfNull(output); + + var serilogLogger = new LoggerConfiguration() + .MinimumLevel.Verbose() // Enable all levels for test debugging + .Enrich.FromLogContext() + .Enrich.With() + .WriteTo.TestOutput(output, formatProvider: CultureInfo.InvariantCulture) + .CreateLogger(); + + var factory = new SerilogLoggerFactory(serilogLogger, dispose: true); + return factory.CreateLogger(); + } + + /// + /// Creates an ILoggerFactory that writes to XUnit test output. + /// Use when you need to create multiple loggers or inject a factory. + /// + /// XUnit test output helper from the test constructor. + /// A configured ILoggerFactory that writes to test output. + /// Thrown when output is null. + public static ILoggerFactory CreateLoggerFactory(ITestOutputHelper output) + { + ArgumentNullException.ThrowIfNull(output); + + var serilogLogger = new LoggerConfiguration() + .MinimumLevel.Verbose() + .Enrich.FromLogContext() + .Enrich.With() + .WriteTo.TestOutput(output, formatProvider: CultureInfo.InvariantCulture) + .CreateLogger(); + + return new SerilogLoggerFactory(serilogLogger, dispose: true); + } +} diff --git a/tests/Core.Tests/Logging/TestLoggerFactoryTests.cs b/tests/Core.Tests/Logging/TestLoggerFactoryTests.cs new file mode 100644 index 000000000..cc1f2eb1f --- /dev/null +++ b/tests/Core.Tests/Logging/TestLoggerFactoryTests.cs @@ -0,0 +1,199 @@ +// Copyright (c) Microsoft. All rights reserved. + +using KernelMemory.Core.Logging; +using Microsoft.Extensions.Logging; +using Xunit.Abstractions; + +namespace KernelMemory.Core.Tests.Logging; + +/// +/// Tests for TestLoggerFactory - validates XUnit output integration for tests. +/// TestLoggerFactory enables test log output visibility for debugging test failures. +/// +public sealed class TestLoggerFactoryTests +{ + private readonly ITestOutputHelper _output; + + /// + /// Initializes test with XUnit output helper. + /// + /// XUnit test output helper. + public TestLoggerFactoryTests(ITestOutputHelper output) + { + this._output = output; + } + + /// + /// Verifies Create returns non-null typed logger. + /// + [Fact] + public void Create_ShouldReturnTypedLogger() + { + // Act + var logger = TestLoggerFactory.Create(this._output); + + // Assert + Assert.NotNull(logger); + } + + /// + /// Verifies logger can log messages without throwing. + /// + [Fact] + public void Create_ShouldCreateFunctionalLogger() + { + // Arrange + var logger = TestLoggerFactory.Create(this._output); + + // Act + var exception = Record.Exception(() => + { + logger.LogDebug("Debug message"); + logger.LogInformation("Info message"); + logger.LogWarning("Warning message"); + logger.LogError("Error message"); + }); + + // Assert + Assert.Null(exception); + } + + /// + /// Verifies logger enables Debug level by default for detailed test output. + /// + [Fact] + public void Create_ShouldEnableDebugLevel() + { + // Arrange + var logger = TestLoggerFactory.Create(this._output); + + // Assert + Assert.True(logger.IsEnabled(LogLevel.Debug)); + } + + /// + /// Verifies logger enables Trace level for maximum verbosity in tests. + /// + [Fact] + public void Create_ShouldEnableTraceLevel() + { + // Arrange + var logger = TestLoggerFactory.Create(this._output); + + // Assert + Assert.True(logger.IsEnabled(LogLevel.Trace)); + } + + /// + /// Verifies logger can log structured data. + /// + [Fact] + public void Create_ShouldSupportStructuredLogging() + { + // Arrange + var logger = TestLoggerFactory.Create(this._output); + + // Act - log with structured parameters + var exception = Record.Exception(() => + { + logger.LogInformation("Processing {ItemCount} items for {UserId}", 42, "user-123"); + }); + + // Assert + Assert.Null(exception); + } + + /// + /// Verifies logger can log exceptions with stack traces. + /// + [Fact] + public void Create_ShouldLogExceptionsWithStackTrace() + { + // Arrange + var logger = TestLoggerFactory.Create(this._output); + var testException = new InvalidOperationException("Test exception"); + + // Act + var exception = Record.Exception(() => + { + logger.LogError(testException, "An error occurred"); + }); + + // Assert + Assert.Null(exception); + } + + /// + /// Verifies CreateLoggerFactory returns functional factory. + /// + [Fact] + public void CreateLoggerFactory_ShouldReturnFunctionalFactory() + { + // Act + using var factory = TestLoggerFactory.CreateLoggerFactory(this._output); + + // Assert + Assert.NotNull(factory); + + var logger = factory.CreateLogger(); + Assert.NotNull(logger); + } + + /// + /// Verifies factory can create multiple loggers for different types. + /// + [Fact] + public void CreateLoggerFactory_ShouldCreateMultipleLoggers() + { + // Arrange + using var factory = TestLoggerFactory.CreateLoggerFactory(this._output); + + // Act + var logger1 = factory.CreateLogger(); + var logger2 = factory.CreateLogger(); + + // Assert + Assert.NotNull(logger1); + Assert.NotNull(logger2); + Assert.NotSame(logger1, logger2); + } + + /// + /// Verifies logger output includes class name from ILogger of T. + /// + [Fact] + public void Create_ShouldIncludeClassName() + { + // Arrange + var logger = TestLoggerFactory.Create(this._output); + + // Act - the logger should automatically include the class name + // This test passes if no exception occurs and output is visible + logger.LogInformation("Message from typed logger"); + + // Assert - logging works (class name visible in output) + Assert.True(true); + } + + /// + /// Verifies null output helper throws appropriate exception. + /// + [Fact] + public void Create_WithNullOutput_ShouldThrowArgumentNullException() + { + // Act & Assert + Assert.Throws(() => + TestLoggerFactory.Create(null!)); + } + + /// + /// Verifies CreateLoggerFactory with null output throws. + /// + [Fact] + public void CreateLoggerFactory_WithNullOutput_ShouldThrowArgumentNullException() + { + // Act & Assert + Assert.Throws(() => + TestLoggerFactory.CreateLoggerFactory(null!)); + } +} diff --git a/tests/Directory.Packages.props b/tests/Directory.Packages.props index cb833b6cd..aeba656f5 100644 --- a/tests/Directory.Packages.props +++ b/tests/Directory.Packages.props @@ -9,6 +9,11 @@ + + + + + diff --git a/tests/Main.Tests/Integration/CliIntegrationTests.cs b/tests/Main.Tests/Integration/CliIntegrationTests.cs index 1a87eaa4b..6b063c329 100644 --- a/tests/Main.Tests/Integration/CliIntegrationTests.cs +++ b/tests/Main.Tests/Integration/CliIntegrationTests.cs @@ -3,6 +3,7 @@ using KernelMemory.Core.Config; using KernelMemory.Core.Config.ContentIndex; using KernelMemory.Main.CLI.Commands; +using Microsoft.Extensions.Logging.Abstractions; using Spectre.Console.Cli; namespace KernelMemory.Main.Tests.Integration; @@ -70,7 +71,7 @@ public async Task UpsertCommand_WithMinimalOptions_CreatesContent() Content = "Test content" }; - var command = new UpsertCommand(config); + var command = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); // Act @@ -95,7 +96,7 @@ public async Task UpsertCommand_WithCustomId_UsesProvidedId() Id = customId }; - var command = new UpsertCommand(config); + var command = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); // Act @@ -112,7 +113,7 @@ public async Task UpsertCommand_WithCustomId_UsesProvidedId() Id = customId }; - var getCommand = new GetCommand(config); + var getCommand = new GetCommand(config, NullLoggerFactory.Instance); var getExitCode = await getCommand.ExecuteAsync(context, getSettings, CancellationToken.None).ConfigureAwait(false); Assert.Equal(Constants.ExitCodeSuccess, getExitCode); } @@ -133,7 +134,7 @@ public async Task UpsertCommand_WithAllMetadata_StoresAllFields() MimeType = "text/markdown" }; - var command = new UpsertCommand(config); + var command = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); // Act @@ -157,7 +158,7 @@ public async Task GetCommand_ExistingId_ReturnsContent() Id = customId }; - var upsertCommand = new UpsertCommand(config); + var upsertCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings, CancellationToken.None).ConfigureAwait(false); @@ -169,7 +170,7 @@ public async Task GetCommand_ExistingId_ReturnsContent() Id = customId }; - var getCommand = new GetCommand(config); + var getCommand = new GetCommand(config, NullLoggerFactory.Instance); var exitCode = await getCommand.ExecuteAsync(context, getSettings, CancellationToken.None).ConfigureAwait(false); // Assert @@ -189,7 +190,7 @@ public async Task GetCommand_NonExistentId_ReturnsUserError() Format = "json", Content = "Some content to create the DB" }; - var upsertCommand = new UpsertCommand(config); + var upsertCommand = new UpsertCommand(config, NullLoggerFactory.Instance); await upsertCommand.ExecuteAsync(CreateTestContext("put"), upsertSettings, CancellationToken.None).ConfigureAwait(false); // Now try to get non-existent ID from existing DB @@ -200,7 +201,7 @@ public async Task GetCommand_NonExistentId_ReturnsUserError() Id = "non-existent-id-12345" }; - var command = new GetCommand(config); + var command = new GetCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("get"); // Act @@ -225,7 +226,7 @@ public async Task GetCommand_WithFullFlag_ReturnsAllDetails() Title = "Full Title" }; - var upsertCommand = new UpsertCommand(config); + var upsertCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings, CancellationToken.None).ConfigureAwait(false); @@ -238,7 +239,7 @@ public async Task GetCommand_WithFullFlag_ReturnsAllDetails() ShowFull = true }; - var getCommand = new GetCommand(config); + var getCommand = new GetCommand(config, NullLoggerFactory.Instance); var exitCode = await getCommand.ExecuteAsync(context, getSettings, CancellationToken.None).ConfigureAwait(false); // Assert @@ -257,7 +258,7 @@ public async Task ListCommand_EmptyDatabase_ReturnsEmptyList() Content = "Temporary content to create database", Id = "temp-id" }; - var upsertCommand = new UpsertCommand(config); + var upsertCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings, CancellationToken.None).ConfigureAwait(false); @@ -268,7 +269,7 @@ public async Task ListCommand_EmptyDatabase_ReturnsEmptyList() Format = "json", Id = "temp-id" }; - var deleteCommand = new DeleteCommand(config); + var deleteCommand = new DeleteCommand(config, NullLoggerFactory.Instance); await deleteCommand.ExecuteAsync(context, deleteSettings, CancellationToken.None).ConfigureAwait(false); // Now test list on empty database @@ -278,7 +279,7 @@ public async Task ListCommand_EmptyDatabase_ReturnsEmptyList() Format = "json" }; - var command = new ListCommand(config); + var command = new ListCommand(config, NullLoggerFactory.Instance); var listContext = CreateTestContext("list"); // Act @@ -304,7 +305,7 @@ public async Task Bug3_ListCommand_EmptyDatabase_HumanFormat_ShouldHandleGracefu Content = "Temporary content to create database", Id = "temp-id-human" }; - var upsertCommand = new UpsertCommand(config); + var upsertCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings, CancellationToken.None).ConfigureAwait(false); @@ -315,7 +316,7 @@ public async Task Bug3_ListCommand_EmptyDatabase_HumanFormat_ShouldHandleGracefu Format = "json", Id = "temp-id-human" }; - var deleteCommand = new DeleteCommand(config); + var deleteCommand = new DeleteCommand(config, NullLoggerFactory.Instance); await deleteCommand.ExecuteAsync(context, deleteSettings, CancellationToken.None).ConfigureAwait(false); // Now test list on empty database with human format @@ -325,7 +326,7 @@ public async Task Bug3_ListCommand_EmptyDatabase_HumanFormat_ShouldHandleGracefu Format = "human" // Test human format, not just JSON }; - var command = new ListCommand(config); + var command = new ListCommand(config, NullLoggerFactory.Instance); var listContext = CreateTestContext("list"); // Act @@ -349,7 +350,7 @@ public async Task ListCommand_WithContent_ReturnsList() Content = "List test content" }; - var upsertCommand = new UpsertCommand(config); + var upsertCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings, CancellationToken.None).ConfigureAwait(false); @@ -360,7 +361,7 @@ public async Task ListCommand_WithContent_ReturnsList() Format = "json" }; - var listCommand = new ListCommand(config); + var listCommand = new ListCommand(config, NullLoggerFactory.Instance); var exitCode = await listCommand.ExecuteAsync(context, listSettings, CancellationToken.None).ConfigureAwait(false); // Assert @@ -372,7 +373,7 @@ public async Task ListCommand_WithPagination_RespectsSkipAndTake() { // Arrange - Insert multiple items var config = ConfigParser.LoadFromFile(this._configPath); - var upsertCommand = new UpsertCommand(config); + var upsertCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); for (int i = 0; i < 5; i++) @@ -395,7 +396,7 @@ public async Task ListCommand_WithPagination_RespectsSkipAndTake() Take = 2 }; - var listCommand = new ListCommand(config); + var listCommand = new ListCommand(config, NullLoggerFactory.Instance); var exitCode = await listCommand.ExecuteAsync(context, listSettings, CancellationToken.None).ConfigureAwait(false); // Assert @@ -416,7 +417,7 @@ public async Task DeleteCommand_ExistingId_DeletesSuccessfully() Id = customId }; - var upsertCommand = new UpsertCommand(config); + var upsertCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings, CancellationToken.None).ConfigureAwait(false); @@ -428,7 +429,7 @@ public async Task DeleteCommand_ExistingId_DeletesSuccessfully() Id = customId }; - var deleteCommand = new DeleteCommand(config); + var deleteCommand = new DeleteCommand(config, NullLoggerFactory.Instance); var exitCode = await deleteCommand.ExecuteAsync(context, deleteSettings, CancellationToken.None).ConfigureAwait(false); // Assert @@ -442,7 +443,7 @@ public async Task DeleteCommand_ExistingId_DeletesSuccessfully() Id = customId }; - var getCommand = new GetCommand(config); + var getCommand = new GetCommand(config, NullLoggerFactory.Instance); var getExitCode = await getCommand.ExecuteAsync(context, getSettings, CancellationToken.None).ConfigureAwait(false); Assert.Equal(Constants.ExitCodeUserError, getExitCode); } @@ -461,7 +462,7 @@ public async Task DeleteCommand_WithQuietVerbosity_SucceedsWithMinimalOutput() Id = customId }; - var upsertCommand = new UpsertCommand(config); + var upsertCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("put"); await upsertCommand.ExecuteAsync(context, upsertSettings, CancellationToken.None).ConfigureAwait(false); @@ -474,7 +475,7 @@ public async Task DeleteCommand_WithQuietVerbosity_SucceedsWithMinimalOutput() Id = customId }; - var deleteCommand = new DeleteCommand(config); + var deleteCommand = new DeleteCommand(config, NullLoggerFactory.Instance); var exitCode = await deleteCommand.ExecuteAsync(context, deleteSettings, CancellationToken.None).ConfigureAwait(false); // Assert @@ -498,7 +499,7 @@ public async Task EndToEndWorkflow_UpsertGetListDelete_AllSucceed() Id = testId, Tags = "e2e,test" }; - var upsertCommand = new UpsertCommand(config); + var upsertCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var upsertExitCode = await upsertCommand.ExecuteAsync(context, upsertSettings, CancellationToken.None).ConfigureAwait(false); Assert.Equal(Constants.ExitCodeSuccess, upsertExitCode); @@ -509,7 +510,7 @@ public async Task EndToEndWorkflow_UpsertGetListDelete_AllSucceed() Format = "json", Id = testId }; - var getCommand = new GetCommand(config); + var getCommand = new GetCommand(config, NullLoggerFactory.Instance); var getExitCode = await getCommand.ExecuteAsync(context, getSettings, CancellationToken.None).ConfigureAwait(false); Assert.Equal(Constants.ExitCodeSuccess, getExitCode); @@ -519,7 +520,7 @@ public async Task EndToEndWorkflow_UpsertGetListDelete_AllSucceed() ConfigPath = this._configPath, Format = "json" }; - var listCommand = new ListCommand(config); + var listCommand = new ListCommand(config, NullLoggerFactory.Instance); var listExitCode = await listCommand.ExecuteAsync(context, listSettings, CancellationToken.None).ConfigureAwait(false); Assert.Equal(Constants.ExitCodeSuccess, listExitCode); @@ -530,7 +531,7 @@ public async Task EndToEndWorkflow_UpsertGetListDelete_AllSucceed() Format = "json", Id = testId }; - var deleteCommand = new DeleteCommand(config); + var deleteCommand = new DeleteCommand(config, NullLoggerFactory.Instance); var deleteExitCode = await deleteCommand.ExecuteAsync(context, deleteSettings, CancellationToken.None).ConfigureAwait(false); Assert.Equal(Constants.ExitCodeSuccess, deleteExitCode); @@ -550,7 +551,7 @@ public async Task NodesCommand_WithJsonFormat_ListsAllNodes() Format = "json" }; - var command = new NodesCommand(config); + var command = new NodesCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("nodes"); // Act @@ -571,7 +572,7 @@ public async Task NodesCommand_WithYamlFormat_ListsAllNodes() Format = "yaml" }; - var command = new NodesCommand(config); + var command = new NodesCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("nodes"); // Act @@ -593,7 +594,7 @@ public async Task ConfigCommand_Default_ShowsCurrentNode() }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = CreateTestContext("config"); // Act @@ -616,7 +617,7 @@ public async Task ConfigCommand_WithShowNodes_ShowsAllNodes() }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = CreateTestContext("config"); // Act @@ -639,7 +640,7 @@ public async Task ConfigCommand_WithShowCache_ShowsCacheConfig() }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = CreateTestContext("config"); // Act @@ -692,7 +693,7 @@ public async Task Bug2_ConfigCommand_HumanFormat_ShouldNotLeakTypeNames() }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = CreateTestContext("config"); // Act diff --git a/tests/Main.Tests/Integration/CommandExecutionTests.cs b/tests/Main.Tests/Integration/CommandExecutionTests.cs index ddf743678..fd0cdc760 100644 --- a/tests/Main.Tests/Integration/CommandExecutionTests.cs +++ b/tests/Main.Tests/Integration/CommandExecutionTests.cs @@ -2,6 +2,7 @@ using KernelMemory.Core.Config; using KernelMemory.Main.CLI.Commands; +using Microsoft.Extensions.Logging.Abstractions; using Spectre.Console.Cli; namespace KernelMemory.Main.Tests.Integration; @@ -64,7 +65,7 @@ public async Task UpsertCommand_WithValidContent_ReturnsSuccess() ConfigPath = this._configPath, Content = "Test content" }; - var command = new UpsertCommand(config); + var command = new UpsertCommand(config, NullLoggerFactory.Instance); var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "put", null); var result = await command.ExecuteAsync(context, settings, CancellationToken.None).ConfigureAwait(false); @@ -83,7 +84,7 @@ public async Task GetCommand_WithNonExistentId_ReturnsError() ConfigPath = this._configPath, Content = "Test content to create DB" }; - var putCommand = new UpsertCommand(config); + var putCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var putContext = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "put", null); await putCommand.ExecuteAsync(putContext, putSettings, CancellationToken.None).ConfigureAwait(false); @@ -93,7 +94,7 @@ public async Task GetCommand_WithNonExistentId_ReturnsError() ConfigPath = this._configPath, Id = "nonexistent-id-12345" }; - var command = new GetCommand(config); + var command = new GetCommand(config, NullLoggerFactory.Instance); var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "get", null); var result = await command.ExecuteAsync(context, settings, CancellationToken.None).ConfigureAwait(false); @@ -112,7 +113,7 @@ public async Task DeleteCommand_WithNonExistentId_ReturnsSuccess() ConfigPath = this._configPath, Id = "nonexistent-id-12345" }; - var command = new DeleteCommand(config); + var command = new DeleteCommand(config, NullLoggerFactory.Instance); var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "delete", null); var result = await command.ExecuteAsync(context, settings, CancellationToken.None).ConfigureAwait(false); @@ -131,7 +132,7 @@ public async Task ListCommand_WithEmptyDatabase_ReturnsSuccess() ConfigPath = this._configPath, Content = "Temp content to create database" }; - var putCommand = new UpsertCommand(config); + var putCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "put", null); await putCommand.ExecuteAsync(context, putSettings, CancellationToken.None).ConfigureAwait(false); @@ -141,7 +142,7 @@ public async Task ListCommand_WithEmptyDatabase_ReturnsSuccess() ConfigPath = this._configPath, Id = "temp-id" }; - var deleteCommand = new DeleteCommand(config); + var deleteCommand = new DeleteCommand(config, NullLoggerFactory.Instance); await deleteCommand.ExecuteAsync(context, deleteSettings, CancellationToken.None).ConfigureAwait(false); // Now test list on empty database @@ -149,7 +150,7 @@ public async Task ListCommand_WithEmptyDatabase_ReturnsSuccess() { ConfigPath = this._configPath }; - var command = new ListCommand(config); + var command = new ListCommand(config, NullLoggerFactory.Instance); var result = await command.ExecuteAsync(context, settings, CancellationToken.None).ConfigureAwait(false); Assert.Equal(0, result); @@ -165,7 +166,7 @@ public async Task NodesCommand_WithValidConfig_ReturnsSuccess() { ConfigPath = this._configPath }; - var command = new NodesCommand(config); + var command = new NodesCommand(config, NullLoggerFactory.Instance); var context = new CommandContext(["--config", this._configPath], new EmptyRemainingArguments(), "nodes", null); var result = await command.ExecuteAsync(context, settings, CancellationToken.None).ConfigureAwait(false); @@ -183,7 +184,7 @@ public async Task ConfigCommand_WithoutFlags_ReturnsSuccess() ConfigPath = this._configPath }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "config", null); var result = await command.ExecuteAsync(context, settings).ConfigureAwait(false); @@ -202,7 +203,7 @@ public async Task ConfigCommand_WithShowNodes_ReturnsSuccess() ShowNodes = true }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "config", null); var result = await command.ExecuteAsync(context, settings).ConfigureAwait(false); @@ -221,7 +222,7 @@ public async Task ConfigCommand_WithShowCache_ReturnsSuccess() ShowCache = true }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "config", null); var result = await command.ExecuteAsync(context, settings).ConfigureAwait(false); @@ -240,7 +241,7 @@ public async Task GetCommand_WithFullFlag_ReturnsSuccess() ConfigPath = this._configPath, Content = "Test content for full flag" }; - var putCommand = new UpsertCommand(config); + var putCommand = new UpsertCommand(config, NullLoggerFactory.Instance); var putContext = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "put", null); await putCommand.ExecuteAsync(putContext, putSettings, CancellationToken.None).ConfigureAwait(false); @@ -252,7 +253,7 @@ public async Task GetCommand_WithFullFlag_ReturnsSuccess() Id = "some-id", ShowFull = true }; - var getCommand = new GetCommand(config); + var getCommand = new GetCommand(config, NullLoggerFactory.Instance); var getContext = new CommandContext(new[] { "--config", this._configPath }, new EmptyRemainingArguments(), "get", null); var result = await getCommand.ExecuteAsync(getContext, getSettings, CancellationToken.None).ConfigureAwait(false); diff --git a/tests/Main.Tests/Integration/ConfigCommandTests.cs b/tests/Main.Tests/Integration/ConfigCommandTests.cs index 36f5f37b0..b1ac3f461 100644 --- a/tests/Main.Tests/Integration/ConfigCommandTests.cs +++ b/tests/Main.Tests/Integration/ConfigCommandTests.cs @@ -3,6 +3,7 @@ using KernelMemory.Core.Config; using KernelMemory.Core.Config.Cache; using KernelMemory.Main.CLI.Commands; +using Microsoft.Extensions.Logging.Abstractions; using Spectre.Console.Cli; namespace KernelMemory.Main.Tests.Integration; @@ -82,7 +83,7 @@ public void ConfigCommand_WithoutFlags_ShouldShowEntireConfiguration() }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = new CommandContext( new[] { "--config", this._configPath }, new EmptyRemainingArguments(), @@ -135,7 +136,7 @@ public void ConfigCommand_OutputStructure_ShouldMatchAppConfigFormat() }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = new CommandContext( new[] { "--config", this._configPath }, new EmptyRemainingArguments(), @@ -193,7 +194,7 @@ public void ConfigCommand_WithShowNodesFlag_ShouldShowAllNodesSummary() }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = new CommandContext( new[] { "--config", this._configPath }, new EmptyRemainingArguments(), @@ -235,7 +236,7 @@ public void ConfigCommand_WithCreate_CreatesConfigFile() var config = ConfigParser.LoadFromFile(this._configPath); var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(newConfigPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var settings = new ConfigCommandSettings { @@ -271,7 +272,7 @@ public void ConfigCommand_WithCreate_WhenFileExists_ReturnsError() var config = ConfigParser.LoadFromFile(this._configPath); var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var settings = new ConfigCommandSettings { @@ -322,7 +323,7 @@ public void ConfigCommand_WithoutConfigFile_StillSucceeds() var config = AppConfig.CreateDefault(); var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(missingConfigPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var settings = new ConfigCommandSettings { @@ -369,7 +370,7 @@ public void ConfigCommand_OutputJson_DoesNotContainNullFields() // Arrange var config = ConfigParser.LoadFromFile(this._configPath); var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var settings = new ConfigCommandSettings { @@ -417,7 +418,7 @@ public void ConfigCommand_OutputJson_ContainsCorrectDiscriminators() // Arrange var config = ConfigParser.LoadFromFile(this._configPath); var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var settings = new ConfigCommandSettings { diff --git a/tests/Main.Tests/Integration/ReadonlyCommandTests.cs b/tests/Main.Tests/Integration/ReadonlyCommandTests.cs index 36c99012b..8b0af4f5d 100644 --- a/tests/Main.Tests/Integration/ReadonlyCommandTests.cs +++ b/tests/Main.Tests/Integration/ReadonlyCommandTests.cs @@ -3,6 +3,7 @@ using KernelMemory.Core.Config; using KernelMemory.Core.Config.ContentIndex; using KernelMemory.Main.CLI.Commands; +using Microsoft.Extensions.Logging.Abstractions; using Spectre.Console.Cli; namespace KernelMemory.Main.Tests.Integration; @@ -81,7 +82,7 @@ public async Task BugA_ListCommand_NonExistentDatabase_ShouldNotCreateDirectory( Format = "json" }; - var command = new ListCommand(config); + var command = new ListCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("list"); // Act @@ -117,7 +118,7 @@ public async Task BugA_GetCommand_NonExistentDatabase_ShouldNotCreateDirectory() Id = "test-id" }; - var command = new GetCommand(config); + var command = new GetCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("get"); // Act @@ -152,7 +153,7 @@ public async Task BugA_NodesCommand_NonExistentDatabase_ShouldNotCreateDirectory Format = "json" }; - var command = new NodesCommand(config); + var command = new NodesCommand(config, NullLoggerFactory.Instance); var context = CreateTestContext("nodes"); // Act @@ -188,7 +189,7 @@ public async Task BugA_ConfigCommand_NonExistentDatabase_ShouldNotCreateDirector }; var configPathService = new KernelMemory.Main.CLI.Infrastructure.ConfigPathService(this._configPath); - var command = new ConfigCommand(config, configPathService); + var command = new ConfigCommand(config, NullLoggerFactory.Instance, configPathService); var context = CreateTestContext("config"); // Act diff --git a/tests/Main.Tests/Integration/UserDataProtectionTests.cs b/tests/Main.Tests/Integration/UserDataProtectionTests.cs index 385768e46..105761038 100644 --- a/tests/Main.Tests/Integration/UserDataProtectionTests.cs +++ b/tests/Main.Tests/Integration/UserDataProtectionTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using KernelMemory.Core.Config; using KernelMemory.Main.CLI.Commands; +using Microsoft.Extensions.Logging.Abstractions; using Spectre.Console.Cli; namespace KernelMemory.Main.Tests.Integration; @@ -86,7 +87,7 @@ public async Task CriticalBug_CommandExecutionTests_MustNotTouchUserData() Content = "CRITICAL BUG: This should NOT go to user data!" }; - var command = new UpsertCommand(config); + var command = new UpsertCommand(config, NullLoggerFactory.Instance); // This context has --config flag, but BaseCommand reads from settings.ConfigPath! var context = new CommandContext( @@ -146,7 +147,7 @@ public async Task Fixed_SettingsWithConfigPath_MustUseTestDirectory() Content = "Test content in temp directory" }; - var command = new UpsertCommand(config); + var command = new UpsertCommand(config, NullLoggerFactory.Instance); var context = new CommandContext( new[] { "--config", this._configPath }, new EmptyRemainingArguments(), diff --git a/tests/Main.Tests/Unit/Commands/BaseCommandTests.cs b/tests/Main.Tests/Unit/Commands/BaseCommandTests.cs index 9c145044c..1147b58ce 100644 --- a/tests/Main.Tests/Unit/Commands/BaseCommandTests.cs +++ b/tests/Main.Tests/Unit/Commands/BaseCommandTests.cs @@ -2,6 +2,7 @@ using KernelMemory.Main.CLI; using KernelMemory.Main.CLI.Commands; using KernelMemory.Main.CLI.OutputFormatters; +using Microsoft.Extensions.Logging.Abstractions; using Moq; using Spectre.Console.Cli; @@ -81,7 +82,7 @@ public void HandleError_WithIOException_ReturnsSystemError() /// private sealed class TestCommand : BaseCommand { - public TestCommand() : base(CreateTestConfig()) + public TestCommand() : base(CreateTestConfig(), NullLoggerFactory.Instance) { }