-
Notifications
You must be signed in to change notification settings - Fork 303
add logging-level support #1123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
67a3921
2451384
3f13ee0
c39943a
c4f5d77
362ea68
7061046
1bb2d53
39935e3
f9041ce
e30bd2b
7c52b7b
867a937
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| using System.Net; | ||
| using Azure.Mcp.Core.Areas.Server.Models; | ||
| using Azure.Mcp.Core.Areas.Server.Options; | ||
| using Azure.Mcp.Core.Areas.Server.Services; | ||
| using Azure.Mcp.Core.Commands; | ||
| using Azure.Mcp.Core.Helpers; | ||
| using Azure.Mcp.Core.Services.Azure; | ||
|
|
@@ -78,6 +79,8 @@ protected override void RegisterOptions(Command command) | |
| command.Options.Add(ServiceOptionDefinitions.DangerouslyDisableHttpIncomingAuth); | ||
| command.Options.Add(ServiceOptionDefinitions.InsecureDisableElicitation); | ||
| command.Options.Add(ServiceOptionDefinitions.OutgoingAuthStrategy); | ||
| command.Options.Add(ServiceOptionDefinitions.LogLevel); | ||
| command.Options.Add(ServiceOptionDefinitions.LogFilePath); | ||
| command.Validators.Add(commandResult => | ||
| { | ||
| string transport = ResolveTransport(commandResult); | ||
|
|
@@ -120,7 +123,9 @@ protected override ServiceStartOptions BindOptions(ParseResult parseResult) | |
| Debug = parseResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.Debug.Name), | ||
| DangerouslyDisableHttpIncomingAuth = parseResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.DangerouslyDisableHttpIncomingAuth.Name), | ||
| InsecureDisableElicitation = parseResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.InsecureDisableElicitation.Name), | ||
| OutgoingAuthStrategy = outgoingAuthStrategy | ||
| OutgoingAuthStrategy = outgoingAuthStrategy, | ||
| LogLevel = parseResult.GetValueOrDefault<string?>(ServiceOptionDefinitions.LogLevel.Name), | ||
| LogFilePath = parseResult.GetValueOrDefault<string?>(ServiceOptionDefinitions.LogFilePath.Name) | ||
| }; | ||
| return options; | ||
| } | ||
|
|
@@ -362,6 +367,9 @@ private IHost CreateStdioHost(ServiceStartOptions serverOptions) | |
| logging.AddFilter("Microsoft.Extensions.Logging.Console.ConsoleLoggerProvider", LogLevel.Debug); | ||
| logging.SetMinimumLevel(LogLevel.Debug); | ||
| } | ||
|
|
||
| // Apply custom logging configuration | ||
| ConfigureLogging(logging, serverOptions); | ||
| }) | ||
| .ConfigureServices(services => | ||
| { | ||
|
|
@@ -389,6 +397,9 @@ private IHost CreateHttpHost(ServiceStartOptions serverOptions) | |
| builder.Logging.AddEventSourceLogger(); | ||
| builder.Logging.AddConsole(); | ||
|
|
||
| // Apply custom logging configuration | ||
| ConfigureLogging(builder.Logging, serverOptions); | ||
|
|
||
| IServiceCollection services = builder.Services; | ||
|
|
||
| // Configure outgoing and incoming authentication and authorization. | ||
|
|
@@ -566,6 +577,9 @@ private IHost CreateIncomingAuthDisabledHttpHost(ServiceStartOptions serverOptio | |
| builder.Logging.AddEventSourceLogger(); | ||
| builder.Logging.AddConsole(); | ||
|
|
||
| // Apply custom logging configuration | ||
| ConfigureLogging(builder.Logging, serverOptions); | ||
|
|
||
| IServiceCollection services = builder.Services; | ||
|
|
||
| // Configure single identity token credential provider for outgoing authentication | ||
|
|
@@ -607,6 +621,57 @@ private IHost CreateIncomingAuthDisabledHttpHost(ServiceStartOptions serverOptio | |
| return app; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Configures logging based on the server options. | ||
| /// </summary> | ||
| /// <param name="logging">The logging builder to configure.</param> | ||
| /// <param name="options">The server configuration options.</param> | ||
| private static void ConfigureLogging(ILoggingBuilder logging, ServiceStartOptions options) | ||
| { | ||
| // Set minimum log level | ||
| // Default to Information to avoid logging sensitive data (Debug/Trace may contain secrets) | ||
| if (!string.IsNullOrWhiteSpace(options.LogLevel)) | ||
| { | ||
| if (Enum.TryParse<LogLevel>(options.LogLevel, ignoreCase: true, out var logLevel)) | ||
| { | ||
| logging.SetMinimumLevel(logLevel); | ||
|
|
||
| // If LogLevel.None is specified, clear all providers to disable logging | ||
| if (logLevel == LogLevel.None) | ||
| { | ||
| logging.ClearProviders(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @xiangyan99, please chat with @srnagar on the changes that he's making for logging. We're trying to improve the situation for remote MCP servers, and Srikanta's changes similarly have a switch to shut off all telemetry. |
||
| return; // Exit early, don't add file logging | ||
| } | ||
| } | ||
| } | ||
| else if (options.Debug) | ||
| { | ||
| // Debug mode overrides default | ||
| logging.SetMinimumLevel(LogLevel.Debug); | ||
| } | ||
| else | ||
| { | ||
| // Default to Information level to prevent logging sensitive data | ||
| // (Debug and Trace levels may contain secrets, tokens, and other sensitive information) | ||
| logging.SetMinimumLevel(LogLevel.Information); | ||
| } | ||
|
|
||
| // Add file logging if path is specified | ||
| if (!string.IsNullOrWhiteSpace(options.LogFilePath)) | ||
| { | ||
| // Ensure directory exists | ||
| var directory = Path.GetDirectoryName(options.LogFilePath); | ||
| if (!string.IsNullOrEmpty(directory)) | ||
| { | ||
| Directory.CreateDirectory(directory); | ||
| } | ||
|
|
||
xiangyan99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Add simple file logger | ||
| // Note: For production scenarios, consider using a more robust file logging provider | ||
| logging.AddProvider(new SimpleFileLoggerProvider(options.LogFilePath)); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Configures the MCP server services. | ||
| /// </summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,8 @@ public static class ServiceOptionDefinitions | |
| public const string DangerouslyDisableHttpIncomingAuthName = "dangerously-disable-http-incoming-auth"; | ||
| public const string InsecureDisableElicitationName = "insecure-disable-elicitation"; | ||
| public const string OutgoingAuthStrategyName = "outgoing-auth-strategy"; | ||
| public const string LogLevelName = "log-level"; | ||
| public const string LogFilePathName = "log-file-path"; | ||
|
|
||
| public static readonly Option<string> Transport = new($"--{TransportName}") | ||
| { | ||
|
|
@@ -91,4 +93,20 @@ public static class ServiceOptionDefinitions | |
| Description = "Outgoing authentication strategy for Azure service requests. Valid values: NotSet, UseHostingEnvironmentIdentity, UseOnBehalfOf.", | ||
| DefaultValueFactory = _ => Options.OutgoingAuthStrategy.NotSet | ||
| }; | ||
|
|
||
| public static readonly Option<string?> LogLevel = new( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we avoid creating a new argument for log levels? Controlling log levels with configurations is built into the ILogger infrastructure with log categories and log levels. These can both be controlled using any number of sources of configurations, including command line and environment variables. |
||
| $"--{LogLevelName}") | ||
| { | ||
| Required = false, | ||
| Description = "Minimum logging level. Valid values: Trace, Debug, Information, Warning, Error, Critical, None. Default is Information (or Debug if --debug is set).", | ||
alzimmermsft marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| DefaultValueFactory = _ => null | ||
| }; | ||
|
|
||
| public static readonly Option<string?> LogFilePath = new( | ||
| $"--{LogFilePathName}") | ||
| { | ||
| Required = false, | ||
| Description = "Path to write log file output. When specified, logs will be written to the specified file in addition to console output.", | ||
| DefaultValueFactory = _ => null | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using Microsoft.Extensions.Logging; | ||
|
|
||
| namespace Azure.Mcp.Core.Areas.Server.Services; | ||
|
|
||
| /// <summary> | ||
| /// A simple file logger provider for writing logs to a file. | ||
| /// </summary> | ||
| internal sealed class SimpleFileLoggerProvider : ILoggerProvider | ||
| { | ||
| private readonly string _filePath; | ||
| private readonly object _lock = new(); | ||
|
|
||
| public SimpleFileLoggerProvider(string filePath) | ||
| { | ||
| _filePath = filePath ?? throw new ArgumentNullException(nameof(filePath)); | ||
| } | ||
|
|
||
| public ILogger CreateLogger(string categoryName) | ||
| { | ||
| return new SimpleFileLogger(categoryName, _filePath, _lock); | ||
| } | ||
|
|
||
| public void Dispose() | ||
| { | ||
| // Nothing to dispose | ||
| } | ||
|
|
||
| private sealed class SimpleFileLogger(string categoryName, string filePath, object lockObject) : ILogger | ||
| { | ||
| private readonly string _categoryName = categoryName; | ||
| private readonly string _filePath = filePath; | ||
| private readonly object _lock = lockObject; | ||
|
|
||
| public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null; | ||
|
|
||
| public bool IsEnabled(LogLevel logLevel) => logLevel != LogLevel.None; | ||
|
|
||
| public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter) | ||
| { | ||
| if (!IsEnabled(logLevel)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var message = formatter(state, exception); | ||
| var logEntry = $"[{DateTime.UtcNow:yyyy-MM-dd HH:mm:ss.fff}] [{logLevel}] [{_categoryName}] {message}"; | ||
|
|
||
| if (exception != null) | ||
| { | ||
| logEntry += Environment.NewLine + exception; | ||
| } | ||
|
|
||
| lock (_lock) | ||
| { | ||
| try | ||
| { | ||
| var directory = Path.GetDirectoryName(_filePath); | ||
| if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory)) | ||
| { | ||
| Directory.CreateDirectory(directory); | ||
| } | ||
| File.AppendAllText(_filePath, logEntry + Environment.NewLine); | ||
| } | ||
| catch | ||
| { | ||
| // Silently fail if we can't write to the log file | ||
| // to avoid breaking the application | ||
| } | ||
xiangyan99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.