Skip to content

Commit ef40e6a

Browse files
xobotyixobotyi
committed
[logger] fix logger name carrying
- Previously in case multiple child logger would change name it would result in multiple fields with the same name. New approach allows for logger name rewrites and custom logger name generation strategies. Package provides two implementations of logger name formatters - hierarchical (with `:` delimiter) and replaced (which completely replaces the name). - Implemented `logger.IsEqual` allowing to compare if two loggers are functionally the same. Method is mostly utilitary and rarely needed for external use therefore not reflected in doc.go. Change-Id: I09d18c3353beaddf696c97f5d8e2ca0db718a3e6
1 parent dbbf7a2 commit ef40e6a

4 files changed

Lines changed: 224 additions & 18 deletions

File tree

‎logger/ctx_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func TestToAndFromCtx(t *testing.T) {
3232
lgrCtx, ok := logger.FromCtx(ctxLogger)
3333
assert.True(t, ok, "should return true for context with logger")
3434
assert.False(t, lgrCtx.IsZero(), "should not return zero-value logger for context with logger")
35-
assert.Equal(t, lgr, lgrCtx, "should return same logger as stored in context")
35+
assert.True(t, logger.IsEqual(lgr, lgrCtx), "should return same logger as stored in context")
3636
}
3737
}
3838

@@ -53,6 +53,6 @@ func TestFromCtxOrNop(t *testing.T) {
5353
{
5454
lgrCtx := logger.FromCtxOrNop(ctxLogger)
5555
assert.False(t, lgrCtx.IsNop(), "should not return nop logger for context with logger")
56-
assert.Equal(t, lgr, lgrCtx, "should return same logger as stored in context")
56+
assert.True(t, logger.IsEqual(lgr, lgrCtx), "should return same logger as stored in context")
5757
}
5858
}

‎logger/doc.go‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@
6464
// dbLogger := lgr.WithName("database")
6565
// dbLogger.Info("connection established") // includes logger-name: database
6666
//
67+
// // Build hierarchical names by chaining WithName calls
68+
// // By default, names are joined with ":" separator
69+
// serviceLogger := lgr.WithName("service")
70+
// handlerLogger := serviceLogger.WithName("handler")
71+
// handlerLogger.Info("processing") // includes logger-name: service:handler
72+
//
6773
// // Attach stack trace to debug issues
6874
// lgr.WithStackTrace(0).Error("unexpected error", err)
6975
//
@@ -88,7 +94,7 @@
8894
//
8995
// To create a custom adapter, implement the [logger.Adapter] interface.
9096
//
91-
// # Customization with Mappers
97+
// # Customization with Mappers and Formatters
9298
//
9399
// Though the logger supports concepts of logger names, errors logging and stack
94100
// traces, etc. - different logging backends may have different conventions for
@@ -105,6 +111,18 @@
105111
// return fields.F("component", name) // Use "component" instead of "logger-name"
106112
// }))
107113
//
114+
// Name Formatter: Controls how logger names are combined when WithName is called
115+
// multiple times. By default, uses NameFormatterHierarchical which joins names
116+
// with ":" separator. Use NameFormatterReplaced to replace names instead:
117+
//
118+
// // Hierarchical naming (default): "service:handler:method"
119+
// lgr := logger.New(adapter)
120+
// lgr.WithName("service").WithName("handler").WithName("method")
121+
//
122+
// // Replacement naming: only "method"
123+
// lgr := logger.New(adapter, logger.WithNameFormatter(logger.NameFormatterReplaced))
124+
// lgr.WithName("service").WithName("handler").WithName("method")
125+
//
108126
// Error Mapper: Converts errors to fields. By default, creates a field with
109127
// key "error" and calls err.Error() to get the string representation. The
110128
// default mapper includes panic recovery for improperly implemented error types:
@@ -120,7 +138,8 @@
120138
// return fields.F("stack", st.String()) // Use "stack" instead of "stacktrace"
121139
// }))
122140
//
123-
// All three mappers can be customized independently during logger creation.
141+
// All mappers and formatters can be customized independently during logger
142+
// creation.
124143
//
125144
// # No-Op Logger
126145
//

