From 655e348870712280eac03d9e32d027e74c119ced Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Thu, 19 Jun 2025 17:00:58 +0300 Subject: Refactor: Extract magic numbers as constants and reduce client code duplication - Created internal/constants package with organized constant files: - timeouts.go: All time duration constants (timeouts, intervals, delays) - channels.go: Channel buffer size constants - limits.go: Numeric limits and configuration values - buffers.go: Buffer size constants in bytes - Replaced all magic numbers throughout codebase with named constants: - Time durations (2s, 3s, 5s, 10s, 100ms, 24h) now use descriptive constants - Buffer sizes (8KB, 64KB, 1MB) extracted to constants - Channel buffer sizes and multipliers - Configuration limits (max connections, concurrency, etc.) - Health check status codes - Percentage calculations - Reduced code duplication in client implementations: - Created CommonClient to share functionality between CatClient, GrepClient, and TailClient - All three clients now inherit from CommonClient - Eliminated duplicate makeHandler() and makeCommands() methods - Simplified client constructors This refactoring improves code maintainability by centralizing configuration values and reducing redundant code across similar client implementations. --- internal/io/dlog/loggers/file.go | 3 ++- internal/io/fs/aggregateprocessor.go | 3 ++- internal/io/fs/chunkedreader.go | 9 +++++---- internal/io/fs/directprocessor.go | 5 +++-- internal/io/fs/stats.go | 12 +++++++----- internal/io/fs/tailprocessor.go | 5 +++-- 6 files changed, 22 insertions(+), 15 deletions(-) (limited to 'internal/io') diff --git a/internal/io/dlog/loggers/file.go b/internal/io/dlog/loggers/file.go index 6a09353..b183843 100644 --- a/internal/io/dlog/loggers/file.go +++ b/internal/io/dlog/loggers/file.go @@ -10,6 +10,7 @@ import ( "time" "github.com/mimecast/dtail/internal/config" + "github.com/mimecast/dtail/internal/constants" ) type fileWriter struct{} @@ -36,7 +37,7 @@ type file struct { func newFile(strategy Strategy) *file { return &file{ - bufferCh: make(chan *fileMessageBuf, runtime.NumCPU()*100), + bufferCh: make(chan *fileMessageBuf, runtime.NumCPU()*constants.LoggerBufferChannelMultiplier), pauseCh: make(chan struct{}), resumeCh: make(chan struct{}), rotateCh: make(chan struct{}), diff --git a/internal/io/fs/aggregateprocessor.go b/internal/io/fs/aggregateprocessor.go index 98d0c31..809f298 100644 --- a/internal/io/fs/aggregateprocessor.go +++ b/internal/io/fs/aggregateprocessor.go @@ -5,6 +5,7 @@ import ( "context" "time" + "github.com/mimecast/dtail/internal/constants" "github.com/mimecast/dtail/internal/io/line" "github.com/mimecast/dtail/internal/lcontext" "github.com/mimecast/dtail/internal/regex" @@ -69,7 +70,7 @@ func (p *AggregateLineProcessor) Flush() []byte { if !p.isTailing { // Close the lines channel to signal end of input // Add a small delay to ensure all lines are processed before closing - time.Sleep(10 * time.Millisecond) + time.Sleep(constants.ProcessorSleepDuration) close(p.linesCh) } return nil diff --git a/internal/io/fs/chunkedreader.go b/internal/io/fs/chunkedreader.go index ab78ba1..7775a58 100644 --- a/internal/io/fs/chunkedreader.go +++ b/internal/io/fs/chunkedreader.go @@ -6,12 +6,13 @@ import ( "io" "time" + "github.com/mimecast/dtail/internal/constants" "github.com/mimecast/dtail/internal/io/dlog" "github.com/mimecast/dtail/internal/io/pool" ) // Reusable timer to reduce allocations - PBO optimization -var sharedTimer = time.NewTimer(10 * time.Millisecond) +var sharedTimer = time.NewTimer(constants.ProcessorSleepDuration) // ChunkedReader reads data in large chunks and processes it line by line // This replaces the byte-by-byte reading approach for better performance @@ -29,14 +30,14 @@ type ChunkedReader struct { // NewChunkedReader creates a new chunked reader with the specified chunk size func NewChunkedReader(reader io.Reader, chunkSize int) *ChunkedReader { if chunkSize <= 0 { - chunkSize = 64 * 1024 // Default 64KB chunks + chunkSize = constants.DefaultChunkSize // Default 64KB chunks } return &ChunkedReader{ reader: reader, buffer: make([]byte, chunkSize), chunkSize: chunkSize, // PBO optimization: Pre-allocate line buffer - lineBuffer: make([]byte, 0, 8192), // 8KB initial capacity + lineBuffer: make([]byte, 0, constants.LineBufferInitialCapacity), // 8KB initial capacity } } @@ -76,7 +77,7 @@ func (cr *ChunkedReader) ProcessLines(ctx context.Context, rawLines chan *bytes. default: } } - sharedTimer.Reset(10 * time.Millisecond) + sharedTimer.Reset(constants.ProcessorSleepDuration) select { case <-ctx.Done(): return ctx.Err() diff --git a/internal/io/fs/directprocessor.go b/internal/io/fs/directprocessor.go index 84b78bb..312ac9c 100644 --- a/internal/io/fs/directprocessor.go +++ b/internal/io/fs/directprocessor.go @@ -9,6 +9,7 @@ import ( "time" "github.com/mimecast/dtail/internal/config" + "github.com/mimecast/dtail/internal/constants" "github.com/mimecast/dtail/internal/lcontext" ) @@ -77,7 +78,7 @@ func (dp *DirectProcessor) ProcessReader(ctx context.Context, reader io.Reader, // Set buffer size respecting MaxLineLength configuration maxLineLength := config.Server.MaxLineLength - initialBufSize := 64 * 1024 + initialBufSize := constants.InitialBufferSize if maxLineLength < initialBufSize { initialBufSize = maxLineLength } @@ -337,7 +338,7 @@ func (dp *DirectProcessor) followFile(ctx context.Context, filePath string) erro select { case <-ctx.Done(): return ctx.Err() - case <-time.After(100 * time.Millisecond): + case <-time.After(constants.ProcessorTimeoutDuration): // Check if file has grown fileInfo, err := os.Stat(filePath) if err != nil { diff --git a/internal/io/fs/stats.go b/internal/io/fs/stats.go index 4121ff7..1990e53 100644 --- a/internal/io/fs/stats.go +++ b/internal/io/fs/stats.go @@ -1,14 +1,16 @@ package fs +import "github.com/mimecast/dtail/internal/constants" + // Used to calculate how many log lines matched the regular expression // and how many log files could be transmitted from the server to the client. // Hit and transmit percentage takes only the last 100 log lines into calculation. type stats struct { pos int lineCount uint64 - matched [100]bool + matched [constants.StatsArraySize]bool matchCount uint64 - transmitted [100]bool + transmitted [constants.StatsArraySize]bool transmitCount int } @@ -25,7 +27,7 @@ func (f *stats) transmittedPerc() int { // Update bucket position. We only take into consideration the last 100 // lines for stats. func (f *stats) updatePosition() { - f.pos = (f.pos + 1) % 100 + f.pos = (f.pos + 1) % constants.StatsArraySize f.lineCount++ } @@ -63,7 +65,7 @@ func (f *stats) updateLineNotTransmitted() { func percentOf(total float64, value float64) float64 { if total == 0 || total == value { - return 100 + return constants.PercentageMultiplier } - return value / (total / 100.0) + return value / (total / constants.PercentageMultiplier) } diff --git a/internal/io/fs/tailprocessor.go b/internal/io/fs/tailprocessor.go index 3bc9029..55934ce 100644 --- a/internal/io/fs/tailprocessor.go +++ b/internal/io/fs/tailprocessor.go @@ -7,6 +7,7 @@ import ( "time" "github.com/mimecast/dtail/internal/config" + "github.com/mimecast/dtail/internal/constants" "github.com/mimecast/dtail/internal/lcontext" "github.com/mimecast/dtail/internal/regex" ) @@ -163,7 +164,7 @@ func (ftp *FollowingTailProcessor) followFile(ctx context.Context, filePath stri func (ftp *FollowingTailProcessor) followReader(ctx context.Context, file *os.File, filePath string) error { // Set buffer size respecting MaxLineLength configuration maxLineLength := config.Server.MaxLineLength - initialBufSize := 64 * 1024 + initialBufSize := constants.InitialBufferSize if maxLineLength < initialBufSize { initialBufSize = maxLineLength } @@ -259,7 +260,7 @@ func (ftp *FollowingTailProcessor) followReader(ctx context.Context, file *os.Fi select { case <-ctx.Done(): return ctx.Err() - case <-time.After(100 * time.Millisecond): + case <-time.After(constants.ProcessorTimeoutDuration): // Continue the loop to check for new content } } -- cgit v1.2.3