Repository navigation
refactor: unify logging finalize + add check to avoid adding slog later again - #1406
Closed
SoWieMarkus wants to merge 2 commits into
Closed
SoWieMarkus wants to merge 2 commits into
SoWieMarkus wants to merge 2 commits into
Conversation
Convert the remaining log/slog usage in the scheduling stack (internal/scheduling
and api/external) to controller-runtime's logr, completing the slog removal from
the service (only tools/ retains slog/stdlib log).
Key interface changes:
- The pipeline step interface Run now takes logr.Logger instead of *slog.Logger;
the trace logger stays a threaded parameter, only its type changed.
- FilterWeigherPipelineRequest.GetTraceLogArgs() now returns []any (flat k/v
pairs for logr.WithValues) instead of []slog.Attr, removing the slog.Attr ->
[]any conversion loop in the pipeline.
- APIMonitor.Respond takes logr.Logger (value); callers pass logr.Logger{} /
logr.Discard() instead of nil, and it falls back to the package logger when
the passed logger has no sink.
Translation rules (unchanged from earlier phases):
- Info/Warn -> Info; Debug -> V(1).Info; Error(msg,"error",err) -> Error(err,msg);
a Warn carrying an error becomes Error so the error keeps its slot.
- Package-level loggers named to avoid shadowing the controllers' local
'log := ctrl.LoggerFrom(ctx)' idiom (pipelineLog in lib, schedulerLog in nova).
Tests pass logr.Discard() as the trace logger; the one buffer-capturing test
installs a zap logger via logf.SetLogger to assert on output.
Signed-off-by: Markus Wieland <markus.wieland@sap.com>
Complete the slog removal started in the previous commits: - Drop the now-dead LOG_LEVEL env from the manager helm template; logLevel renders only --zap-log-level. Guard the env block so it is omitted when no custom env is set. - Update helm values.yaml and cortex.secrets.example.yaml comments that referenced 'both zap and slog' to describe the single controller-runtime (zap) logger. - Add a forbidigo rule to .golangci.yaml forbidding any slog.* usage, with a message pointing at sigs.k8s.io/controller-runtime/pkg/log, so log/slog cannot re-enter the service. - Add ADR 003 recording the decision, translation rules, and consequences (notably the loss of the warn level, which was slog-only). Signed-off-by: Markus Wieland <markus.wieland@sap.com>
SoWieMarkus
requested review from
PhilippMatthes,
auhlig,
juliusclausnitzer,
mblos and
umswmayj
as code owners
October 21, 2026 10:13
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
1 participant
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.
No description provided.