Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

logging: switch to using log/slog #138

Merged
merged 1 commit into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/actions/terraform-linter/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ runs:
- id: 'setup-go'
uses: 'actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753' # ratchet:actions/setup-go@v4
with:
go-version: '1.20'
go-version: '1.21'

- id: 'run-linter'
shell: 'bash'
working-directory: 'abcxyz-pkg'
run: 'go run ./cmd/terraform-linter ${GITHUB_WORKSPACE}/${{inputs.directory}}'
run: |-
go run ./cmd/terraform-linter ${GITHUB_WORKSPACE}/${{inputs.directory}}
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ jobs:
go_lint:
uses: './.github/workflows/go-lint.yml'
with:
go_version: '1.20'
go_version: '1.21'

go_test:
uses: './.github/workflows/go-test.yml'
with:
go_version: '1.20'
go_version: '1.21'

terraform_lint:
uses: './.github/workflows/terraform-lint.yml'
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/go-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ on:
golangci_lint_version:
description: 'Version of golangci linter to use'
type: 'string'
default: 'v1.53'
default: 'v1.54'

jobs:
# modules checks if the go modules are all up-to-date. While rare with modern
Expand All @@ -51,7 +51,6 @@ jobs:
uses: 'actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753' # ratchet:actions/setup-go@v4
with:
go-version: '${{ inputs.go_version }}'
cache: false

- name: 'Check modules'
shell: 'bash'
Expand Down Expand Up @@ -82,6 +81,7 @@ jobs:
uses: 'actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753' # ratchet:actions/setup-go@v4
with:
go-version: '${{ inputs.go_version }}'
cache: false

- name: 'Lint (download default configuration)'
id: 'load-default-config'
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
lint:
uses: 'abcxyz/pkg/.github/workflows/go-lint.yml@main'
with:
go_version: '1.20'
go_version: '1.21'
```

Linting is done via [golangci-lint](https://golangci-lint.run/). If a
Expand Down Expand Up @@ -68,7 +68,7 @@ jobs:
lint:
uses: 'abcxyz/pkg/.github/workflows/go-test.yml@main'
with:
go_version: '1.20'
go_version: '1.21'
```

