Skip to content

Commit 8d144c2

Browse files
authored
Create way to add key-value pairs to new sinks (#4582)
Counterintuitively, a new sink added to an existing logger will not inherit the existing logger's key-value pairs. This has only caused a problem in practice with respect to a small number of known key-value pairs, so this PR creates a way to add specific key-value pairs to new sinks. This supersedes #4579.
1 parent 2d008a5 commit 8d144c2

File tree

2 files changed

+43
-4
lines changed

2 files changed

+43
-4
lines changed

pkg/log/log.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,34 @@ func AddSentry(l logr.Logger, opts sentry.ClientOptions, tags map[string]string)
136136
return AddSink(l, WithSentry(opts, tags))
137137
}
138138

139-
// AddSink extends an existing logr.Logger with a new sink. It returns the new
140-
// logr.Logger, a cleanup function, and an error.
141-
func AddSink(l logr.Logger, sink logConfig) (logr.Logger, func() error, error) {
139+
// AddSink extends an existing logr.Logger with a new sink. It returns the new logr.Logger, a cleanup function, and an
140+
// error.
141+
//
142+
// The new sink will not inherit any of the existing logger's key-value pairs. Key-value pairs can be added to the new
143+
// sink specifically by passing them to this function.
144+
func AddSink(l logr.Logger, sink logConfig, keysAndValues ...any) (logr.Logger, func() error, error) {
142145
if sink.err != nil {
143146
return l, nil, sink.err
144147
}
148+
149+
// New key-value pairs cannot be ergonomically added directly to cores. logr has code to do it, but that code is not
150+
// exported. Rather than replicating it ourselves, we indirectly use it by creating a temporary logger for the new
151+
// core, adding the key-value pairs to the temporary logger, and then extracting the temporary logger's modified
152+
// core.
153+
newSinkLogger := zapr.NewLogger(zap.New(sink.core))
154+
newSinkLogger = newSinkLogger.WithValues(keysAndValues...)
155+
newCoreLogger, err := getZapLogger(newSinkLogger)
156+
if err != nil {
157+
return l, nil, fmt.Errorf("error setting up new key-value pairs: %w", err)
158+
}
159+
newSinkCore := newCoreLogger.Core()
160+
145161
zapLogger, err := getZapLogger(l)
146162
if err != nil {
147163
return l, nil, errors.New("unsupported logr implementation")
148164
}
149165
zapLogger = zapLogger.WithOptions(zap.WrapCore(func(core zapcore.Core) zapcore.Core {
150-
return zapcore.NewTee(core, sink.core)
166+
return zapcore.NewTee(core, newSinkCore)
151167
}))
152168
return zapr.NewLogger(zapLogger), firstErrorFunc(zapLogger.Sync, sink.cleanup), nil
153169
}

pkg/log/log_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,29 @@ func TestAddSink(t *testing.T) {
163163
assert.Contains(t, buf2.String(), "line 2")
164164
}
165165

166+
func TestAddSink_WithKeyValuePairs(t *testing.T) {
167+
// Arrange: Create a logger with a key-value pair
168+
var buf1 bytes.Buffer
169+
logger, cleanup := New("service-name", WithConsoleSink(&buf1))
170+
t.Cleanup(func() { _ = cleanup })
171+
logger = logger.WithValues("sink 1 key", "sink 1 value")
172+
173+
// Arrange: Add a second sink with a new key-value pair
174+
var buf2 bytes.Buffer
175+
logger, flush, err := AddSink(logger, WithConsoleSink(&buf2), "sink 2 key", "sink 2 value")
176+
require.NoError(t, err)
177+
178+
// Act
179+
logger.Info("something")
180+
require.NoError(t, flush())
181+
182+
// Assert: Confirm that each sink received only its own key-value pair
183+
assert.Contains(t, buf1.String(), "sink 1")
184+
assert.NotContains(t, buf1.String(), "sink 2")
185+
assert.Contains(t, buf2.String(), "sink 2")
186+
assert.NotContains(t, buf2.String(), "sink 1")
187+
}
188+
166189
func TestStaticLevelSink(t *testing.T) {
167190
var buf1, buf2 bytes.Buffer
168191
l1 := zap.NewAtomicLevel()

0 commit comments

Comments
 (0)