Cherry-pick: LogLevel bug fixes#3511
Draft
RubenCerna2079 wants to merge 4 commits intorelease/2.0from
Draft
Conversation
## Why make this change? Closes #3274 - MCP Server returns "Method not found: logging/setLevel" error when clients send the standard MCP logging/setLevel request. Closes #3275 - Control output in MCP stdio mode (default to `LogLevel.None`, redirect/suppress console output). ## What is this change? ### MCP `logging/setLevel` Handler - Added handler for `logging/setLevel` JSON-RPC method in `McpStdioServer.cs` - Implemented `DynamicLogLevelProvider` with `ILogLevelController` interface to allow MCP to update log levels dynamically - Added `IsCliOverridden` and `IsConfigOverridden` properties to enforce precedence rules ### Log Level Precedence System **Precedence (highest to lowest):** 1. **CLI `--LogLevel` flag** - cannot be changed by MCP 2. **Config `runtime.telemetry.log-level`** - cannot be changed by MCP 3. **MCP `logging/setLevel`** - only works if neither CLI nor config set a level 4. Default (LogLevel.None for MCP stdio mode) If CLI or config set a level, MCP requests are accepted but silently ignored (no error returned per MCP spec). ### Early Config Reading for MCP Mode - Added `TryGetLogLevelFromConfig()` in `Program.cs` to read config file early (before host build) - This ensures config log level is detected before Console redirect decision - Console redirect for MCP stdio mode now respects config log level ### CLI Log Level Handling - Added `Utils.CliLogLevel` property to track the parsed `--LogLevel` value - CLI's `CustomLoggerProvider` now respects the `--LogLevel` value for its own logging ### Config Helpers - Added `HasExplicitLogLevel()` helper to `RuntimeConfig` to correctly detect when config actually pins a log level - This properly handles null values in telemetry section (null values don't count as explicit override) ## How was this tested? - [x] Unit Tests (`DynamicLogLevelProviderTests` - 5 tests) - [x] Manual Testing ### Manual Test 1: No override (MCP can change level) 1. Start MCP server without `--LogLevel` and without config `log-level` 2. MCP sends `logging/setLevel` with `level: info` 3. Result: Log level changes to info ### Manual Test 2: CLI override (MCP blocked) 1. Start MCP server with `--LogLevel Warning` 2. MCP sends `logging/setLevel` with `level: info` 3. Result: Log level stays at Warning, MCP request accepted silently ### Manual Test 3: Config override (MCP blocked) 1. Add `"telemetry": { "log-level": { "default": "Warning" } }` to config 2. Start MCP server without `--LogLevel` 3. MCP sends `logging/setLevel` with `level: info` 5. Result: Log level stays at Warning, MCP request accepted silently ### Manual Test 4: Config with null values (MCP can change level) 1. Add `"telemetry": { "log-level": { "default": null } }` to config 2. Start MCP server without `--LogLevel` 3. MCP sends `logging/setLevel` with `level: info` 4. Result: Log level changes to info (null values don't count as override) ## Sample Request(s) MCP client sends: ```json { "jsonrpc": "2.0", "id": 1, "method": "logging/setLevel", "params": { "level": "info" } } ``` Server responds with empty result (success per MCP spec) and updates log level if no CLI/config override is active. --------- Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com>
## Why make this change? Closes #3268 ## What is this change? When `dab validate` or `dab start` encounters a config parsing error (e.g. missing entities/autoentities), the CLI previously dumped the full exception message and stack trace to stderr. This made the output noisy and unhelpful. After this change, only a clean, descriptive validation message is shown. The key design changes: - `TryParseConfig` is now a pure function that returns an `out string? parseError` message on failure instead of writing to `Console.Error` or `ILogger` internally. Error reporting is the caller's responsibility. - `FileSystemRuntimeConfigLoader.TryLoadConfig` writes the `parseError` to `Console.Error` (because config is parsed before the DI container and logger are available, so the log buffer would never be flushed on a parse failure) and sets an instance-scoped `IsParseErrorEmitted` flag so CLI callers (`ConfigGenerator`) can avoid logging duplicate messages. - `ConfigGenerator.IsConfigValid` now has an explicit early-return path when config parsing fails (via `runtimeConfigProvider.TryGetConfig`), using `IsParseErrorEmitted` to suppress duplicate output. - `ValidateOptions.Handler` uses `LogError("Config is invalid.")`. - Comments referencing which method emits the error to `Console.Error` corrected to `TryLoadConfig` (not `TryParseConfig`). ## How was this tested? * `ValidateConfigTests.cs` — Added `TestValidateConfigWithNoEntitiesProducesCleanError` (new test verifying clean error message, no stack traces). * `EnvironmentTests.cs` — Updated `FailureToStartEngineWhenEnvVarNamedWrong` to match the new single-line clean stderr format. * `EndToEndTests.cs` — Simplified assertions in `TestExitOfRuntimeEngineWithInvalidConfig` (this test is Ignored but updated for consistency). * `RuntimeConfigLoaderTests.cs` — Updated `FailLoadMultiDataSourceConfigDuplicateEntities` to assert `loader.IsParseErrorEmitted` is true. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Souvik Ghosh <souvikofficial04@gmail.com>
…label on startup (#3307) ## Why make this change? Closes #3269 ## What is this change? Align the CLI logger labels with the labels used in ASP.Net Core. Downgraded internal plumbing message from `Information` to `Debug` Add a test case to cover the changed behavior. ## How was this tested? Added a test case to validate the new behavior.
- Closes issue #3262 The logger for the Startup class is not initialized properly, since this logger is special due to the nature of the Startup class it needs to be continuously updated as DAB initializes. This causes two problems: - Some logs appear even when LogLevel is set to some value that would impede those logs to appear. - Some logs don't appear at all, even when LogLevel is set to a value that should allow them to be logged. - Closes issue #3256 & #3255 The CLI logger still outputs some logs even when the LogLevel is set to `none`. It is expected that if the LogLevel set is `none` or some other level that shouldn't output the `information` level, the logs will not appear. Important Note: These changes currently only allow us to change the LogLevel from the CLI with the `default` namespace in the config file. An task was created to solve this issue: #3451 In order to solve issue #3262: - We removed the LogBuffer from the services inside of `Startup.cs`, this is necessary since we wanted each class to have its own LogBuffer so that we are able to tell from which logger the logs are being outputted. - Then, we also correctly initialized the `Startup` logger by changing the method that it was using to initialize the logger, it now uses `CreateLoggerFactoryForHostedAndNonHostedScenario` which checks if there are any LogLevel namespaces from the config file that can be applicable for the specific logger. It is important to note that there are multiple places where the logs are flushed in order to cover for the cases in which an exception is found and causes DAB to end abruptly, and when we there is an IsLateConfigured scenario. - We also changed the logger for the LogBuffer in all the missing places where it creates logs before the logger is able to properly initialize to add those logs to the LogBuffer and only flush them after the loggers are initialized. In order to solve issue #3256 & #3255: - We changed the CLI so that we add all the logs go to a single global LogBuffer that is created inside the `StartOptions.cs` until it is able to deserialize the RuntimeConfig and find which level to set the `LogLevel` in order to flush all the logs. - This is something that we only want to happen when we use the `dab start` command, which is why we only make this change in the `StartOptions.cs` file, on the function `TryStartEngineWithOptions` inside of `ConfigGenerator.cs`, and a few functions from `Utils.cs` and `ConfigMerger.cs` that are used inside the `TryStartEngine` function. - [ ] Integration Tests - [x] Unit Tests - dab start --LogLevel none - dab start --LogLevel error --------- Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
RubenCerna2079
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why make this change?
This change cherry-picks the PRs that fix the LogLevel bugs
What is this change?
Cherry-picked PRs:
nonebug #3318How was this tested?
Existing tests in cherry-pick cover new changes.
Sample Request(s)
N/A