From fc3c21ade4c37e95bdfa80af88d9bbab18917862 Mon Sep 17 00:00:00 2001 From: Mansur Besleney Date: Fri, 16 Jan 2026 11:10:17 +0300 Subject: [PATCH] Refactor MCP logging and tool initialization logic Refactored McpLogger to use ILogger and Serilog for file logging, removing manual file handling and rotation. Enhanced debug logging in McpHttpClientService and added explicit tool name initialization from cache. Updated Program.cs to use a separate log file for MCP mode. Improved error logging in McpServerService for tool execution failures. --- .../Commands/Services/McpHttpClientService.cs | 15 ++- .../Abp/Cli/Commands/Services/McpLogger.cs | 106 +++++------------- .../Cli/Commands/Services/McpServerService.cs | 2 +- .../Commands/Services/McpToolsCacheService.cs | 2 + .../src/Volo.Abp.Cli/Volo/Abp/Cli/Program.cs | 23 +++- 5 files changed, 59 insertions(+), 89 deletions(-) diff --git a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpHttpClientService.cs b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpHttpClientService.cs index e4c4226248..b27c3740d6 100644 --- a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpHttpClientService.cs +++ b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpHttpClientService.cs @@ -15,7 +15,7 @@ using Volo.Abp.IO; namespace Volo.Abp.Cli.Commands.Services; -public class McpHttpClientService : ITransientDependency +public class McpHttpClientService : ISingletonDependency { private static readonly JsonSerializerOptions JsonSerializerOptionsWeb = new(JsonSerializerDefaults.Web); @@ -80,8 +80,17 @@ public class McpHttpClientService : ITransientDependency public string ServerUrl { get; set; } } + public void InitializeToolNames(List tools) + { + _validToolNames = tools.Select(t => t.Name).ToList(); + _toolDefinitionsLoaded = true; + _mcpLogger.Debug(LogSource, $"Initialized tool names from cache. Count={tools.Count}, Instance={GetHashCode()}"); + } + public async Task CallToolAsync(string toolName, JsonElement arguments) { + _mcpLogger.Debug(LogSource, $"CallToolAsync called for '{toolName}'. _toolDefinitionsLoaded={_toolDefinitionsLoaded}, Instance={GetHashCode()}"); + if (!_toolDefinitionsLoaded) { throw new CliUsageException("Tool definitions have not been loaded yet. This is an internal error."); @@ -193,6 +202,8 @@ public class McpHttpClientService : ITransientDependency public async Task> GetToolDefinitionsAsync() { + _mcpLogger.Debug(LogSource, $"GetToolDefinitionsAsync called. Instance={GetHashCode()}"); + var baseUrl = await GetMcpServerUrlAsync(); var url = $"{baseUrl}/tools"; @@ -220,6 +231,8 @@ public class McpHttpClientService : ITransientDependency _validToolNames = tools.Select(t => t.Name).ToList(); _toolDefinitionsLoaded = true; + _mcpLogger.Debug(LogSource, $"Tool definitions loaded successfully. _toolDefinitionsLoaded={_toolDefinitionsLoaded}, Tool count={tools.Count}, Instance={GetHashCode()}"); + return tools; } catch (HttpRequestException ex) diff --git a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpLogger.cs b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpLogger.cs index 1a81486241..20d6fce469 100644 --- a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpLogger.cs +++ b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpLogger.cs @@ -1,26 +1,25 @@ using System; -using System.IO; -using System.Text; +using Microsoft.Extensions.Logging; using Volo.Abp.DependencyInjection; namespace Volo.Abp.Cli.Commands.Services; /// -/// MCP logger implementation that writes to both file and stderr. -/// - All logs at or above the configured level are written to file +/// MCP logger implementation that writes to both file (via Serilog) and stderr. +/// - All logs at or above the configured level are written to file via ILogger /// - Warning and Error logs are also written to stderr /// - Log level is controlled via ABP_MCP_LOG_LEVEL environment variable /// public class McpLogger : IMcpLogger, ISingletonDependency { - private const long MaxLogFileSizeBytes = 5 * 1024 * 1024; // 5MB private const string LogPrefix = "[MCP]"; - private readonly object _fileLock = new(); + private readonly ILogger _logger; private readonly McpLogLevel _configuredLogLevel; - public McpLogger() + public McpLogger(ILogger logger) { + _logger = logger; _configuredLogLevel = GetConfiguredLogLevel(); } @@ -56,48 +55,34 @@ public class McpLogger : IMcpLogger, ISingletonDependency private void Log(McpLogLevel level, string source, string message) { - if (_configuredLogLevel == McpLogLevel.None) + if (_configuredLogLevel == McpLogLevel.None || level < _configuredLogLevel) { return; } - if (level < _configuredLogLevel) - { - return; - } - - var timestamp = DateTime.Now.ToString("yyyy-MM-dd HH:mm:ss"); - var levelStr = level.ToString().ToUpperInvariant(); - var formattedMessage = $"[{timestamp}][{levelStr}][{source}] {message}"; + var mcpFormattedMessage = $"{LogPrefix}[{source}] {message}"; - // Write to file (all levels at or above configured level) - WriteToFile(formattedMessage); - - // Write to stderr for Warning and Error levels - if (level >= McpLogLevel.Warning) + // File logging via Serilog + switch (level) { - WriteToStderr(levelStr, message); + case McpLogLevel.Debug: + _logger.LogDebug(mcpFormattedMessage); + break; + case McpLogLevel.Info: + _logger.LogInformation(mcpFormattedMessage); + break; + case McpLogLevel.Warning: + _logger.LogWarning(mcpFormattedMessage); + break; + case McpLogLevel.Error: + _logger.LogError(mcpFormattedMessage); + break; } - } - private void WriteToFile(string formattedMessage) - { - try - { - lock (_fileLock) - { - EnsureLogDirectoryExists(); - RotateLogFileIfNeeded(); - - File.AppendAllText( - CliPaths.McpLog, - formattedMessage + Environment.NewLine, - Encoding.UTF8); - } - } - catch + // Stderr output for MCP protocol (Warning/Error only) + if (level >= McpLogLevel.Warning) { - // Silently ignore file write errors to not disrupt MCP operations + WriteToStderr(level.ToString().ToUpperInvariant(), message); } } @@ -114,47 +99,6 @@ public class McpLogger : IMcpLogger, ISingletonDependency } } - private void EnsureLogDirectoryExists() - { - var directory = Path.GetDirectoryName(CliPaths.McpLog); - if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory)) - { - Directory.CreateDirectory(directory); - } - } - - private void RotateLogFileIfNeeded() - { - try - { - if (!File.Exists(CliPaths.McpLog)) - { - return; - } - - var fileInfo = new FileInfo(CliPaths.McpLog); - if (fileInfo.Length < MaxLogFileSizeBytes) - { - return; - } - - var backupPath = CliPaths.McpLog + ".1"; - - // Delete old backup if exists - if (File.Exists(backupPath)) - { - File.Delete(backupPath); - } - - // Rename current log to backup - File.Move(CliPaths.McpLog, backupPath); - } - catch - { - // Silently ignore rotation errors - } - } - private static McpLogLevel GetConfiguredLogLevel() { var envValue = Environment.GetEnvironmentVariable(CliConsts.McpLogLevelEnvironmentVariable); diff --git a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpServerService.cs b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpServerService.cs index c77443b1f1..a591c42371 100644 --- a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpServerService.cs +++ b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpServerService.cs @@ -161,7 +161,7 @@ public class McpServerService : ITransientDependency } catch (Exception ex) { - _mcpLogger.Error(LogSource, $"Tool '{toolName}' execution failed", ex); + _mcpLogger.Error(LogSource, $"Tool '{toolName}' execution failed '{ex.Message}'", ex); return CreateErrorResult(ToolErrorMessages.UnexpectedError); } } diff --git a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpToolsCacheService.cs b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpToolsCacheService.cs index 2a24f585d2..146f5e4ba9 100644 --- a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpToolsCacheService.cs +++ b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpToolsCacheService.cs @@ -43,6 +43,8 @@ public class McpToolsCacheService : ITransientDependency if (cachedTools != null) { _mcpLogger.Debug(LogSource, "Using cached tool definitions"); + // Initialize the HTTP client's tool names list from cache + _mcpHttpClient.InitializeToolNames(cachedTools); return cachedTools; } } diff --git a/framework/src/Volo.Abp.Cli/Volo/Abp/Cli/Program.cs b/framework/src/Volo.Abp.Cli/Volo/Abp/Cli/Program.cs index 61f2ef9de8..58d4d43442 100644 --- a/framework/src/Volo.Abp.Cli/Volo/Abp/Cli/Program.cs +++ b/framework/src/Volo.Abp.Cli/Volo/Abp/Cli/Program.cs @@ -1,4 +1,4 @@ -using System; +using System; using Microsoft.Extensions.DependencyInjection; using Serilog; using Serilog.Events; @@ -15,7 +15,7 @@ public class Program Console.OutputEncoding = System.Text.Encoding.UTF8; var loggerOutputTemplate = "{Message:lj}{NewLine}{Exception}"; - Log.Logger = new LoggerConfiguration() + var config = new LoggerConfiguration() .MinimumLevel.Information() .MinimumLevel.Override("Microsoft", LogEventLevel.Warning) .MinimumLevel.Override("Volo.Abp", LogEventLevel.Warning) @@ -26,10 +26,21 @@ public class Program #else .MinimumLevel.Override("Volo.Abp.Cli", LogEventLevel.Information) #endif - .Enrich.FromLogContext() - .WriteTo.File(Path.Combine(CliPaths.Log, "abp-cli-logs.txt"), outputTemplate: loggerOutputTemplate) - .WriteTo.Console(theme: AnsiConsoleTheme.Sixteen, outputTemplate: loggerOutputTemplate) - .CreateLogger(); + .Enrich.FromLogContext(); + + if (args.Length > 0 && args[0] == "mcp") + { + Log.Logger = config + .WriteTo.File(Path.Combine(CliPaths.Log, "abp-cli-mcp-logs.txt"), outputTemplate: loggerOutputTemplate) + .CreateLogger(); + } + else + { + Log.Logger = config + .WriteTo.File(Path.Combine(CliPaths.Log, "abp-cli-logs.txt"), outputTemplate: loggerOutputTemplate) + .WriteTo.Console(theme: AnsiConsoleTheme.Sixteen, outputTemplate: loggerOutputTemplate) + .CreateLogger(); + } using (var application = AbpApplicationFactory.Create( options =>