feat: enhance log reading with ANSI color stripping and comprehensive documentation

- Add newLogReader function with optional ANSI color code stripping - Implement logReaderNoColor() and logReaderRaw() methods for different use cases - Add comprehensive documentation for logReader struct and all related methods - Add extensive test coverage with 16+ test cases covering edge cases The new functionality allows consumers to choose between raw log data (with ANSI color codes) or stripped content (without color codes), making logs more suitable for different processing pipelines and display environments.
2026-02-03 22:18:39 +00:00 · 2025-09-08 16:46:16 +07:00
parent f6be1ab1fb
commit 59b98245d3
3 changed files with 378 additions and 7 deletions
--- 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
--- 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())
+}
--- 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)
+			}
+		})
+	}
+}