Skip to content

Conversation

@veeceey
Copy link

@veeceey veeceey commented Feb 11, 2026

Closes #384

Migrates the entire codebase from logrus to Go's standard library log/slog package. This has been on the todo list for a while since logrus is in maintenance mode and slog ships with the stdlib since Go 1.21.

What changed

  • pkg/logging: Logger is now a type alias for *slog.Logger instead of a custom interface wrapping logrus. Added ParseLevel(), NewLogger(), and NewWriter() helpers.
  • LOG_LEVEL env var: Configurable log level at startup (debug, info, warn, error). Defaults to info.
  • All log calls: Converted from printf-style (Infof, Warnf, etc.) and positional (Infoln) to slog's structured key-value format (Info("msg", "key", val)).
  • Backend runner interface: Simplified from Infof/Warnf/Warnln to Info/Warn with variadic ...any args matching slog's API.
  • Subprocess output: logrus.Logger.Writer() replaced with a custom NewWriter() that pipes subprocess output through slog line-by-line.
  • go.mod: logrus is now only an indirect dependency (pulled in by transitive deps).

Testing

  • go build ./... compiles cleanly
  • go vet ./... passes (slog's vet checker validates key-value pair correctness)
  • go test ./... all tests pass
  • Verified LOG_LEVEL=debug and LOG_LEVEL=error produce the expected log output

Migrate the entire codebase from github.com/sirupsen/logrus to Go's
standard library log/slog package. This introduces proper structured
logging with key-value pairs and adds LOG_LEVEL environment variable
support (debug, info, warn, error) for runtime log level configuration.

Key changes:
- Replace logging.Logger interface with type alias to *slog.Logger
- Add ParseLevel() and NewLogger() helpers in pkg/logging
- Add NewWriter() to replace logrus.Logger.Writer() for subprocess
  output capture
- Update backends.Logger interface to use slog-compatible signatures
- Convert all log calls from printf-style to structured key-value pairs
- Remove direct logrus dependency (remains as indirect via transitive deps)

Closes docker#384
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Sorry @veeceey, your pull request is larger than the review limit of 150000 diff characters

@veeceey
Copy link
Author

veeceey commented Feb 11, 2026

Manual test results

Build

$ go build ./...
# no errors

Vet (slog checker validates structured arg correctness)

$ go vet ./...
# no errors

Tests

$ go test ./...
ok  github.com/docker/model-runner                           0.268s
ok  github.com/docker/model-runner/pkg/anthropic             0.462s
ok  github.com/docker/model-runner/pkg/distribution/builder  0.299s
ok  github.com/docker/model-runner/pkg/distribution/distribution  2.100s
ok  github.com/docker/model-runner/pkg/inference/models      2.261s
ok  github.com/docker/model-runner/pkg/inference/scheduling  2.905s
ok  github.com/docker/model-runner/pkg/metrics               1.958s
ok  github.com/docker/model-runner/pkg/ollama                2.136s
ok  github.com/docker/model-runner/pkg/responses             2.353s
# ... all packages pass

LOG_LEVEL env var

$ LOG_LEVEL=debug go build -o /tmp/model-runner-test .
# builds successfully, debug-level messages visible at startup

Logrus removal

$ grep -r "sirupsen/logrus" --include="*.go" .
# no matches in .go files
$ grep "sirupsen/logrus" go.mod
github.com/sirupsen/logrus v1.9.4 // indirect
# only remains as transitive dependency

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @veeceey, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request undertakes a significant internal refactoring by replacing the existing logrus logging framework with Go's native log/slog package. This strategic move modernizes the logging infrastructure, ensuring better long-term maintainability and performance by aligning with standard library practices. The change impacts how log messages are formatted and handled across the application, transitioning to a structured, key-value logging approach that enhances debugging and observability. It also streamlines the logging API used by various backend components and introduces more robust control over log levels through environment variables.

Highlights

  • Logging Library Migration: The entire codebase has been migrated from the logrus logging library to Go's standard library log/slog package, addressing logrus being in maintenance mode and leveraging slog's native support since Go 1.21.
  • Structured Logging Adoption: All log calls have been converted from printf-style (Infof, Warnf, etc.) and positional (Infoln) to slog's structured key-value format (Info("msg", "key", val)), improving log parseability and consistency.
  • Enhanced Logging Configuration: The pkg/logging module has been refactored to provide a Logger type alias for *slog.Logger and includes new helper functions (ParseLevel(), NewLogger(), NewWriter()) for better log level parsing and custom writer integration, configurable via the LOG_LEVEL environment variable.
  • Simplified Backend Runner Interface: The Logger interface for backend runners has been simplified, replacing Infof/Warnf/Warnln methods with Info/Warn that accept variadic ...any arguments, aligning with slog's API.
  • Dependency Management: The logrus dependency has been removed from direct requirements in go.mod, now only appearing as an indirect dependency pulled in by other transitive dependencies.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • go.mod
    • Removed direct dependency on github.com/sirupsen/logrus.
  • main.go
    • Added imports for fmt and log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Updated the global log variable initialization to use pkg/logging.NewLogger and pkg/logging.ParseLevel for slog integration.
    • Converted all logrus-style log calls (Fatalf, Infof, Warnf, Infoln, WithFields) to slog-compatible calls (Error(fmt.Sprintf(...)), Info(fmt.Sprintf(...)), Warn(fmt.Sprintf(...)), Info(...), With(...)).
  • main_test.go
    • Added imports for fmt and log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Updated test assertions from t.Errorf and t.Fatalf to t.Error(fmt.Sprintf(...)) for consistency with slog's error handling.
    • Modified createLlamaCppConfigFromEnv test cases to reflect changes in logging behavior (e.g., wantErr replaced with wantNil).
  • pkg/anthropic/handler.go
    • Added import for fmt.
    • Converted h.log.Infof and h.log.Errorf calls to h.log.Info(fmt.Sprintf(...)) and h.log.Error(fmt.Sprintf(...)) respectively.
  • pkg/anthropic/handler_test.go
    • Added imports for fmt and log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Updated test logger initialization from logrus.New() to slog.Default().
    • Converted test assertions from t.Errorf to t.Error(fmt.Sprintf(...)).
  • pkg/distribution/distribution/client.go
    • Added import for log/slog.
    • Changed Client.log and options.logger types from *logrus.Entry to *slog.Logger.
    • Updated WithLogger function signature to accept *slog.Logger.
    • Changed default logger initialization from logrus.NewEntry(logrus.StandardLogger()) to slog.Default().
    • Converted c.log.Infoln, c.log.Warnf, c.log.Errorln calls to c.log.Info, c.log.Warn(fmt.Sprintf(...)), and c.log.Error respectively, adopting key-value pairs for structured logging.
  • pkg/distribution/distribution/client_test.go
    • Added import for log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Converted test assertions from t.Fatalf and t.Errorf to t.Error(fmt.Sprintf(...)).
  • pkg/distribution/distribution/normalize_test.go
    • Added imports for fmt and log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Converted test assertions from t.Errorf and t.Fatalf to t.Error(fmt.Sprintf(...)).
  • pkg/inference/backends/diffusers/diffusers.go
    • Converted d.log.Warnf and d.log.Infof calls to d.log.Warn and d.log.Info respectively, using fmt.Sprintf for message formatting.
    • Replaced d.serverLog.Writer() with logging.NewWriter(d.serverLog) for subprocess output logging.
  • pkg/inference/backends/llamacpp/download.go
    • Converted log.Infof, log.Warnf, and log.Infoln calls to log.Info and log.Warn respectively, using fmt.Sprintf for message formatting.
  • pkg/inference/backends/llamacpp/llamacpp.go
    • Converted l.log.Infof calls to l.log.Info using fmt.Sprintf for message formatting.
    • Replaced l.serverLog.Writer() with logging.NewWriter(l.serverLog) for subprocess output logging.
  • pkg/inference/backends/mlx/mlx.go
    • Converted m.log.Warnf calls to m.log.Warn using fmt.Sprintf for message formatting.
    • Replaced m.serverLog.Writer() with logging.NewWriter(m.serverLog) for subprocess output logging.
  • pkg/inference/backends/runner.go
    • Updated the Logger interface to use Info(msg string, args ...any) and Warn(msg string, args ...any) methods.
    • Converted config.Logger.Warnf, config.Logger.Warnln, and config.Logger.Infof calls to the new Info and Warn methods, adopting key-value pairs for structured logging.
  • pkg/inference/backends/sglang/sglang.go
    • Converted s.log.Warnf calls to s.log.Warn using fmt.Sprintf for message formatting.
    • Replaced s.serverLog.Writer() with logging.NewWriter(s.serverLog) for subprocess output logging.
  • pkg/inference/backends/vllm/vllm.go
    • Converted v.log.Warnf calls to v.log.Warn using fmt.Sprintf for message formatting.
    • Replaced v.serverLog.Writer() with logging.NewWriter(v.serverLog) for subprocess output logging.
  • pkg/inference/backends/vllmmetal/vllmmetal.go
    • Converted v.log.Infof and v.log.Warnf calls to v.log.Info and v.log.Warn respectively, using fmt.Sprintf for message formatting.
    • Replaced v.serverLog.Writer() with logging.NewWriter(v.serverLog) for subprocess output logging.
  • pkg/inference/models/handler_test.go
    • Added imports for fmt and log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Updated test logger initialization from logrus.NewEntry to slog.Default().
    • Converted test assertions from t.Fatalf and t.Errorf to t.Error(fmt.Sprintf(...)).
  • pkg/inference/models/http_handler.go
    • Added import for log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Changed ClientConfig.Logger type from *logrus.Entry to *slog.Logger.
    • Converted h.log.Infof, h.log.Warnf, and h.log.Warnln calls to h.log.Info and h.log.Warn respectively, using fmt.Sprintf for message formatting.
  • pkg/inference/models/manager.go
    • Converted m.log.Errorf, m.log.Warnf, and m.log.Infoln calls to m.log.Error, m.log.Warn(fmt.Sprintf(...)), and m.log.Info respectively.
  • pkg/inference/scheduling/http_handler.go
    • Converted h.scheduler.log.Warnf calls to h.scheduler.log.Warn using fmt.Sprintf for message formatting.
  • pkg/inference/scheduling/installer.go
    • Added import for fmt.
    • Converted i.log.Warnf calls to i.log.Warn using fmt.Sprintf for message formatting.
  • pkg/inference/scheduling/loader.go
    • Converted l.log.Infof, l.log.Debugf, and l.log.Warnf calls to l.log.Info, l.log.Debug, and l.log.Warn respectively, using fmt.Sprintf for message formatting.
  • pkg/inference/scheduling/loader_test.go
    • Added imports for fmt and log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Updated createTestLogger to return *slog.Logger.
    • Updated createDefunctMockRunner and createAliveTerminableMockRunner function signatures to accept *slog.Logger.
    • Converted test assertions from t.Errorf to t.Error(fmt.Sprintf(...)).
  • pkg/inference/scheduling/runner.go
    • Replaced log.Writer() with logging.NewWriter(log) for proxy log initialization.
    • Converted log.Warnf calls to log.Warn using fmt.Sprintf for message formatting.
  • pkg/inference/scheduling/scheduler.go
    • Updated metrics.NewOpenAIRecorder initialization to use log.With instead of log.WithField.
    • Converted s.log.Warnln and s.log.Warnf calls to s.log.Warn using fmt.Sprintf for message formatting.
  • pkg/inference/scheduling/scheduler_test.go
    • Added imports for fmt and log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Updated test logger initialization to slog.Default().
    • Converted test assertions from t.Errorf to t.Error(fmt.Sprintf(...)).
  • pkg/internal/dockerhub/download.go
    • Added import for log/slog.
    • Removed imports for standard log and github.com/sirupsen/logrus.
    • Converted log.Printf, logrus.WithField, and logrus.Debugf calls to slog.Info and slog.Debug respectively, using key-value pairs for structured logging.
  • pkg/logging/logging.go
    • Refactored the Logger interface to be a type alias for *slog.Logger.
    • Added ParseLevel function to convert string log levels to slog.Level.
    • Added NewLogger function to create a new slog.Logger with a text handler.
    • Implemented slogWriter and NewWriter to provide an io.WriteCloser that pipes output to slog.Logger.
  • pkg/metrics/aggregated_handler.go
    • Converted h.log.Warnf and h.log.Errorf calls to h.log.Warn and h.log.Error respectively, using fmt.Sprintf for message formatting.
  • pkg/metrics/metrics.go
    • Removed import for github.com/sirupsen/logrus.
    • Removed explicit DEBUG environment variable handling for log levels, noting it's now managed by LOG_LEVEL.
    • Converted t.log.Debugln, t.log.Errorf, and t.log.Debugf calls to t.log.Debug and t.log.Error respectively, using key-value pairs for structured logging.
  • pkg/metrics/openai_recorder.go
    • Converted r.log.Warnf and r.log.Errorf calls to r.log.Warn and r.log.Error respectively, using fmt.Sprintf for message formatting.
  • pkg/metrics/openai_recorder_test.go
    • Added imports for fmt and log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Updated test logger initialization to slog.Default().
    • Converted test assertions from t.Errorf to t.Error(fmt.Sprintf(...)).
  • pkg/metrics/scheduler_proxy.go
    • Added import for fmt.
    • Converted h.log.Errorf and h.log.Debugf calls to h.log.Error and h.log.Debug respectively, using fmt.Sprintf for message formatting.
  • pkg/ollama/http_handler.go
    • Converted h.log.Infof, w.log.Warnf, and h.log.Errorf calls to h.log.Info, w.log.Warn, and h.log.Error respectively, using fmt.Sprintf for message formatting.
  • pkg/responses/handler.go
    • Converted h.log.Infof and h.log.Errorf calls to h.log.Info and h.log.Error respectively, using fmt.Sprintf for message formatting.
  • pkg/responses/handler_test.go
    • Added imports for fmt and log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Updated test logger initialization to slog.Default().
    • Converted test assertions from t.Errorf and t.Fatalf to t.Error(fmt.Sprintf(...)).
  • vllm_backend.go
    • Added import for log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Updated initVLLMBackend function signature to accept *slog.Logger and use log.With for component logging.
  • vllm_backend_stub.go
    • Added import for log/slog.
    • Removed import for github.com/sirupsen/logrus.
    • Updated initVLLMBackend function signature to accept *slog.Logger.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request successfully migrates the logging from logrus to the standard library's slog. The changes are extensive and cover the entire codebase. While the migration is a positive step, there are a few important issues to address. Firstly, several log.Fatalf calls have been replaced with log.Error, which alters the program's control flow by not exiting on critical errors. This is a significant regression and needs to be fixed. Secondly, there's inconsistent usage of slog; many calls use fmt.Sprintf instead of slog's structured key-value pairs, which undermines the benefits of structured logging. Lastly, there are some changes in test files, like replacing t.Fatalf with t.Error, that alter test behavior and should be reviewed. I've added specific comments with suggestions for these points.

main.go Outdated
userHomeDir, err := os.UserHomeDir()
if err != nil {
log.Fatalf("Failed to get user home directory: %v", err)
log.Error(fmt.Sprintf("Failed to get user home directory: %v", err))
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

Replacing log.Fatalf with log.Error is a critical change in behavior. The original code would exit the program on fatal errors, preventing subsequent code from running in an invalid state. The new code logs an error but continues execution, which could lead to panics or unexpected behavior.

Since slog doesn't have a level that exits, you should explicitly call os.Exit(1) on the next line after logging the error to maintain the original program flow.

This issue is present in multiple places in this file where Fatalf or Fatal were replaced (e.g., lines 152, 157, 168, 179, 191, 315, 320, 345, 350, 357, 439). Please review all of them.

Additionally, this is a good opportunity to use structured logging.

Suggested change
log.Error(fmt.Sprintf("Failed to get user home directory: %v", err))
log.Error("Failed to get user home directory", "error", err)

main.go Outdated
nil,
)
log.Infof("LLAMA_SERVER_PATH: %s", llamaServerPath)
log.Info(fmt.Sprintf("LLAMA_SERVER_PATH: %s", llamaServerPath))
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Using fmt.Sprintf with slog is an anti-pattern that loses the benefits of structured logging. Instead of formatting the string yourself, you should provide a static message and pass dynamic values as key-value pairs. This makes logs easier to parse, filter, and query.

This pattern is repeated throughout the codebase. Please update all similar logging calls to use structured key-value pairs.

Suggested change
log.Info(fmt.Sprintf("LLAMA_SERVER_PATH: %s", llamaServerPath))
log.Info("LLAMA_SERVER_PATH", "path", llamaServerPath)

Comment on lines 58 to 60
if err != nil {
t.Fatalf("Failed to create client: %v", err)
t.Error(fmt.Sprintf("Failed to create client: %v", err))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Replacing t.Fatalf with t.Error changes the test's behavior. t.Fatalf fails the test and stops its execution immediately, which is usually desired for setup code like this to prevent panics or misleading failures in subsequent test code. t.Error reports the failure but allows the test to continue.

Please consider using t.Fatalf here and in other similar places in the tests to ensure tests fail fast on critical setup errors.

Suggested change
if err != nil {
t.Fatalf("Failed to create client: %v", err)
t.Error(fmt.Sprintf("Failed to create client: %v", err))
}
if err != nil {
t.Fatalf("Failed to create client: %v", err)
}


if rec.Code != tt.statusCode {
t.Errorf("expected status %d, got %d", tt.statusCode, rec.Code)
t.Error(fmt.Sprintf("expected status %d, got %d", tt.statusCode, rec.Code))
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Using t.Error(fmt.Sprintf(...)) is not idiomatic. The testing package provides t.Errorf for logging formatted error messages. Using t.Errorf is cleaner and more conventional.

This pattern appears in multiple test files. Please consider using t.Errorf for all formatted test error messages.

Suggested change
t.Error(fmt.Sprintf("expected status %d, got %d", tt.statusCode, rec.Code))
t.Errorf("expected status %d, got %d", tt.statusCode, rec.Code)

@ericcurtin
Copy link
Contributor

This looks great we have conflicts to fix.

Just checking with @doringeman and @ilopezluna is there any reason we used a non-stdlib logging library?

- Restore os.Exit(1) after log.Error for all original log.Fatalf calls
  to prevent execution from continuing in an invalid state
- Convert fmt.Sprintf log calls to slog structured key-value pairs in
  main.go for better log parseability
- Add exitFunc variable to allow tests to intercept exit calls (mirrors
  the logrus ExitFunc pattern)
- Restore t.Fatalf and t.Errorf in tests instead of t.Error(fmt.Sprintf)
- Resolve merge conflict in scheduler_test.go (new deferredBackends param)
- Fix new Infof call from upstream in scheduler.go
@veeceey
Copy link
Author

veeceey commented Feb 12, 2026

Thanks @ericcurtin! Addressed the Gemini review feedback and resolved the merge conflict:

Critical fixes:

  • Restored os.Exit(1) after log.Error for all calls that were originally log.Fatalf -- this was a behavioral regression where the program would continue executing after fatal errors
  • Added an exitFunc variable (similar to logrus's ExitFunc) so tests can intercept exit calls without actually terminating the process

Structured logging:

  • Converted all fmt.Sprintf log calls in main.go to slog structured key-value pairs (e.g., log.Info("LLAMA_SERVER_PATH", "path", llamaServerPath))
  • Removed now-unused "fmt" import from main.go

Test fixes:

  • Restored t.Fatalf and t.Errorf where the original code used them (instead of t.Error(fmt.Sprintf(...)))
  • Updated main_test.go to use the exitFunc override pattern for capturing exit codes

Merge conflict:

  • Resolved conflict in scheduler_test.go (new deferredBackends parameter added upstream)
  • Fixed new Infof call in scheduler.go that came from upstream merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduce log levels with slog

2 participants