Repository: snowflakedb/gosnowflake
Pull request #1358 (2025-04-07 08:08:40)

NO-SNOW Hide logger enabled flag behind mutex
### Description

SNOW-XXX Please explain the changes you made here.

### Checklist
- [ ] Created tests which fail without the change (if possible)
- [ ] Extended the README / documentation, if necessary


sfc-gh-snowflakedb-snyk-sa: ![](https://res.cloudinary.com/snyk/image/upload/r-d/scm-platform/snyk-pull-requests/pr-banner-default.svg)

### :tada: **Snyk checks have passed. No issues have been found so far.**

:white_check_mark: **security/snyk** check is complete. No issues have been found. [(View Details)](https://app.snyk.io/org/snowflakedb-sca-scanning-public-repo/pr-checks/cf348e4a-7d67-491a-8e87-dd29d6ef98c5?refs=41b14a45c6ef63db1c05b6c45c62fe89623920a8&source=prChecksComment)

:white_check_mark: **license/snyk** check is complete. No issues have been found. [(View Details)](https://app.snyk.io/org/snowflakedb-sca-scanning-public-repo/pr-checks/cf348e4a-7d67-491a-8e87-dd29d6ef98c5/license?refs=41b14a45c6ef63db1c05b6c45c62fe89623920a8&source=prChecksComment)



codecov[bot]: ## [Codecov](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1358?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 82.20%. Comparing base [(`47db2ad`)](https://app.codecov.io/gh/snowflakedb/gosnowflake/commit/47db2ad2f558b6142beb519379ae726c4139e82d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) to head [(`41b14a4`)](https://app.codecov.io/gh/snowflakedb/gosnowflake/commit/41b14a45c6ef63db1c05b6c45c62fe89623920a8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb).

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##           master    #1358   +/-   ##
=======================================
  Coverage   82.20%   82.20%           
=======================================
  Files          57       57           
  Lines       13911    13911           
=======================================
  Hits        11435    11435           
  Misses       2476     2476           
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1358?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/log.go b/log.go
index cb21d1b..2cce545 100644
--- a/log.go
+++ b/log.go
@@ -8,6 +8,7 @@ import (
 	"path"
 	"runtime"
 	"strings"
+	"sync"
 	"time"
 
 	rlog "github.com/sirupsen/logrus"
@@ -58,6 +59,7 @@ type defaultLogger struct {
 	inner   *rlog.Logger
 	enabled bool
 	file    *os.File
+	mu      sync.Mutex
 }
 
 type sfTextFormatter struct {
@@ -73,7 +75,11 @@ func (f *sfTextFormatter) Format(entry *rlog.Entry) ([]byte, error) {
 // SetLogLevel set logging level for calling defaultLogger
 func (log *defaultLogger) SetLogLevel(level string) error {
 	newEnabled := strings.ToUpper(level) != "OFF"
-	log.enabled = newEnabled
+	func() {
+		log.mu.Lock()
+		defer log.mu.Unlock()
+		log.enabled = newEnabled
+	}()
 	if newEnabled {
 		actualLevel, err := rlog.ParseLevel(level)
 		if err != nil {
@@ -84,9 +90,15 @@ func (log *defaultLogger) SetLogLevel(level string) error {
 	return nil
 }
 
+func (log *defaultLogger) isEnabled() bool {
+	log.mu.Lock()
+	defer log.mu.Unlock()
+	return log.enabled
+}
+
 // GetLogLevel return current log level
 func (log *defaultLogger) GetLogLevel() string {
-	if !log.enabled {
+	if !log.isEnabled() {
 		return "OFF"
 	}
 	return log.inner.GetLevel().String()
@@ -142,19 +154,19 @@ func (log *defaultLogger) WithField(key string, value interface{}) *rlog.Entry {
 
 }
 
-// Adds a struct of fields to the log entry. All it does is call `WithField` for
+// WithFields adds a struct of fields to the log entry. All it does is call `WithField` for
 // each `Field`.
 func (log *defaultLogger) WithFields(fields rlog.Fields) *rlog.Entry {
 	return log.inner.WithFields(fields)
 }
 
-// Add an error as single field to the log entry.  All it does is call
+// WithError adds an error as single field to the log entry.  All it does is call
 // `WithError` for the given `error`.
 func (log *defaultLogger) WithError(err error) *rlog.Entry {
 	return log.inner.WithError(err)
 }
 
-// Overrides the time of the log entry.
+// WithTime overrides the time of the log entry.
 func (log *defaultLogger) WithTime(t time.Time) *rlog.Entry {
 	return log.inner.WithTime(t)
 }
@@ -164,55 +176,55 @@ func (log *defaultLogger) Logf(level rlog.Level, format string, args ...interfac
 }
 
 func (log *defaultLogger) Tracef(format string, args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Tracef(format, args...)
 	}
 }
 
 func (log *defaultLogger) Debugf(format string, args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Debugf(format, args...)
 	}
 }
 
 func (log *defaultLogger) Infof(format string, args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Infof(format, args...)
 	}
 }
 
 func (log *defaultLogger) Printf(format string, args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Printf(format, args...)
 	}
 }
 
 func (log *defaultLogger) Warnf(format string, args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Warnf(format, args...)
 	}
 }
 
 func (log *defaultLogger) Warningf(format string, args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Warningf(format, args...)
 	}
 }
 
 func (log *defaultLogger) Errorf(format string, args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Errorf(format, args...)
 	}
 }
 
 func (log *defaultLogger) Fatalf(format string, args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Fatalf(format, args...)
 	}
 }
 
 func (log *defaultLogger) Panicf(format string, args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Panicf(format, args...)
 	}
 }
@@ -226,109 +238,109 @@ func (log *defaultLogger) LogFn(level rlog.Level, fn rlog.LogFunction) {
 }
 
 func (log *defaultLogger) Trace(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Trace(args...)
 	}
 }
 
 func (log *defaultLogger) Debug(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Debug(args...)
 	}
 }
 
 func (log *defaultLogger) Info(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Info(args...)
 	}
 }
 
 func (log *defaultLogger) Print(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Print(args...)
 	}
 }
 
 func (log *defaultLogger) Warn(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Warn(args...)
 	}
 }
 
 func (log *defaultLogger) Warning(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Warning(args...)
 	}
 }
 
 func (log *defaultLogger) Error(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Error(args...)
 	}
 }
 
 func (log *defaultLogger) Fatal(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Fatal(args...)
 	}
 }
 
 func (log *defaultLogger) Panic(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Panic(args...)
 	}
 }
 
 func (log *defaultLogger) TraceFn(fn rlog.LogFunction) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.TraceFn(fn)
 	}
 }
 
 func (log *defaultLogger) DebugFn(fn rlog.LogFunction) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.DebugFn(fn)
 	}
 }
 
 func (log *defaultLogger) InfoFn(fn rlog.LogFunction) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.InfoFn(fn)
 	}
 }
 
 func (log *defaultLogger) PrintFn(fn rlog.LogFunction) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.PrintFn(fn)
 	}
 }
 
 func (log *defaultLogger) WarnFn(fn rlog.LogFunction) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.PrintFn(fn)
 	}
 }
 
 func (log *defaultLogger) WarningFn(fn rlog.LogFunction) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.WarningFn(fn)
 	}
 }
 
 func (log *defaultLogger) ErrorFn(fn rlog.LogFunction) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.ErrorFn(fn)
 	}
 }
 
 func (log *defaultLogger) FatalFn(fn rlog.LogFunction) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.FatalFn(fn)
 	}
 }
 
 func (log *defaultLogger) PanicFn(fn rlog.LogFunction) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.PanicFn(fn)
 	}
 }
