From 358c97c438a32151f114e67323d3eee43bef748f Mon Sep 17 00:00:00 2001 From: Ankhit Bala Venkata <57641537+abalaven@users.noreply.github.com> Date: Thu, 26 Sep 2024 16:35:24 -0700 Subject: [PATCH 1/2] Added Optional Logging support (#34) --- chanGroup_test.go | 4 +++- driver.go | 41 ++++++++++++++++++++++++++++------------- fsnotify/filewatcher.go | 12 ++++++------ go.mod | 1 - go.sum | 7 ------- logger/logger.go | 8 ++++++++ 6 files changed, 45 insertions(+), 28 deletions(-) create mode 100644 logger/logger.go diff --git a/chanGroup_test.go b/chanGroup_test.go index faf3a63..983436b 100644 --- a/chanGroup_test.go +++ b/chanGroup_test.go @@ -3,9 +3,10 @@ package hotload import ( "context" "database/sql/driver" + "sync" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "sync" ) type testConn struct { @@ -50,6 +51,7 @@ var _ = Describe("Driver", func() { sqlDriver: nil, mu: sync.RWMutex{}, conns: conns, + log: DefaultLogger{}, } }) diff --git a/driver.go b/driver.go index 3def12c..cd92160 100644 --- a/driver.go +++ b/driver.go @@ -55,7 +55,7 @@ import ( "sort" "sync" - "github.com/sirupsen/logrus" + "github.com/infobloxopen/hotload/logger" ) // Strategy is the plugin interface for hotload. @@ -78,7 +78,7 @@ var ( sqlDrivers = make(map[string]*driverInstance) strategies = make(map[string]Strategy) - logger *logrus.Logger + log logger.Logger ) type driverInstance struct { @@ -86,6 +86,12 @@ type driverInstance struct { options map[string]string } +type DefaultLogger struct{} + +func (d DefaultLogger) Debug(args ...any) {} +func (d DefaultLogger) Info(args ...any) {} +func (d DefaultLogger) Error(args ...any) {} + type driverOption func(*driverInstance) // WithDriverOptions allows you to specify query parameters to the underlying driver. @@ -169,14 +175,8 @@ func Strategies() []string { return list } -// SetLogLevel specifies the logrus.Level for the hotload driver's logger -func SetLogLevel(level logrus.Level) { - logger.SetLevel(level) -} - func init() { sql.Register("hotload", &hdriver{ctx: context.Background(), cgroup: make(map[string]*chanGroup)}) - logger = logrus.New() } // hdriver is the hotload driver. @@ -197,6 +197,7 @@ type chanGroup struct { mu sync.RWMutex forceKill bool conns []*managedConn + log logger.Logger } // monitor the location for changes @@ -205,7 +206,7 @@ func (cg *chanGroup) run() { select { case <-cg.parentCtx.Done(): cg.cancel() - logger.Debug("cancelling chanGroup context") + cg.log.Debug("cancelling chanGroup context") return case v := <-cg.values: if v == cg.value { @@ -213,7 +214,7 @@ func (cg *chanGroup) run() { continue } cg.valueChanged(v) - logger.Debug("connection information changed") + cg.log.Debug("connection information changed") } } } @@ -292,11 +293,11 @@ func (cg *chanGroup) remove(conn *managedConn) { func (cg *chanGroup) parseValues(vs url.Values) { cg.mu.Lock() defer cg.mu.Unlock() - logger.WithFields(logrus.Fields{"urlValues": vs}).Debug("parsing values") + cg.log.Debug("parsing values", vs) if v, ok := vs[forceKill]; ok { firstValue := v[0] cg.forceKill = firstValue == "true" - logger.Debug("forceKill set to true") + cg.log.Debug("forceKill set to true") } } @@ -315,7 +316,6 @@ func (h *hdriver) Open(name string) (driver.Conn, error) { if !ok { return nil, ErrUnsupportedStrategy } - sqlDriver, ok := sqlDrivers[uri.Host] if !ok { return nil, ErrUnknownDriver @@ -334,6 +334,7 @@ func (h *hdriver) Open(name string) (driver.Conn, error) { cancel: cancel, sqlDriver: sqlDriver, conns: make([]*managedConn, 0), + log: GetLogger(), } cgroup.parseValues(queryParams) h.cgroup[name] = cgroup @@ -341,3 +342,17 @@ func (h *hdriver) Open(name string) (driver.Conn, error) { } return cgroup.Open() } + +func WithLogger(l logger.Logger) { + log = l + if log == nil { + log = DefaultLogger{} + } +} + +func GetLogger() logger.Logger { + if log == nil { + return DefaultLogger{} + } + return log +} diff --git a/fsnotify/filewatcher.go b/fsnotify/filewatcher.go index f68fead..5b8b787 100644 --- a/fsnotify/filewatcher.go +++ b/fsnotify/filewatcher.go @@ -2,7 +2,6 @@ package fsnotify import ( "context" - "log" "net/url" "os" "strings" @@ -19,6 +18,7 @@ func init() { } var resyncPeriod = time.Second * 2 +var log = hotload.GetLogger() // NewStrategy implements a hotload strategy that monitors config changes // in a file using fsnotify. @@ -52,7 +52,7 @@ func readConfigFile(path string) (v []byte, err error) { } func resync(w watcher, pth string) (string, error) { - log.Printf("fsnotify: Path Name-Resync=%s", pth) + log.Debug("fsnotify: Path Name-Resync ", pth) err := w.Remove(pth) if err != nil && !errors.Is(err, rfsnotify.ErrNonExistentWatch) { return "", err @@ -69,7 +69,7 @@ func (s *Strategy) run() { for { select { case e := <-s.watcher.GetEvents(): - log.Printf("fsnotify: Path Name-Run=%s", e.Name) + log.Debug("fsnotify: Path Name-Run ", e.Name) if e.Op != rfsnotify.Write && e.Op != rfsnotify.Remove { continue } @@ -82,7 +82,7 @@ func (s *Strategy) run() { s.setVal(e.Name, val) case e := <-s.watcher.GetErrors(): - log.Printf("got error: %s", e) + log.Debug("got error: ", e) break case <-time.After(resyncPeriod): var fixedPaths []string @@ -104,7 +104,7 @@ func (s *Strategy) setVal(pth string, val string) { s.mu.Lock() defer s.mu.Unlock() if _, ok := s.paths[pth]; !ok { - log.Printf("fsnotify: Path not in map=%s", pth) + log.Debug("fsnotify: Path not in map ", pth) return } s.paths[pth].value = val @@ -129,7 +129,7 @@ func (s *Strategy) Watch(ctx context.Context, pth string, options url.Values) (v } notifier, found := s.paths[pth] if !found { - log.Printf("fsnotify: Path Name-Init=%s", pth) + log.Debug("fsnotify: Path Name-Init ", pth) if err := s.watcher.Add(pth); err != nil { return "", nil, err } diff --git a/go.mod b/go.mod index b3c2ebb..f46fcf4 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,6 @@ require ( github.com/onsi/gomega v1.27.6 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.18.0 - github.com/sirupsen/logrus v1.9.0 ) require ( diff --git a/go.sum b/go.sum index db6e06b..6b8fb4c 100644 --- a/go.sum +++ b/go.sum @@ -52,7 +52,6 @@ github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE= github.com/onsi/gomega v1.27.6/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_golang v1.18.0 h1:HzFfmkOzH5Q8L8G+kSJKUx5dtG87sewO+FoDDqP5Tbk= github.com/prometheus/client_golang v1.18.0/go.mod h1:T+GXkCk5wSJyOqMIzVgvvjFDlkOQntgjkJWKrN5txjA= @@ -63,12 +62,8 @@ github.com/prometheus/common v0.45.0/go.mod h1:YJmSTw9BoKxJplESWWxlbyttQR4uaEcGy github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k6Bo= github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= -github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0= -github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= -github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= @@ -93,7 +88,6 @@ golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= @@ -125,6 +119,5 @@ gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWD gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/logger/logger.go b/logger/logger.go new file mode 100644 index 0000000..089437e --- /dev/null +++ b/logger/logger.go @@ -0,0 +1,8 @@ +package logger + +// Logger defines the interface for logging. +type Logger interface { + Info(args ...interface{}) + Error(args ...interface{}) + Debug(args ...interface{}) +} From 3a92f6e85a5457da69d896f6b71a98dec851ee2b Mon Sep 17 00:00:00 2001 From: Ankhit Bala Venkata <57641537+abalaven@users.noreply.github.com> Date: Thu, 26 Sep 2024 17:56:22 -0700 Subject: [PATCH 2/2] Simplify the logging structure (#35) --- chanGroup_test.go | 3 ++- driver.go | 18 ++++++------------ fsnotify/filewatcher.go | 10 +++++----- logger/logger.go | 9 ++++----- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/chanGroup_test.go b/chanGroup_test.go index 983436b..0e7cdea 100644 --- a/chanGroup_test.go +++ b/chanGroup_test.go @@ -5,6 +5,7 @@ import ( "database/sql/driver" "sync" + "github.com/infobloxopen/hotload/logger" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -51,7 +52,7 @@ var _ = Describe("Driver", func() { sqlDriver: nil, mu: sync.RWMutex{}, conns: conns, - log: DefaultLogger{}, + log: logger.DefaultLogger, } }) diff --git a/driver.go b/driver.go index cd92160..9d89179 100644 --- a/driver.go +++ b/driver.go @@ -86,12 +86,6 @@ type driverInstance struct { options map[string]string } -type DefaultLogger struct{} - -func (d DefaultLogger) Debug(args ...any) {} -func (d DefaultLogger) Info(args ...any) {} -func (d DefaultLogger) Error(args ...any) {} - type driverOption func(*driverInstance) // WithDriverOptions allows you to specify query parameters to the underlying driver. @@ -206,7 +200,7 @@ func (cg *chanGroup) run() { select { case <-cg.parentCtx.Done(): cg.cancel() - cg.log.Debug("cancelling chanGroup context") + cg.log("cancelling chanGroup context") return case v := <-cg.values: if v == cg.value { @@ -214,7 +208,7 @@ func (cg *chanGroup) run() { continue } cg.valueChanged(v) - cg.log.Debug("connection information changed") + cg.log("connection information changed") } } } @@ -293,11 +287,11 @@ func (cg *chanGroup) remove(conn *managedConn) { func (cg *chanGroup) parseValues(vs url.Values) { cg.mu.Lock() defer cg.mu.Unlock() - cg.log.Debug("parsing values", vs) + cg.log("parsing values", vs) if v, ok := vs[forceKill]; ok { firstValue := v[0] cg.forceKill = firstValue == "true" - cg.log.Debug("forceKill set to true") + cg.log("forceKill set to true") } } @@ -346,13 +340,13 @@ func (h *hdriver) Open(name string) (driver.Conn, error) { func WithLogger(l logger.Logger) { log = l if log == nil { - log = DefaultLogger{} + log = logger.DefaultLogger } } func GetLogger() logger.Logger { if log == nil { - return DefaultLogger{} + return logger.DefaultLogger } return log } diff --git a/fsnotify/filewatcher.go b/fsnotify/filewatcher.go index 5b8b787..59b4885 100644 --- a/fsnotify/filewatcher.go +++ b/fsnotify/filewatcher.go @@ -52,7 +52,7 @@ func readConfigFile(path string) (v []byte, err error) { } func resync(w watcher, pth string) (string, error) { - log.Debug("fsnotify: Path Name-Resync ", pth) + log("fsnotify: Path Name-Resync ", pth) err := w.Remove(pth) if err != nil && !errors.Is(err, rfsnotify.ErrNonExistentWatch) { return "", err @@ -69,7 +69,7 @@ func (s *Strategy) run() { for { select { case e := <-s.watcher.GetEvents(): - log.Debug("fsnotify: Path Name-Run ", e.Name) + log("fsnotify: Path Name-Run ", e.Name) if e.Op != rfsnotify.Write && e.Op != rfsnotify.Remove { continue } @@ -82,7 +82,7 @@ func (s *Strategy) run() { s.setVal(e.Name, val) case e := <-s.watcher.GetErrors(): - log.Debug("got error: ", e) + log("got error: ", e) break case <-time.After(resyncPeriod): var fixedPaths []string @@ -104,7 +104,7 @@ func (s *Strategy) setVal(pth string, val string) { s.mu.Lock() defer s.mu.Unlock() if _, ok := s.paths[pth]; !ok { - log.Debug("fsnotify: Path not in map ", pth) + log("fsnotify: Path not in map ", pth) return } s.paths[pth].value = val @@ -129,7 +129,7 @@ func (s *Strategy) Watch(ctx context.Context, pth string, options url.Values) (v } notifier, found := s.paths[pth] if !found { - log.Debug("fsnotify: Path Name-Init ", pth) + log("fsnotify: Path Name-Init ", pth) if err := s.watcher.Add(pth); err != nil { return "", nil, err } diff --git a/logger/logger.go b/logger/logger.go index 089437e..a73d6d3 100644 --- a/logger/logger.go +++ b/logger/logger.go @@ -1,8 +1,7 @@ package logger // Logger defines the interface for logging. -type Logger interface { - Info(args ...interface{}) - Error(args ...interface{}) - Debug(args ...interface{}) -} +type Logger func(...interface{}) + +// DefaultLogger is a no-op logger function that does nothing +var DefaultLogger Logger = func(args ...interface{}) {}