‎logger/logger.go‎

Lines changed: 80 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,39 @@ func WithNameMapper(fn func(name string) fields.Field) Option {
6464
})
6565
}
6666

67+
// NameFormatterHierarchical is the default formatter for combining new logger name
68+
// with existing, using `:` separator (e.g., "parent:child").
69+
func NameFormatterHierarchical(prev, next string) string {
70+
const sep = ":"
71+
72+
if prev == "" {
73+
return next
74+
}
75+
76+
return prev + sep + next
77+
}
78+
79+
// NameFormatterReplaced is a name formatter that always replaces the previous
80+
// name with the new name.
81+
func NameFormatterReplaced(_, next string) string {
82+
return next
83+
}
84+
85+
// WithNameFormatter sets a custom name formatter for the logger.
86+
//
87+
// The name formatter controls how logger names are combined when
88+
// [Logger.WithName] is called multiple times on a logger chain. By default,
89+
// [NameFormatterHierarchical] is used.
90+
//
91+
// Custom formatters can implement hierarchical naming (e.g., "parent:child") or
92+
// other naming strategies. The formatter receives the previous name (empty
93+
// string if no name was set) and the next name, and returns the final name.
94+
func WithNameFormatter(fn func(prev, next string) string) Option {
95+
return func(l *Logger) {
96+
l.nameFormatter = fn
97+
}
98+
}
99+
67100
// DefaultErrorMapper is the default mapper for converting errors to fields.
68101
// It creates a field with key "error" and the error message string as value.
69102
//
@@ -167,6 +200,9 @@ type Logger struct {
167200
// in opposition to logger itself, mappers are used by-pointer since it never
168201
// changes and there is no need to copy it on every method call.
169202
mappers *mappers
203+
204+
name string
205+
nameFormatter func(prev, next string) string
170206
}
171207

172208
// New creates new [Logger] with maximum log-level set to LevelInfo and default
@@ -179,9 +215,11 @@ func New(adapter Adapter, opts ...Option) Logger {
179215
}
180216

181217
l := Logger{
182-
maxLevel: LevelInfo,
183-
adapter: adapter,
184-
mappers: defaultMappers(),
218+
maxLevel: LevelInfo,
219+
adapter: adapter,
220+
mappers: defaultMappers(),
221+
name: "",
222+
nameFormatter: NameFormatterHierarchical,
185223
}
186224

