Merge pull request #55 from rstudio/mnv-fix-debug-log-enabled-inherit…

…ance Fix debug child loggers not being enabled
rstudio · Feb 1, 2022 · e43ea28 · e43ea28
2 parents 0b352bf + 42f9027
commit e43ea28
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 4 deletions.
diff --git a/pkg/rslog/debug.go b/pkg/rslog/debug.go
@@ -178,8 +178,9 @@ func (l *debugLogger) Tracef(message string, args ...interface{}) {
 func (l *debugLogger) WithFields(fields Fields) DebugLogger {
 	newLgr := l.Logger.WithFields(fields)
 	dbglgr := &debugLogger{
-		Logger: newLgr,
-		region: l.region,
+		Logger:  newLgr,
+		region:  l.region,
+		enabled: l.enabled,
 	}
 	registerLoggerCb(l.region, dbglgr.enable)
 	return dbglgr
@@ -190,8 +191,9 @@ func (l *debugLogger) WithFields(fields Fields) DebugLogger {
 func (l *debugLogger) WithSubRegion(subregion string) DebugLogger {
 	newLgr := l.Logger.WithField("sub_region", subregion)
 	dbglgr := &debugLogger{
-		Logger: newLgr,
-		region: l.region,
+		Logger:  newLgr,
+		region:  l.region,
+		enabled: l.enabled,
 	}
 	registerLoggerCb(l.region, dbglgr.enable)
 	return dbglgr

diff --git a/pkg/rslog/rslogtest/debug_test.go b/pkg/rslog/rslogtest/debug_test.go
@@ -53,6 +53,7 @@ func (s *DebugLoggerSuite) TestInitLog() {
 }
 
 func (s *DebugLoggerSuite) TestNewDebugLogger() {
+	loggerMock.On("Debugf", "test call %s", mock.Anything).Return(loggerMock)
 	loggerMock.On("WithFields", rslog.Fields{"region": ""}).Return(loggerMock)
 	loggerMock.On("WithFields", rslog.Fields{"id": "654-987"}).Return(loggerMock)
 	loggerMock.On("WithField", "sub_region", "balancer").Return(loggerMock)
@@ -62,15 +63,32 @@ func (s *DebugLoggerSuite) TestNewDebugLogger() {
 	s.Equal(lgr.Enabled(), false)
 
 	rslog.Enable(Proxy)
+	lgr.Debugf("test call %s", "base parent")
 	s.Equal(lgr.Enabled(), true)
+	s.Equal(loggerMock.LastCall(), "test call base parent")
 
 	// Logger with fields should be under same region, new callback
 	fieldslgr := lgr.WithFields(rslog.Fields{"id": "654-987"})
+	fieldslgr.Debugf("test call %s", "with-fields")
 	s.Equal(fieldslgr.Enabled(), true)
+	s.Equal(loggerMock.LastCall(), "test call with-fields")
 
 	// SubRegion Logger should be under same region, new callback
 	sublgr := lgr.WithSubRegion("balancer")
+	sublgr.Debugf("test call %s", "subregion")
 	s.Equal(sublgr.Enabled(), true)
+	s.Equal(loggerMock.LastCall(), "test call subregion")
+
+	// Last call, no new calls after disable
+	lgr.Debugf("test call %s", "LAST")
+	s.Equal(loggerMock.LastCall(), "test call LAST")
+
+	rslog.Disable(Proxy)
+	lgr.Debugf("test call %s", "base parent")
+	fieldslgr.Debugf("test call %s", "with-fields")
+	sublgr.Debugf("test call %s", "subregion")
+
+	s.Equal(loggerMock.LastCall(), "test call LAST")
 
 	// For a totally different region
 	another := rslog.NewDebugLogger(LDAP)