diff --git a/cmd/cli/control_server.go b/cmd/cli/control_server.go index ffacea3..1c9d37c 100644 --- a/cmd/cli/control_server.go +++ b/cmd/cli/control_server.go @@ -283,7 +283,7 @@ func (p *prog) registerControlServerHandler() { } })) p.cs.register(viewLogsPath, http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { - lr, err := p.logReader() + lr, err := p.logReaderRaw() if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return @@ -309,7 +309,7 @@ func (p *prog) registerControlServerHandler() { w.WriteHeader(http.StatusServiceUnavailable) return } - r, err := p.logReader() + r, err := p.logReaderNoColor() if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return diff --git a/cmd/cli/log_writer.go b/cmd/cli/log_writer.go index ff5eb8e..13b3cf3 100644 --- a/cmd/cli/log_writer.go +++ b/cmd/cli/log_writer.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "regexp" "strings" "sync" "time" @@ -84,8 +85,19 @@ type logSentResponse struct { Error string `json:"error"` } -// logReader provides read access to log data with size information -// This encapsulates the log reading functionality for external consumers +// logReader provides read access to log data with size information. +// +// This struct encapsulates log reading functionality for external consumers, +// providing both the log content and metadata about the log size. It supports +// reading from both internal log buffers (when no external logging is configured) +// and external log files (when logging to file is enabled). +// +// Fields: +// - r: An io.ReadCloser that provides access to the log content +// - size: The total size of the log data in bytes +// +// The logReader is used by the control server to serve log content to clients +// and by various CLI commands that need to display or process log data. type logReader struct { r io.ReadCloser size int64 @@ -213,7 +225,69 @@ func (p *prog) needInternalLogging() bool { return true } -func (p *prog) logReader() (*logReader, error) { +// logReaderNoColor returns a logReader with ANSI color codes stripped from the log content. +// +// This method is useful when log content needs to be processed by tools that don't +// handle ANSI escape sequences properly, or when storing logs in plain text format. +// It internally calls logReader(true) to strip color codes. +// +// Returns: +// - *logReader: A logReader instance with color codes removed, or nil if no logs available +// - error: Any error encountered during log reading (e.g., empty logs, file access issues) +// +// Use cases: +// - Log processing pipelines that require plain text +// - Storing logs in databases or text files +// - Displaying logs in environments that don't support color +func (p *prog) logReaderNoColor() (*logReader, error) { + return p.logReader(true) +} + +// logReaderRaw returns a logReader with ANSI color codes preserved in the log content. +// +// This method maintains the original formatting of log entries including color codes, +// which is useful for displaying logs in terminals that support ANSI colors or when +// the original visual formatting needs to be preserved. It internally calls logReader(false). +// +// Returns: +// - *logReader: A logReader instance with color codes preserved, or nil if no logs available +// - error: Any error encountered during log reading (e.g., empty logs, file access issues) +// +// Use cases: +// - Terminal-based log viewers that support color +// - Interactive debugging sessions +// - Preserving original log formatting for display +func (p *prog) logReaderRaw() (*logReader, error) { + return p.logReader(false) +} + +// logReader creates a logReader instance for accessing log content with optional color stripping. +// +// This is the core method that handles log reading from different sources based on the +// current logging configuration. It supports both internal logging (when no external +// logging is configured) and external file logging (when logging to file is enabled). +// +// Behavior: +// - Internal logging: Reads from internal log buffers (normal logs + warning logs) +// and combines them with appropriate markers for separation +// - External logging: Reads directly from the configured log file +// - Empty logs: Returns appropriate error messages when no log content is available +// +// Parameters: +// - stripColor: If true, removes ANSI color codes from log content; if false, preserves them +// +// Returns: +// - *logReader: A logReader instance providing access to log content and size metadata +// - error: Any error encountered during log reading, including: +// - "nil internal log writer" - Internal logging not properly initialized +// - "nil internal warn log writer" - Warning log writer not properly initialized +// - "internal log is empty" - No content in internal log buffers +// - "log file is empty" - External log file exists but contains no data +// - File system errors when accessing external log files +// +// The method handles thread-safe access to internal log buffers and provides +// comprehensive error handling for various edge cases. +func (p *prog) logReader(stripColor bool) (*logReader, error) { if p.needInternalLogging() { p.mu.Lock() lw := p.internalLogWriter @@ -225,14 +299,15 @@ func (p *prog) logReader() (*logReader, error) { if wlw == nil { return nil, errors.New("nil internal warn log writer") } + // Normal log content. lw.mu.Lock() - lwReader := bytes.NewReader(lw.buf.Bytes()) + lwReader := newLogReader(&lw.buf, stripColor) lwSize := lw.buf.Len() lw.mu.Unlock() // Warn log content. wlw.mu.Lock() - wlwReader := bytes.NewReader(wlw.buf.Bytes()) + wlwReader := newLogReader(&wlw.buf, stripColor) wlwSize := wlw.buf.Len() wlw.mu.Unlock() reader := io.MultiReader(lwReader, bytes.NewReader([]byte(logWriterLogEndMarker)), wlwReader) @@ -307,3 +382,26 @@ func newMachineFriendlyZapCore(w io.Writer, level zapcore.Level) zapcore.Core { encoder := zapcore.NewConsoleEncoder(encoderConfig) return zapcore.NewCore(encoder, zapcore.AddSync(w), level) } + +// ansiRegex is a regular expression to match ANSI color codes. +var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`) + +// newLogReader creates a reader for log buffer content with optional ANSI color stripping. +// +// This function provides flexible log content access by allowing consumers to choose +// between raw log data (with ANSI color codes) or stripped content (without color codes). +// The color stripping is useful when logs need to be processed by tools that don't +// handle ANSI escape sequences properly, or when storing logs in plain text format. +// +// Parameters: +// - buf: The log buffer containing the log data to read +// - stripColor: If true, strips ANSI color codes from the log content; +// if false, returns raw log content with color codes preserved +// +// Returns an io.Reader that provides access to the processed log content. +func newLogReader(buf *bytes.Buffer, stripColor bool) io.Reader { + if stripColor { + return strings.NewReader(ansiRegex.ReplaceAllString(buf.String(), "")) + } + return strings.NewReader(buf.String()) +} diff --git a/cmd/cli/log_writer_test.go b/cmd/cli/log_writer_test.go index 1138fca..5af5c13 100644 --- a/cmd/cli/log_writer_test.go +++ b/cmd/cli/log_writer_test.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "io" "strings" "sync" "testing" @@ -142,3 +143,275 @@ func TestNoticeLevel(t *testing.T) { t.Logf("Log output with NOTICE level:\n%s", output) } + +func TestNewLogReader(t *testing.T) { + tests := []struct { + name string + bufContent string + stripColor bool + expected string + description string + }{ + { + name: "empty_buffer_no_color_strip", + bufContent: "", + stripColor: false, + expected: "", + description: "Empty buffer should return empty reader", + }, + { + name: "empty_buffer_with_color_strip", + bufContent: "", + stripColor: true, + expected: "", + description: "Empty buffer with color strip should return empty reader", + }, + { + name: "plain_text_no_color_strip", + bufContent: "This is plain text without any color codes", + stripColor: false, + expected: "This is plain text without any color codes", + description: "Plain text should be returned as-is when not stripping colors", + }, + { + name: "plain_text_with_color_strip", + bufContent: "This is plain text without any color codes", + stripColor: true, + expected: "This is plain text without any color codes", + description: "Plain text should be returned as-is when stripping colors", + }, + { + name: "text_with_ansi_codes_no_strip", + bufContent: "Normal text \x1b[31mred text\x1b[0m normal again", + stripColor: false, + expected: "Normal text \x1b[31mred text\x1b[0m normal again", + description: "ANSI color codes should be preserved when not stripping", + }, + { + name: "text_with_ansi_codes_with_strip", + bufContent: "Normal text \x1b[31mred text\x1b[0m normal again", + stripColor: true, + expected: "Normal text red text normal again", + description: "ANSI color codes should be removed when stripping colors", + }, + { + name: "multiple_ansi_codes_no_strip", + bufContent: "\x1b[1mBold\x1b[0m \x1b[32mGreen\x1b[0m \x1b[34mBlue\x1b[0m text", + stripColor: false, + expected: "\x1b[1mBold\x1b[0m \x1b[32mGreen\x1b[0m \x1b[34mBlue\x1b[0m text", + description: "Multiple ANSI codes should be preserved when not stripping", + }, + { + name: "multiple_ansi_codes_with_strip", + bufContent: "\x1b[1mBold\x1b[0m \x1b[32mGreen\x1b[0m \x1b[34mBlue\x1b[0m text", + stripColor: true, + expected: "Bold Green Blue text", + description: "Multiple ANSI codes should be removed when stripping colors", + }, + { + name: "complex_ansi_sequences_no_strip", + bufContent: "\x1b[1;31;42mBold red on green\x1b[0m \x1b[38;5;208mOrange\x1b[0m", + stripColor: false, + expected: "\x1b[1;31;42mBold red on green\x1b[0m \x1b[38;5;208mOrange\x1b[0m", + description: "Complex ANSI sequences should be preserved when not stripping", + }, + { + name: "complex_ansi_sequences_with_strip", + bufContent: "\x1b[1;31;42mBold red on green\x1b[0m \x1b[38;5;208mOrange\x1b[0m", + stripColor: true, + expected: "Bold red on green Orange", + description: "Complex ANSI sequences should be removed when stripping colors", + }, + { + name: "ansi_codes_with_newlines_no_strip", + bufContent: "Line 1\n\x1b[31mRed line\x1b[0m\nLine 3", + stripColor: false, + expected: "Line 1\n\x1b[31mRed line\x1b[0m\nLine 3", + description: "ANSI codes with newlines should be preserved when not stripping", + }, + { + name: "ansi_codes_with_newlines_with_strip", + bufContent: "Line 1\n\x1b[31mRed line\x1b[0m\nLine 3", + stripColor: true, + expected: "Line 1\nRed line\nLine 3", + description: "ANSI codes with newlines should be removed when stripping colors", + }, + { + name: "malformed_ansi_codes_no_strip", + bufContent: "Text \x1b[invalidm \x1b[0m normal", + stripColor: false, + expected: "Text \x1b[invalidm \x1b[0m normal", + description: "Malformed ANSI codes should be preserved when not stripping", + }, + { + name: "malformed_ansi_codes_with_strip", + bufContent: "Text \x1b[invalidm \x1b[0m normal", + stripColor: true, + expected: "Text \x1b[invalidm normal", + description: "Non-matching ANSI sequences should be preserved when stripping colors", + }, + { + name: "large_buffer_no_strip", + bufContent: strings.Repeat("A", 10000) + "\x1b[31m" + strings.Repeat("B", 1000) + "\x1b[0m", + stripColor: false, + expected: strings.Repeat("A", 10000) + "\x1b[31m" + strings.Repeat("B", 1000) + "\x1b[0m", + description: "Large buffer should handle ANSI codes correctly when not stripping", + }, + { + name: "large_buffer_with_strip", + bufContent: strings.Repeat("A", 10000) + "\x1b[31m" + strings.Repeat("B", 1000) + "\x1b[0m", + stripColor: true, + expected: strings.Repeat("A", 10000) + strings.Repeat("B", 1000), + description: "Large buffer should remove ANSI codes correctly when stripping", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a buffer with the test content + buf := &bytes.Buffer{} + buf.WriteString(tt.bufContent) + + // Create the log reader + reader := newLogReader(buf, tt.stripColor) + + // Read all content from the reader + content, err := io.ReadAll(reader) + if err != nil { + t.Fatalf("Failed to read from log reader: %v", err) + } + + // Verify the content matches expected + actual := string(content) + if actual != tt.expected { + t.Errorf("Expected content: %q, got: %q", tt.expected, actual) + t.Logf("Description: %s", tt.description) + } + }) + } +} + +func TestNewLogReader_ReaderBehavior(t *testing.T) { + // Test that the returned reader behaves correctly + buf := &bytes.Buffer{} + buf.WriteString("Test content with \x1b[31mred\x1b[0m text") + + // Test with color stripping + reader := newLogReader(buf, true) + + // Test reading in chunks + chunk1 := make([]byte, 10) + n1, err := reader.Read(chunk1) + if err != nil && err != io.EOF { + t.Fatalf("Unexpected error reading first chunk: %v", err) + } + if n1 != 10 { + t.Errorf("Expected to read 10 bytes, got %d", n1) + } + + // Test reading remaining content + remaining, err := io.ReadAll(reader) + if err != nil { + t.Fatalf("Failed to read remaining content: %v", err) + } + + // Verify total content + totalContent := string(chunk1[:n1]) + string(remaining) + expected := "Test content with red text" + if totalContent != expected { + t.Errorf("Expected total content: %q, got: %q", expected, totalContent) + } +} + +func TestNewLogReader_ConcurrentAccess(t *testing.T) { + // Test concurrent access to the same buffer + buf := &bytes.Buffer{} + buf.WriteString("Concurrent test with \x1b[32mgreen\x1b[0m text") + + var wg sync.WaitGroup + numGoroutines := 10 + results := make(chan string, numGoroutines) + + for i := 0; i < numGoroutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + reader := newLogReader(buf, true) + content, err := io.ReadAll(reader) + if err != nil { + t.Errorf("Failed to read content: %v", err) + return + } + results <- string(content) + }() + } + + wg.Wait() + close(results) + + // Verify all goroutines got the same result + expected := "Concurrent test with green text" + for result := range results { + if result != expected { + t.Errorf("Expected: %q, got: %q", expected, result) + } + } +} + +func TestNewLogReader_ANSIRegexEdgeCases(t *testing.T) { + // Test edge cases for ANSI regex matching + tests := []struct { + name string + input string + expected string + }{ + { + name: "empty_escape_sequence", + input: "Text \x1b[m normal", + expected: "Text normal", + }, + { + name: "multiple_semicolons", + input: "Text \x1b[1;2;3;4m normal", + expected: "Text normal", + }, + { + name: "numeric_only", + input: "Text \x1b[123m normal", + expected: "Text normal", + }, + { + name: "mixed_numeric_semicolon", + input: "Text \x1b[1;23;456m normal", + expected: "Text normal", + }, + { + name: "no_closing_bracket", + input: "Text \x1b[31 normal", + expected: "Text \x1b[31 normal", + }, + { + name: "no_opening_bracket", + input: "Text 31m normal", + expected: "Text 31m normal", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := &bytes.Buffer{} + buf.WriteString(tt.input) + + reader := newLogReader(buf, true) + content, err := io.ReadAll(reader) + if err != nil { + t.Fatalf("Failed to read content: %v", err) + } + + actual := string(content) + if actual != tt.expected { + t.Errorf("Expected: %q, got: %q", tt.expected, actual) + } + }) + } +}