logger: reduce go routine generation (thrasher-corp#992)

* logger: reduce go routine generation * logger: shift most of processing and prep work to the worker pool, add pool for fields because each log we are pushing the struct to the heap, has better segregation now and includes a buffer in scope instead of relying on a pool * logger: shift fmt package calls to worker pool * logger: conform tests to new design * linter: fix issues * Update log/logger_test.go Co-authored-by: Scott <[email protected]> * Update log/logger_test.go Co-authored-by: Scott <[email protected]> * UN-GLORIOUS: nits * logger: Handle config variable * logger: NITERINOS BY GLORIOUS CODE * logger: revert * glorious: nits * Panic at the disco: fix * Panic at the disco: fix * logger: make sure logger closed and job channel emptied on start up error * fix tests * logger: reduce globals * logger: finished reduces globals, reduce workers to one too keep everything in line. * logger: remove comments * logger/exhchange: linter issues * db/test: fix linter * logger: add tests shift wait before unlock * logger: consolidate worker code; fix linter issue and make sure we can sustain writing for external testing. * logger: fix race and warn for conflict in config * logger: fix name and add to tests * logger: remove zero value field * glorious: panic fix and removal of code * logger: reinstate channels in close * logger: shift reinstate processing to SetupGlobalLogger * logger: segregate config.json from internal log.Config * logger: fix silly mistake that is silly * engine: Add protection for nil issues and implement new constructor in tests * logger: Force singular mutex usage throughout package, throw away funcs that are not used outside of this package, unexport a bunch. Fix tests. * logger: actually set advanced settings * Update log/loggers.go Co-authored-by: Scott <[email protected]> * Update log/loggers.go Co-authored-by: Scott <[email protected]> * Update log/loggers.go Co-authored-by: Scott <[email protected]> * Update log/loggers.go Co-authored-by: Scott <[email protected]> * Update log/logger_multiwriter.go Co-authored-by: Scott <[email protected]> * glorious: nits * logger: test issue when not purging temp file and contents * loggertest: add more protections for the panics * linter: fix * glorious: nits * cleanup * logger: linter fix * linter: fix(?) :/ * linter: revert change * linter: fix Co-authored-by: Scott <[email protected]> Co-authored-by: Ryan O'Hara-Reid <[email protected]>
0x1be20 · Oct 24, 2022 · 4e135c9 · 4e135c9
1 parent f5ee675
commit 4e135c9
Show file tree

Hide file tree

Showing 28 changed files with 913 additions and 619 deletions.
diff --git a/README.md b/README.md
@@ -124,7 +124,7 @@ go build
 copy config_example.json %APPDATA%\GoCryptoTrader\config.json
 ```
 
-+ Make any neccessary changes to the `config.json` file.
++ Make any necessary changes to the `config.json` file.
 + Run the `gocryptotrader` binary file inside your GOPATH bin folder.
 
 ## Donations

diff --git a/backtester/main.go b/backtester/main.go
@@ -11,6 +11,7 @@ import (
 	"github.com/thrasher-corp/gocryptotrader/backtester/config"
 	backtest "github.com/thrasher-corp/gocryptotrader/backtester/engine"
 	"github.com/thrasher-corp/gocryptotrader/backtester/plugins/strategies"
+	"github.com/thrasher-corp/gocryptotrader/common/convert"
 	"github.com/thrasher-corp/gocryptotrader/common/file"
 	"github.com/thrasher-corp/gocryptotrader/engine"
 	"github.com/thrasher-corp/gocryptotrader/log"
@@ -110,12 +111,18 @@ func main() {
 		common.PurgeColours()
 	}
 
-	log.GlobalLogConfig = log.GenDefaultSettings()
-	log.GlobalLogConfig.AdvancedSettings.ShowLogSystemName = &logSubHeader
-	log.GlobalLogConfig.AdvancedSettings.Headers.Info = common.CMDColours.Info + "[INFO]" + common.CMDColours.Default
-	log.GlobalLogConfig.AdvancedSettings.Headers.Warn = common.CMDColours.Warn + "[WARN]" + common.CMDColours.Default
-	log.GlobalLogConfig.AdvancedSettings.Headers.Debug = common.CMDColours.Debug + "[DEBUG]" + common.CMDColours.Default
-	log.GlobalLogConfig.AdvancedSettings.Headers.Error = common.CMDColours.Error + "[ERROR]" + common.CMDColours.Default
+	defaultLogSettings := log.GenDefaultSettings()
+	defaultLogSettings.AdvancedSettings.ShowLogSystemName = convert.BoolPtr(logSubHeader)
+	defaultLogSettings.AdvancedSettings.Headers.Info = common.CMDColours.Info + "[INFO]" + common.CMDColours.Default
+	defaultLogSettings.AdvancedSettings.Headers.Warn = common.CMDColours.Warn + "[WARN]" + common.CMDColours.Default
+	defaultLogSettings.AdvancedSettings.Headers.Debug = common.CMDColours.Debug + "[DEBUG]" + common.CMDColours.Default
+	defaultLogSettings.AdvancedSettings.Headers.Error = common.CMDColours.Error + "[ERROR]" + common.CMDColours.Default
+	err = log.SetGlobalLogConfig(defaultLogSettings)
+	if err != nil {
+		fmt.Printf("Could not setup global logger. Error: %v.\n", err)
+		os.Exit(1)
+	}
+
 	err = log.SetupGlobalLogger()
 	if err != nil {
 		fmt.Printf("Could not setup global logger. Error: %v.\n", err)

diff --git a/backtester/report/report.go b/backtester/report/report.go
@@ -141,7 +141,7 @@ func (d *Data) enhanceCandles() error {
 			Asset:     lookup.Asset,
 			Pair:      lookup.Pair,
 			Interval:  lookup.Interval,
-			Watermark: fmt.Sprintf("%s - %s - %s", strings.Title(lookup.Exchange), lookup.Asset.String(), lookup.Pair.Upper()), //nolint // Title usage
+			Watermark: fmt.Sprintf("%s - %s - %s", strings.Title(lookup.Exchange), lookup.Asset.String(), lookup.Pair.Upper()), //nolint:staticcheck // Ignore Title usage warning
 		}
 
 		statsForCandles :=

diff --git a/cmd/apichecker/apicheck.go b/cmd/apichecker/apicheck.go
@@ -101,10 +101,13 @@ func main() {
 	flag.BoolVar(&verbose, "verbose", false, "increases logging verbosity for API Update Checker")
 	flag.BoolVar(&create, "create", false, "specifies whether to automatically create trello list, card and checklist in a given board")
 	flag.Parse()
-	var err error
-	log.RWM.Lock()
-	log.GlobalLogConfig = log.GenDefaultSettings()
-	log.RWM.Unlock()
+
+	err := log.SetGlobalLogConfig(log.GenDefaultSettings())
+	if err != nil {
+		fmt.Printf("Could not setup global logger. Error: %v.\n", err)
+		os.Exit(1)
+	}
+
 	err = log.SetupGlobalLogger()
 	if err != nil {
 		fmt.Printf("Could not setup global logger. Error: %v.\n", err)

diff --git a/cmd/apichecker/apicheck_test.go b/cmd/apichecker/apicheck_test.go
@@ -6,7 +6,6 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/thrasher-corp/gocryptotrader/common/convert"
 	gctfile "github.com/thrasher-corp/gocryptotrader/common/file"
 	exchange "github.com/thrasher-corp/gocryptotrader/exchanges"
 	"github.com/thrasher-corp/gocryptotrader/log"
@@ -25,14 +24,12 @@ var (
 
 func TestMain(m *testing.M) {
 	setTestVars()
-	testMode = true
-	c := log.GenDefaultSettings()
-	c.Enabled = convert.BoolPtr(true)
-	log.RWM.Lock()
-	log.GlobalLogConfig = c
-	log.RWM.Unlock()
+	err := log.SetGlobalLogConfig(log.GenDefaultSettings())
+	if err != nil {
+		log.Error(log.Global, err)
+		os.Exit(1)
+	}
 	log.Infoln(log.Global, "set verbose to true for more detailed output")
-	var err error
 	configData, err = readFileData(jsonFile)
 	if err != nil {
 		log.Error(log.Global, err)

diff --git a/cmd/documentation/documentation.go b/cmd/documentation/documentation.go
@@ -486,7 +486,7 @@ func GetPackageName(name string, capital bool) string {
 		i = len(newStrings) - 1
 	}
 	if capital {
-		return strings.Replace(strings.Title(newStrings[i]), "_", " ", -1) //nolint // ignore staticcheck strings.Title warning
+		return strings.Replace(strings.Title(newStrings[i]), "_", " ", -1) //nolint:staticcheck // Ignore Title usage warning
 	}
 	return newStrings[i]
 }

diff --git a/cmd/documentation/root_templates/root_readme.tmpl b/cmd/documentation/root_templates/root_readme.tmpl
@@ -125,7 +125,7 @@ go build
 copy config_example.json %APPDATA%\GoCryptoTrader\config.json
 ```
 
-+ Make any neccessary changes to the `config.json` file.
++ Make any necessary changes to the `config.json` file.
 + Run the `gocryptotrader` binary file inside your GOPATH bin folder.
 
 {{template "donations" .}}

diff --git a/config/config.go b/config/config.go
@@ -1188,20 +1188,20 @@ func (c *Config) CheckLoggerConfig() error {
 			log.Warnf(log.ConfigMgr, "Logger rotation size invalid, defaulting to %v", log.DefaultMaxFileSize)
 			c.Logging.LoggerFileConfig.MaxSize = log.DefaultMaxFileSize
 		}
-		log.FileLoggingConfiguredCorrectly = true
+		log.SetFileLoggingState( /*Is correctly configured*/ true)
 	}
