move to functional options

coder · johnstcn · Dec 19, 2023 · Dec 19, 2023 · Dec 19, 2023 · Dec 19, 2023
commit 1963c14a9f91d4fa0b61d0f53f10f61cacc54d71
diff --git a/cli/clilog/clilog.go b/cli/clilog/clilog.go
@@ -29,51 +29,62 @@ type Builder struct {
 	Verbose     bool
 }
 
-func New() *Builder {
-	return &Builder{
+func New(opts ...Option) *Builder {
+	b := &Builder{
 		Human:  "/dev/stderr",
 		Filter: []string{},
 	}
+	for _, opt := range opts {
+		opt(b)
+	}
+	return b
 }
 
-func (b *Builder) WithFilter(filters ...string) *Builder {
-	b.Filter = filters
-	return b
+func WithFilter(filters ...string) Option {
+	return func(b *Builder) {
+		b.Filter = filters
+	}
 }
 
-func (b *Builder) WithHuman(loc string) *Builder {
-	b.Human = loc
-	return b
+func WithHuman(loc string) Option {
+	return func(b *Builder) {
+		b.Human = loc
+	}
 }
 
-func (b *Builder) WithJSON(loc string) *Builder {
-	b.JSON = loc
-	return b
+func WithJSON(loc string) Option {
+	return func(b *Builder) {
+		b.JSON = loc
+	}
 }
 
-func (b *Builder) WithStackdriver(loc string) *Builder {
-	b.Stackdriver = loc
-	return b
+func WithStackdriver(loc string) Option {
+	return func(b *Builder) {
+		b.Stackdriver = loc
+	}
 }
 
-func (b *Builder) WithTrace() *Builder {
-	b.Trace = true
-	return b
+func WithTrace() Option {
+	return func(b *Builder) {
+		b.Trace = true
+	}
 
 }
-func (b *Builder) WithVerbose() *Builder {
-	b.Verbose = true
-	return b
+func WithVerbose() Option {
+	return func(b *Builder) {
+		b.Verbose = true
+	}
 }
 
-func (b *Builder) FromDeploymentValues(vals *codersdk.DeploymentValues) *Builder {
-	b.Filter = vals.Logging.Filter.Value()
-	b.Human = vals.Logging.Human.Value()
-	b.JSON = vals.Logging.JSON.Value()
-	b.Stackdriver = vals.Logging.Stackdriver.Value()
-	b.Trace = vals.Trace.Enable.Value()
-	b.Verbose = vals.Verbose.Value()
-	return b
+func FromDeploymentValues(vals *codersdk.DeploymentValues) Option {
+	return func(b *Builder) {
+		b.Filter = vals.Logging.Filter.Value()
+		b.Human = vals.Logging.Human.Value()
+		b.JSON = vals.Logging.JSON.Value()
+		b.Stackdriver = vals.Logging.Stackdriver.Value()
+		b.Trace = vals.Trace.Enable.Value()
+		b.Verbose = vals.Verbose.Value()
+	}
 }
 
 func (b *Builder) Build(inv *clibase.Invocation) (slog.Logger, func(), error) {

diff --git a/cli/clilog/clilog_test.go b/cli/clilog/clilog_test.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/coder/coder/v2/cli/clibase"
 	"github.com/coder/coder/v2/cli/clilog"
+	"github.com/coder/coder/v2/codersdk"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -24,11 +25,11 @@ func TestBuilder(t *testing.T) {
 		cmd := &clibase.Cmd{
 			Use: "test",
 			Handler: func(inv *clibase.Invocation) error {
-				logger, closeLog, err := clilog.New().
-					WithFilter("foo", "baz").
-					WithHuman(tempFile).
-					WithVerbose().
-					Build(inv)
+				logger, closeLog, err := clilog.New(
+					clilog.WithFilter("foo", "baz"),
+					clilog.WithHuman(tempFile),
+					clilog.WithVerbose(),
+				).Build(inv)
 				if err != nil {
 					return err
 				}
@@ -58,8 +59,8 @@ func TestBuilder(t *testing.T) {
 		cmd := &clibase.Cmd{
 			Use: "test",
 			Handler: func(inv *clibase.Invocation) error {
-				logger, closeLog, err := clilog.New().
-					WithHuman(tempFile).
+				logger, closeLog, err := clilog.New(
+					clilog.WithHuman(tempFile)).
 					Build(inv)
 				if err != nil {
 					return err
@@ -90,8 +91,8 @@ func TestBuilder(t *testing.T) {
 		cmd := &clibase.Cmd{
 			Use: "test",
 			Handler: func(inv *clibase.Invocation) error {
-				logger, closeLog, err := clilog.New().
-					WithJSON(tempFile).
+				logger, closeLog, err := clilog.New(
+					clilog.WithJSON(tempFile)).
 					Build(inv)
 				if err != nil {
 					return err
@@ -124,15 +125,26 @@ func TestBuilder(t *testing.T) {
 		require.Equal(t, "bar is also not a useful message", entry.Message)
 	})
 
-	t.Run("WithStackdriver", func(t *testing.T) {
+	t.Run("FromDeploymentValues", func(t *testing.T) {
 		t.Parallel()
 
 		tempFile := filepath.Join(t.TempDir(), "test.log")
+		tempJSON := filepath.Join(t.TempDir(), "test.json")
+		dv := &codersdk.DeploymentValues{
+			Logging: codersdk.LoggingConfig{
+				Filter: []string{"foo", "baz"},
+				Human:  clibase.String(tempFile),
+				JSON:   clibase.String(tempJSON),
+			},
+			Verbose: true,
+			Trace: codersdk.TraceConfig{
+				Enable: true,
+			},
+		}
 		cmd := &clibase.Cmd{
 			Use: "test",
 			Handler: func(inv *clibase.Invocation) error {
-				logger, closeLog, err := clilog.New().
-					WithStackdriver(tempFile).
+				logger, closeLog, err := clilog.New(clilog.FromDeploymentValues(dv)).
 					Build(inv)
 				if err != nil {
 					return err
@@ -149,20 +161,21 @@ func TestBuilder(t *testing.T) {
 		data, err := os.ReadFile(tempFile)
 		require.NoError(t, err)
 		logs := strings.Split(strings.TrimSpace(string(data)), "\n")
-		if !assert.Len(t, logs, 1) {
+		if !assert.Len(t, logs, 2) {
 			t.Logf(string(data))
 			t.FailNow()
 		}
-		require.Contains(t, logs[0], "bar is also not a useful message")
-
-		var entry struct {
-			Severity string `json:"severity"`
-			Message  string `json:"message"`
-		}
+		require.Contains(t, logs[0], "foo is not a useful message")
+		require.Contains(t, logs[1], "bar is also not a useful message")
 
-		err = json.NewDecoder(strings.NewReader(logs[0])).Decode(&entry)
+		data, err = os.ReadFile(tempJSON)
 		require.NoError(t, err)
-		require.Equal(t, "INFO", entry.Severity)
-		require.Equal(t, "bar is also not a useful message", entry.Message)
+		logs = strings.Split(strings.TrimSpace(string(data)), "\n")
+		if !assert.Len(t, logs, 2) {
+			t.Logf(string(data))
+			t.FailNow()
+		}
+		require.Contains(t, logs[0], "foo is not a useful message")
+		require.Contains(t, logs[1], "bar is also not a useful message")
 	})
 }
diff --git a/cli/clilog/doc.go b/cli/clilog/doc.go
@@ -0,0 +1,2 @@
+// Package clilog provides a fluent API for configuring structured logging.
+package clilog
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		// Package clilog provides a fluent API for configuring structured logging.
		package clilog