Skip to content

Conversation

@itzlambda
Copy link
Collaborator

@itzlambda itzlambda commented Dec 22, 2025

Cloud log aggregators don't support ANSI escape codes, causing ASCII artifacts in logs.

This PR:

  • Adds NO_COLOR environment variable support to the logging module
  • Sets NO_COLOR=1 for billing, payments, and API services in cloud compute config

Summary by CodeRabbit

Release Notes

  • New Features
    • Added support for the NO_COLOR environment variable to disable colored output in logging.

✏️ Tip: You can customize this high-level summary in your review settings.

Cloud log aggregators don't support ANSI escape codes, causing ASCII
artifacts in logs. This adds NO_COLOR environment variable support
and sets it for all cloud services.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 22, 2025

Walkthrough

These changes implement NO_COLOR environment variable support in the Rust logging layer to respect color preferences, then configure three ECS services in Terraform to disable ANSI colors in their logs by setting NO_COLOR="1".

Changes

Cohort / File(s) Summary
Logging ANSI override support
crates/basilica-common/src/logging/mod.rs
Added resolve_ansi_override function to check NO_COLOR environment variable and disable ANSI formatting when set. Refactored fmt layer configuration to apply the override conditionally.
Terraform ECS service configuration
scripts/cloud/compute.tf
Added NO_COLOR="1" environment variable to three ECS service modules (billing_service, payments_service, basilica_api_service) to disable colored log output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • The logging module change introduces straightforward environment variable checking with no complex branching logic
  • Terraform changes are repetitive configuration additions (same variable applied 3 times)
  • No API modifications or intricate control flow alterations

Poem

🐰 No colors for logs, just plain text delight,
NO_COLOR sets in, making debugging light,
Three services aligned with respectful grace,
ANSI stays quiet in this peaceful place! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main change: adding NO_COLOR environment variable support to disable ANSI colors in cloud deployments.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/disable-ansi-colors-cloud-logs

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d32e2df and b77c122.

📒 Files selected for processing (2)
  • crates/basilica-common/src/logging/mod.rs
  • scripts/cloud/compute.tf
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: lint-complex
  • GitHub Check: test-python-sdk (3.12)
  • GitHub Check: test-python-sdk (3.11)
  • GitHub Check: test-python-sdk (3.13)
  • GitHub Check: test-python-sdk (3.10)
  • GitHub Check: build-payments (stable)
  • GitHub Check: build-api (stable)
  • GitHub Check: build-cli (stable)
  • GitHub Check: build-validator (stable)
  • GitHub Check: build-miner (stable)
  • GitHub Check: build-billing (stable)
🔇 Additional comments (4)
scripts/cloud/compute.tf (1)

179-179: LGTM! Consistent NO_COLOR configuration across all services.

The addition of NO_COLOR="1" to all three ECS services (billing, payments, and API) is implemented consistently and will properly disable ANSI escape codes in cloud logs. The placement after RUST_LOG is logical and maintains clean organization.

Also applies to: 275-275, 414-414

crates/basilica-common/src/logging/mod.rs (3)

46-50: LGTM! Well-configured formatting layer.

The fmt layer configuration is appropriate for cloud deployments:

  • with_target(true) provides module path context
  • .compact() reduces log verbosity
  • Commented options for file/line numbers are preserved for future use

52-55: LGTM! Correct ANSI override integration.

The conditional application of the ANSI override is implemented correctly:

  • When NO_COLOR is set, with_ansi(false) disables color output
  • When NO_COLOR is absent, default behavior is preserved
  • Clean integration with the existing registry setup

Also applies to: 59-59


65-71: LGTM! Correct NO_COLOR standard implementation.

The implementation correctly follows the NO_COLOR standard:

  • Uses var_os().is_some() to check for variable presence regardless of value
  • Returns Some(false) to disable ANSI when NO_COLOR is set
  • Returns None to use default behavior otherwise

This ensures colors are disabled when NO_COLOR is present with any value (including empty string), which aligns with the standard specification.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@itzlambda itzlambda marked this pull request as ready for review December 22, 2025 08:18
@itzlambda itzlambda merged commit d026fe3 into main Dec 22, 2025
17 checks passed
@itzlambda itzlambda deleted the fix/disable-ansi-colors-cloud-logs branch December 22, 2025 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants