-
-
Notifications
You must be signed in to change notification settings - Fork 0
Closed
Labels
Description
Problem
Configuration includes a buffer option that is parsed but never used, creating confusion for users.
Current Implementation
// pkg/config/config.go:136
type OutputConfig struct {
Format string `yaml:"format"`
Buffer string `yaml:"buffer"` // Parsed but NEVER used!
}Configuration example:
output:
format: text
buffer: line # This does nothing!Issue
The buffer field is:
- ✅ Defined in the config struct
- ✅ Parsed from YAML
- ✅ Validated (sort of)
- ❌ NEVER used in any code path
User impact: Users may set buffer: none expecting unbuffered output, but it has no effect.
Decision Required
We need to decide: Implement it or remove it?
Option 1: Implement Buffering Modes (Recommended)
Add three buffering modes:
1. buffer: line (default)
- Buffer per line (current behavior)
- Use
bufio.Scannerwith line splitting - Output after each complete line
2. buffer: none
- Unbuffered output (byte-by-byte)
- Use raw pipe reads without buffering
- Maximum real-time responsiveness
- Higher syscall overhead
3. buffer: full
- Buffer entire output until command completes
- Useful for formatting/sorting entire output
- Not real-time
Implementation:
// pkg/processor/processor.go
func (p *Processor) ProcessStreams(stdout, stderr io.Reader, ...) error {
switch p.config.Output.Buffer {
case "none":
return p.processUnbuffered(stdout, stderr)
case "line":
return p.processLineBuffered(stdout, stderr) // Current
case "full":
return p.processFullyBuffered(stdout, stderr)
default:
return fmt.Errorf("invalid buffer mode: %s", p.config.Output.Buffer)
}
}
func (p *Processor) processUnbuffered(stdout, stderr io.Reader) error {
// Read byte-by-byte, no buffering
buf := make([]byte, 1024)
for {
n, err := stdout.Read(buf)
if n > 0 {
fmt.Print(string(buf[:n])) // No line splitting
}
if err != nil {
return err
}
}
}Option 2: Remove It (Simpler)
If buffering modes aren't needed:
1. Remove from config struct:
type OutputConfig struct {
Format string `yaml:"format"`
// buffer: removed
}2. Update validation:
// Remove buffer validation from validation.go3. Update documentation:
- Remove from example config files
- Add migration note if any users rely on it
Recommendation
Implement it (Option 1) because:
- Users may genuinely need different buffering modes
buffer: noneis useful for debugging (truly real-time)buffer: fullenables future features (sorting, filtering)- It's already half-implemented (parsing exists)
Use Cases
buffer: none - Unbuffered:
- Interactive debugging
- Progress bars that need immediate updates
- Tailing logs in real-time
buffer: line - Line buffered (current):
- Standard log processing
- Good balance of performance and real-time
buffer: full - Fully buffered:
- Post-processing entire output
- Sorting log lines by level
- Filtering before output
Implementation Checklist
If implementing (Option 1):
- Implement
processUnbuffered()method - Implement
processFullyBuffered()method - Update validation to allow: none, line, full
- Add tests for each buffering mode
- Update documentation with examples
- Add performance notes (unbuffered has overhead)
If removing (Option 2):
- Remove
Bufferfield from config struct - Remove buffer validation
- Update example config files
- Add migration note to CHANGELOG
- Search for any references to buffer config
Testing
func TestBufferingModes(t *testing.T) {
tests := []struct {
mode string
expected string
}{
{"line", "line-by-line output"},
{"none", "unbuffered byte stream"},
{"full", "complete buffered output"},
}
for _, tt := range tests {
cfg := &config.Config{
Output: config.OutputConfig{
Buffer: tt.mode,
},
}
proc := New(cfg, formatter)
// Test buffering behavior
}
}Performance Considerations
Unbuffered mode overhead:
- More syscalls (read byte-by-byte)
- Higher CPU usage
- Lower throughput
- Document tradeoffs
Fully buffered mode:
- Delayed output (not real-time)
- Higher memory usage
- Better for post-processing
Breaking Changes
If implementing:
- None - backward compatible (default stays
line)
If removing:
- Minor - users with
buffer:in config will get unknown field warning - Add migration guide
Related Issues
- Add comprehensive package-level documentation #14 - Package documentation (document buffering modes)
- Add end-to-end integration tests for complete pipeline #9 - Integration tests (test different buffering modes)
References
- Go bufio package: https://pkg.go.dev/bufio
- Unix buffering modes: https://www.gnu.org/software/libc/manual/html_node/Buffering-Concepts.html
Reactions are currently unavailable