Conversation
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
anushakolan
requested review from
Alekhya-Polavarapu,
Aniruddh25,
JerryNixon,
RubenCerna2079,
aaronburtle,
rusamant,
sourabh1007,
souvikghosh04,
stuartpa and
vadeveka
as code owners
There was a problem hiding this comment.
Pull request overview
This PR addresses MCP stdio clients receiving Method not found: logging/setLevel by adding support for the logging/setLevel JSON-RPC method and wiring it to DAB’s dynamic log-level infrastructure with explicit precedence rules (CLI > config > MCP).
Changes:
- Added
logging/setLeveldispatch + handler inMcpStdioServerto accept the standard MCP request and (when allowed) update runtime log level. - Introduced
ILogLevelControllerand implemented it inDynamicLogLevelProvider, including CLI/config precedence tracking. - Updated CLI engine-launch argument construction to only pass
--LogLevelwhen explicitly provided, plus added unit tests for MCP log-level updates.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Service/Telemetry/DynamicLogLevelProvider.cs | Implements ILogLevelController, adds MCP level mapping and precedence handling (CLI/config/MCP). |
| src/Service/Program.cs | Registers ILogLevelController in DI so MCP can resolve the controller. |
| src/Service.Tests/UnitTests/DynamicLogLevelProviderTests.cs | Adds unit tests for MCP-driven log-level changes and override behavior. |
| src/Core/Telemetry/ILogLevelController.cs | New interface to decouple MCP from the concrete log-level provider. |
| src/Cli/ConfigGenerator.cs | Only forwards --LogLevel when explicitly set by the user (to avoid treating defaults as CLI overrides). |
| src/Azure.DataApiBuilder.Mcp/Core/McpStdioServer.cs | Adds logging/setLevel handler that calls into ILogLevelController. |
| if (!IsCliOverridden) | ||
| { | ||
| CurrentLogLevel = runtimeConfig.GetConfiguredLogLevel(); | ||
|
|
||
| // Track if config explicitly set a log level (not just using defaults) | ||
| IsConfigOverridden = !runtimeConfig.IsLogLevelNull(); | ||
| } |
There was a problem hiding this comment.
IsConfigOverridden is derived from !runtimeConfig.IsLogLevelNull(), but the schema allows runtime.telemetry.log-level values to be null (meaning “use host-mode defaults”). In that case IsLogLevelNull() returns false (because the dictionary exists), causing MCP logging/setLevel to be blocked even though config did not actually pin a log level. Consider treating config as “overridden” only when at least one configured log-level value is non-null (e.g., any entry value != null), or add a dedicated RuntimeConfig helper for this distinction.
| minimumLogLevel = deserializedRuntimeConfig.GetConfiguredLogLevel(); | ||
| HostMode hostModeType = deserializedRuntimeConfig.IsDevelopmentMode() ? HostMode.Development : HostMode.Production; | ||
|
|
||
| _logger.LogInformation($"Setting default minimum LogLevel: {minimumLogLevel} for {hostModeType} mode.", minimumLogLevel, hostModeType); |
There was a problem hiding this comment.
In the default log-level branch, _logger.LogInformation($"Setting default minimum LogLevel: {minimumLogLevel} for {hostModeType} mode.", minimumLogLevel, hostModeType); uses an interpolated string while also passing structured args—those args won’t be captured as structured fields. Use a message template without $"..." (or remove the extra args) so logging behaves as intended.
| _logger.LogInformation($"Setting default minimum LogLevel: {minimumLogLevel} for {hostModeType} mode.", minimumLogLevel, hostModeType); | |
| _logger.LogInformation("Setting default minimum LogLevel: {minimumLogLevel} for {hostModeType} mode.", minimumLogLevel, hostModeType); |
| @@ -2597,17 +2598,22 @@ public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRun | |||
|
|
|||
| minimumLogLevel = (LogLevel)options.LogLevel; | |||
| _logger.LogInformation("Setting minimum LogLevel: {minimumLogLevel}.", minimumLogLevel); | |||
|
|
|||
| // Only add --LogLevel when user explicitly specified it via CLI. | |||
| // This allows MCP logging/setLevel to work when no CLI override is present. | |||
| args.Add("--LogLevel"); | |||
| args.Add(minimumLogLevel.ToString()); | |||
| } | |||
| else | |||
| { | |||
| minimumLogLevel = deserializedRuntimeConfig.GetConfiguredLogLevel(); | |||
| HostMode hostModeType = deserializedRuntimeConfig.IsDevelopmentMode() ? HostMode.Development : HostMode.Production; | |||
|
|
|||
| _logger.LogInformation($"Setting default minimum LogLevel: {minimumLogLevel} for {hostModeType} mode.", minimumLogLevel, hostModeType); | |||
| } | |||
|
|
|||
| args.Add("--LogLevel"); | |||
| args.Add(minimumLogLevel.ToString()); | |||
| // Don't add --LogLevel arg since user didn't explicitly set it. | |||
| // Service will determine default log level based on config or host mode. | |||
| } | |||
There was a problem hiding this comment.
After this change, when options.LogLevel is not provided the CLI no longer passes --LogLevel to the engine. The Service defaults to LogLevel.Error when --LogLevel is absent (Program.GetLogLevelFromCommandLineArgs), so early startup logs will be suppressed even in Development until DynamicLogLevelProvider.UpdateFromRuntimeConfig(...) runs. If the intent is to keep the existing “Debug in Development / Error in Production” default behavior while still allowing MCP to change the level, consider setting the initial log level from the loaded config earlier in host construction (or introducing a non-override mechanism distinct from the CLI override flag).
Why make this change?
Closes #3274 - MCP Server returns "Method not found: logging/setLevel" error when clients send the standard MCP logging/setLevel request.
What is this change?
logging/setLevelJSON-RPC method inMcpStdioServer.csILogLevelControllerinterface to allow MCP to update log levels dynamicallyIsConfigOverriddenproperty to enforce precedence rulesPrecedence (highest to lowest):
--LogLevelflag - cannot be changedruntime.telemetry.log-level- cannot be changed by MCPlogging/setLevel- only works if neither CLI nor config set a levelIf CLI or config set a level, MCP requests are accepted but silently ignored (no error returned).
How was this tested?
Manual Test 1: No override (MCP can change level)
--LogLeveland without configlog-levellogging/setLevelwithlevel: infoManual Test 2: CLI override (MCP blocked)
--LogLevel Nonelogging/setLevelwithlevel: infoManual Test 3: Config override (MCP blocked)
"telemetry": { "log-level": { "default": "Warning" } }to config--LogLevellogging/setLevelwithlevel: infoSample Request(s)
MCP client sends:
{ "jsonrpc": "2.0", "id": 1, "method": "logging/setLevel", "params": { "level": "info" } }Server responds with empty result (success) and updates log level if no CLI/config override is active.