Testing is done via the `go test` command with:
Expand Down
6 changes: 3 additions & 3 deletions bqutil/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,13 @@ func (q *bqQuery[T]) Execute(ctx context.Context) ([]T, error) {
// result, err := RetryQueryEntries(ctx, q, 1, backoff)
func RetryQueryEntries[T any](ctx context.Context, q Query[T], wantCount int, backoff retry.Backoff) ([]T, error) {
logger := logging.FromContext(ctx)
logger.Debugw("Start retrying query", "query", q.String())
logger.DebugContext(ctx, "start retrying query", "query", q.String())

var result []T
if err := retry.Do(ctx, backoff, func(ctx context.Context) error {
entries, err := q.Execute(ctx)
if err != nil {
logger.Debugw("Query failed; will retry", "error", err)
logger.DebugContext(ctx, "query failed; will retry", "error", err)
return retry.RetryableError(err)
}

Expand All @@ -126,7 +126,7 @@ func RetryQueryEntries[T any](ctx context.Context, q Query[T], wantCount int, ba
return nil
}

logger.Debugw("Not enough entries; will retry", "gotCount", gotCount, "wantCount", wantCount)
logger.DebugContext(ctx, "not enough entries; will retry", "got_count", gotCount, "want_count", wantCount)
return retry.RetryableError(fmt.Errorf("not enough entries"))
}); err != nil {
return nil, fmt.Errorf("retry backoff exhausted: %w", err)
Expand Down
5 changes: 1 addition & 4 deletions bqutil/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ import (
"github.com/abcxyz/pkg/testutil"
"github.com/google/go-cmp/cmp"
"github.com/sethvargo/go-retry"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest"
)

type fakeQuery[T any] struct {
Expand Down Expand Up @@ -94,8 +92,7 @@ func TestRetryQueryEntries(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

ctx := logging.WithLogger(context.Background(),
logging.TestLogger(t, zaptest.Level(zapcore.DebugLevel)))
ctx := logging.WithLogger(context.Background(), logging.TestLogger(t))

wantCount := len(tc.wantEntries)

Expand Down
40 changes: 40 additions & 0 deletions cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,16 @@ import (
"flag"
"fmt"
"io"
"log/slog"
"os"
"sort"
"strconv"
"strings"
"time"

"github.com/abcxyz/pkg/logging"
"github.com/abcxyz/pkg/timeutil"

"github.com/kr/text"
"github.com/posener/complete/v2"
"github.com/posener/complete/v2/predict"
Expand Down Expand Up @@ -868,6 +871,43 @@ func (f *FlagSection) Uint64Var(i *Uint64Var) {
})
}

type LogLevelVar struct {
Logger *slog.Logger
}

func (f *FlagSection) LogLevelVar(i *LogLevelVar) {
parser := func(s string) (slog.Level, error) {
v, err := logging.LookupLevel(s)
if err != nil {
return 0, err
}
return v, nil
}

printer := func(v slog.Level) string { return logging.LevelString(v) }

setter := func(_ *slog.Level, val slog.Level) { logging.SetLevel(i.Logger, val) }

// trick the CLI into thinking we need a value to set.
var fake slog.Level

levelNames := logging.LevelNames()

Flag(f, &Var[slog.Level]{
Name: "log-level",
Aliases: []string{"l"},
Usage: `Sets the logging verbosity. Valid values include: ` +
strings.Join(levelNames, ",") + `.`,
Example: "warn",
Default: slog.LevelInfo,
Predict: predict.Set(levelNames),
Target: &fake,
Parser: parser,
Printer: printer,
Setter: setter,
})
}

// wrapAtLengthWithPadding wraps the given text at the maxLineLength, taking
// into account any provided left padding.
func wrapAtLengthWithPadding(s string, pad int) string {
Expand Down
64 changes: 64 additions & 0 deletions cli/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@
package cli

import (
"context"
"flag"
"fmt"
"io"
"log/slog"
"reflect"
"strings"
"testing"

"github.com/abcxyz/pkg/logging"
"github.com/abcxyz/pkg/testutil"
"github.com/google/go-cmp/cmp"
)
Expand Down Expand Up @@ -314,3 +317,64 @@ func ExampleFlagSet_AfterParse_checkIfError() {
func ptrTo[T any](v T) *T {
return &v
}

func TestLogLevelVar(t *testing.T) {
t.Parallel()

ctx := context.Background()

cases := []struct {
name string
args []string

wantLevel slog.Level
wantError string
}{
{
name: "empty",
args: nil,
wantLevel: slog.LevelInfo,
},
{
name: "long",
args: []string{"-log-level", "debug"},
wantLevel: slog.LevelDebug,
},
{
name: "short",
args: []string{"-l", "debug"},
wantLevel: slog.LevelDebug,
},
{
name: "invalid",
args: []string{"-log-level", "pants"},
wantError: `invalid value "pants" for flag -log-level`,
},
}

for _, tc := range cases {
tc := tc

t.Run(tc.name, func(t *testing.T) {
t.Parallel()

logger := logging.DefaultLogger()

set := NewFlagSet()
f := set.NewSection("FLAGS")

f.LogLevelVar(&LogLevelVar{
Logger: logger,
})

err := set.Parse(tc.args)
if diff := testutil.DiffErrString(err, tc.wantError); diff != "" {
t.Error(diff)
}

if !logger.Handler().Enabled(ctx, tc.wantLevel) {
t.Errorf("expected handler to be at least %s", tc.wantLevel)
}
})
}
}
4 changes: 2 additions & 2 deletions containertest/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ jobs:
steps:
- uses: actions/setup-go@v4
with:
go-version: '1.20' # Optional
go-version: '1.21' # Optional
```

... and *don't* do this:
Expand All @@ -111,5 +111,5 @@ jobs:
test:
name: Go Test
runs-on: ubuntu-latest
container: golang:1.20 # DON'T DO THIS
container: golang:1.21 # DON'T DO THIS
```
2 changes: 1 addition & 1 deletion gcputil/gcputil.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func ProjectID(ctx context.Context) string {

v, err := metadata.ProjectID()
if err != nil {
logging.FromContext(ctx).Errorw("failed to get project id", "error", err)
logging.FromContext(ctx).ErrorContext(ctx, "failed to get project id", "error", err)
return ""
}
return v
Expand Down
Loading