feat(redact): use a scan regex for default redact rule of lines to improve cpu usage and reduce time cost (#1291)

Author: DexterYan

pkg/collect/redact.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 
 	"github.com/pkg/errors"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
@@ -17,97 +18,128 @@ import (
 )
 
 func RedactResult(bundlePath string, input CollectorResult, additionalRedactors []*troubleshootv1beta2.Redact) error {
+	wg := &sync.WaitGroup{}
+
+	// Error channel to capture errors from goroutines
+	errorCh := make(chan error, len(input))
+
 	for k, v := range input {
-		file := k
 
-		var reader io.Reader
-		if v == nil {
-			// Collected contents are in a file. Get a reader to the file.
-			info, err := os.Lstat(filepath.Join(bundlePath, file))
-			if err != nil {
-				if os.IsNotExist(errors.Cause(err)) {
-					// File not found, moving on.
-					continue
-				}
-				return errors.Wrap(err, "failed to stat file")
-			}
+		wg.Add(1)
 
-			// Redact the target file of a symlink
-			// There is an opportunity for improving performance here by skipping symlinks
-			// if a target has been redacted already, but that would require
-			// some extra logic to ensure that a spec filtering only symlinks still works.
-			if info.Mode().Type() == os.ModeSymlink {
-				symlink := file
-				target, err := os.Readlink(filepath.Join(bundlePath, symlink))
+		go func(file string, data []byte) {
+			defer wg.Done()
+			var reader io.Reader
+			if data == nil {
+
+				// Collected contents are in a file. Get a reader to the file.
+				info, err := os.Lstat(filepath.Join(bundlePath, file))
 				if err != nil {
-					return errors.Wrap(err, "failed to read symlink")
+					if os.IsNotExist(errors.Cause(err)) {
+						// File not found, moving on.
+						return
+					}
+					errorCh <- errors.Wrap(err, "failed to stat file")
+					return
 				}
 
-				// Get the relative path to the target file to conform with
-				// the path formats of the CollectorResult
-				file, err = filepath.Rel(bundlePath, target)
+				// Redact the target file of a symlink
+				// There is an opportunity for improving performance here by skipping symlinks
+				// if a target has been redacted already, but that would require
+				// some extra logic to ensure that a spec filtering only symlinks still works.
+				if info.Mode().Type() == os.ModeSymlink {
+					symlink := file
+					target, err := os.Readlink(filepath.Join(bundlePath, symlink))
+					if err != nil {
+						errorCh <- errors.Wrap(err, "failed to read symlink")
+						return
+					}
+					// Get the relative path to the target file to conform with
+					// the path formats of the CollectorResult
+					file, err = filepath.Rel(bundlePath, target)
+					if err != nil {
+						errorCh <- errors.Wrap(err, "failed to get relative path")
+						return
+					}
+					klog.V(2).Infof("Redacting %s (symlink => %s)\n", file, symlink)
+				} else {
+					klog.V(2).Infof("Redacting %s\n", file)
+				}
+				r, err := input.GetReader(bundlePath, file)
 				if err != nil {
-					return errors.Wrap(err, "failed to get relative path")
+					if os.IsNotExist(errors.Cause(err)) {
+						return
+					}
+					errorCh <- errors.Wrap(err, "failed to get reader")
+					return
 				}
-				klog.V(2).Infof("Redacting %s (symlink => %s)\n", file, symlink)
+				defer r.Close()
+
+				reader = r
 			} else {
-				klog.V(2).Infof("Redacting %s\n", file)
+				// Collected contents are in memory. Get a reader to the memory buffer.
+				reader = bytes.NewBuffer(data)
 			}
-			r, err := input.GetReader(bundlePath, file)
-			if err != nil {
-				if os.IsNotExist(errors.Cause(err)) {
-					continue
+
+			// If the file is .tar, .tgz or .tar.gz, it must not be redacted. Instead it is
+			// decompressed and each file inside the tar redacted and compressed back into the archive.
+			if filepath.Ext(file) == ".tar" || filepath.Ext(file) == ".tgz" || strings.HasSuffix(file, ".tar.gz") {
+				tmpDir, err := ioutil.TempDir("", "troubleshoot-subresult-")
+				if err != nil {
+					errorCh <- errors.Wrap(err, "failed to create temp dir")
+					return
 				}
-				return errors.Wrap(err, "failed to get reader")
-			}
-			defer r.Close()
+				defer os.RemoveAll(tmpDir)
 
-			reader = r
-		} else {
-			// Collected contents are in memory. Get a reader to the memory buffer.
-			reader = bytes.NewBuffer(v)
-		}
+				subResult, tarHeaders, err := decompressFile(tmpDir, reader, file)
+				if err != nil {
+					errorCh <- errors.Wrap(err, "failed to decompress file")
+					return
+				}
+				err = RedactResult(tmpDir, subResult, additionalRedactors)
+				if err != nil {
+					errorCh <- errors.Wrap(err, "failed to redact file")
+					return
+				}
 
-		// If the file is .tar, .tgz or .tar.gz, it must not be redacted. Instead it is
-		// decompressed and each file inside the tar redacted and compressed back into the archive.
-		if filepath.Ext(file) == ".tar" || filepath.Ext(file) == ".tgz" || strings.HasSuffix(file, ".tar.gz") {
-			tmpDir, err := ioutil.TempDir("", "troubleshoot-subresult-")
-			if err != nil {
-				return errors.Wrap(err, "failed to create temp dir")
-			}
-			defer os.RemoveAll(tmpDir)
+				dstFilename := filepath.Join(bundlePath, file)
+				err = compressFiles(tmpDir, subResult, tarHeaders, dstFilename)
+				if err != nil {
+					errorCh <- errors.Wrap(err, "failed to re-compress file")
+					return
+				}
 
-			subResult, tarHeaders, err := decompressFile(tmpDir, reader, file)
-			if err != nil {
-				return errors.Wrap(err, "failed to decompress file")
+				os.RemoveAll(tmpDir) // ensure clean up on each iteration in addition to the defer
+
+				//Content of the tar file was redacted. return to next file.
+				return
 			}
-			err = RedactResult(tmpDir, subResult, additionalRedactors)
+
+			redacted, err := redact.Redact(reader, file, additionalRedactors)
 			if err != nil {
-				return errors.Wrap(err, "failed to redact file")
+				errorCh <- errors.Wrap(err, "failed to redact io stream")
+				return
 			}
 
-			dstFilename := filepath.Join(bundlePath, file)
-			err = compressFiles(tmpDir, subResult, tarHeaders, dstFilename)
+			err = input.ReplaceResult(bundlePath, file, redacted)
 			if err != nil {
-				return errors.Wrap(err, "failed to re-compress file")
+				errorCh <- errors.Wrap(err, "failed to create redacted result")
+				return
 			}
+		}(k, v)
+	}
 
-			os.RemoveAll(tmpDir) // ensure clean up on each iteration in addition to the defer
-
-			//Content of the tar file was redacted. Continue to next file.
-			continue
-		}
-
-		redacted, err := redact.Redact(reader, file, additionalRedactors)
-		if err != nil {
-			return errors.Wrap(err, "failed to redact io stream")
-		}
+	go func() {
+		wg.Wait()
+		close(errorCh)
+	}()
 
-		err = input.ReplaceResult(bundlePath, file, redacted)
+	for err := range errorCh {
 		if err != nil {
-			return errors.Wrap(err, "failed to create redacted result")
+			return err
 		}
 	}
+
 	return nil
 }
 

pkg/collect/result.go

@@ -127,7 +127,12 @@ func (r CollectorResult) SaveResult(bundlePath string, relativePath string, read
 		return errors.Wrap(err, "failed to copy data")
 	}
 
-	klog.V(2).Infof("Added %q to bundle output", relativePath)
+	fileInfo, err := f.Stat()
+	if err != nil {
+		return errors.Wrap(err, "failed to stat file")
+	}
+
+	klog.V(2).Infof("Added %q (%d MB) to bundle output", relativePath, fileInfo.Size()/(1024*1024))
 	return nil
 }
 

pkg/constants/constants.go

@@ -82,4 +82,7 @@ const (
 	// TermUI Display Constants
 	MESSAGE_TEXT_PADDING                = 4
 	MESSAGE_TEXT_LINES_MARGIN_TO_BOTTOM = 4
+
+	// Bufio Reader Constants
+	MAX_BUFFER_CAPACITY = 1024 * 1024
 )

pkg/redact/multi_line.go

@@ -5,9 +5,11 @@ import (
 	"fmt"
 	"io"
 	"regexp"
+	"strings"
 )
 
 type MultiLineRedactor struct {
+	scan       *regexp.Regexp
 	re1        *regexp.Regexp
 	re2        *regexp.Regexp
 	maskText   string
@@ -16,16 +18,26 @@ type MultiLineRedactor struct {
 	isDefault  bool
 }
 
-func NewMultiLineRedactor(re1, re2, maskText, path, name string, isDefault bool) (*MultiLineRedactor, error) {
-	compiled1, err := regexp.Compile(re1)
+func NewMultiLineRedactor(re1 LineRedactor, re2 string, maskText, path, name string, isDefault bool) (*MultiLineRedactor, error) {
+	var scanCompiled *regexp.Regexp
+	compiled1, err := regexp.Compile(re1.regex)
 	if err != nil {
 		return nil, err
 	}
+
+	if re1.scan != "" {
+		scanCompiled, err = regexp.Compile(re1.scan)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	compiled2, err := regexp.Compile(re2)
 	if err != nil {
 		return nil, err
 	}
-	return &MultiLineRedactor{re1: compiled1, re2: compiled2, maskText: maskText, filePath: path, redactName: name, isDefault: isDefault}, nil
+
+	return &MultiLineRedactor{scan: scanCompiled, re1: compiled1, re2: compiled2, maskText: maskText, filePath: path, redactName: name, isDefault: isDefault}, nil
 }
 
 func (r *MultiLineRedactor) Redact(input io.Reader, path string) io.Reader {
@@ -52,6 +64,17 @@ func (r *MultiLineRedactor) Redact(input io.Reader, path string) io.Reader {
 		for err == nil {
 			lineNum++ // the first line that can be redacted is line 2
 
+			// is scan is not nil, then check if line1 matches scan by lowercasing it
+			if r.scan != nil {
+				lowerLine1 := strings.ToLower(line1)
+				if !r.scan.MatchString(lowerLine1) {
+					fmt.Fprintf(writer, "%s\n", line1)
+					line1, line2, err = getNextTwoLines(reader, &line2)
+					flushLastLine = true
+					continue
+				}
+			}
+
 			// If line1 matches re1, then transform line2 using re2
 			if !r.re1.MatchString(line1) {
 				fmt.Fprintf(writer, "%s\n", line1)
@@ -60,7 +83,6 @@ func (r *MultiLineRedactor) Redact(input io.Reader, path string) io.Reader {
 				continue
 			}
 			flushLastLine = false
-
 			clean := r.re2.ReplaceAllString(line2, substStr)
 
 			// io.WriteString would be nicer, but reader strips new lines