Skip to content

Change CLI flag LogLevel to log-level#3650

Open
RubenCerna2079 wants to merge 2 commits into
Azure:mainfrom
RubenCerna2079:dev/rubencerna/fix-cli-loglevel-flag
Open

Change CLI flag LogLevel to log-level#3650
RubenCerna2079 wants to merge 2 commits into
Azure:mainfrom
RubenCerna2079:dev/rubencerna/fix-cli-loglevel-flag

Conversation

@RubenCerna2079
Copy link
Copy Markdown
Contributor

@RubenCerna2079 RubenCerna2079 commented Jun 5, 2026

Why make this change?

What is this change?

We changed the dab start --LogLevel flag to --log-level in order to have it follow the conventions we already have. We also allow for the CLI to still use the previous --LogLevel flag but add a warning sign to ensure the user knows it is deprecated. Lastly, also changed all of the comments that mention the previous --LogLevel and updated it to use the new --log-level.

How was this tested?

  • Integration Tests
  • Unit Tests
    Changed the tests that were for --LogLevel so they now use --log-level and added a few extra cases to ensure the --LogLevel flag still works

Sample Request(s)

dab start --log-level information
dab start --LogLevel warning

@RubenCerna2079 RubenCerna2079 changed the title Change LogLevel to log-level Change CLI flag LogLevel to log-level Jun 6, 2026
@RubenCerna2079 RubenCerna2079 marked this pull request as ready for review June 6, 2026 00:19
Copilot AI review requested due to automatic review settings June 6, 2026 00:19
@RubenCerna2079 RubenCerna2079 modified the milestones: July 2026, June 2026 Jun 6, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the dab start CLI flag for engine log level from --LogLevel to the convention-aligned --log-level, while keeping --LogLevel as a hidden legacy alias and updating related comments/tests across CLI, Service, and MCP components.

Changes:

  • Rename CLI option from --LogLevel to --log-level (with legacy alias retained).
  • Update CLI-to-Service argument forwarding to emit --log-level and add a deprecation warning for --LogLevel.
  • Update comments and unit/e2e tests to reference --log-level, including MCP log-level precedence documentation.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Service/Startup.cs Updates inline comments to reference --log-level.
src/Service/Program.cs Updates service-side log-level extraction to look for --log-level.
src/Service.Tests/UnitTests/DynamicLogLevelProviderTests.cs Updates test comments to reference --log-level.
src/Core/Telemetry/ILogLevelController.cs Updates interface documentation to reference --log-level.
src/Cli/Utils.cs Updates CLI documentation strings to reference --log-level.
src/Cli/Program.cs Updates early-flag scan to recognize --log-level.
src/Cli/Exporter.cs Updates StartOptions construction to include new legacy parameter.
src/Cli/CustomLoggerProvider.cs Updates comments to reference --log-level behavior in MCP mode.
src/Cli/ConfigGenerator.cs Adds legacy handling/deprecation warning and forwards --log-level to the service.
src/Cli/Commands/StartOptions.cs Adds --log-level option and hidden legacy --LogLevel option.
src/Cli.Tests/EndToEndTests.cs Updates e2e coverage to use --log-level and adds legacy cases.
src/Cli.Tests/CustomLoggerTests.cs Updates test docs to reference --log-level.
src/Azure.DataApiBuilder.Mcp/Core/McpStdioServer.cs Updates MCP docs/comments to reference --log-level.
Comments suppressed due to low confidence (1)

src/Cli/Program.cs:67

  • ParseEarlyFlags only recognizes the new "--log-level" flag. When users pass the legacy "--LogLevel" (which this PR still supports), MCP stdio logging behavior will be wrong because Utils.IsCliOverriding/CliLogLevel won’t be set before logger creation (leading to suppressed stderr output in MCP mode).
                if (string.Equals(arg, "--mcp-stdio", StringComparison.OrdinalIgnoreCase))
                {
                    Utils.IsMcpStdioMode = true;
                }
                else if (string.Equals(arg, "--log-level", StringComparison.OrdinalIgnoreCase) && i + 1 < args.Length)
                {
                    Utils.IsCliOverriding = true;
                    if (Enum.TryParse<LogLevel>(args[i + 1], ignoreCase: true, out LogLevel cliLogLevel))
                    {

Comment thread src/Service/Program.cs
Comment on lines +240 to 242
// Check if --log-level was explicitly specified via CLI (case-insensitive parsing)
int logLevelIndex = Array.FindIndex(args, a => string.Equals(a, "--log-level", StringComparison.OrdinalIgnoreCase));
bool hasCliLogLevel = logLevelIndex >= 0 && logLevelIndex + 1 < args.Length;
Comment on lines +833 to +835
[DataRow("--log-level information", DisplayName = "Case sensitivity: LogLevel Information from command line.")]
[DataRow("--LogLevel 0", DisplayName = "Case sensitivity: LogLevel legacy Information from command line.")]
[DataRow("--LogLevel information", DisplayName = "Case sensitivity: LogLevel legacy Information from command line.")]
Comment on lines +41 to +45
[Option("log-level", SetName = "loglevel", Required = false, HelpText = LOGLEVEL_HELPTEXT)]
public LogLevel? LogLevel { get; }

[Option("LogLevel", SetName = "LogLevel", Required = false, HelpText = LOGLEVEL_HELPTEXT, Hidden = true)]
public LogLevel? LogLevelLegacy { get; }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

4 participants