Skip to content

operator: Replace zap logger with global ContextFree logger#1436

Closed
RafalKorepta wants to merge 1 commit intomainfrom
rk/fix-logger
Closed

operator: Replace zap logger with global ContextFree logger#1436
RafalKorepta wants to merge 1 commit intomainfrom
rk/fix-logger

Conversation

@RafalKorepta
Copy link
Copy Markdown
Contributor

The multicluster command constructed its own ctrlzap.New() logger and passed it to RaftConfiguration.Logger, which forwarded it to the controller-runtime manager via ctrl.Options.Logger.

Because this logger had a non-nil sink, controller-runtime skipped the global logger fallback:

if options.Logger.GetSink() == nil {
    options.Logger = log.Log  // <-- never reached
}

The manager then used this raw zap sink to build per-reconciliation loggers in reconcileHandler:

log := c.LogConstructor(&req)
ctx = logf.IntoContext(ctx, log)

When reconciler code called otelutil/log.FromContext(ctx), which appends "ctx", ctx to keysAndValues, the zap sink serialized context.Context as the full wrapper chain:

"ctx":"context.Background.WithCancel.WithCancel.WithValue(logr...)"

The global logger set by main.go via log.SetGlobals(logger.NewLogger()) wraps non-OTEL sinks with ContextFree, which strips context.Context values from keysAndValues before they reach the underlying sink. But the per-command zap logger bypassed SetGlobals entirely, so it never got the ContextFree wrapper.

Fix by replacing the standalone ctrlzap.New() with log.FromContext(ctx), which retrieves the global logger already wrapped with ContextFree. Remove the now-unused --log-level flag and parseLogLevel helper, as log level is already configured globally via the root command's logOptions.

Copy link
Copy Markdown
Contributor

@andrewstucki andrewstucki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the proper fix to me, probably want to name the logger based on the subsystem rather than the node though?

The multicluster command constructed its own ctrlzap.New() logger and
passed it to RaftConfiguration.Logger, which forwarded it to the
controller-runtime manager via ctrl.Options.Logger.

Because this logger had a non-nil sink, controller-runtime skipped the
global logger fallback:

    if options.Logger.GetSink() == nil {
        options.Logger = log.Log  // <-- never reached
    }

The manager then used this raw zap sink to build per-reconciliation
loggers in reconcileHandler:

    log := c.LogConstructor(&req)
    ctx = logf.IntoContext(ctx, log)

When reconciler code called otelutil/log.FromContext(ctx), which appends
"ctx", ctx to keysAndValues, the zap sink serialized context.Context as
the full wrapper chain:

    "ctx":"context.Background.WithCancel.WithCancel.WithValue(logr...)"

The global logger set by main.go via log.SetGlobals(logger.NewLogger())
wraps non-OTEL sinks with ContextFree, which strips context.Context
values from keysAndValues before they reach the underlying sink. But the
per-command zap logger bypassed SetGlobals entirely, so it never got the
ContextFree wrapper.

Fix by replacing the standalone ctrlzap.New() with log.FromContext(ctx),
which retrieves the global logger already wrapped with ContextFree.
Remove the now-unused --log-level flag and parseLogLevel helper, as log
level is already configured globally via the root command's logOptions.
@RafalKorepta RafalKorepta enabled auto-merge (rebase) April 9, 2026 17:23
@RafalKorepta RafalKorepta disabled auto-merge April 9, 2026 17:23
@RafalKorepta RafalKorepta enabled auto-merge (rebase) April 9, 2026 17:23
@RafalKorepta RafalKorepta disabled auto-merge April 9, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants