feat: simplify green path logging

moov-io · Jul 18, 2024 · 748de34 · 748de34
1 parent 0da821d
commit 748de34
Show file tree

Hide file tree

Showing 11 changed files with 23 additions and 39 deletions.
diff --git a/README.md b/README.md
@@ -69,6 +69,7 @@ ACHTestHarness:
       Bind:
         Address: ":3333"
   Matching:
+    # Enable for verbose logging of incoming entries and the matches/actions.
     Debug: false
   # ValidateOpts can use all values from https://pkg.go.dev/github.com/moov-io/ach#ValidateOpts
   ValidateOpts: {}

diff --git a/cmd/ach-test-harness/main.go b/cmd/ach-test-harness/main.go
@@ -33,12 +33,12 @@ func main() {
 	defer stopServers()
 
 	// Initialize our responders
-	entryRepository := entries.NewFTPRepository(env.Logger, env.Config.Servers.FTP)
+	entryRepository := entries.NewFTPRepository(env.Config.Servers.FTP)
 	entryService := entries.NewEntryService(entryRepository)
 	entryController := entries.NewEntryController(env.Logger, entryService)
 	entryController.AppendRoutes(env.Router)
 
-	batchRepository := batches.NewFTPRepository(env.Logger, env.Config.Servers.FTP)
+	batchRepository := batches.NewFTPRepository(env.Config.Servers.FTP)
 	batchService := batches.NewBatchService(batchRepository)
 	batchController := batches.NewBatchController(env.Logger, batchService)
 	batchController.AppendRoutes(env.Router)

diff --git a/pkg/batches/api_batch_test.go b/pkg/batches/api_batch_test.go
@@ -19,7 +19,7 @@ func TestBatchController(t *testing.T) {
 	logger := log.NewDefaultLogger()
 
 	t.Run("/batches returns list of batches", func(t *testing.T) {
-		repo := NewFTPRepository(logger, &service.FTPConfig{
+		repo := NewFTPRepository(&service.FTPConfig{
 			RootPath: "./testdata",
 		})
 		newBatchService := NewBatchService(repo)

diff --git a/pkg/batches/repository_batch.go b/pkg/batches/repository_batch.go
@@ -9,21 +9,18 @@ import (
 	"github.com/moov-io/ach"
 	"github.com/moov-io/ach-test-harness/pkg/response/match"
 	"github.com/moov-io/ach-test-harness/pkg/service"
-	"github.com/moov-io/base/log"
 )
 
 type BatchRepository interface {
 	Search(opts SearchOptions) ([]ach.Batcher, error)
 }
 
 type batchRepository struct {
-	logger   log.Logger
 	rootPath string
 }
 
-func NewFTPRepository(logger log.Logger, cfg *service.FTPConfig) *batchRepository {
+func NewFTPRepository(cfg *service.FTPConfig) *batchRepository {
 	return &batchRepository{
-		logger:   logger,
 		rootPath: cfg.RootPath,
 	}
 }
@@ -38,11 +35,9 @@ func (r *batchRepository) Search(opts SearchOptions) ([]ach.Batcher, error) {
 			return nil
 		}
 		if err != nil {
-			r.logger.Logf("error walking dir %s. %v", d.Name(), err)
 			return nil
 		}
 
-		r.logger.Logf("reading %s", path)
 		// read only *.ach files
 		if strings.ToLower(filepath.Ext(path)) != ".ach" {
 			return nil
@@ -61,7 +56,6 @@ func (r *batchRepository) Search(opts SearchOptions) ([]ach.Batcher, error) {
 		walkingPath = filepath.Join(r.rootPath, opts.Path)
 	}
 
-	r.logger.Logf("Waling directory %s", walkingPath)
 	if err := filepath.WalkDir(walkingPath, search); err != nil {
 		return nil, fmt.Errorf("failed reading directory content %s: %v", walkingPath, err)
 	}

diff --git a/pkg/batches/repository_batch_test.go b/pkg/batches/repository_batch_test.go
@@ -4,14 +4,11 @@ import (
 	"testing"
 
 	"github.com/moov-io/ach-test-harness/pkg/service"
-	"github.com/moov-io/base/log"
 	"github.com/stretchr/testify/require"
 )
 
 func TestRepository(t *testing.T) {
-	logger := log.NewDefaultLogger()
-
-	repo := NewFTPRepository(logger, &service.FTPConfig{
+	repo := NewFTPRepository(&service.FTPConfig{
 		RootPath: "./testdata",
 		Paths: service.Paths{
 			Files:  "/outbound/",

diff --git a/pkg/entries/api_test.go b/pkg/entries/api_test.go
@@ -19,7 +19,7 @@ func TestEntryController(t *testing.T) {
 	logger := log.NewDefaultLogger()
 
 	t.Run("/entries returns list of entries", func(t *testing.T) {
-		repo := NewFTPRepository(logger, &service.FTPConfig{
+		repo := NewFTPRepository(&service.FTPConfig{
 			RootPath: "./testdata",
 		})
 		service := NewEntryService(repo)

diff --git a/pkg/entries/repository_entry.go b/pkg/entries/repository_entry.go
@@ -9,21 +9,18 @@ import (
 	"github.com/moov-io/ach"
 	"github.com/moov-io/ach-test-harness/pkg/response/match"
 	"github.com/moov-io/ach-test-harness/pkg/service"
-	"github.com/moov-io/base/log"
 )
 
 type EntryRepository interface {
 	Search(opts SearchOptions) ([]*ach.EntryDetail, error)
 }
 
 type ftpRepository struct {
-	logger   log.Logger
 	rootPath string
 }
 
-func NewFTPRepository(logger log.Logger, cfg *service.FTPConfig) *ftpRepository {
+func NewFTPRepository(cfg *service.FTPConfig) *ftpRepository {
 	return &ftpRepository{
-		logger:   logger,
 		rootPath: cfg.RootPath,
 	}
 }
@@ -41,7 +38,6 @@ func (r *ftpRepository) Search(opts SearchOptions) ([]*ach.EntryDetail, error) {
 			return nil
 		}
 
-		r.logger.Logf("reading %s", path)
 		// read only *.ach files
 		if strings.ToLower(filepath.Ext(path)) != ".ach" {
 			return nil
@@ -60,7 +56,6 @@ func (r *ftpRepository) Search(opts SearchOptions) ([]*ach.EntryDetail, error) {
 		walkingPath = filepath.Join(r.rootPath, opts.Path)
 	}
 
-	r.logger.Logf("Waling directory %s", walkingPath)
 	if err := filepath.WalkDir(walkingPath, search); err != nil {
 		return nil, fmt.Errorf("failed reading directory content %s: %v", walkingPath, err)
 	}

diff --git a/pkg/entries/repository_entry_test.go b/pkg/entries/repository_entry_test.go
@@ -4,14 +4,11 @@ import (
 	"testing"
 
 	"github.com/moov-io/ach-test-harness/pkg/service"
-	"github.com/moov-io/base/log"
 	"github.com/stretchr/testify/require"
 )
 
 func TestRepository(t *testing.T) {
-	logger := log.NewDefaultLogger()
-
-	repo := NewFTPRepository(logger, &service.FTPConfig{
+	repo := NewFTPRepository(&service.FTPConfig{
 		RootPath: "./testdata",
 		Paths: service.Paths{
 			Files:  "/outbound/",

diff --git a/pkg/response/file_transformer.go b/pkg/response/file_transformer.go
@@ -55,16 +55,10 @@ func (ft *FileTransfomer) Transform(file *ach.File) error {
 			// Check if there's a matching Action and perform it. There may also be a future-dated action to execute.
 			copyAction, processAction := ft.Matcher.FindAction(entries[j])
 			if copyAction != nil {
-				logger := ft.Matcher.Logger.With(copyAction)
-				logger.Log("Processing matched action")
-
 				// Save this Entry
 				mirror.saveEntry(&file.Batches[i], copyAction.Copy, entries[j])
 			}
 			if processAction != nil {
-				logger := ft.Matcher.Logger.With(processAction)
-				logger.Log("Processing matched action")
-
 				entry, err := ft.Entry.MorphEntry(file.Header, bh, entries[j], processAction)
 				if err != nil {
 					return fmt.Errorf("transform batch[%d] morph entry[%d] error: %v", i, j, err)

diff --git a/pkg/response/ftp_watcher.go b/pkg/response/ftp_watcher.go
@@ -22,7 +22,7 @@ func Register(
 			transformer:  transformer,
 		})
 	} else {
-		logger.Info().Log("unable to register transformer")
+		logger.Error().Log("unable to register transformer")
 	}
 }
 
@@ -44,25 +44,27 @@ func (notify *FTPWatcher) AfterFilePut(conn *ftp.Conn, dstPath string, size int6
 	// Grab a file descriptor
 	driver, err := conn.ServerOpts().Factory.NewDriver()
 	if err != nil {
-		notify.logger.Info().Log(fmt.Sprintf("ftp: error getting driver for file %s: %v", dstPath, err))
+		notify.logger.Error().Log(fmt.Sprintf("ftp: error getting driver for file %s: %v", dstPath, err))
 	}
 	_, fd, err := driver.GetFile(dstPath, 0)
 	if err != nil {
-		notify.logger.Info().Log(fmt.Sprintf("ftp: error reading file %s: %v", dstPath, err))
+		notify.logger.Error().Log(fmt.Sprintf("ftp: error reading file %s: %v", dstPath, err))
 	}
 	// Read the file that was uploaded
 	reader := ach.NewReader(fd)
 	reader.SetValidation(notify.validateOpts)
 
+	// TODO(adam): ACH file Iterator
+
 	file, err := reader.Read()
 	if err != nil {
-		notify.logger.Info().Log(fmt.Sprintf("ftp: error reading ACH file %s: %v", dstPath, err))
+		notify.logger.Error().Log(fmt.Sprintf("ftp: error reading ACH file %s: %v", dstPath, err))
 	}
 	if err := file.Create(); err != nil {
-		notify.logger.Info().Log(fmt.Sprintf("ftp: error creating file %s: %v", dstPath, err))
+		notify.logger.Error().Log(fmt.Sprintf("ftp: error creating file %s: %v", dstPath, err))
 	}
 
 	if err := notify.transformer.Transform(&file); err != nil {
-		notify.logger.Info().Log(fmt.Sprintf("ftp: error transforming file %s: %v", dstPath, err))
+		notify.logger.Error().Log(fmt.Sprintf("ftp: error transforming file %s: %v", dstPath, err))
 	}
 }
diff --git a/pkg/response/match/matcher.go b/pkg/response/match/matcher.go
@@ -36,7 +36,9 @@ func (m Matcher) FindAction(ed *ach.EntryDetail) (copyAction *service.Action, pr
 		logger := m.Logger.With(log.Fields{
 			"entry_trace_number": log.String(ed.TraceNumber),
 		})
-		logger.Log("starting EntryDetail matching")
+		if m.Debug {
+			logger.Info().Log("starting EntryDetail matching")
+		}
 
 		positive, negative := 0, 0 // Matchers are AND'd together
 
@@ -151,7 +153,9 @@ func (m Matcher) FindAction(ed *ach.EntryDetail) (copyAction *service.Action, pr
 			b.WriteString(fmt.Sprintf(" (%s)", strings.Join(positiveMatchers, ", ")))
 		}
 
-		logger.Log(b.String())
+		if m.Debug {
+			logger.Log(b.String())
+		}
 
 		// Return the Action if we've still matched
 		if negative == 0 && positive > 0 {