Fix ANSI leaks in plain logs and telemetry output#16225
Conversation
sebastienros
requested review from
JamesNK,
davidfowl and
mitchdenny
as code owners
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sebastienros
force-pushed
the
sebastienros/sebros-fix-plain-logs-ansi-output
branch
from
f1b9697 to
fab8032
Compare
There was a problem hiding this comment.
Pull request overview
Fixes ANSI/styling “leaks” in plain-text CLI scenarios (notably aspire logs and aspire otel logs) by making non-interactive and non-ANSI outputs reliably control-sequence-free, while preserving JSON output for automation.
Changes:
- Make
--non-interactiveforceSupportsAnsi=false(and update related tests). - Configure Spectre.Console to default to plain rendering unless ANSI is explicitly enabled via resolved host capabilities; disable hyperlinks when ANSI is off.
- Strip embedded ANSI/control sequences from human-readable
logs/otel logsoutput when ANSI is disabled; add coverage to ensure JSON preserves raw content.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Utils/CliHostEnvironmentTests.cs | Adds/updates tests to ensure non-interactive mode disables ANSI. |
| tests/Aspire.Cli.Tests/Commands/TelemetryLogsCommandTests.cs | Adds test to ensure telemetry log body strips ANSI when ANSI is disabled. |
| tests/Aspire.Cli.Tests/Commands/LogsCommandTests.cs | Adds tests for stripping ANSI in text output and preserving ANSI in JSON output; improves test injection of log lines. |
| src/Aspire.Cli/Utils/CliHostEnvironment.cs | Makes --non-interactive explicitly disable ANSI support. |
| src/Aspire.Cli/Program.cs | Adjusts Spectre console settings to honor resolved ANSI capability and disables hyperlink capability when ANSI is off. |
| src/Aspire.Cli/Commands/TelemetryLogsCommand.cs | Strips embedded control sequences from log bodies when ANSI is disabled. |
| src/Aspire.Cli/Commands/LogsCommand.cs | Strips embedded control sequences from log content when ANSI is disabled (while keeping JSON intact). |
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16225Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16225" |
|
I agree with the change to strip ANSI from external sources (logs) when not supported, but why should If an environment doesn't support ANSI then existing built-in detection should turn it off. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Because we also use The log-payload stripping is independent of that. This part just keeps the CLI's own formatting behavior aligned with |
|
But non-interactive is just saying no interaction please. It shouldn't control what the output is. If stdout is redirected to a then ANSI is disabled. And if a terminal doesn't support ANSI then it would use the standard NO_COLOR env var when it runs the process to suppress ANSI. I think there are lots of options already. Non-interactive doesn't need to control more than it needs to. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Agreed. I updated the branch so |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
Clean refactoring and good test coverage for the ANSI stripping in plain-text output. Two items flagged: one potential unintended color downgrade in playground mode, one test robustness suggestion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ix-plain-logs-ansi-output
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🎬 CLI E2E Test Recordings — 72 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24540519621 |
Description
Fixes the plain-text output leak reported in #15845. In non-ANSI and automation-oriented scenarios, the CLI could still emit Spectre styling or pass through embedded ANSI sequences, which showed up as raw fragments in
aspire logsand telemetry output.This change makes
--non-interactiveforce plain console rendering, updates the CLI console setup to honor the resolved ANSI capability instead of falling back to auto-detection, disables hyperlinks when ANSI is off, and strips embedded control sequences from human-readableaspire logsandaspire otel logsoutput while preserving JSON output.Validation:
./dotnet.sh test tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj -- --filter-class "*.LogsCommandTests" --filter-class "*.TelemetryLogsCommandTests" --filter-class "*.TelemetryTracesCommandTests" --filter-class "*.CliHostEnvironmentTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"Fixes #15845
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: