Nov 12, 2024 · Nov 8, 2024 · Nov 12, 2024 · Nov 12, 2024 · Nov 12, 2024 · Nov 8, 2024
diff --git a/cli/clilog/clilog.go b/cli/clilog/clilog.go
 "context"
 "fmt"
 "io"
 "os"
 "regexp"
 "strings"

 "golang.org/x/xerrors"
 "gopkg.in/natefinch/lumberjack.v2"

 "cdr.dev/slog"
 "cdr.dev/slog/sloggers/sloghuman"
 addSinkIfProvided := func(sinkFn func(io.Writer) slog.Sink, loc string) error {
 switch loc {
 case "":

 case "/dev/stdout":
 sinks = append(sinks, sinkFn(inv.Stdout))

 case "/dev/stderr":
 sinks = append(sinks, sinkFn(inv.Stderr))

 default:
 fi, err := os.OpenFile(loc, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o644)
 if err != nil {
 return xerrors.Errorf("open log file %q: %w", loc, err)
 logWriter := &lumberjack.Logger{
 Filename: loc,
 MaxSize:  5, // MB
 // Without this, rotated logs will never be deleted.
 MaxBackups: 1,
 }
 closers = append(closers,fi.Close)
 sinks = append(sinks, sinkFn(fi))
 closers = append(closers,logWriter.Close)
 sinks = append(sinks, sinkFn(logWriter))
 }
 return nil
 }
diff --git a/cli/clilog/clilog_test.go b/cli/clilog/clilog_test.go

 import (
 "encoding/json"
 "io/fs"
 "os"
 "path/filepath"
 "strings"
 assertLogsJSON(t, tempJSON, info, infoLog, warn, warnLog)
 })
 })

 t.Run("NotFound", func(t *testing.T) {
 t.Parallel()

 tempFile := filepath.Join(t.TempDir(), "doesnotexist", "test.log")
 cmd := &serpent.Command{
 Use: "test",
 Handler: func(inv *serpent.Invocation) error {
 logger, closeLog, err := clilog.New(
 clilog.WithFilter("foo", "baz"),
 clilog.WithHuman(tempFile),
 clilog.WithVerbose(),
 ).Build(inv)
 if err != nil {
 return err
 }
 defer closeLog()
 logger.Error(inv.Context(), "you will never see this")
 return nil
 },
 }
 err := cmd.Invoke().Run()
 require.ErrorIs(t, err, fs.ErrNotExist)
 })
 }

 var (
Original file line number	Diff line number	Diff line change
Expand Up		@@ -4,11 +4,11 @@ import (
		"context"
		"fmt"
		"io"
		"os"
		"regexp"
		"strings"

		"golang.org/x/xerrors"
		"gopkg.in/natefinch/lumberjack.v2"

		"cdr.dev/slog"
		"cdr.dev/slog/sloggers/sloghuman"
Expand DownExpand Up		@@ -104,20 +104,21 @@ func (b Builder) Build(inv serpent.Invocation) (log slog.Logger, closeLog func
		addSinkIfProvided := func(sinkFn func(io.Writer) slog.Sink, loc string) error {
		switch loc {
		case "":

		case "/dev/stdout":
		sinks = append(sinks, sinkFn(inv.Stdout))

		case "/dev/stderr":
		sinks = append(sinks, sinkFn(inv.Stderr))

		default:
		fi, err := os.OpenFile(loc, os.O_WRONLY\|os.O_CREATE\|os.O_APPEND, 0o644)
		if err != nil {
		return xerrors.Errorf("open log file %q: %w", loc, err)
		logWriter := &lumberjack.Logger{
		Filename: loc,
		MaxSize: 5, // MB
		// Without this, rotated logs will never be deleted.
		MaxBackups: 1,
		}
		closers = append(closers,fi.Close)
		sinks = append(sinks, sinkFn(fi))
		closers = append(closers,logWriter.Close)
Copy link Contributor dannykoppingNov 8, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Does this give us the equivalent behaviour of`lumberjackWriteCloseFixer` in`cli/agent.go`? Copy link Member johnstcnNov 8, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I think we should probably re-use that same fix here, although the main reason for that seems to have been to reduce CI flakes due to racy writes after close. We probably won't be logging to a file in CI in the general case.
		sinks = append(sinks, sinkFn(logWriter))
		}
		return nil
		}
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -2,7 +2,6 @@ package clilog_test

		import (
		"encoding/json"
		"io/fs"
		"os"
		"path/filepath"
		"strings"
Expand DownExpand Up		@@ -145,30 +144,6 @@ func TestBuilder(t *testing.T) {
		assertLogsJSON(t, tempJSON, info, infoLog, warn, warnLog)
		})
		})

		t.Run("NotFound", func(t *testing.T) {
		t.Parallel()

		tempFile := filepath.Join(t.TempDir(), "doesnotexist", "test.log")
		cmd := &serpent.Command{
		Use: "test",
		Handler: func(inv *serpent.Invocation) error {
		logger, closeLog, err := clilog.New(
		clilog.WithFilter("foo", "baz"),
		clilog.WithHuman(tempFile),
		clilog.WithVerbose(),
		).Build(inv)
		if err != nil {
		return err
		}
		defer closeLog()
		logger.Error(inv.Context(), "you will never see this")
		return nil
		},
		}
		err := cmd.Invoke().Run()
		require.ErrorIs(t, err, fs.ErrNotExist)
		})
		}

		var (
Expand Down