Skip to content

Commit b5ab209

Browse files
josecelanojosecelano
committed
docs: add follow-up issue #5 for ANSI codes in file logging
Add comprehensive specification for removing ANSI color codes from log files when using compact and pretty formats. Recommends independent format control (--log-file-format, --log-stderr-format) to give users full control over logging behavior. Discovered during PR #4 review. JSON format already works correctly. Related to #2, #3
1 parent deb2883 commit b5ab209

File tree

2 files changed

+377
-0
lines changed

2 files changed

+377
-0
lines changed

‎docs/issues/5-remove-ansi-codes-from-file-logging.md‎

Lines changed: 375 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,375 @@
1+
# Remove ANSI Color Codes from File Logging
2+
3+
**Issue**: [#5](https://github.com/torrust/torrust-tracker-deployer/issues/5) (follow-up to #3)
4+
**Parent Epic**: [#2](https://github.com/torrust/torrust-tracker-deployer/issues/2) - Scaffolding for main app
5+
**Related**:
6+
7+
- Issue [#3](https://github.com/torrust/torrust-tracker-deployer/issues/3) - Setup logging for production CLI
8+
- [User Output vs Logging Separation](../research/UX/user-output-vs-logging-separation.md)
9+
- [Development Principles](../development-principles.md)
10+
11+
## Overview
12+
13+
Remove ANSI color codes from log files when using `compact` and `pretty` formats, or implement independent format control for file vs stderr outputs. Currently, both `compact` and `pretty` formats write ANSI escape sequences to log files, making them harder to parse with standard text tools (grep, awk, editors).
14+
15+
This is a follow-up refinement to Issue #3, discovered during PR review. The JSON format correctly omits ANSI codes, but the text-based formats include them regardless of output destination.
16+
17+
## Problem Statement
18+
19+
### Current Behavior
20+
21+
When using `--log-format compact` or `--log-format pretty` with file logging:
22+
23+
```bash
24+
# Logs contain ANSI escape codes in files
25+
$ torrust-tracker-deployer --log-format compact --log-dir /tmp/logs
26+
$ hexdump -C /tmp/logs/log.txt | head -5
27+
00000000 1b 5b 32 6d 32 30 32 35 2d 31 30 2d 31 35 54 31 |.[2m2025-10-15T1|
28+
00000010 35 3a 30 39 3a 33 31 2e 34 39 39 36 35 32 5a 1b |5:09:31.499652Z.|
29+
00000020 5b 30 6d 20 1b 5b 33 32 6d 20 49 4e 46 4f 1b 5b |[0m .[32m INFO.[|
30+
00000030 30 6d 20 1b 5b 32 6d 74 6f 72 72 75 73 74 5f 74 |0m .[2mtorrust_t|
31+
```
32+
33+
ANSI codes present:
34+
35+
- `\x1b[2m` - dim text
36+
- `\x1b[32m` - green color
37+
- `\x1b[0m` - reset formatting
38+
39+
### Why This Is Problematic
40+
41+
1. **Parsing difficulty**: Tools like `grep`, `awk`, `sed` see escape sequences as part of the text
42+
2. **File size**: ANSI codes add ~15-20% overhead to log file size
43+
3. **Readability**: Opening in plain text editors shows escape codes instead of text
44+
4. **Processing**: Log aggregation tools may not handle ANSI codes correctly
45+
5. **Searching**: Searching for patterns becomes harder with embedded codes
46+
47+
### What Works Correctly
48+
49+
✅ **JSON format**: No ANSI codes, clean machine-readable output
50+
51+
```json
52+
{
53+
"timestamp": "2025-10-15T15:09:48.417627Z",
54+
"level": "INFO",
55+
"fields": { "message": "Application started" }
56+
}
57+
```
58+
59+
## Goals
60+
61+
- [ ] Remove ANSI codes from log files when using `compact` format
62+
- [ ] Remove ANSI codes from log files when using `pretty` format
63+
- [ ] Keep ANSI codes for stderr output (development mode)
64+
- [ ] Maintain JSON format as-is (already correct)
65+
- [ ] Update documentation to reflect behavior
66+
- [ ] Verify backwards compatibility with existing log files
67+
68+
## Specifications
69+
70+
### Option A: Independent Format Control (Recommended)
71+
72+
Allow users to specify different formats for file and stderr, giving them full control over what format they want in each destination:
73+
74+
```rust
75+
#[derive(Parser)]
76+
pub struct Cli {
77+
/// Format for file logging (default: compact without ANSI)
78+
#[arg(long, value_enum, default_value = "compact")]
79+
pub log_file_format: LogFormat,
80+
81+
/// Format for stderr logging (default: pretty with ANSI)
82+
#[arg(long, value_enum, default_value = "pretty")]
83+
pub log_stderr_format: LogFormat,
84+
85+
// ... other args
86+
}
87+
```
88+
89+
Example usage:
90+
91+
```bash
92+
# JSON to file, pretty to stderr (common for development)
93+
torrust-tracker-deployer \
94+
--log-file-format json \
95+
--log-stderr-format pretty \
96+
--log-output file-and-stderr
97+
98+
# Compact to file, compact to stderr (production with real-time monitoring)
99+
torrust-tracker-deployer \
100+
--log-file-format compact \
101+
--log-stderr-format compact \
102+
--log-output file-and-stderr
103+
104+
# JSON to file only (production default)
105+
torrust-tracker-deployer --log-file-format json --log-output file-only
106+
```
107+
108+
**Benefits:**
109+
110+
- Users choose exactly what format they want for each destination
111+
- No assumptions about user preferences (some may want compact in files, others JSON)
112+
- ANSI codes automatically disabled for file formats (compact/pretty write without ANSI)
113+
- ANSI codes automatically enabled for stderr formats (compact/pretty write with ANSI)
114+
- Backwards compatible: can deprecate old `--log-format` in favor of new flags
115+
116+
### Option B: Auto-detect and Force No-ANSI for Files (Simpler but Less Flexible)
117+
118+
Automatically disable ANSI codes when writing to files, regardless of format chosen:
119+
120+
```rust
121+
// Pseudocode - actual implementation in src/logging.rs
122+
match output_mode {
123+
LogOutput::FileOnly => {
124+
// File layer: no ANSI codes (forced)
125+
let file_layer = fmt::layer()
126+
.with_ansi(false) // Always disable ANSI for files
127+
.with_format(self.format)
128+
.with_writer(file);
129+
130+
subscriber.with(file_layer)
131+
},
132+
LogOutput::FileAndStderr => {
133+
// File layer: no ANSI codes (forced)
134+
let file_layer = fmt::layer()
135+
.with_ansi(false)
136+
.with_format(self.format)
137+
.with_writer(file);
138+
139+
// Stderr layer: with ANSI codes (forced)
140+
let stderr_layer = fmt::layer()
141+
.with_ansi(true)
142+
.with_format(self.format)
143+
.with_writer(stderr);
144+
145+
subscriber.with(file_layer).with(stderr_layer)
146+
}
147+
}
148+
```
149+
150+
**Drawbacks:**
151+
152+
- Forces same format for both file and stderr
153+
- Removes user choice (what if they want pretty in files with ANSI?)
154+
- Less flexible for advanced use cases
155+
156+
### Recommendation
157+
158+
**Implement Option A** (independent format control):
159+
160+
- Gives users full control over logging behavior
161+
- Most flexible solution
162+
- Better aligns with principle of not making assumptions
163+
- Can support diverse production scenarios (JSON for aggregation, compact for grep, pretty for debugging)
164+
165+
## Implementation Plan
166+
167+
### Phase 1: Update CLI Arguments (1-2 hours)
168+
169+
- [ ] Task 1.1: Add `--log-file-format` argument to `src/app.rs`
170+
- [ ] Task 1.2: Add `--log-stderr-format` argument to `src/app.rs`
171+
- [ ] Task 1.3: Deprecate old `--log-format` argument (or keep for backwards compat)
172+
- [ ] Task 1.4: Update `LoggingBuilder` to accept separate file and stderr formats
173+
- [ ] Task 1.5: Update help text to explain independent format control
174+
175+
### Phase 2: Modify Logging Layer (1-2 hours)
176+
177+
- [ ] Task 2.1: Update `src/logging.rs` to support two separate formats
178+
- [ ] Task 2.2: Add `.with_ansi(false)` for file writers (all formats)
179+
- [ ] Task 2.3: Add `.with_ansi(true)` for stderr writers (all formats)
180+
- [ ] Task 2.4: Handle `FileOnly` mode with single format
181+
- [ ] Task 2.5: Handle `FileAndStderr` mode with dual formats
182+
- [ ] Task 2.6: Test all format combinations (compact, pretty, json × file-only, file-and-stderr)
183+
184+
### Phase 3: Update Tests (45 minutes)
185+
186+
- [ ] Task 3.1: Update `tests/logging_integration.rs` to verify no ANSI in files
187+
- [ ] Task 3.2: Add test to verify ANSI codes present in stderr output
188+
- [ ] Task 3.3: Test different format combinations (json file + pretty stderr, etc.)
189+
- [ ] Task 3.4: Verify backwards compatibility if keeping old `--log-format`
190+
- [ ] Task 3.5: Review E2E test binaries (`src/bin/e2e_*.rs`) - verify if they need updates or if defaults are acceptable
191+
192+
### Phase 4: Documentation Updates (45 minutes)
193+
194+
- [ ] Task 4.1: Update `docs/user-guide/logging.md` to explain independent format control
195+
- [ ] Task 4.2: Add examples showing different format combinations
196+
- [ ] Task 4.3: Add note about ANSI codes (files clean, stderr colored)
197+
- [ ] Task 4.4: Update `README.md` with new argument examples
198+
- [ ] Task 4.5: Update `docs/contributing/logging-guide.md` for contributors
199+
- [ ] Task 4.6: Document migration path from old `--log-format` (if deprecated)
200+
201+
### Phase 5: Verification (30 minutes)
202+
203+
- [ ] Task 5.1: Manual testing with all format combinations
204+
- [ ] Task 5.2: Verify file sizes reduced (no ANSI overhead in files)
205+
- [ ] Task 5.3: Test grep/awk/sed work cleanly on log files
206+
- [ ] Task 5.4: Verify stderr colors display correctly in terminal
207+
- [ ] Task 5.5: Test backwards compatibility (if keeping old args)
208+
- [ ] Task 5.6: Run full test suite (`./scripts/pre-commit.sh`)
209+
210+
## Technical Details
211+
212+
### Current Implementation Location
213+
214+
The logging configuration is in `src/logging.rs`, specifically in the `LoggingBuilder` implementation:
215+
216+
```rust
217+
// Current code (approximate location)
218+
pub fn init(self) {
219+
// ... setup code ...
220+
221+
let file_layer = fmt::layer()
222+
.with_format(self.format)
223+
.with_writer(file);
224+
225+
// Need to add: .with_ansi(false) here for files
226+
}
227+
```
228+
229+
### Key Files to Modify
230+
231+
1. **`src/app.rs`**: Add new CLI arguments (`--log-file-format`, `--log-stderr-format`)
232+
2. **`src/logging.rs`**: Main implementation (add `.with_ansi()` calls, support dual formats)
233+
3. **`src/bin/e2e_*.rs`**: E2E test binaries (verify logging setup, may need updates)
234+
4. **`tests/logging_integration.rs`**: Add/update tests for ANSI detection
235+
5. **`docs/user-guide/logging.md`**: Document clean file output behavior
236+
6. **`docs/contributing/logging-guide.md`**: Update contributor guide
237+
238+
### Backward Compatibility
239+
240+
**Breaking changes** (requires major version bump or deprecation period):
241+
242+
- Old `--log-format` argument replaced with `--log-file-format` and `--log-stderr-format`
243+
- **Migration path**: Keep `--log-format` for backwards compatibility, applies to both outputs if new args not specified
244+
245+
**Recommended approach** (backwards compatible):
246+
247+
```rust
248+
// If new args not provided, fall back to old --log-format for both
249+
let file_format = cli.log_file_format.unwrap_or(cli.log_format);
250+
let stderr_format = cli.log_stderr_format.unwrap_or(cli.log_format);
251+
```
252+
253+
**No data breaking changes**:
254+
255+
- Existing log files with ANSI codes remain valid
256+
- New log files will be cleaner and easier to process
257+
- JSON format unchanged
258+
- E2E test binaries continue to work
259+
260+
## Acceptance Criteria
261+
262+
- [ ] Compact format files contain no ANSI escape codes
263+
- [ ] Pretty format files contain no ANSI escape codes
264+
- [ ] JSON format files remain unchanged (no ANSI codes)
265+
- [ ] Stderr output shows colors when using `--log-output file-and-stderr`
266+
- [ ] File-only mode produces clean, grep-friendly logs
267+
- [ ] Log file sizes reduced (less ANSI overhead)
268+
- [ ] Documentation updated to explain ANSI behavior
269+
- [ ] Integration tests verify ANSI presence/absence
270+
- [ ] All existing tests pass (`./scripts/pre-commit.sh`)
271+
- [ ] Manual testing confirms grep/awk work cleanly on log files
272+
273+
## Example Outputs
274+
275+
### Before (Current - with ANSI codes in files)
276+
277+
```bash
278+
$ torrust-tracker-deployer --log-format compact
279+
$ cat data/logs/log.txt
280+
^[[2m2025-10-15T15:09:31.499652Z^[[0m ^[[32m INFO^[[0m ^[[2mtorrust_tracker_deployer::app^[[0m...
281+
# Hard to grep, includes escape sequences
282+
```
283+
284+
### After (Fixed - no ANSI codes in files)
285+
286+
```bash
287+
$ torrust-tracker-deployer --log-format compact
288+
$ cat data/logs/log.txt
289+
2025-10-15T15:09:31.499652Z INFO torrust_tracker_deployer::app: Application started...
290+
# Clean, grep-friendly output
291+
292+
$ grep "Application started" data/logs/log.txt
293+
2025-10-15T15:09:31.499652Z INFO torrust_tracker_deployer::app: Application started...
294+
# Works perfectly!
295+
```
296+
297+
### Stderr Still Has Colors (Development Mode)
298+
299+
```bash
300+
$ torrust-tracker-deployer --log-format pretty --log-output file-and-stderr
301+
2025-10-15T15:09:31.499652Z INFO torrust_tracker_deployer::app: Application started...
302+
# Terminal shows colored output for real-time monitoring
303+
# But file remains clean for post-mortem analysis
304+
```
305+
306+
## Related Documentation
307+
308+
- [User Output vs Logging Separation](../research/UX/user-output-vs-logging-separation.md) - Design principles
309+
- [Development Principles](../development-principles.md) - Observability and user-friendliness
310+
- [Logging User Guide](../user-guide/logging.md) - End-user documentation
311+
- [Logging Contributor Guide](../contributing/logging-guide.md) - Developer documentation
312+
- Issue #3 - Parent issue that introduced logging CLI
313+
- [tracing-subscriber documentation](https://docs.rs/tracing-subscriber/) - Upstream library reference
314+
315+
## Notes
316+
317+
### Why This Wasn't Caught Earlier
318+
319+
- The implementation in Issue #3 correctly followed standard Rust logging patterns
320+
- ANSI codes in files is common behavior for many Rust logging libraries
321+
- The focus was on getting logging infrastructure working (achieved successfully)
322+
- This is a refinement, not a bug - JSON format works correctly for production
323+
324+
### Production Workaround (Until Fixed)
325+
326+
Users who need clean log files immediately can:
327+
328+
```bash
329+
# Option 1: Use JSON format (already clean)
330+
torrust-tracker-deployer --log-format json
331+
332+
# Option 2: Strip ANSI codes with sed (post-processing)
333+
sed 's/\x1b\[[0-9;]*m//g' data/logs/log.txt > data/logs/log-clean.txt
334+
```
335+
336+
### E2E Test Binaries Consideration
337+
338+
The E2E test binaries (`src/bin/e2e_provision_tests.rs`, `src/bin/e2e_config_tests.rs`, etc.) currently initialize logging directly:
339+
340+
```rust
341+
// Example from e2e_provision_tests.rs
342+
LoggingBuilder::new(std::path::Path::new("./data/logs"))
343+
.with_format(LogFormat::Compact)
344+
.with_output(LogOutput::FileAndStderr)
345+
.init();
346+
```
347+
348+
**Action needed**: Review whether E2E tests should:
349+
350+
- **Option 1**: Keep current behavior (compact format for both file and stderr) - likely acceptable since they need file+stderr for CI visibility
351+
- **Option 2**: Update to use new dual format API once available
352+
- **Option 3**: Use JSON for files, pretty for stderr (better for debugging)
353+
354+
**Recommendation**: Start with Option 1 (no changes needed) unless testing reveals issues with ANSI codes in E2E log files affecting CI/CD tooling.
355+
356+
### Future Enhancements (Out of Scope)
357+
358+
- Automatic log rotation based on size
359+
- Compression of old log files
360+
- Integration with log aggregation services (CloudWatch, DataDog, etc.)
361+
- Separate verbosity control for file vs stderr
362+
- Custom format templates
363+
364+
## Estimated Effort
365+
366+
**Total**: 5-6 hours
367+
368+
- Phase 1: 1-2 hours (CLI arguments update)
369+
- Phase 2: 1-2 hours (logging layer modifications)
370+
- Phase 3: 45 minutes (tests)
371+
- Phase 4: 45 minutes (documentation)
372+
- Phase 5: 30 minutes (verification)
373+
- Buffer: 1 hour
374+
375+
This is a moderate refactoring that adds new functionality while maintaining backwards compatibility.

‎project-words.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ exitcode
5050
getent
5151
Gossman
5252
handleable
53+
hexdump
5354
Hostnames
5455
hotfixes
5556
htdocs
@@ -69,6 +70,7 @@ maxbytes
6970
mgmt
7071
millis
7172
mocksecret
73+
mtorrust
7274
multiprocess
7375
MVVM
7476
myapp

0 commit comments

Comments
 (0)