@@ -338,55 +350,55 @@ func (log *defaultLogger) Logln(level rlog.Level, args ...interface{}) {
 }
 
 func (log *defaultLogger) Traceln(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Traceln(args...)
 	}
 }
 
 func (log *defaultLogger) Debugln(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Debugln(args...)
 	}
 }
 
 func (log *defaultLogger) Infoln(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Infoln(args...)
 	}
 }
 
 func (log *defaultLogger) Println(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Println(args...)
 	}
 }
 
 func (log *defaultLogger) Warnln(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Warnln(args...)
 	}
 }
 
 func (log *defaultLogger) Warningln(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Warningln(args...)
 	}
 }
 
 func (log *defaultLogger) Errorln(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Errorln(args...)
 	}
 }
 
 func (log *defaultLogger) Fatalln(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Fatalln(args...)
 	}
 }
 
 func (log *defaultLogger) Panicln(args ...interface{}) {
-	if log.enabled {
+	if log.isEnabled() {
 		log.inner.Panicln(args...)
 	}
 }


Test patch:
diff --git a/driver_ocsp_test.go b/driver_ocsp_test.go
index 030e659..ff486f9 100644
--- a/driver_ocsp_test.go
+++ b/driver_ocsp_test.go
@@ -33,9 +33,9 @@ func deleteOCSPCacheFile() {
 
 // deleteOCSPCacheAll deletes all entries in the OCSP response cache on memory
 func deleteOCSPCacheAll() {
-	ocspResponseCacheLock.Lock()
-	defer ocspResponseCacheLock.Unlock()
-	ocspResponseCache = make(map[certIDKey]*certCacheValue)
+	syncUpdateOcspResponseCache(func() {
+		ocspResponseCache = make(map[certIDKey]*certCacheValue)
+	})
 }
 
 func cleanup() {
diff --git a/log_test.go b/log_test.go
index 962dada..bd1d2b9 100644
--- a/log_test.go
+++ b/log_test.go
@@ -12,10 +12,10 @@ import (
 	rlog "github.com/sirupsen/logrus"
 )
 
-func createTestLogger() defaultLogger {
+func createTestLogger() *defaultLogger {
 	var rLogger = rlog.New()
 	var ret = defaultLogger{inner: rLogger}
-	return ret
+	return &ret
 }
 
 func TestIsLevelEnabled(t *testing.T) {
@@ -59,7 +59,6 @@ func TestDefaultLogLevel(t *testing.T) {
 	logger := CreateDefaultLogger()
 	buf := &bytes.Buffer{}
 	logger.SetOutput(buf)
-	SetLogger(&logger)
 
 	// default logger level is info
 	logger.Info("info")
@@ -112,7 +111,6 @@ func TestOffLogLevel(t *testing.T) {
 	logger.SetOutput(buf)
 	err := logger.SetLogLevel("OFF")
 	assertNilF(t, err)
-	SetLogger(&logger)
 
 	logger.Info("info")
 	logger.Infof("info%v", "f")
diff --git a/ocsp_test.go b/ocsp_test.go
index 1fc1c2e..0904234 100644
--- a/ocsp_test.go
+++ b/ocsp_test.go
@@ -40,7 +40,9 @@ func TestOCSP(t *testing.T) {
 		for _, tgt := range targetURL {
 			_ = os.Setenv(cacheServerEnabledEnv, enabled)
 			_ = os.Remove(cacheFileName) // clear cache file
-			ocspResponseCache = make(map[certIDKey]*certCacheValue)
+			syncUpdateOcspResponseCache(func() {
+				ocspResponseCache = make(map[certIDKey]*certCacheValue)
+			})
 			for _, tr := range transports {
 				t.Run(fmt.Sprintf("%v_%v", tgt, enabled), func(t *testing.T) {
 					c := &http.Client{
@@ -186,7 +188,9 @@ func TestUnitCheckOCSPResponseCache(t *testing.T) {
 	}
 	b64Key := base64.StdEncoding.EncodeToString([]byte("DUMMY_VALUE"))
 	currentTime := float64(time.Now().UTC().Unix())
-	ocspResponseCache[dummyKey0] = &certCacheValue{currentTime, b64Key}
+	syncUpdateOcspResponseCache(func() {
+		ocspResponseCache[dummyKey0] = &certCacheValue{currentTime, b64Key}
+	})
 	subject := &x509.Certificate{}
 	issuer := &x509.Certificate{}
 	ost := checkOCSPResponseCache(&dummyKey, subject, issuer)
@@ -194,13 +198,17 @@ func TestUnitCheckOCSPResponseCache(t *testing.T) {
 		t.Fatalf("should have failed. expected: %v, got: %v", ocspMissedCache, ost.code)
 	}
 	// old timestamp
-	ocspResponseCache[dummyKey] = &certCacheValue{float64(1395054952), b64Key}
+	syncUpdateOcspResponseCache(func() {
+		ocspResponseCache[dummyKey] = &certCacheValue{float64(1395054952), b64Key}
+	})
 	ost = checkOCSPResponseCache(&dummyKey, subject, issuer)
 	if ost.code != ocspCacheExpired {
 		t.Fatalf("should have failed. expected: %v, got: %v", ocspCacheExpired, ost.code)
 	}
 	// future timestamp
-	ocspResponseCache[dummyKey] = &certCacheValue{float64(1805054952), b64Key}
+	syncUpdateOcspResponseCache(func() {
+		ocspResponseCache[dummyKey] = &certCacheValue{float64(1805054952), b64Key}
+	})
 	ost = checkOCSPResponseCache(&dummyKey, subject, issuer)
 	if ost.code != ocspFailedParseResponse {
 		t.Fatalf("should have failed. expected: %v, got: %v", ocspFailedDecodeResponse, ost.code)
@@ -214,13 +222,17 @@ func TestUnitCheckOCSPResponseCache(t *testing.T) {
 		"koRzw/UU7zKsqiTB0ZN/rgJp+MocTdqQSGKvbZyR8d4u8eNQqi1x4Pk3yO/pftANFaJKGB+JPgKS3PQAqJaXcipNcEfqtl7y4PO6kqA" + // pragma: allowlist secret
 		"Jb4xI/OTXIrRA5TsT4cCioE"
 	// issuer is not a true issuer certificate
-	ocspResponseCache[dummyKey] = &certCacheValue{float64(currentTime - 1000), actualOcspResponse}
+	syncUpdateOcspResponseCache(func() {
+		ocspResponseCache[dummyKey] = &certCacheValue{float64(currentTime - 1000), actualOcspResponse}
+	})
 	ost = checkOCSPResponseCache(&dummyKey, subject, issuer)
 	if ost.code != ocspFailedParseResponse {
 		t.Fatalf("should have failed. expected: %v, got: %v", ocspFailedParseResponse, ost.code)
 	}
 	// invalid validity
-	ocspResponseCache[dummyKey] = &certCacheValue{float64(currentTime - 1000), actualOcspResponse}
+	syncUpdateOcspResponseCache(func() {
+		ocspResponseCache[dummyKey] = &certCacheValue{float64(currentTime - 1000), actualOcspResponse}
+	})
 	ost = checkOCSPResponseCache(&dummyKey, subject, nil)
 	if ost.code != ocspInvalidValidity {
 		t.Fatalf("should have failed. expected: %v, got: %v", ocspInvalidValidity, ost.code)
@@ -235,11 +247,9 @@ func TestOcspCacheClearer(t *testing.T) {
 		initOCSPCache()
 		initOCSPCacheClearer()
 	}()
-	func() {
-		ocspResponseCacheLock.Lock()
-		defer ocspResponseCacheLock.Unlock()
+	syncUpdateOcspResponseCache(func() {
 		ocspResponseCache[certIDKey{}] = nil
-	}()
+	})
 	func() {
 		ocspParsedRespCacheLock.Lock()
 		defer ocspParsedRespCacheLock.Unlock()
@@ -249,11 +259,9 @@ func TestOcspCacheClearer(t *testing.T) {
 	os.Setenv(ocspResponseCacheClearingIntervalInSecondsEnv, "1")
 	initOCSPCacheClearer()
 	time.Sleep(2 * time.Second)
-	func() {
-		ocspResponseCacheLock.Lock()
-		defer ocspResponseCacheLock.Unlock()
+	syncUpdateOcspResponseCache(func() {
 		assertEqualE(t, len(ocspResponseCache), 0)
-	}()
+	})
 	func() {
 		ocspParsedRespCacheLock.Lock()
 		defer ocspParsedRespCacheLock.Unlock()
@@ -598,3 +606,9 @@ func TestInitOCSPCacheFileCreation(t *testing.T) {
 		t.Error(err)
 	}
 }
+
+func syncUpdateOcspResponseCache(f func()) {
+	ocspResponseCacheLock.Lock()
+	defer ocspResponseCacheLock.Unlock()
+	f()
+}
diff --git a/secure_storage_manager_test.go b/secure_storage_manager_test.go
index 51b42fb..25f88ea 100644
--- a/secure_storage_manager_test.go
+++ b/secure_storage_manager_test.go
@@ -242,7 +242,7 @@ func TestSkipStoringCredentialIfHostIsEmpty(t *testing.T) {
 	}
 }
 
-func TestStoreTemporaryCredental(t *testing.T) {
+func TestStoreTemporaryCredential(t *testing.T) {
 	if runningOnGithubAction() {
 		t.Skip("cannot write to github file system")
 	}