-	log.RWM.Lock()
-	log.GlobalLogConfig = &c.Logging
-	log.RWM.Unlock()
 
-	logPath := c.GetDataPath("logs")
-	err := common.CreateDir(logPath)
+	err := log.SetGlobalLogConfig(&c.Logging)
 	if err != nil {
 		return err
 	}
-	log.LogPath = logPath
 
-	return nil
+	logPath := c.GetDataPath("logs")
+	err = common.CreateDir(logPath)
+	if err != nil {
+		return err
+	}
+	return log.SetLogPath(logPath)
 }
 
 func (c *Config) checkGCTScriptConfig() error {

diff --git a/currency/pairs.go b/currency/pairs.go
@@ -100,22 +100,24 @@ func (p Pairs) MarshalJSON() ([]byte, error) {
 	return json.Marshal(p.Join())
 }
 
-// Upper updates the original pairs and returns the pairs for convenience if
-// needed.
+// Upper updates and returns the entire slice of pairs to upper casing
+// NOTE: Do not duplicate slice reference as this can cause race issues.
 func (p Pairs) Upper() Pairs {
+	newSlice := make(Pairs, len(p))
 	for i := range p {
-		p[i] = p[i].Upper()
+		newSlice[i] = p[i].Upper()
 	}
-	return p
+	return newSlice
 }
 
-// Lower updates the original pairs and returns the pairs for convenience if
-// needed.
+// Lower updates and returns the entire slice of pairs to upper casing
+// NOTE: Do not duplicate slice reference as this can cause race issues.
 func (p Pairs) Lower() Pairs {
+	newSlice := make(Pairs, len(p))
 	for i := range p {
-		p[i] = p[i].Lower()
+		newSlice[i] = p[i].Lower()
 	}
-	return p
+	return newSlice
 }
 
 // Contains checks to see if a specified pair exists inside a currency pair

diff --git a/currency/pairs_test.go b/currency/pairs_test.go
@@ -14,7 +14,7 @@ func TestPairsUpper(t *testing.T) {
 	}
 	if expected := "BTC_USD,BTC_AUD,BTC_LTC"; pairs.Upper().Join() != expected {
 		t.Errorf("Pairs Join() error expected %s but received %s",
-			expected, pairs.Join())
+			expected, pairs.Upper().Join())
 	}
 }
 
