Skip to content

fix(stackable-telemetry): Disable ANSI colors when stdout is not a terminal#1183

Merged
Techassi merged 2 commits into
mainfrom
push-tsylnwpxvwrr
Mar 30, 2026
Merged

fix(stackable-telemetry): Disable ANSI colors when stdout is not a terminal#1183
Techassi merged 2 commits into
mainfrom
push-tsylnwpxvwrr

Conversation

@lfrancke

@lfrancke lfrancke commented Mar 30, 2026

Copy link
Copy Markdown
Member

Summary

  • Only emit ANSI escape codes in console log output when stdout is actually a terminal
  • Applies to both Plain and Json console log formats

Problem

tracing_subscriber::fmt::layer() enables ANSI colors by default whenever the ansi feature is enabled and NO_COLOR is unset (source):

let ansi = cfg!(feature = "ansi") && env::var("NO_COLOR").map_or(true, |v| v.is_empty());

There is no automatic TTY detection — this is a known upstream issue open since 2021. As a result, containerdebug (and any other consumer of stackable-telemetry) emits raw ANSI codes when running in containers where logs are collected by aggregation systems like Graylog or Vector:

[2m2026-03-29T18:26:23.658052Z[0m [32m INFO[0m [1mcontainerdebug run[0m...

Fix

Explicitly check std::io::stdout().is_terminal() before configuring the subscriber, so ANSI is only used when writing to an actual terminal.

Test plan

  • cargo check -p stackable-telemetry passes
  • Verify in a terminal: colored output still works
  • Verify in a container/pipe: ANSI codes are suppressed

@lfrancke lfrancke self-assigned this Mar 30, 2026
@lfrancke lfrancke moved this to Refinement Acceptance: Waiting for in Stackable Engineering Mar 30, 2026
@lfrancke lfrancke moved this from Refinement Acceptance: Waiting for to Development: Waiting for Review in Stackable Engineering Mar 30, 2026
NickLarsenNZ
NickLarsenNZ previously approved these changes Mar 30, 2026

@NickLarsenNZ NickLarsenNZ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@NickLarsenNZ NickLarsenNZ moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Mar 30, 2026

@Techassi Techassi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should include a changelog entry. Otherwise looks good.

@NickLarsenNZ

Copy link
Copy Markdown
Member

I will try to get around to testing this with containerdebug.

@Techassi Techassi added this pull request to the merge queue Mar 30, 2026
@Techassi Techassi removed this pull request from the merge queue due to a manual request Mar 30, 2026
@Techassi

Copy link
Copy Markdown
Member

Okay I have confirmed that both

cargo run -- --output state.json > logs.txt
cargo run -- --output state.json

produce the expected output:

  • When logging to a terminal, the ANSI escape sequences are emitted which results in colored console output.
  • When piped to a text file, the log messages are written as plain text.

This PR can be merged and released which allows us to bump to a never stackable-telemetry version in containerdebug.

@Techassi Techassi added this pull request to the merge queue Mar 30, 2026
Merged via the queue into main with commit 957e602 Mar 30, 2026
5 checks passed
@Techassi Techassi deleted the push-tsylnwpxvwrr branch March 30, 2026 13:25
@sbernauer sbernauer moved this from Development: In Review to Development: Done in Stackable Engineering Apr 1, 2026
@lfrancke lfrancke moved this from Development: Done to Done in Stackable Engineering Apr 9, 2026
@sbernauer

Copy link
Copy Markdown
Member

Sadly this means we loose colored output, e.g. in k9s.

Is this intentional or can you think of any way to restore it? :)

image

@sbernauer

Copy link
Copy Markdown
Member

Raised #1237 with a different approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants