-
Notifications
You must be signed in to change notification settings - Fork 79
Fix extra logging in non-root commands #1500
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
Fix extra logging in non-root commands #1500
Conversation
Co-authored-by: waldekmastykarz <11164679+waldekmastykarz@users.noreply.github.com>
waldekmastykarz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot is this the best way to implement this? The stdio command has something similar implemented too. Is there a way to configure it centrally and avoid duplication? Only the root command should have the extended logging. All other commands should only print console.writelines. All other logs should be invisible
Co-authored-by: waldekmastykarz <11164679+waldekmastykarz@users.noreply.github.com>
Refactored in ccf902b. The logging is now configured centrally in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
waldekmastykarz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot ok, it must be done differently, because now we're surpressing all output which isn't great. jwt and stdio shouldn't log anything to console. Other commands should
Co-authored-by: waldekmastykarz <11164679+waldekmastykarz@users.noreply.github.com>
Fixed in eb12d79. Updated the logging configuration:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot is this the best way to solve it? |
The current approach is a reasonable solution given the constraint that logging must be configured early in app startup, before System.CommandLine parses commands. An alternative would be to configure all subcommands to have no console logging by default (like Would you prefer that simpler approach? It would mean commands like |
|
@copilot let's stick with the current approach. We can always change it in the future |
There was a problem hiding this 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 fixes extra logging in non-root commands by configuring logging centrally based on command type. The issue was that plugin messages and other diagnostic logs were appearing in the output of utility commands like jwt create, interfering with their intended output.
Changes:
- Added
IsJwtCommandproperty to detect when the JWT command is being run and suppress all console logging - Added
IsRootCommandproperty to detect when the root proxy command is being invoked (vs. subcommands) - Refactored logging configuration to use rich logging for root command, no logging for JWT command, and simple console logging with filtered plugin messages for other subcommands
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| DevProxy/Commands/DevProxyCommand.cs | Added IsJwtCommand and IsRootCommand static properties to detect which command is being executed based on command-line arguments |
| DevProxy/Extensions/ILoggingBuilderExtensions.cs | Refactored logging configuration to use conditional logic based on command type, with different logging strategies for stdio, jwt, root, and other subcommands |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…nd adding console formatter
…/github.com/dotnet/dev-proxy into copilot/fix-extra-logging-non-root-commands
@copilot include outdated in the list of commands that should use simplified logging |
Co-authored-by: waldekmastykarz <11164679+waldekmastykarz@users.noreply.github.com>
Fixed in 8ecf3a4. The subcommands branch was incorrectly using the same rich logging as the root command. Now it properly uses
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot revert the last change. For the |
Co-authored-by: waldekmastykarz <11164679+waldekmastykarz@users.noreply.github.com>
Fixed in c7c1793. Reverted to use rich logging with
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|


Fixes extra logging in non-root commands by configuring logging centrally based on command type.
Changes Made
ProxyConsoleFormatter(unchanged)stdiocommand: Uses file logging only (unchanged)jwtcommand: No console logging - all logging suppressed so only the token output is visiblecert,config,outdated,msgraphdb): Rich logging withProxyConsoleFormatterbut with plugin messages filtered outAdded
IsRootCommandproperty that detects if the first argument starts with-(option) or there are no arguments, automatically working for any new subcommand without code changes.Added
IsJwtCommandproperty to handle the jwt command's special case where no logging should be visible.Testing
jwt createoutputs only the token without extra loggingoutdatedshows its output correctly with rich formatting (no plugin messages)cert --helpandconfig --helpwork correctly--helpdisplays correctly with rich formattingOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.