@@ -26,7 +26,7 @@ func TestPairsLower(t *testing.T) {
 	}
 	if expected := "btc_usd,btc_aud,btc_ltc"; pairs.Lower().Join() != expected {
 		t.Errorf("Pairs Join() error expected %s but received %s",
-			expected, pairs.Join())
+			expected, pairs.Lower().Join())
 	}
 }
 

diff --git a/database/testhelpers/test_helpers.go b/database/testhelpers/test_helpers.go
@@ -11,7 +11,6 @@ import (
 	psqlConn "github.com/thrasher-corp/gocryptotrader/database/drivers/postgres"
 	sqliteConn "github.com/thrasher-corp/gocryptotrader/database/drivers/sqlite3"
 	"github.com/thrasher-corp/gocryptotrader/database/repository"
-	"github.com/thrasher-corp/gocryptotrader/log"
 	"github.com/thrasher-corp/goose"
 	"github.com/thrasher-corp/sqlboiler/boil"
 )
@@ -119,14 +118,7 @@ func migrateDB(db *sql.DB) error {
 
 // EnableVerboseTestOutput enables debug output for SQL queries
 func EnableVerboseTestOutput() error {
-	log.RWM.Lock()
-	log.GlobalLogConfig = log.GenDefaultSettings()
-	log.RWM.Unlock()
-	if err := log.SetupGlobalLogger(); err != nil {
-		return err
-	}
-	DBLogger := database.Logger{}
 	boil.DebugMode = true
-	boil.DebugWriter = DBLogger
+	boil.DebugWriter = database.Logger{}
 	return nil
 }
diff --git a/engine/datahistory_manager.go b/engine/datahistory_manager.go
@@ -77,6 +77,9 @@ func (m *DataHistoryManager) Start() error {
 	if m == nil {
 		return ErrNilSubsystem
 	}
+	if m.databaseConnectionInstance == nil {
+		return errNilDatabaseConnectionManager
+	}
 	if !atomic.CompareAndSwapInt32(&m.started, 0, 1) {
 		return ErrSubSystemAlreadyStarted
 	}
@@ -225,7 +228,7 @@ func (m *DataHistoryManager) run() {
 			case <-m.shutdown:
 				return
 			case <-m.interval.C:
-				if m.databaseConnectionInstance.IsConnected() {
+				if m.databaseConnectionInstance != nil && m.databaseConnectionInstance.IsConnected() {
 					go func() {
 						if err := m.runJobs(); err != nil {
 							log.Error(log.DataHistory, err)

diff --git a/engine/datahistory_manager_test.go b/engine/datahistory_manager_test.go
@@ -979,6 +979,7 @@ func createDHM(t *testing.T) (*DataHistoryManager, *datahistoryjob.DataHistoryJo
 		},
 	}
 	m := &DataHistoryManager{
+		databaseConnectionInstance: &dataBaseConnection{},
 		jobDB: dataHistoryJobService{
 			job: j,
 		},
@@ -993,6 +994,20 @@ func createDHM(t *testing.T) (*DataHistoryManager, *datahistoryjob.DataHistoryJo
 	return m, j
 }
 
+type dataBaseConnection struct{}
+
+func (d *dataBaseConnection) IsConnected() bool {
+	return false
+}
+
+func (d *dataBaseConnection) GetSQL() (*sql.DB, error) {
+	return nil, errors.New("not implemented")
+}
+
+func (d *dataBaseConnection) GetConfig() *database.Config {
+	return nil
+}
+
 func TestProcessCandleData(t *testing.T) {
 	t.Parallel()
 	m, _ := createDHM(t)

diff --git a/engine/engine.go b/engine/engine.go
@@ -395,8 +395,11 @@ func (bot *Engine) Start() error {
 	gctlog.Debugf(gctlog.Global, "Bot '%s' started.\n", bot.Config.Name)
 	gctlog.Debugf(gctlog.Global, "Using data dir: %s\n", bot.Settings.DataDir)
 	if *bot.Config.Logging.Enabled && strings.Contains(bot.Config.Logging.Output, "file") {
-		gctlog.Debugf(gctlog.Global, "Using log file: %s\n",
-			filepath.Join(gctlog.LogPath, bot.Config.Logging.LoggerFileConfig.FileName))
+		gctlog.Debugf(gctlog.Global,
+			"Using log file: %s\n",
+			filepath.Join(gctlog.GetLogPath(),
+				bot.Config.Logging.LoggerFileConfig.FileName),
+		)
 	}
 	gctlog.Debugf(gctlog.Global,
 		"Using %d out of %d logical processors for runtime performance\n",
@@ -725,6 +728,7 @@ func (bot *Engine) Stop() {
 
 	// Wait for services to gracefully shutdown
 	bot.ServicesWG.Wait()
+	gctlog.Infoln(gctlog.Global, "Exiting.")
 	if err := gctlog.CloseLogger(); err != nil {
 		log.Printf("Failed to close logger. Error: %v\n", err)
 	}

diff --git a/exchanges/exchange_test.go b/exchanges/exchange_test.go
@@ -3,9 +3,7 @@ package exchange
 import (
 	"context"
 	"errors"
-	"fmt"
 	"net"
-	"os"
 	"testing"
 	"time"
 
@@ -18,7 +16,6 @@ import (
 	"github.com/thrasher-corp/gocryptotrader/exchanges/protocol"
 	"github.com/thrasher-corp/gocryptotrader/exchanges/request"
 	"github.com/thrasher-corp/gocryptotrader/exchanges/stream"
-	"github.com/thrasher-corp/gocryptotrader/log"
 	"github.com/thrasher-corp/gocryptotrader/portfolio/banking"
 )
 
@@ -27,17 +24,6 @@ const (
 	defaultTestCurrencyPair = "BTC-USD"
 )
 
-func TestMain(m *testing.M) {
-	log.RWM.Lock()
-	log.GlobalLogConfig = log.GenDefaultSettings()
-	log.RWM.Unlock()
-	if err := log.SetupGlobalLogger(); err != nil {
-		fmt.Println("Cannot setup global logger. Error:", err)
-		os.Exit(1)
-	}
-	os.Exit(m.Run())
-}
-
 func TestSupportsRESTTickerBatchUpdates(t *testing.T) {
 	t.Parallel()
 

diff --git a/gctscript/vm/vm_test.go b/gctscript/vm/vm_test.go
@@ -9,8 +9,6 @@ import (
 	"time"
 
 	"github.com/gofrs/uuid"
-	"github.com/thrasher-corp/gocryptotrader/common/convert"
-	"github.com/thrasher-corp/gocryptotrader/log"
 )
 
 const (
@@ -29,15 +27,6 @@ var (
 	testScriptRunnerInvalid  = filepath.Join("..", "..", "testdata", "gctscript", "invalid_timer.gct")
 )
 
-func TestMain(m *testing.M) {
-	c := log.GenDefaultSettings()
-	c.Enabled = convert.BoolPtr(false)
-	log.RWM.Lock()
-	log.GlobalLogConfig = c
-	log.RWM.Unlock()
-	os.Exit(m.Run())
-}
-
 func TestNewVM(t *testing.T) {
 	manager := GctScriptManager{
 		config: configHelper(true, true, maxTestVirtualMachines),

diff --git a/gctscript/wrappers/gct/exchange/exchange_test.go b/gctscript/wrappers/gct/exchange/exchange_test.go
@@ -34,13 +34,9 @@ const (
 
 var (
 	settings = engine.Settings{
-		ConfigFile:          filepath.Join("..", "..", "..", "..", "testdata", "configtest.json"),
-		EnableDryRun:        true,
-		DataDir:             filepath.Join("..", "..", "..", "..", "testdata", "gocryptotrader"),
-		Verbose:             false,
-		EnableGRPC:          false,
-		EnableDeprecatedRPC: false,
-		EnableWebsocketRPC:  false,
+		ConfigFile:   filepath.Join("..", "..", "..", "..", "testdata", "configtest.json"),
+		EnableDryRun: true,
+		DataDir:      filepath.Join("..", "..", "..", "..", "testdata", "gocryptotrader"),
 	}
 	exchangeTest = Exchange{}
 )