Skip to content

Commit 852f2c3

Browse files
author
Evan Phoenix
authored
Merge pull request #134 from hashicorp/f-better-levels
Implement the ability to more logically share level hierarchies
2 parents 3d50de2 + 4911d46 commit 852f2c3

File tree

3 files changed

+177
-12
lines changed

3 files changed

+177
-12
lines changed

intlogger.go

Lines changed: 77 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,16 @@ type intLogger struct {
7979
writer *writer
8080
level *int32
8181

82+
// The value of curEpoch the last time we performed the level sync process
83+
ownEpoch uint64
84+
85+
// Shared amongst all the loggers created in this hierachy, used to determine
86+
// if the level sync process should be run by comparing it with ownEpoch
87+
curEpoch *uint64
88+
89+
// The logger this one was created from. Only set when syncParentLevel is set
90+
parent *intLogger
91+
8292
headerColor ColorOption
8393
fieldColor ColorOption
8494

@@ -88,6 +98,7 @@ type intLogger struct {
8898

8999
// create subloggers with their own level setting
90100
independentLevels bool
101+
syncParentLevel bool
91102

92103
subloggerHook func(sub Logger) Logger
93104
}
@@ -129,9 +140,9 @@ func newLogger(opts *LoggerOptions) *intLogger {
129140
}
130141

131142
var (
132-
primaryColor ColorOption = ColorOff
133-
headerColor ColorOption = ColorOff
134-
fieldColor ColorOption = ColorOff
143+
primaryColor = ColorOff
144+
headerColor = ColorOff
145+
fieldColor = ColorOff
135146
)
136147
switch {
137148
case opts.ColorHeaderOnly:
@@ -152,8 +163,10 @@ func newLogger(opts *LoggerOptions) *intLogger {
152163
mutex: mutex,
153164
writer: newWriter(output, primaryColor),
154165
level: new(int32),
166+
curEpoch: new(uint64),
155167
exclude: opts.Exclude,
156168
independentLevels: opts.IndependentLevels,
169+
syncParentLevel: opts.SyncParentLevel,
157170
headerColor: headerColor,
158171
fieldColor: fieldColor,
159172
subloggerHook: opts.SubloggerHook,
@@ -194,7 +207,7 @@ const offsetIntLogger = 3
194207
// Log a message and a set of key/value pairs if the given level is at
195208
// or more severe that the threshold configured in the Logger.
196209
func (l *intLogger) log(name string, level Level, msg string, args ...interface{}) {
197-
if level < Level(atomic.LoadInt32(l.level)) {
210+
if level < l.GetLevel() {
198211
return
199212
}
200213

@@ -597,7 +610,7 @@ func (l *intLogger) logJSON(t time.Time, name string, level Level, msg string, a
597610
vals := l.jsonMapEntry(t, name, level, msg)
598611
args = append(l.implied, args...)
599612

600-
if args != nil && len(args) > 0 {
613+
if len(args) > 0 {
601614
if len(args)%2 != 0 {
602615
cs, ok := args[len(args)-1].(CapturedStacktrace)
603616
if ok {
@@ -718,27 +731,27 @@ func (l *intLogger) Error(msg string, args ...interface{}) {
718731

719732
// Indicate that the logger would emit TRACE level logs
720733
func (l *intLogger) IsTrace() bool {
721-
return Level(atomic.LoadInt32(l.level)) == Trace
734+
return l.GetLevel() == Trace
722735
}
723736

724737
// Indicate that the logger would emit DEBUG level logs
725738
func (l *intLogger) IsDebug() bool {
726-
return Level(atomic.LoadInt32(l.level)) <= Debug
739+
return l.GetLevel() <= Debug
727740
}
728741

729742
// Indicate that the logger would emit INFO level logs
730743
func (l *intLogger) IsInfo() bool {
731-
return Level(atomic.LoadInt32(l.level)) <= Info
744+
return l.GetLevel() <= Info
732745
}
733746

734747
// Indicate that the logger would emit WARN level logs
735748
func (l *intLogger) IsWarn() bool {
736-
return Level(atomic.LoadInt32(l.level)) <= Warn
749+
return l.GetLevel() <= Warn
737750
}
738751

739752
// Indicate that the logger would emit ERROR level logs
740753
func (l *intLogger) IsError() bool {
741-
return Level(atomic.LoadInt32(l.level)) <= Error
754+
return l.GetLevel() <= Error
742755
}
743756

744757
const MissingKey = "EXTRA_VALUE_AT_END"
@@ -854,12 +867,62 @@ func (l *intLogger) resetOutput(opts *LoggerOptions) error {
854867
// Update the logging level on-the-fly. This will affect all subloggers as
855868
// well.
856869
func (l *intLogger) SetLevel(level Level) {
857-
atomic.StoreInt32(l.level, int32(level))
870+
if !l.syncParentLevel {
871+
atomic.StoreInt32(l.level, int32(level))
872+
return
873+
}
874+
875+
nsl := new(int32)
876+
*nsl = int32(level)
877+
878+
l.level = nsl
879+
880+
l.ownEpoch = atomic.AddUint64(l.curEpoch, 1)
881+
}
882+
883+
func (l *intLogger) searchLevelPtr() *int32 {
884+
p := l.parent
885+
886+
ptr := l.level
887+
888+
max := l.ownEpoch
889+
890+
for p != nil {
891+
if p.ownEpoch > max {
892+
max = p.ownEpoch
893+
ptr = p.level
894+
}
895+
896+
p = p.parent
897+
}
898+
899+
return ptr
858900
}
859901

860902
// Returns the current level
861903
func (l *intLogger) GetLevel() Level {
862-
return Level(atomic.LoadInt32(l.level))
904+
// We perform the loads immediately to keep the CPU pipeline busy, which
905+
// effectively makes the second load cost nothing. Once loaded into registers
906+
// the comparison returns the already loaded value. The comparison is almost
907+
// always true, so the branch predictor should hit consistently with it.
908+
var (
909+
curEpoch = atomic.LoadUint64(l.curEpoch)
910+
level = Level(atomic.LoadInt32(l.level))
911+
own = l.ownEpoch
912+
)
913+
914+
if curEpoch == own {
915+
return level
916+
}
917+
918+
// Perform the level sync process. We'll avoid doing this next time by seeing the
919+
// epoch as current.
920+
921+
ptr := l.searchLevelPtr()
922+
l.level = ptr
923+
l.ownEpoch = curEpoch
924+
925+
return Level(atomic.LoadInt32(ptr))
863926
}
864927

865928
// Create a *log.Logger that will send it's data through this Logger. This
@@ -912,6 +975,8 @@ func (l *intLogger) copy() *intLogger {
912975
if l.independentLevels {
913976
sl.level = new(int32)
914977
*sl.level = *l.level
978+
} else if l.syncParentLevel {
979+
sl.parent = l
915980
}
916981

917982
return &sl

logger.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,24 @@ type LoggerOptions struct {
304304
// will not affect the parent or sibling loggers.
305305
IndependentLevels bool
306306

307+
// When set, changing the level of a logger effects only it's direct sub-loggers
308+
// rather than all sub-loggers. For example:
309+
// a := logger.Named("a")
310+
// a.SetLevel(Error)
311+
// b := a.Named("b")
312+
// c := a.Named("c")
313+
// b.GetLevel() => Error
314+
// c.GetLevel() => Error
315+
// b.SetLevel(Info)
316+
// a.GetLevel() => Error
317+
// b.GetLevel() => Info
318+
// c.GetLevel() => Error
319+
// a.SetLevel(Warn)
320+
// a.GetLevel() => Warn
321+
// b.GetLevel() => Warn
322+
// c.GetLevel() => Warn
323+
SyncParentLevel bool
324+
307325
// SubloggerHook registers a function that is called when a sublogger via
308326
// Named, With, or ResetNamed is created. If defined, the function is passed
309327
// the newly created Logger and the returned Logger is returned from the

logger_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,88 @@ func TestLogger(t *testing.T) {
586586
t.Fatal("output from disabled logger:", str)
587587
}
588588
})
589+
590+
t.Run("sub-loggers levels don't bubble upward", func(t *testing.T) {
591+
var buf bytes.Buffer
592+
593+
logger := New(&LoggerOptions{
594+
Name: "root",
595+
Output: &buf,
596+
SyncParentLevel: true,
597+
})
598+
599+
another := logger.Named("sublogger")
600+
another.SetLevel(Error)
601+
602+
logger.Info("this is test")
603+
str := buf.String()
604+
dataIdx := strings.IndexByte(str, ' ')
605+
rest := str[dataIdx+1:]
606+
assert.Equal(t, "[INFO] root: this is test\n", rest)
607+
608+
buf.Reset()
609+
610+
a := logger.Named("a")
611+
b := a.Named("b")
612+
c := a.Named("c")
613+
614+
a.SetLevel(Error)
615+
616+
b.Info("this is a test")
617+
618+
require.Empty(t, buf.String())
619+
620+
b.SetLevel(Info)
621+
622+
assert.Equal(t, Error, a.GetLevel())
623+
624+
a.SetLevel(Error)
625+
626+
assert.Equal(t, Error, b.GetLevel())
627+
628+
assert.Equal(t, Error, c.GetLevel())
629+
630+
// Make sure that setting a sibling logger doesn't confuse
631+
// when b had previously had it's own level.
632+
c.SetLevel(Info)
633+
634+
assert.Equal(t, Error, b.GetLevel())
635+
})
636+
637+
t.Run("level sync example", func(t *testing.T) {
638+
var buf bytes.Buffer
639+
640+
logger := New(&LoggerOptions{
641+
Name: "root",
642+
Output: &buf,
643+
SyncParentLevel: true,
644+
})
645+
646+
s := assert.New(t)
647+
648+
a := logger.Named("a")
649+
a.SetLevel(Error)
650+
b := a.Named("b")
651+
c := a.Named("c")
652+
s.Equal(Error, b.GetLevel())
653+
s.Equal(Error, c.GetLevel())
654+
655+
b.SetLevel(Info)
656+
s.Equal(Error, a.GetLevel())
657+
s.Equal(Info, b.GetLevel())
658+
s.Equal(Error, c.GetLevel())
659+
660+
a.SetLevel(Warn)
661+
s.Equal(Warn, a.GetLevel())
662+
s.Equal(Warn, b.GetLevel())
663+
s.Equal(Warn, c.GetLevel())
664+
665+
logger.SetLevel(Trace)
666+
s.Equal(Trace, logger.GetLevel())
667+
s.Equal(Trace, a.GetLevel())
668+
s.Equal(Trace, b.GetLevel())
669+
s.Equal(Trace, c.GetLevel())
670+
})
589671
}
590672

591673
func TestLogger_leveledWriter(t *testing.T) {

0 commit comments

Comments
 (0)