187225
lp := &l
@@ -196,9 +234,11 @@ func New(adapter Adapter, opts ...Option) Logger {
196234
// will also create no-op loggers.
197235
func NewNop() Logger {
198236
return Logger{
199-
maxLevel: math.MaxInt,
200-
adapter: nil,
201-
mappers: nil,
237+
maxLevel: math.MaxInt,
238+
adapter: nil,
239+
mappers: nil,
240+
name: "",
241+
nameFormatter: nil,
202242
}
203243
}
204244

@@ -304,6 +344,10 @@ func (l Logger) Log(level int, msg string, err error, fs ...fields.Field) {
304344
return
305345
}
306346

347+
if l.name != "" {
348+
fs = append(fs, l.mappers.name(l.name))
349+
}
350+
307351
if err != nil {
308352
fs = append(fs, l.mappers.error(err))
309353
}
@@ -356,14 +400,19 @@ func (l Logger) WithStackTrace(skip int) Logger {
356400
// from this logger. This is useful for identifying which component or module
357401
// generated a log entry.
358402
//
403+
// When called multiple times on a logger chain, the name formatter (set via
404+
// [WithNameFormatter]) determines how names are combined. By default, names
405+
// are concatenated with a `:` separator (e.g., "parent:child").
406+
//
359407
// The parent logger remains unaffected. For no-op loggers, this method returns
360408
// the same no-op logger.
361409
func (l Logger) WithName(name string) Logger {
362410
if l.IsNop() {
363411
return l
364412
}
365413

366-
l.adapter = l.adapter.WithFields(l.mappers.name(name))
414+
//revive:disable-next-line:modifies-value-receiver
415+
l.name = l.nameFormatter(l.name, name)
367416

368417
return l
369418
}
@@ -389,6 +438,30 @@ func (l Logger) IsZero() bool {
389438
return l.adapter == nil && l.maxLevel == 0 && l.mappers == nil
390439
}
391440

441+
// formatterIsEqual checks if f1 is equal to f2.
442+
func formatterIsEqual(f1, f2 func(string, string) string) bool {
443+
if f1 == nil && f2 == nil {
444+
return true
445+
}
446+
447+
if f1 == nil || f2 == nil {
448+
return false
449+
}
450+
451+
return reflect.ValueOf(f1).Pointer() == reflect.ValueOf(f2).Pointer()
452+
}
453+
454+
// IsEqual returns true if two loggers are functionally equal. Two loggers are
455+
// considered equal if they have the same maxLevel, adapter, mappers, name, and
456+
// nameFormatter.
457+
func IsEqual(l1, l2 Logger) bool {
458+
return l1.maxLevel == l2.maxLevel &&
459+
l1.adapter == l2.adapter &&
460+
l1.name == l2.name &&
461+
l1.mappers == l2.mappers &&
462+
formatterIsEqual(l1.nameFormatter, l2.nameFormatter)
463+
}
464+
392465
// NewErrorLogger creates a new [e.ErrorLogger] that logs errors with the
393466
// given log-level.
394467
//

‎logger/logger_test.go‎

Lines changed: 121 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,22 @@ func TestMappers(t *testing.T) {
8484
assert.NotZero(t, entry2.Fields.ToDict()["stacktrace"], "should have stacktrace field")
8585
}
8686

87+
func TestNameFormatterHierarchical(t *testing.T) {
88+
t.Parallel()
89+
90+
assert.Empty(t, logger.NameFormatterHierarchical("", ""))
91+
assert.Equal(t, "prev:", logger.NameFormatterHierarchical("prev", ""))
92+
assert.Equal(t, "next", logger.NameFormatterHierarchical("", "next"))
93+
assert.Equal(t, "prev:next", logger.NameFormatterHierarchical("prev", "next"))
94+
assert.Equal(t, "prev:next:other", logger.NameFormatterHierarchical("prev:next", "other"))
95+
}
96+
97+
func TestNameFormatterReplaced(t *testing.T) {
98+
t.Parallel()
99+
100+
assert.Equal(t, "next", logger.NameFormatterReplaced("prev", "next"))
101+
}
102+
87103
func TestNopLogger(t *testing.T) {
88104
t.Parallel()
89105

@@ -417,18 +433,16 @@ func TestLogger(t *testing.T) {
417433
assert.Contains(t, adapter.shared.logCalls[0].fields, fields.F("key2", 42))
418434
})
419435

420-
t.Run("with_name_calls_adapter_with_name_field", func(t *testing.T) {
436+
t.Run("with_name_prepends_name_field_to_logs", func(t *testing.T) {
421437
t.Parallel()
422438

423439
adapter := newTrackingAdapter()
424440
lgr := logger.New(adapter)
425441

426-
childLgr := lgr.WithName("test-logger")
442+
childLgr := lgr.WithFields(fields.F("foo", "bar")).WithName("test-logger")
427443
childLgr.Info("test message")
428444

429-
require.Len(t, adapter.shared.withFieldsCalls, 1, "should have 1 WithFields call for name")
430-
assert.Contains(t, adapter.shared.withFieldsCalls[0].fields, fields.F("logger-name", "test-logger"))
431-
445+
require.Len(t, adapter.shared.withFieldsCalls, 1)
432446
require.Len(t, adapter.shared.logCalls, 1)
433447
assert.Contains(t, adapter.shared.logCalls[0].fields, fields.F("logger-name", "test-logger"))
434448
})
@@ -476,10 +490,10 @@ func TestLogger(t *testing.T) {
476490

477491
childLgr.Info("test message", fields.F("key3", "value3"))
478492

479-
require.Len(t, adapter.shared.withFieldsCalls, 3, "should have 3 WithFields calls")
493+
require.Len(t, adapter.shared.withFieldsCalls, 2, "should have 2 WithFields calls")
480494

481495
require.Len(t, adapter.shared.logCalls, 1)
482-
// All fields should be accumulated
496+
// All fields should be accumulated including the logger name
483497
assert.Contains(t, adapter.shared.logCalls[0].fields, fields.F("key1", "value1"))
484498
assert.Contains(t, adapter.shared.logCalls[0].fields, fields.F("logger-name", "test-logger"))
485499
assert.Contains(t, adapter.shared.logCalls[0].fields, fields.F("key2", "value2"))
@@ -526,3 +540,103 @@ func TestLogger(t *testing.T) {
526540
assert.Contains(t, adapter.shared.logCalls[1].fields, fields.F("another", "field"))
527541
})
528542
}
543+
544+
func TestLogger_IsEqual(t *testing.T) {
545+
t.Parallel()
546+
547+
adapter1, _ := bufferadapter.New()
548+
adapter2, _ := bufferadapter.New()
549+
550+
parent := logger.New(adapter1)
551+
lgr1 := logger.New(adapter1)
552+
553+
nameFormatter := func(prev, next string) string {
554+
if prev == "" {
555+
return next
556+
}
557+
return prev + "/" + next
558+
}
559+
560+
tests := []struct {
561+
name string
562+
l1 logger.Logger
563+
l2 logger.Logger
564+
expected bool
565+
}{
566+
{
567+
name: "same_logger_is_equal",
568+
l1: lgr1,
569+
l2: lgr1,
570+
expected: true,
571+
},
572+
{
573+
name: "different_maxLevel",
574+
l1: logger.New(adapter1, logger.WithLevel(logger.LevelInfo)),
575+
l2: logger.New(adapter1, logger.WithLevel(logger.LevelDebug)),
576+
expected: false,
577+
},
578+
{
579+
name: "different_adapter",
580+
l1: logger.New(adapter1),
581+
l2: logger.New(adapter2),
582+
expected: false,
583+
},
584+
{
585+
name: "different_name",
586+
l1: logger.New(adapter1).WithName("logger1"),
587+
l2: logger.New(adapter1).WithName("logger2"),
588+
expected: false,
589+
},
590+
{
591+
name: "same_name_from_parent",
592+
l1: parent.WithName("test-logger"),
593+
l2: parent.WithName("test-logger"),
594+
expected: true,
595+
},
596+
{
597+
name: "child_equal_to_itself",
598+
l1: parent.WithName("test-logger"),
599+
l2: parent.WithName("test-logger"),
600+
expected: true,
601+
},
602+
{
603+
name: "different_mappers_not_equal",
604+
l1: logger.New(adapter1),
605+
l2: logger.New(adapter1),
606+
expected: false,
607+
},
608+
{
609+
name: "children_different_names_not_equal",
610+
l1: parent.WithName("child1"),
611+
l2: parent.WithName("child2"),
612+
expected: false,
613+
},
614+
{
615+
name: "children_different_formatters_not_equal",
616+
l1: logger.New(adapter1).WithName("child"),
617+
l2: logger.New(adapter1, logger.WithNameFormatter(nameFormatter)).WithName("child"),
618+
expected: false,
619+
},
620+
{
621+
name: "nop_loggers_are_equal",
622+
l1: logger.NewNop(),
623+
l2: logger.NewNop(),
624+
expected: true,
625+
},
626+
{
627+
name: "child_loggers_with_fields_different_adapters",
628+
l1: parent.WithFields(fields.F("key", "value")),
629+
l2: parent.WithFields(fields.F("key", "value")),
630+
expected: false,
631+
},
632+
}
633+
634+
for _, tt := range tests {
635+
t.Run(tt.name, func(t *testing.T) {
636+
t.Parallel()
637+
638+
result := logger.IsEqual(tt.l1, tt.l2)
639+
assert.Equal(t, tt.expected, result)
640+
})
641+
}
642+
}

0 commit comments

Comments
 (0)