logging: switch to using log/slog (#138)

.github/actions/terraform-linter/action.yml

@@ -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}}

.github/workflows/ci.yml

@@ -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'

.github/workflows/go-lint.yml

@@ -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
@@ -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'
@@ -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'

README.md

@@ -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
@@ -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:

bqutil/query.go

@@ -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)
 		}
 
@@ -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)

bqutil/query_test.go

@@ -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 {
@@ -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)
 

cli/flags.go

@@ -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"
@@ -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 {

cli/flags_test.go

@@ -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"
 )
@@ -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)
+			}
+		})
+	}
+}

containertest/README.md

@@ -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:
@@ -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
 ```

gcputil/gcputil.go

@@ -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