operator: Replace zap logger with global ContextFree logger#1436
Closed
RafalKorepta wants to merge 1 commit intomainfrom
Closed
operator: Replace zap logger with global ContextFree logger#1436RafalKorepta wants to merge 1 commit intomainfrom
RafalKorepta wants to merge 1 commit intomainfrom
Conversation
andrewstucki
approved these changes
Apr 9, 2026
Contributor
andrewstucki
left a comment
There was a problem hiding this comment.
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.
6174d11 to
7607623
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
The manager then used this raw zap sink to build per-reconciliation loggers in reconcileHandler:
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:
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.