From d219098916140466493fcca942a98b51582e32ca Mon Sep 17 00:00:00 2001 From: Preston Vasquez Date: Tue, 1 Aug 2023 10:29:15 -0600 Subject: [PATCH] GODRIVER-2881 Enable logging for ComponentAll (#1340) --- internal/logger/logger.go | 20 +++- internal/logger/logger_test.go | 176 +++++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 2 deletions(-) diff --git a/internal/logger/logger.go b/internal/logger/logger.go index c4053ea3df..07dcffe66b 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -75,9 +75,25 @@ func (logger *Logger) Close() error { } // LevelComponentEnabled will return true if the given LogLevel is enabled for -// the given LogComponent. +// the given LogComponent. If the ComponentLevels on the logger are enabled for +// "ComponentAll", then this function will return true for any level bound by +// the level assigned to "ComponentAll". +// +// If the level is not enabled (i.e. LevelOff), then false is returned. This is +// to avoid false positives, such as returning "true" for a component that is +// not enabled. For example, without this condition, an empty LevelComponent +// would be considered "enabled" for "LevelOff". func (logger *Logger) LevelComponentEnabled(level Level, component Component) bool { - return logger.ComponentLevels[component] >= level + if level == LevelOff { + return false + } + + if logger.ComponentLevels == nil { + return false + } + + return logger.ComponentLevels[component] >= level || + logger.ComponentLevels[ComponentAll] >= level } // Print will synchronously print the given message to the configured LogSink. diff --git a/internal/logger/logger_test.go b/internal/logger/logger_test.go index eead29c96c..8629a10748 100644 --- a/internal/logger/logger_test.go +++ b/internal/logger/logger_test.go @@ -14,6 +14,8 @@ import ( "reflect" "sync" "testing" + + "go.mongodb.org/mongo-driver/internal/assert" ) type mockLogSink struct{} @@ -334,3 +336,177 @@ func TestTruncate(t *testing.T) { } } + +func TestLogger_LevelComponentEnabled(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + logger Logger + level Level + component Component + want bool + }{ + { + name: "zero", + logger: Logger{}, + level: LevelOff, + component: ComponentCommand, + want: false, + }, + { + name: "empty", + logger: Logger{ + ComponentLevels: map[Component]Level{}, + }, + level: LevelOff, + component: ComponentCommand, + want: false, // LevelOff should never be considered enabled. + }, + { + name: "one level below", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentCommand: LevelDebug, + }, + }, + level: LevelInfo, + component: ComponentCommand, + want: true, + }, + { + name: "equal levels", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentCommand: LevelDebug, + }, + }, + level: LevelDebug, + component: ComponentCommand, + want: true, + }, + { + name: "one level above", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentCommand: LevelInfo, + }, + }, + level: LevelDebug, + component: ComponentCommand, + want: false, + }, + { + name: "component mismatch", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentCommand: LevelDebug, + }, + }, + level: LevelDebug, + component: ComponentTopology, + want: false, + }, + { + name: "component all enables with topology", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentAll: LevelDebug, + }, + }, + level: LevelDebug, + component: ComponentTopology, + want: true, + }, + { + name: "component all enables with server selection", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentAll: LevelDebug, + }, + }, + level: LevelDebug, + component: ComponentServerSelection, + want: true, + }, + { + name: "component all enables with connection", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentAll: LevelDebug, + }, + }, + level: LevelDebug, + component: ComponentConnection, + want: true, + }, + { + name: "component all enables with command", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentAll: LevelDebug, + }, + }, + level: LevelDebug, + component: ComponentCommand, + want: true, + }, + { + name: "component all enables with all", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentAll: LevelDebug, + }, + }, + level: LevelDebug, + component: ComponentAll, + want: true, + }, + { + name: "component all does not enable with lower level", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentAll: LevelInfo, + }, + }, + level: LevelDebug, + component: ComponentCommand, + want: false, + }, + { + name: "component all has a lower log level than command", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentAll: LevelInfo, + ComponentCommand: LevelDebug, + }, + }, + level: LevelDebug, + component: ComponentCommand, + want: true, + }, + { + name: "component all has a higher log level than command", + logger: Logger{ + ComponentLevels: map[Component]Level{ + ComponentAll: LevelDebug, + ComponentCommand: LevelInfo, + }, + }, + level: LevelDebug, + component: ComponentCommand, + want: true, + }, + } + + for _, tcase := range tests { + tcase := tcase // Capture the range variable. + + t.Run(tcase.name, func(t *testing.T) { + t.Parallel() + + got := tcase.logger.LevelComponentEnabled(tcase.level, tcase.component) + assert.Equal(t, tcase.want, got, "unexpected result for LevelComponentEnabled") + }) + } +}