From 887d3f17b19df3fe29d2b4217397274c3c717f91 Mon Sep 17 00:00:00 2001 From: Marco Molteni Date: Thu, 14 Mar 2024 17:16:45 +0100 Subject: [PATCH 1/2] replace github.com/hashicorp/go-hclog with stdlib log/slog --- CHANGELOG.md | 4 + cmd/cogito/main.go | 26 ++++-- cmd/cogito/main_test.go | 9 +- cogito/check.go | 9 +- cogito/check_test.go | 7 +- cogito/gchatsink.go | 5 +- cogito/gchatsink_test.go | 9 +- cogito/get.go | 9 +- cogito/get_test.go | 7 +- cogito/ghcommitsink.go | 7 +- cogito/ghcommitsink_test.go | 5 +- cogito/protocol_test.go | 19 ++-- cogito/put.go | 5 +- cogito/put_test.go | 31 ++++--- cogito/putter.go | 16 ++-- github/commitstatus.go | 7 +- github/commitstatus_test.go | 35 +++----- go.mod | 5 -- go.sum | 26 +----- internal/hclogslog/adapter.go | 159 ---------------------------------- testhelp/testlog.go | 32 +++++++ 21 files changed, 129 insertions(+), 303 deletions(-) delete mode 100644 internal/hclogslog/adapter.go create mode 100644 testhelp/testlog.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 88cafc9c..cd6936d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Update to Go 1.22 +### Removed + +- Replaced module github.com/hashicorp/go-hclog with stdlib log/slog. + ## [v0.12.0] - 2023-11-03 ### Added diff --git a/cmd/cogito/main.go b/cmd/cogito/main.go index c8403786..6655d776 100644 --- a/cmd/cogito/main.go +++ b/cmd/cogito/main.go @@ -6,11 +6,10 @@ import ( "encoding/json" "fmt" "io" + "log/slog" "os" "path" - "github.com/hashicorp/go-hclog" - "github.com/Pix4D/cogito/cogito" "github.com/Pix4D/cogito/sets" ) @@ -37,16 +36,27 @@ func mainErr(in io.Reader, out io.Writer, logOut io.Writer, args []string) error if err != nil { return fmt.Errorf("reading stdin: %s", err) } + logLevel, err := peekLogLevel(input) if err != nil { return err } - log := hclog.New(&hclog.LoggerOptions{ - Name: "cogito", - Level: hclog.LevelFromString(logLevel), - Output: logOut, - DisableTime: true, - }) + var level slog.Level + if err := level.UnmarshalText([]byte(logLevel)); err != nil { + return fmt.Errorf("%s. (valid: debug, info, warn, error)", err) + } + removeTime := func(groups []string, a slog.Attr) slog.Attr { + if a.Key == slog.TimeKey { + return slog.Attr{} + } + return a + } + log := slog.New(slog.NewTextHandler( + logOut, + &slog.HandlerOptions{ + Level: level, + ReplaceAttr: removeTime, + })) log.Info(cogito.BuildInfo()) switch cmd { diff --git a/cmd/cogito/main_test.go b/cmd/cogito/main_test.go index e869348a..1a49049f 100644 --- a/cmd/cogito/main_test.go +++ b/cmd/cogito/main_test.go @@ -129,9 +129,9 @@ func TestRunPutSuccessIntegration(t *testing.T) { assert.NilError(t, err, "\nout:\n%s\nlogOut:\n%s", out.String(), logOut.String()) assert.Assert(t, cmp.Contains(logOut.String(), - "cogito.put.ghCommitStatus: commit status posted successfully")) + `level=INFO msg="commit status posted successfully" name=cogito.put name=ghCommitStatus state=error`)) assert.Assert(t, cmp.Contains(logOut.String(), - "cogito.put.gChat: state posted successfully to chat")) + `level=INFO msg="state posted successfully to chat" name=cogito.put name=gChat state=error`)) } func TestRunFailure(t *testing.T) { @@ -199,11 +199,12 @@ func TestRunPrintsBuildInformation(t *testing.T) { } }`) var logBuf bytes.Buffer - wantLog := "cogito: This is the Cogito GitHub status resource. unknown" + wantLog := "This is the Cogito GitHub status resource. unknown" err := mainErr(in, io.Discard, &logBuf, []string{"check"}) assert.NilError(t, err) haveLog := logBuf.String() - assert.Assert(t, strings.Contains(haveLog, wantLog), haveLog) + assert.Assert(t, strings.Contains(haveLog, wantLog), + "\nhave: %s\nwant: %s", haveLog, wantLog) } diff --git a/cogito/check.go b/cogito/check.go index 0bd4a22c..647b40d9 100644 --- a/cogito/check.go +++ b/cogito/check.go @@ -4,8 +4,7 @@ import ( "encoding/json" "fmt" "io" - - "github.com/hashicorp/go-hclog" + "log/slog" ) // Check implements the "check" step (the "check" executable). @@ -17,10 +16,8 @@ import ( // It is given the configured source and current version on stdin, and must print the // array of new versions, in chronological order (oldest first), to stdout, including // the requested version if it is still valid. -func Check(log hclog.Logger, input []byte, out io.Writer, args []string) error { - log = log.Named("check") - log.Debug("started") - defer log.Debug("finished") +func Check(log *slog.Logger, input []byte, out io.Writer, args []string) error { + log = log.With("name", "cogito.check") request, err := NewCheckRequest(input) if err != nil { diff --git a/cogito/check_test.go b/cogito/check_test.go index 49f7d37b..23f4d3bb 100644 --- a/cogito/check_test.go +++ b/cogito/check_test.go @@ -5,7 +5,6 @@ import ( "io" "testing" - "github.com/hashicorp/go-hclog" "gotest.tools/v3/assert" "github.com/Pix4D/cogito/cogito" @@ -22,7 +21,7 @@ func TestCheckSuccess(t *testing.T) { test := func(t *testing.T, tc testCase) { in := testhelp.ToJSON(t, tc.request) var out bytes.Buffer - log := hclog.NewNullLogger() + log := testhelp.MakeTestLog() err := cogito.Check(log, in, &out, nil) @@ -74,7 +73,7 @@ func TestCheckFailure(t *testing.T) { test := func(t *testing.T, tc testCase) { assert.Assert(t, tc.wantErr != "") in := testhelp.ToJSON(t, cogito.CheckRequest{Source: tc.source}) - log := hclog.NewNullLogger() + log := testhelp.MakeTestLog() err := cogito.Check(log, in, tc.writer, nil) @@ -126,7 +125,7 @@ func TestCheckFailure(t *testing.T) { } func TestCheckInputFailure(t *testing.T) { - log := hclog.NewNullLogger() + log := testhelp.MakeTestLog() err := cogito.Check(log, nil, io.Discard, nil) diff --git a/cogito/gchatsink.go b/cogito/gchatsink.go index f40b7322..f75aad77 100644 --- a/cogito/gchatsink.go +++ b/cogito/gchatsink.go @@ -4,17 +4,16 @@ import ( "context" "fmt" "io/fs" + "log/slog" "strings" "time" - "github.com/hashicorp/go-hclog" - "github.com/Pix4D/cogito/googlechat" ) // GoogleChatSink is an implementation of [Sinker] for the Cogito resource. type GoogleChatSink struct { - Log hclog.Logger + Log *slog.Logger InputDir fs.FS GitRef string Request PutRequest diff --git a/cogito/gchatsink_test.go b/cogito/gchatsink_test.go index 24fb248f..8099d020 100644 --- a/cogito/gchatsink_test.go +++ b/cogito/gchatsink_test.go @@ -7,7 +7,6 @@ import ( "testing" "testing/fstest" - "github.com/hashicorp/go-hclog" "gotest.tools/v3/assert" "gotest.tools/v3/assert/cmp" @@ -38,7 +37,7 @@ func TestSinkGoogleChatSendSuccess(t *testing.T) { tc.setWebHook(&request, ts.URL) assert.NilError(t, request.Source.Validate()) sink := cogito.GoogleChatSink{ - Log: hclog.NewNullLogger(), + Log: testhelp.MakeTestLog(), GitRef: wantGitRef, Request: request, } @@ -80,7 +79,7 @@ func TestSinkGoogleChatDecidesNotToSendSuccess(t *testing.T) { test := func(t *testing.T, tc testCase) { sink := cogito.GoogleChatSink{ - Log: hclog.NewNullLogger(), + Log: testhelp.MakeTestLog(), Request: tc.request, } @@ -120,7 +119,7 @@ func TestSinkGoogleChatSendBackendFailure(t *testing.T) { request.Source.GChatWebHook = ts.URL assert.NilError(t, request.Source.Validate()) sink := cogito.GoogleChatSink{ - Log: hclog.NewNullLogger(), + Log: testhelp.MakeTestLog(), Request: request, } @@ -136,7 +135,7 @@ func TestSinkGoogleChatSendInputFailure(t *testing.T) { request.Source.GChatWebHook = "dummy-url" assert.NilError(t, request.Source.Validate()) sink := cogito.GoogleChatSink{ - Log: hclog.NewNullLogger(), + Log: testhelp.MakeTestLog(), InputDir: fstest.MapFS{"bar/msg.txt": {Data: []byte("from-custom-file")}}, Request: request, } diff --git a/cogito/get.go b/cogito/get.go index 555ad13e..2b46b671 100644 --- a/cogito/get.go +++ b/cogito/get.go @@ -4,8 +4,7 @@ import ( "encoding/json" "fmt" "io" - - "github.com/hashicorp/go-hclog" + "log/slog" ) // Get implements the "get" step (the "in" executable). @@ -24,10 +23,8 @@ import ( // The program must emit a JSON object containing the fetched version, and may emit // metadata as a list of key-value pairs. // This data is intended for public consumption and will be shown on the build page. -func Get(log hclog.Logger, input []byte, out io.Writer, args []string) error { - log = log.Named("get") - log.Debug("started") - defer log.Debug("finished") +func Get(log *slog.Logger, input []byte, out io.Writer, args []string) error { + log = log.With("name", "cogito.get") request, err := NewGetRequest(input) if err != nil { diff --git a/cogito/get_test.go b/cogito/get_test.go index a43a549d..54327277 100644 --- a/cogito/get_test.go +++ b/cogito/get_test.go @@ -5,7 +5,6 @@ import ( "io" "testing" - "github.com/hashicorp/go-hclog" "gotest.tools/v3/assert" "github.com/Pix4D/cogito/cogito" @@ -22,7 +21,7 @@ func TestGetSuccess(t *testing.T) { test := func(t *testing.T, tc testCase) { in := testhelp.ToJSON(t, tc.request) var out bytes.Buffer - log := hclog.NewNullLogger() + log := testhelp.MakeTestLog() err := cogito.Get(log, in, &out, []string{"dummy-dir"}) @@ -73,7 +72,7 @@ func TestGetFailure(t *testing.T) { Source: tc.source, Version: tc.version, }) - log := hclog.NewNullLogger() + log := testhelp.MakeTestLog() err := cogito.Get(log, in, tc.writer, tc.args) @@ -144,7 +143,7 @@ func TestGetNonEmptyParamsFailure(t *testing.T) { }`) wantErr := `get: parsing request: json: unknown field "params"` - err := cogito.Get(hclog.NewNullLogger(), in, io.Discard, []string{}) + err := cogito.Get(testhelp.MakeTestLog(), in, io.Discard, []string{}) assert.Error(t, err, wantErr) } diff --git a/cogito/ghcommitsink.go b/cogito/ghcommitsink.go index ff429400..94079667 100644 --- a/cogito/ghcommitsink.go +++ b/cogito/ghcommitsink.go @@ -4,10 +4,7 @@ import ( "log/slog" "time" - "github.com/hashicorp/go-hclog" - "github.com/Pix4D/cogito/github" - "github.com/Pix4D/cogito/internal/hclogslog" "github.com/Pix4D/cogito/retry" ) @@ -26,7 +23,7 @@ const ( // GitHubCommitStatusSink is an implementation of [Sinker] for the Cogito resource. type GitHubCommitStatusSink struct { - Log hclog.Logger + Log *slog.Logger GitRef string Request PutRequest } @@ -46,7 +43,7 @@ func (sink GitHubCommitStatusSink) Send() error { FirstDelay: retryFirstDelay, BackoffLimit: retryBackoffLimit, UpTo: retryUpTo, - Log: slog.New(hclogslog.Adapt(sink.Log)), + Log: sink.Log, }, } commitStatus := github.NewCommitStatus(target, sink.Request.Source.AccessToken, diff --git a/cogito/ghcommitsink_test.go b/cogito/ghcommitsink_test.go index a429ca49..cd077cac 100644 --- a/cogito/ghcommitsink_test.go +++ b/cogito/ghcommitsink_test.go @@ -7,7 +7,6 @@ import ( "path" "testing" - "github.com/hashicorp/go-hclog" "gotest.tools/v3/assert" "github.com/Pix4D/cogito/cogito" @@ -26,7 +25,7 @@ func TestSinkGitHubCommitStatusSendSuccess(t *testing.T) { gitHubSpyURL, err := url.Parse(ts.URL) assert.NilError(t, err, "error parsing SpyHttpServer URL: %s", err) sink := cogito.GitHubCommitStatusSink{ - Log: hclog.NewNullLogger(), + Log: testhelp.MakeTestLog(), GitRef: wantGitRef, Request: cogito.PutRequest{ Source: cogito.Source{GhHostname: gitHubSpyURL.Host}, @@ -53,7 +52,7 @@ func TestSinkGitHubCommitStatusSendFailure(t *testing.T) { assert.NilError(t, err, "error parsing SpyHttpServer URL: %s", err) defer ts.Close() sink := cogito.GitHubCommitStatusSink{ - Log: hclog.NewNullLogger(), + Log: testhelp.MakeTestLog(), GitRef: "deadbeefdeadbeef", Request: cogito.PutRequest{ Source: cogito.Source{GhHostname: gitHubSpyURL.Host}, diff --git a/cogito/protocol_test.go b/cogito/protocol_test.go index b6dec2d3..289848b5 100644 --- a/cogito/protocol_test.go +++ b/cogito/protocol_test.go @@ -4,14 +4,15 @@ import ( "bytes" "encoding/json" "fmt" + "log/slog" "strings" "testing" - "github.com/hashicorp/go-hclog" "gotest.tools/v3/assert" "gotest.tools/v3/assert/cmp" "github.com/Pix4D/cogito/cogito" + "github.com/Pix4D/cogito/testhelp" ) func TestSourceValidationSuccess(t *testing.T) { @@ -247,15 +248,16 @@ sinks: []` assert.Equal(t, have, want) }) - t.Run("hclog redacts fields", func(t *testing.T) { + t.Run("slog redacts fields", func(t *testing.T) { var logBuf bytes.Buffer - log := hclog.New(&hclog.LoggerOptions{Output: &logBuf}) + log := slog.New(slog.NewTextHandler(&logBuf, + &slog.HandlerOptions{ReplaceAttr: testhelp.RemoveTime})) log.Info("log test", "source", source) have := logBuf.String() - assert.Assert(t, cmp.Contains(have, "| access_token: ***REDACTED***")) - assert.Assert(t, cmp.Contains(have, "| gchat_webhook: ***REDACTED***")) + assert.Assert(t, cmp.Contains(have, "access_token: ***REDACTED***")) + assert.Assert(t, cmp.Contains(have, "gchat_webhook: ***REDACTED***")) assert.Assert(t, !strings.Contains(have, "sensitive")) }) } @@ -302,14 +304,15 @@ sinks: []` assert.Equal(t, have, want) }) - t.Run("hclog redacts fields", func(t *testing.T) { + t.Run("slog redacts fields", func(t *testing.T) { var logBuf bytes.Buffer - log := hclog.New(&hclog.LoggerOptions{Output: &logBuf}) + log := slog.New(slog.NewTextHandler(&logBuf, + &slog.HandlerOptions{ReplaceAttr: testhelp.RemoveTime})) log.Info("log test", "params", params) have := logBuf.String() - assert.Assert(t, cmp.Contains(have, "| gchat_webhook: ***REDACTED***")) + assert.Assert(t, cmp.Contains(have, "gchat_webhook: ***REDACTED***")) assert.Assert(t, !strings.Contains(have, "sensitive")) }) } diff --git a/cogito/put.go b/cogito/put.go index 6ca058c0..fc8c16ce 100644 --- a/cogito/put.go +++ b/cogito/put.go @@ -3,9 +3,8 @@ package cogito import ( "fmt" "io" + "log/slog" "strings" - - "github.com/hashicorp/go-hclog" ) // Putter represents the put step of a Concourse resource. @@ -40,7 +39,7 @@ type Sinker interface { // Additionally, the script may emit metadata as a list of key-value pairs. This data is // intended for public consumption and will make it upstream, intended to be shown on the // build's page. -func Put(log hclog.Logger, input []byte, out io.Writer, args []string, putter Putter) error { +func Put(log *slog.Logger, input []byte, out io.Writer, args []string, putter Putter) error { if err := putter.LoadConfiguration(input, args); err != nil { return fmt.Errorf("put: %s", err) } diff --git a/cogito/put_test.go b/cogito/put_test.go index 61311cef..79b747f8 100644 --- a/cogito/put_test.go +++ b/cogito/put_test.go @@ -7,7 +7,6 @@ import ( "path/filepath" "testing" - "github.com/hashicorp/go-hclog" "gotest.tools/v3/assert" "github.com/Pix4D/cogito/cogito" @@ -65,7 +64,7 @@ func (ms MockSinker) Send() error { func TestPutSuccess(t *testing.T) { putter := MockPutter{sinkers: []cogito.Sinker{MockSinker{}}} - err := cogito.Put(hclog.NewNullLogger(), nil, nil, nil, putter) + err := cogito.Put(testhelp.MakeTestLog(), nil, nil, nil, putter) assert.NilError(t, err) } @@ -78,7 +77,7 @@ func TestPutFailure(t *testing.T) { } test := func(t *testing.T, tc testCase) { - err := cogito.Put(hclog.NewNullLogger(), nil, nil, nil, tc.putter) + err := cogito.Put(testhelp.MakeTestLog(), nil, nil, nil, tc.putter) assert.ErrorContains(t, err, tc.wantErr) } @@ -124,7 +123,7 @@ func TestPutFailure(t *testing.T) { func TestPutterLoadConfigurationSuccess(t *testing.T) { in := testhelp.ToJSON(t, basePutRequest) - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) err := putter.LoadConfiguration(in, []string{"dummy-dir"}) @@ -143,7 +142,7 @@ func TestPutterLoadConfigurationSinksOverrideSuccess(t *testing.T) { }, "params": {"sinks": ["gchat"]} }`) - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) inputDir := []string{""} err := putter.LoadConfiguration(in, inputDir) assert.NilError(t, err) @@ -164,7 +163,7 @@ func TestPutterLoadConfigurationFailure(t *testing.T) { test := func(t *testing.T, tc testCase) { in := testhelp.ToJSON(t, tc.putInput) - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) err := putter.LoadConfiguration(in, tc.args) @@ -205,7 +204,7 @@ func TestPutterLoadConfigurationInvalidParamsFailure(t *testing.T) { "params": {"pizza": "margherita"} }`) wantErr := `put: parsing request: json: unknown field "pizza"` - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) err := putter.LoadConfiguration(in, nil) @@ -219,7 +218,7 @@ func TestPutterLoadConfigurationMissingGchatwebHook(t *testing.T) { "params": {} }`) wantErr := `put: source: missing keys: gchat_webhook` - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) err := putter.LoadConfiguration(in, nil) @@ -233,7 +232,7 @@ func TestPutterLoadConfigurationUnknownSink(t *testing.T) { "params": {} }`) wantErr := `put: source: invalid sink(s): [pizza]` - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) err := putter.LoadConfiguration(in, nil) @@ -247,7 +246,7 @@ func TestPutterLoadConfigurationUnknownSinkPutParams(t *testing.T) { "params": {"sinks": ["pizza"]} }`) wantErr := `put: arguments: unsupported sink(s): [pizza]` - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) err := putter.LoadConfiguration(in, nil) @@ -263,7 +262,7 @@ func TestPutterProcessInputDirSuccess(t *testing.T) { } test := func(t *testing.T, tc testCase) { - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) tmpDir := testhelp.MakeGitRepoFromTestdata(t, tc.inputDir, "https://github.com/dummy-owner/dummy-repo", "dummySHA", "banana") putter.InputDir = filepath.Join(tmpDir, filepath.Base(tc.inputDir)) @@ -318,7 +317,7 @@ func TestPutterProcessInputDirFailure(t *testing.T) { test := func(t *testing.T, tc testCase) { tmpDir := testhelp.MakeGitRepoFromTestdata(t, tc.inputDir, "https://github.com/dummy-owner/dummy-repo", "dummySHA", "banana mango") - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) putter.Request = cogito.PutRequest{ Source: cogito.Source{GhHostname: github.GhDefaultHostname, Owner: "dummy-owner", Repo: "dummy-repo"}, Params: tc.params, @@ -395,7 +394,7 @@ func TestPutterProcessInputDirNonExisting(t *testing.T) { } func TestPutterSinks(t *testing.T) { - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) sinks := putter.Sinks() assert.Assert(t, len(sinks) == 2) @@ -407,7 +406,7 @@ func TestPutterSinks(t *testing.T) { } func TestPutterCustomSinks(t *testing.T) { - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) putter.Request = cogito.PutRequest{ Params: cogito.PutParams{Sinks: []string{"gchat"}}, } @@ -418,7 +417,7 @@ func TestPutterCustomSinks(t *testing.T) { } func TestPutterOutputSuccess(t *testing.T) { - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) err := putter.Output(io.Discard) @@ -426,7 +425,7 @@ func TestPutterOutputSuccess(t *testing.T) { } func TestPutterOutputFailure(t *testing.T) { - putter := cogito.NewPutter(hclog.NewNullLogger()) + putter := cogito.NewPutter(testhelp.MakeTestLog()) err := putter.Output(&testhelp.FailingWriter{}) diff --git a/cogito/putter.go b/cogito/putter.go index 27d58607..e10a4d1f 100644 --- a/cogito/putter.go +++ b/cogito/putter.go @@ -4,13 +4,13 @@ import ( "encoding/json" "fmt" "io" + "log/slog" "net/url" "os" "path" "path/filepath" "strings" - "github.com/hashicorp/go-hclog" "github.com/sasbury/mini" "github.com/Pix4D/cogito/github" @@ -23,22 +23,18 @@ type ProdPutter struct { Request PutRequest InputDir string // Cogito specific fields. - log hclog.Logger + log *slog.Logger gitRef string } // NewPutter returns a Cogito ProdPutter. -func NewPutter(log hclog.Logger) *ProdPutter { +func NewPutter(log *slog.Logger) *ProdPutter { return &ProdPutter{ - log: log, + log: log.With("name", "cogito.put"), } } func (putter *ProdPutter) LoadConfiguration(input []byte, args []string) error { - putter.log = putter.log.Named("put") - putter.log.Debug("started") - defer putter.log.Debug("finished") - request, err := NewPutRequest(input) if err != nil { return err @@ -152,12 +148,12 @@ func (putter *ProdPutter) ProcessInputDir() error { func (putter *ProdPutter) Sinks() []Sinker { supportedSinkers := map[string]Sinker{ "github": GitHubCommitStatusSink{ - Log: putter.log.Named("ghCommitStatus"), + Log: putter.log.With("name", "ghCommitStatus"), GitRef: putter.gitRef, Request: putter.Request, }, "gchat": GoogleChatSink{ - Log: putter.log.Named("gChat"), + Log: putter.log.With("name", "gChat"), // TODO putter.InputDir itself should be of type fs.FS. InputDir: os.DirFS(putter.InputDir), GitRef: putter.gitRef, diff --git a/github/commitstatus.go b/github/commitstatus.go index 6c39eff8..e4aea5cf 100644 --- a/github/commitstatus.go +++ b/github/commitstatus.go @@ -10,14 +10,13 @@ import ( "errors" "fmt" "io" + "log/slog" "net/http" "path" "regexp" "strings" "time" - "github.com/hashicorp/go-hclog" - "github.com/Pix4D/cogito/retry" ) @@ -56,7 +55,7 @@ type CommitStatus struct { repo string context string - log hclog.Logger + log *slog.Logger } // NewCommitStatus returns a CommitStatus object associated to a specific GitHub owner @@ -70,7 +69,7 @@ type CommitStatus struct { // // See also: // - https://docs.github.com/en/rest/commits/statuses -func NewCommitStatus(target *Target, token, owner, repo, context string, log hclog.Logger) CommitStatus { +func NewCommitStatus(target *Target, token, owner, repo, context string, log *slog.Logger) CommitStatus { return CommitStatus{ target: target, token: token, diff --git a/github/commitstatus_test.go b/github/commitstatus_test.go index 2449ac51..b3da4f84 100644 --- a/github/commitstatus_test.go +++ b/github/commitstatus_test.go @@ -3,18 +3,15 @@ package github_test import ( "errors" "fmt" - "log/slog" "net/http" "net/http/httptest" "strconv" "testing" "time" - "github.com/hashicorp/go-hclog" "gotest.tools/v3/assert" "github.com/Pix4D/cogito/github" - "github.com/Pix4D/cogito/internal/hclogslog" "github.com/Pix4D/cogito/retry" "github.com/Pix4D/cogito/testhelp" ) @@ -77,10 +74,7 @@ func TestGitHubStatusSuccessMockAPI(t *testing.T) { } ts := httptest.NewServer(http.HandlerFunc(handler)) defer ts.Close() - log := hclog.NewNullLogger() - if testing.Verbose() { - log = hclog.Default() - } + log := testhelp.MakeTestLog() sleepSpy := SleepSpy{} target := &github.Target{ Server: ts.URL, @@ -89,7 +83,7 @@ func TestGitHubStatusSuccessMockAPI(t *testing.T) { BackoffLimit: retryBackoffLimit, UpTo: retryUpTo, SleepFn: sleepSpy.Sleep, - Log: slog.New(hclogslog.Adapt(log)), + Log: log, }, } ghStatus := github.NewCommitStatus(target, cfg.Token, cfg.Owner, cfg.Repo, context, log) @@ -220,10 +214,7 @@ func TestGitHubStatusFailureMockAPI(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(handler)) defer ts.Close() wantErr := fmt.Sprintf(tc.wantErr, ts.URL) - log := hclog.NewNullLogger() - if testing.Verbose() { - log = hclog.Default() - } + log := testhelp.MakeTestLog() sleepSpy := SleepSpy{} target := &github.Target{ Server: ts.URL, @@ -232,7 +223,7 @@ func TestGitHubStatusFailureMockAPI(t *testing.T) { BackoffLimit: retryBackoffLimit, UpTo: upTo, SleepFn: sleepSpy.Sleep, - Log: slog.New(hclogslog.Adapt(log)), + Log: log, }, } ghStatus := github.NewCommitStatus(target, cfg.Token, cfg.Owner, cfg.Repo, context, log) @@ -332,20 +323,17 @@ func TestGitHubStatusSuccessIntegration(t *testing.T) { targetURL := "https://cogito.invalid/builds/job/42" desc := time.Now().Format("15:04:05") state := "success" - log := hclog.NewNullLogger() - if testing.Verbose() { - log = hclog.Default() - } + log := testhelp.MakeTestLog() target := &github.Target{ Server: github.ApiRoot(github.GhDefaultHostname), Retry: retry.Retry{ FirstDelay: retryFirstDelay, BackoffLimit: retryBackoffLimit, UpTo: retryUpTo, - Log: slog.New(hclogslog.Adapt(log)), + Log: log, }, } - ghStatus := github.NewCommitStatus(target, cfg.Token, cfg.Owner, cfg.Repo, context, hclog.NewNullLogger()) + ghStatus := github.NewCommitStatus(target, cfg.Token, cfg.Owner, cfg.Repo, context, log) err := ghStatus.Add(cfg.SHA, state, targetURL, desc) @@ -369,10 +357,7 @@ func TestGitHubStatusFailureIntegration(t *testing.T) { cfg := testhelp.GitHubSecretsOrFail(t) state := "success" - log := hclog.NewNullLogger() - if testing.Verbose() { - log = hclog.Default() - } + log := testhelp.MakeTestLog() run := func(t *testing.T, tc testCase) { // zero values are defaults @@ -395,10 +380,10 @@ func TestGitHubStatusFailureIntegration(t *testing.T) { FirstDelay: retryFirstDelay, BackoffLimit: retryBackoffLimit, UpTo: retryUpTo, - Log: slog.New(hclogslog.Adapt(log)), + Log: log, }, } - ghStatus := github.NewCommitStatus(target, tc.token, tc.owner, tc.repo, "dummy-context", hclog.NewNullLogger()) + ghStatus := github.NewCommitStatus(target, tc.token, tc.owner, tc.repo, "dummy-context", log) err := ghStatus.Add(tc.sha, state, "dummy-url", "dummy-desc") assert.Error(t, err, tc.wantErr) diff --git a/go.mod b/go.mod index 1e3496a6..59371cae 100644 --- a/go.mod +++ b/go.mod @@ -7,18 +7,13 @@ require ( github.com/alexflint/go-arg v1.4.3 github.com/go-quicktest/qt v1.101.0 github.com/google/go-cmp v0.6.0 - github.com/hashicorp/go-hclog v1.6.2 github.com/sasbury/mini v0.0.0-20181226232755-dc74af49394b gotest.tools/v3 v3.5.1 ) require ( github.com/alexflint/go-scalar v1.2.0 // indirect - github.com/fatih/color v1.16.0 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/kr/text v0.2.0 // indirect - github.com/mattn/go-colorable v0.1.13 // indirect - github.com/mattn/go-isatty v0.0.20 // indirect github.com/rogpeppe/go-internal v1.12.0 // indirect - golang.org/x/sys v0.17.0 // indirect ) diff --git a/go.sum b/go.sum index 6ffc7507..6926d49e 100644 --- a/go.sum +++ b/go.sum @@ -9,28 +9,14 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= -github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= -github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= github.com/go-quicktest/qt v1.101.0 h1:O1K29Txy5P2OK0dGo59b7b0LR6wKfIhttaAhHUyn7eI= github.com/go-quicktest/qt v1.101.0/go.mod h1:14Bz/f7NwaXPtdYEgzsx46kqSxVwTbzVZsDC26tQJow= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/hashicorp/go-hclog v1.6.2 h1:NOtoftovWkDheyUM/8JW3QMiXyxJK3uHRK7wV04nD2I= -github.com/hashicorp/go-hclog v1.6.2/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= -github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= -github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= -github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= -github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= -github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= -github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= -github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= -github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -41,18 +27,8 @@ github.com/sasbury/mini v0.0.0-20181226232755-dc74af49394b h1:E0nPZOFGK8IsGxpckE github.com/sasbury/mini v0.0.0-20181226232755-dc74af49394b/go.mod h1:wYn/TPCowpxu/JFfDkzQH7E+IJGE20xICM5dNd1QN8c= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.7.2 h1:4jaiDzPyXQvSd7D0EjG45355tLlV3VOECpq10pLC+8s= -github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= -golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220503163025-988cb79eb6c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/hclogslog/adapter.go b/internal/hclogslog/adapter.go deleted file mode 100644 index eec6ef26..00000000 --- a/internal/hclogslog/adapter.go +++ /dev/null @@ -1,159 +0,0 @@ -// Taken from https://github.com/evanphx/go-hclog-slog and modified to use -// stdlib/log/slog instead of golang.org/x/exp/slog -// -// Will be removed as soon as we migrate the totality of cogito to stdlib/log/slog. - -package hclogslog - -import ( - "context" - "log/slog" - - "github.com/hashicorp/go-hclog" -) - -func Adapt(l hclog.Logger) slog.Handler { - return &Handler{l: l} -} - -type Handler struct { - l hclog.Logger - prefix string -} - -// Enabled reports whether the handler handles records at the given level. -// The handler ignores records whose level is lower. -// It is called early, before any arguments are processed, -// to save effort if the log event should be discarded. -// If called from a Logger method, the first argument is the context -// passed to that method, or context.Background() if nil was passed -// or the method does not take a context. -// The context is passed so Enabled can use its values -// to make a decision. -func (h *Handler) Enabled(ctx context.Context, lvl slog.Level) bool { - switch { - case lvl < slog.LevelDebug: - return h.l.IsTrace() - case lvl < slog.LevelInfo: - return h.l.IsDebug() - case lvl < slog.LevelWarn: - return h.l.IsInfo() - case lvl < slog.LevelError: - return h.l.IsWarn() - default: - return h.l.IsError() - } -} - -var basicTranslate = map[slog.Level]hclog.Level{ - slog.LevelDebug - 4: hclog.Trace, - slog.LevelDebug: hclog.Debug, - slog.LevelInfo: hclog.Info, - slog.LevelWarn: hclog.Warn, - slog.LevelError: hclog.Error, -} - -func (h *Handler) translateLevel(lvl slog.Level) hclog.Level { - if tl, ok := basicTranslate[lvl]; ok { - return tl - } - - switch { - case lvl < slog.LevelDebug: - return hclog.Trace - case lvl < slog.LevelInfo: - return hclog.Debug - case lvl < slog.LevelWarn: - return hclog.Info - case lvl < slog.LevelError: - return hclog.Warn - default: - return hclog.Error - } -} - -// Handle handles the Record. -// It will only be called when Enabled returns true. -// The Context argument is as for Enabled. -// It is present solely to provide Handlers access to the context's values. -// Canceling the context should not affect record processing. -// (Among other things, log messages may be necessary to debug a -// cancellation-related problem.) -// -// Handle methods that produce output should observe the following rules: -// - If r.Time is the zero time, ignore the time. -// - If r.PC is zero, ignore it. -// - Attr's values should be resolved. -// - If an Attr's key and value are both the zero value, ignore the Attr. -// This can be tested with attr.Equal(Attr{}). -// - If a group's key is empty, inline the group's Attrs. -// - If a group has no Attrs (even if it has a non-empty key), -// ignore it. -func (h *Handler) Handle(_ context.Context, rec slog.Record) error { - attrs := make([]any, 0, rec.NumAttrs()*2) - - rec.Attrs(func(a slog.Attr) bool { - if a.Value.Kind() == slog.KindGroup { - for _, subA := range a.Value.Group() { - attrs = append(attrs, h.prefix+a.Key+"."+subA.Key, subA.Value.Any()) - } - } else { - attrs = append(attrs, h.prefix+a.Key, a.Value.Any()) - } - return true - }) - - h.l.Log(h.translateLevel(rec.Level), rec.Message, attrs...) - return nil -} - -// WithAttrs returns a new Handler whose attributes consist of -// both the receiver's attributes and the arguments. -// The Handler owns the slice: it may retain, modify or discard it. - -func (h *Handler) WithAttrs(attrs []slog.Attr) slog.Handler { - args := make([]any, 0, len(attrs)) - - for _, a := range attrs { - if a.Value.Kind() == slog.KindGroup { - for _, subA := range a.Value.Group() { - args = append(args, h.prefix+a.Key+"."+subA.Key, subA.Value.Any()) - } - } else { - args = append(args, h.prefix+a.Key, a.Value.Any()) - } - } - - return &Handler{ - l: h.l.With(args...), - prefix: h.prefix, - } -} - -// WithGroup returns a new Handler with the given group appended to -// the receiver's existing groups. -// The keys of all subsequent attributes, whether added by With or in a -// Record, should be qualified by the sequence of group names. -// -// How this qualification happens is up to the Handler, so long as -// this Handler's attribute keys differ from those of another Handler -// with a different sequence of group names. -// -// A Handler should treat WithGroup as starting a Group of Attrs that ends -// at the end of the log event. That is, -// -// logger.WithGroup("s").LogAttrs(level, msg, slog.Int("a", 1), slog.Int("b", 2)) -// -// should behave like -// -// logger.LogAttrs(level, msg, slog.Group("s", slog.Int("a", 1), slog.Int("b", 2))) -// -// If the name is empty, WithGroup returns the receiver. -func (h *Handler) WithGroup(name string) slog.Handler { - return &Handler{ - l: h.l, - prefix: name + ".", - } -} - -var _ slog.Handler = &Handler{} diff --git a/testhelp/testlog.go b/testhelp/testlog.go new file mode 100644 index 00000000..ce66b5d3 --- /dev/null +++ b/testhelp/testlog.go @@ -0,0 +1,32 @@ +package testhelp + +import ( + "io" + "log/slog" + "os" + "testing" +) + +// MakeTestLog returns a *slog.Logger adapted for tests: it never reports the +// timestamp and by default it discards all the output. If on the other hand +// the tests are invoked in verbose mode (go test -v), then the logger will +// log normally. +func MakeTestLog() *slog.Logger { + out := io.Discard + if testing.Verbose() { + out = os.Stdout + } + return slog.New(slog.NewTextHandler( + out, + &slog.HandlerOptions{ + ReplaceAttr: RemoveTime, + })) +} + +// RemoveTime removes the "time" attribute from the output of a slog.Logger. +func RemoveTime(groups []string, a slog.Attr) slog.Attr { + if a.Key == slog.TimeKey { + return slog.Attr{} + } + return a +} From f500a01b2d57cacf040d5e35e4e6e3b6a70b4c1c Mon Sep 17 00:00:00 2001 From: Marco Molteni Date: Fri, 15 Mar 2024 10:24:09 +0100 Subject: [PATCH 2/2] update README and CHANGELOG --- CHANGELOG.md | 7 +++++++ README.md | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd6936d8..0346aec1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [v0.12.1] - UNRELEASED +### Minor breaking change + +- `source.log_level`. + - The README was wrongly still mentioning `silent` as a way to silent the logging. Actually `silent` was remapped to `info` since [v0.8.2](#v082---2022-11-18). + - Starting from this version, levels `silent` and `off` will cause an error. + - The only supported log levels are `debug`, `info`, `warn`, `error`. The default level, `info`, is recommended for normal operation. + ### Changed - Update to Go 1.22 diff --git a/README.md b/README.md index 801e7dbe..34f10175 100644 --- a/README.md +++ b/README.md @@ -195,8 +195,8 @@ With reference to the [GitHub Commit status API], the `POST` parameters (`state` See also: the optional `context` in the [put step](#the-put-step). - `log_level`:\ - The log level (one of `debug`, `info`, `warn`, `error`, `silent`).\ - Default: `info`. + The log level (one of `debug`, `info`, `warn`, `error`).\ + Default: `info`, recommended for normal operation. - `github_hostname`:\ GitHub hostname. This allows to post commit statuses to repositories hosted by GitHub Enterprise (GHE) instances. For example: github.mycompany.org will be expanded by cogito to https://github.mycompany.org/api/v3 \