Skip to content

Conversation

@devin-ai-integration
Copy link

@devin-ai-integration devin-ai-integration bot commented Sep 26, 2025

Closes: MBA-243

Summary

This PR removes the Logrus dependency (github.com/sirupsen/logrus v1.9.3) and replaces all logging with Go's standard library slog. This eliminates an external dependency while maintaining equivalent logging functionality.

Key Changes

Core Logging Migration (internal/ghmcp/server.go)

  • Replaced logrus.New() with slog.New(slog.NewTextHandler(...))
  • Migrated from formatted string logging (logrus.Infof) to structured logging (slog.Info with key-value pairs)
  • Critical fix: Ensured both slogLogger and stdLogger use the same output destination when LogFilePath is configured (initially had regression where stdLogger always wrote to stderr)

IOLogger Refactoring (pkg/log/io.go)

  • Updated IOLogger struct to accept *slog.Logger instead of *logrus.Logger
  • Converted logging calls to structured format:
    • Before: l.logger.Infof("[stdin]: received %d bytes: %s", n, string(p[:n]))
    • After: l.logger.Info("[stdin]: received bytes", "count", n, "data", string(p[:n]))

Dependency Cleanup

  • Removed github.com/sirupsen/logrus v1.9.3 from go.mod
  • Updated go.sum via go mod tidy
  • Removed Logrus license references from all platform-specific license files

Tradeoffs

  • Log format change: Output format shifts from formatted strings to structured key-value pairs, which may impact existing log parsing/monitoring
  • Dependency reduction: Eliminates external dependency in favor of standard library, improving maintainability
  • Performance: slog is generally more performant than Logrus, especially for structured logging

Testing

  • All existing tests pass with updated logger configuration
  • Build and lint checks pass
  • Verified logging works correctly with and without LogFilePath configuration

Human Review Checklist

Critical areas to verify:

  1. File logging behavior: Test that when LogFilePath is set, both slogLogger and stdLogger write to the same file (not stderr)
  2. Log output format: Confirm structured logging format is acceptable for existing monitoring/parsing
  3. Error handling: Verify file opening/writing error cases still work correctly
  4. Configuration compatibility: Test logging behavior with various EnableCommandLogging and LogFilePath combinations

Link to Devin run: https://app.devin.ai/sessions/f6e4a889c2af459ead9c3e94ebcea9e9
Requested by: @jakexcosme
JIRA: MBA-243

- Replace github.com/sirupsen/logrus with Go's standard library slog
- Update internal/ghmcp/server.go to use slog.Logger instead of logrus.Logger
- Update pkg/log/io.go to use structured logging with slog
- Update pkg/log/io_test.go to use slog for testing
- Remove logrus dependency from go.mod and run go mod tidy
- Remove Logrus license references from all platform-specific license files
- Preserve conditional logging behavior for EnableCommandLogging and LogFilePath
- All tests pass and project builds successfully

Co-Authored-By: Jake Cosme <jake@cognition.ai>
@devin-ai-integration
Copy link
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

…ath is configured

- Both stdLogger and slogLogger now use the same output destination
- When LogFilePath is set, both loggers write to the file instead of stderr
- Preserves original behavior where stdLogger used logrusLogger.Writer()
- Fixes critical regression identified in PR review

Co-Authored-By: Jake Cosme <jake@cognition.ai>
@sonarqubecloud
Copy link

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.

1 participant