Skip to content

Commit 3479d02

Browse files
ricochetmattdhollowayJoannaaKL
authored
fix: replace logrus with slog (#781)
- Adjust some logs to use structured outputs - Set stdioserver log prefix as const - Do not export test func removeTimeAttr Signed-off-by: Bailey Hayes <behayes2@gmail.com> Co-authored-by: Matt Holloway <mattdholloway@github.com> Co-authored-by: JoannaaKL <joannaakl@github.com>
1 parent 842003a commit 3479d02

File tree

9 files changed

+33
-54
lines changed

9 files changed

+33
-54
lines changed

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ require (
77
github.com/josephburnett/jd v1.9.2
88
github.com/mark3labs/mcp-go v0.32.0
99
github.com/migueleliasweb/go-github-mock v1.3.0
10-
github.com/sirupsen/logrus v1.9.3
1110
github.com/spf13/cobra v1.9.1
1211
github.com/spf13/viper v1.20.1
1312
github.com/stretchr/testify v1.10.0

go.sum

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,6 @@ github.com/shurcooL/githubv4 v0.0.0-20240727222349-48295856cce7 h1:cYCy18SHPKRkv
6666
github.com/shurcooL/githubv4 v0.0.0-20240727222349-48295856cce7/go.mod h1:zqMwyHmnN/eDOZOdiTohqIUKUrTFX62PNlu7IJdu0q8=
6767
github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466 h1:17JxqqJY66GmZVHkmAsGEkcIu0oCe3AM420QDgGwZx0=
6868
github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466/go.mod h1:9dIRpgIY7hVhoqfe0/FcYp0bpInZaT7dc3BYOprrIUE=
69-
github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
70-
github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
7169
github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo=
7270
github.com/sourcegraph/conc v0.3.0/go.mod h1:Sdozi7LEKbFPqYX2/J+iBAM6HpqSLTASQIKqDmF7Mt0=
7371
github.com/spf13/afero v1.14.0 h1:9tH6MapGnn/j0eb0yIXiLjERO8RB6xIVZRDCX7PtqWA=
@@ -83,7 +81,6 @@ github.com/spf13/viper v1.20.1/go.mod h1:P9Mdzt1zoHIG8m2eZQinpiBjo6kCmZSKBClNNqj
8381
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
8482
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
8583
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
86-
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
8784
github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
8885
github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
8986
github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8=
@@ -98,7 +95,6 @@ golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0
9895
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY=
9996
golang.org/x/oauth2 v0.29.0 h1:WdYw2tdTK1S8olAzWHdgeqfy+Mtm9XNhv/xJsY65d98=
10097
golang.org/x/oauth2 v0.29.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8=
101-
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
10298
golang.org/x/sys v0.31.0 h1:ioabZlmFYtWhL+TRYpcnNlLwhyxaM9kWTDEmfnprqik=
10399
golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
104100
golang.org/x/text v0.23.0 h1:D71I7dUrlY+VX0gQShAThNGHFxZ13dGLBHQLVl1mJlY=

internal/ghmcp/server.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"io"
77
"log"
8+
"log/slog"
89
"net/http"
910
"net/url"
1011
"os"
@@ -21,7 +22,6 @@ import (
2122
"github.com/mark3labs/mcp-go/mcp"
2223
"github.com/mark3labs/mcp-go/server"
2324
"github.com/shurcooL/githubv4"
24-
"github.com/sirupsen/logrus"
2525
)
2626

2727
type MCPServerConfig struct {
@@ -49,6 +49,8 @@ type MCPServerConfig struct {
4949
Translator translations.TranslationHelperFunc
5050
}
5151

52+
const stdioServerLogPrefix = "stdioserver"
53+
5254
func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
5355
apiHost, err := parseAPIHost(cfg.Host)
5456
if err != nil {
@@ -203,17 +205,22 @@ func RunStdioServer(cfg StdioServerConfig) error {
203205

204206
stdioServer := server.NewStdioServer(ghServer)
205207

206-
logrusLogger := logrus.New()
208+
var slogHandler slog.Handler
209+
var logOutput io.Writer
207210
if cfg.LogFilePath != "" {
208211
file, err := os.OpenFile(cfg.LogFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600)
209212
if err != nil {
210213
return fmt.Errorf("failed to open log file: %w", err)
211214
}
212-
213-
logrusLogger.SetLevel(logrus.DebugLevel)
214-
logrusLogger.SetOutput(file)
215-
}
216-
stdLogger := log.New(logrusLogger.Writer(), "stdioserver", 0)
215+
logOutput = file
216+
slogHandler = slog.NewTextHandler(logOutput, &slog.HandlerOptions{Level: slog.LevelDebug})
217+
} else {
218+
logOutput = os.Stderr
219+
slogHandler = slog.NewTextHandler(logOutput, &slog.HandlerOptions{Level: slog.LevelInfo})
220+
}
221+
logger := slog.New(slogHandler)
222+
logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "dynamicToolsets", cfg.DynamicToolsets, "readOnly", cfg.ReadOnly)
223+
stdLogger := log.New(logOutput, stdioServerLogPrefix, 0)
217224
stdioServer.SetErrorLogger(stdLogger)
218225

219226
if cfg.ExportTranslations {
@@ -227,7 +234,7 @@ func RunStdioServer(cfg StdioServerConfig) error {
227234
in, out := io.Reader(os.Stdin), io.Writer(os.Stdout)
228235

229236
if cfg.EnableCommandLogging {
230-
loggedIO := mcplog.NewIOLogger(in, out, logrusLogger)
237+
loggedIO := mcplog.NewIOLogger(in, out, logger)
231238
in, out = loggedIO, loggedIO
232239
}
233240
// enable GitHub errors in the context
@@ -241,9 +248,10 @@ func RunStdioServer(cfg StdioServerConfig) error {
241248
// Wait for shutdown signal
242249
select {
243250
case <-ctx.Done():
244-
logrusLogger.Infof("shutting down server...")
251+
logger.Info("shutting down server", "signal", "context done")
245252
case err := <-errC:
246253
if err != nil {
254+
logger.Error("error running server", "error", err)
247255
return fmt.Errorf("error running server: %w", err)
248256
}
249257
}

pkg/log/io.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,19 @@ package log
33
import (
44
"io"
55

6-
log "github.com/sirupsen/logrus"
6+
"log/slog"
77
)
88

99
// IOLogger is a wrapper around io.Reader and io.Writer that can be used
1010
// to log the data being read and written from the underlying streams
1111
type IOLogger struct {
1212
reader io.Reader
1313
writer io.Writer
14-
logger *log.Logger
14+
logger *slog.Logger
1515
}
1616

1717
// NewIOLogger creates a new IOLogger instance
18-
func NewIOLogger(r io.Reader, w io.Writer, logger *log.Logger) *IOLogger {
18+
func NewIOLogger(r io.Reader, w io.Writer, logger *slog.Logger) *IOLogger {
1919
return &IOLogger{
2020
reader: r,
2121
writer: w,
@@ -30,7 +30,7 @@ func (l *IOLogger) Read(p []byte) (n int, err error) {
3030
}
3131
n, err = l.reader.Read(p)
3232
if n > 0 {
33-
l.logger.Infof("[stdin]: received %d bytes: %s", n, string(p[:n]))
33+
l.logger.Info("[stdin]: received bytes", "count", n, "data", string(p[:n]))
3434
}
3535
return n, err
3636
}
@@ -40,6 +40,6 @@ func (l *IOLogger) Write(p []byte) (n int, err error) {
4040
if l.writer == nil {
4141
return 0, io.ErrClosedPipe
4242
}
43-
l.logger.Infof("[stdout]: sending %d bytes: %s", len(p), string(p))
43+
l.logger.Info("[stdout]: sending bytes", "count", len(p), "data", string(p))
4444
return l.writer.Write(p)
4545
}

pkg/log/io_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import (
55
"strings"
66
"testing"
77

8-
log "github.com/sirupsen/logrus"
8+
"log/slog"
9+
910
"github.com/stretchr/testify/assert"
1011
)
1112

@@ -17,11 +18,7 @@ func TestLoggedReadWriter(t *testing.T) {
1718

1819
// Create logger with buffer to capture output
1920
var logBuffer bytes.Buffer
20-
logger := log.New()
21-
logger.SetOutput(&logBuffer)
22-
logger.SetFormatter(&log.TextFormatter{
23-
DisableTimestamp: true,
24-
})
21+
logger := slog.New(slog.NewTextHandler(&logBuffer, &slog.HandlerOptions{ReplaceAttr: removeTimeAttr}))
2522

2623
lrw := NewIOLogger(reader, nil, logger)
2724

@@ -44,11 +41,7 @@ func TestLoggedReadWriter(t *testing.T) {
4441

4542
// Create logger with buffer to capture output
4643
var logBuffer bytes.Buffer
47-
logger := log.New()
48-
logger.SetOutput(&logBuffer)
49-
logger.SetFormatter(&log.TextFormatter{
50-
DisableTimestamp: true,
51-
})
44+
logger := slog.New(slog.NewTextHandler(&logBuffer, &slog.HandlerOptions{ReplaceAttr: removeTimeAttr}))
5245

5346
lrw := NewIOLogger(nil, &writeBuffer, logger)
5447

@@ -63,3 +56,10 @@ func TestLoggedReadWriter(t *testing.T) {
6356
assert.Contains(t, logBuffer.String(), outputData)
6457
})
6558
}
59+
60+
func removeTimeAttr(groups []string, a slog.Attr) slog.Attr {
61+
if a.Key == slog.TimeKey && len(groups) == 0 {
62+
return slog.Attr{}
63+
}
64+
return a
65+
}

third-party-licenses.darwin.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ Some packages may only be included on certain architectures or operating systems
2626
- [github.com/sagikazarmark/locafero](https://pkg.go.dev/github.com/sagikazarmark/locafero) ([MIT](https://github.com/sagikazarmark/locafero/blob/v0.9.0/LICENSE))
2727
- [github.com/shurcooL/githubv4](https://pkg.go.dev/github.com/shurcooL/githubv4) ([MIT](https://github.com/shurcooL/githubv4/blob/48295856cce7/LICENSE))
2828
- [github.com/shurcooL/graphql](https://pkg.go.dev/github.com/shurcooL/graphql) ([MIT](https://github.com/shurcooL/graphql/blob/ed46e5a46466/LICENSE))
29-
- [github.com/sirupsen/logrus](https://pkg.go.dev/github.com/sirupsen/logrus) ([MIT](https://github.com/sirupsen/logrus/blob/v1.9.3/LICENSE))
3029
- [github.com/sourcegraph/conc](https://pkg.go.dev/github.com/sourcegraph/conc) ([MIT](https://github.com/sourcegraph/conc/blob/v0.3.0/LICENSE))
3130
- [github.com/spf13/afero](https://pkg.go.dev/github.com/spf13/afero) ([Apache-2.0](https://github.com/spf13/afero/blob/v1.14.0/LICENSE.txt))
3231
- [github.com/spf13/cast](https://pkg.go.dev/github.com/spf13/cast) ([MIT](https://github.com/spf13/cast/blob/v1.7.1/LICENSE))

third-party-licenses.linux.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ Some packages may only be included on certain architectures or operating systems
2626
- [github.com/sagikazarmark/locafero](https://pkg.go.dev/github.com/sagikazarmark/locafero) ([MIT](https://github.com/sagikazarmark/locafero/blob/v0.9.0/LICENSE))
2727
- [github.com/shurcooL/githubv4](https://pkg.go.dev/github.com/shurcooL/githubv4) ([MIT](https://github.com/shurcooL/githubv4/blob/48295856cce7/LICENSE))
2828
- [github.com/shurcooL/graphql](https://pkg.go.dev/github.com/shurcooL/graphql) ([MIT](https://github.com/shurcooL/graphql/blob/ed46e5a46466/LICENSE))
29-
- [github.com/sirupsen/logrus](https://pkg.go.dev/github.com/sirupsen/logrus) ([MIT](https://github.com/sirupsen/logrus/blob/v1.9.3/LICENSE))
3029
- [github.com/sourcegraph/conc](https://pkg.go.dev/github.com/sourcegraph/conc) ([MIT](https://github.com/sourcegraph/conc/blob/v0.3.0/LICENSE))
3130
- [github.com/spf13/afero](https://pkg.go.dev/github.com/spf13/afero) ([Apache-2.0](https://github.com/spf13/afero/blob/v1.14.0/LICENSE.txt))
3231
- [github.com/spf13/cast](https://pkg.go.dev/github.com/spf13/cast) ([MIT](https://github.com/spf13/cast/blob/v1.7.1/LICENSE))

third-party-licenses.windows.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ Some packages may only be included on certain architectures or operating systems
2727
- [github.com/sagikazarmark/locafero](https://pkg.go.dev/github.com/sagikazarmark/locafero) ([MIT](https://github.com/sagikazarmark/locafero/blob/v0.9.0/LICENSE))
2828
- [github.com/shurcooL/githubv4](https://pkg.go.dev/github.com/shurcooL/githubv4) ([MIT](https://github.com/shurcooL/githubv4/blob/48295856cce7/LICENSE))
2929
- [github.com/shurcooL/graphql](https://pkg.go.dev/github.com/shurcooL/graphql) ([MIT](https://github.com/shurcooL/graphql/blob/ed46e5a46466/LICENSE))
30-
- [github.com/sirupsen/logrus](https://pkg.go.dev/github.com/sirupsen/logrus) ([MIT](https://github.com/sirupsen/logrus/blob/v1.9.3/LICENSE))
3130
- [github.com/sourcegraph/conc](https://pkg.go.dev/github.com/sourcegraph/conc) ([MIT](https://github.com/sourcegraph/conc/blob/v0.3.0/LICENSE))
3231
- [github.com/spf13/afero](https://pkg.go.dev/github.com/spf13/afero) ([Apache-2.0](https://github.com/spf13/afero/blob/v1.14.0/LICENSE.txt))
3332
- [github.com/spf13/cast](https://pkg.go.dev/github.com/spf13/cast) ([MIT](https://github.com/spf13/cast/blob/v1.7.1/LICENSE))

third-party/github.com/sirupsen/logrus/LICENSE

Lines changed: 0 additions & 21 deletions
This file was deleted.

0 commit comments

Comments
 (0)