Skip to content

Remove Loguru color formatting#8079

Open
achamayou wants to merge 8 commits into
mainfrom
achamayou-remove-log-colors
Open

Remove Loguru color formatting#8079
achamayou wants to merge 8 commits into
mainfrom
achamayou-remove-log-colors

Conversation

@achamayou

@achamayou achamayou commented Jul 17, 2026

Copy link
Copy Markdown
Member

Summary

  • remove every explicit Loguru color option and markup producer from the Python test infrastructure
  • replace colored sink formats and request/response rendering with plain text
  • render levels in an aligned four-character field: TRC , DBG , INFO, SUCC, WARN, ERR , and CRIT
  • prefix success, warning, and error messages with ✅, ⚠️, and ❌ respectively; leave trace, debug, info, and critical messages unmarked
  • delete the Loguru tag-escaping helper, its fuzzing call sites, status-color selection, and the trailing-backslash workaround

This is an experimental alternative to #8078. The goal is to compare the simpler plain-text path with hardening the existing colored-markup path. No tests are added by this PR.

Scope and complexity

The repository-wide audit found 16 explicit Loguru color/markup sites across 10 files. After this change, searches for colors=True, colorize=, Loguru color tags, and the escaping helper return no production matches.

Before adding the requested compact level display and severity markers, the color-removal diff was 19 additions and 39 deletions: 20 fewer lines. The display mapping adds 30 lines, making the final diff 49 additions and 39 deletions: 10 more lines overall. The escaping, status-color, and backslash-workaround simplifications remain.

Visual comparison

These captures use the same intentional assertion at the start of recovery_snapshot_ledger_offset against the parent revision and the final formatter. The assertion existed only in temporary source copies and is not part of this PR.

Side-by-side terminal output showing colored Loguru failure output before the change and plain compact failure output after the change

Measurements

Stored size

A representative client-log workload emitted 100,000 request/response pairs (200,000 info-level lines) using the exact before/after runner formats and request/response message shapes with Loguru 0.7.3.

Sink Existing colored path Final plain path Reduction
Non-TTY/file 25,400,000 bytes 24,600,000 bytes 800,000 bytes (3.1%)
TTY color output captured to disk 45,400,000 bytes 24,600,000 bytes 20,800,000 bytes (45.8%)

Loguru already suppresses ANSI sequences for a non-TTY sink, so the non-TTY reduction comes from shortening the level field from eight characters to four. Capturing TTY-colored output stores 4,600,000 ANSI sequences totaling 20,000,000 bytes in this workload.

Rough runtime cost

A local process-CPU microbenchmark emitted 60,000 representative lines per sample into an in-memory counting sink, with 11 repetitions and median results. The final plain path includes the callable formatter that maps level labels and markers.

Path Median CPU time Throughput Overhead vs final plain
Final plain 1.281 s 46,829 lines/s -
Colored markup, non-TTY 1.750 s 34,286 lines/s 36.6%
Colored markup and ANSI generation, TTY 1.859 s 32,269 lines/s 45.1%

This is intentionally a rough microbenchmark rather than an application benchmark. It suggests markup parsing remains measurable for short, heavily marked-up client lines, although the level/marker formatter reduces the final plain path's advantage compared with a static plain format.

Recovery workload

For a real test-infrastructure workload, I ran recovery_snapshot_ledger_offset from recovery_test against the same released CCF 7.0.10 binary, once from the parent revision and once with the final formatter. Both runs completed successfully under a pseudo-terminal and emitted exactly 1,168 output records.

Capture Stored bytes ANSI sequences ANSI bytes
Existing colored path 327,388 16,238 70,738
Final plain path 252,252 0 0

The final plain capture is 75,136 bytes smaller (22.95%). ANSI sequences account for 70,738 bytes of that reduction; compact level fields and run-specific plain text account for the remaining 4,398-byte net reduction. The final run included 12 success markers. Single-run wall times were too noisy to attribute to formatting, so the isolated repeated microbenchmark above is the runtime comparison.

The matching VMSS Virtual B CI job and all other required ACI and VMSS jobs passed on the color-removal commit. Fresh CI is running for the final formatting and no-test cleanup. CTest job logs cannot provide a stored-size comparison because successful test stdout is hidden and CTest uses a non-TTY.

Validation

  • Black and Ruff on all changed Python files
  • no tests added

achamayou and others added 3 commits July 17, 2026 09:28
Replace colored sink formats and message markup with plain text, then delete the escaping and trailing-backslash handling that Loguru colors required.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ea7056a8-6011-4c84-b93d-099bb9b8094f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ea7056a8-6011-4c84-b93d-099bb9b8094f
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ea7056a8-6011-4c84-b93d-099bb9b8094f
@achamayou achamayou changed the title Experiment: remove Loguru color formatting Remove Loguru color formatting Jul 17, 2026
@achamayou
achamayou marked this pull request as ready for review July 17, 2026 10:06
@achamayou
achamayou requested a review from a team as a code owner July 17, 2026 10:06
Copilot AI review requested due to automatic review settings July 17, 2026 10:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 removes Loguru color/markup usage from the Python test infrastructure and switches logging output to a plain-text format with compact, aligned level display and optional severity markers. This fits into the test/infra tooling by simplifying log rendering and avoiding markup parsing overhead and escaping complexity.

Changes:

  • Removed colors=True usage and Loguru color tags from test/infra logging call sites and sink formats.
  • Introduced a shared format_log_record() formatter to render compact 4-character level labels in key test runners.
  • Removed the Loguru tag-escaping helper and updated request/response rendering to plain text.

Custom instructions used:

  • .github/copilot-instructions.md
  • .github/instructions/reviewing.instructions.md

Reviewed changes

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

Show a summary per file
File Description
tests/suite/test_requirements.py Removes colored “Test:” banner logging.
tests/start_network.py Switches stdout sink format to plain text.
tests/infra/runner.py Uses shared callable formatter for runner stdout logs.
tests/infra/piccolo/analyzer.py Removes color tags from analyzer output formats.
tests/infra/log_capture.py Drops colors=True when flushing captured lines.
tests/infra/e2e_args.py Adds shared compact level/marker formatter and uses it as default sink format.
tests/infra/clients.py Removes Loguru tag escaping and switches request/response string rendering to plain text.
tests/governance.py Removes colored “Test:” log line in single-node governance run.
tests/fuzzing.py Stops escaping sent/received fuzzing log lines for Loguru markup.
tests/cleanup_iptables.py Switches stdout sink format to plain text.

Comment thread tests/infra/e2e_args.py
Comment thread tests/infra/e2e_args.py
Comment thread tests/infra/clients.py
achamayou and others added 2 commits July 17, 2026 11:44
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@achamayou

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>

Copilot AI commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Fixed in 513b525. I pulled in the main-side tests/infra/clients.py binary content-type handling so the branch now merges cleanly locally, then re-ran ./scripts/python-format-checks.sh, ./scripts/python-lint-checks.sh, python3 -m py_compile on the PR's changed Python files, and parallel validation.

@achamayou

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>

Copilot AI commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Fixed in 3455a0d. I aligned tests/infra/clients.py with the current main branch and verified a clean local merge against the latest origin/main, then re-ran ./scripts/python-format-checks.sh, ./scripts/python-lint-checks.sh, python3 -m py_compile on the PR's changed Python files, and parallel validation.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants