Skip to content

Commit

Permalink
change file setting semantics
Browse files Browse the repository at this point in the history
1. Always default logging to stderr.
2. allow users to overwrite output to stdout or a file.  However, to
   provide defaults, this setting is unexported and has no hooks for
   globally declared files or env varibles.  Users must instead pass a
   file to the settings struct via a method instead.

This PR is to solve concerns about the capacity for discrete logging
files to appear on the same machine when the intent is for
initialization
to use the same file every time.  It requires users to be more aware of
the file locations they're working with in order to set up clog
correctly.
  • Loading branch information
ryanfkeepers committed Nov 7, 2024
1 parent d74116c commit e45ccce
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 80 deletions.
13 changes: 10 additions & 3 deletions clog/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,18 @@ func genLogger(set Settings) *zap.SugaredLogger {
}
)

// set the file handling
toFile := Stderr

if len(set.fileOverride) > 0 {
toFile = set.fileOverride
}

switch set.Format {
// JSON means each row should appear as a single json object.
case FormatToJSON:
zcfg = setLevel(zap.NewProductionConfig(), set.Level)
zcfg.OutputPaths = []string{set.File}
zcfg.OutputPaths = []string{toFile}
// by default we'll use the columnar non-json format, which uses tab
// separated values within each line, and may contain multiple json objs.
default:
Expand All @@ -65,12 +72,12 @@ func genLogger(set Settings) *zap.SugaredLogger {
zcfg.EncoderConfig.EncodeTime = zapcore.TimeEncoderOfLayout(time.StampMilli)

// when printing to stdout/stderr, colorize things!
if set.File == Stderr || set.File == Stdout {
if toFile == Stderr || toFile == Stdout {
zcfg.EncoderConfig.EncodeLevel = zapcore.CapitalColorLevelEncoder
}
}

zcfg.OutputPaths = []string{set.File}
zcfg.OutputPaths = []string{toFile}

zlog, err := zcfg.Build(zopts...)
if err != nil {
Expand Down
2 changes: 0 additions & 2 deletions clog/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@ func (suite *LoggerUnitSuite) TestSettings_ensureDefaults() {
t := suite.T()

s := clog.Settings{}
require.Empty(t, s.File, "file")
require.Empty(t, s.Level, "level")
require.Empty(t, s.Format, "format")
require.Empty(t, s.SensitiveInfoHandling, "piialg")
require.Empty(t, s.OnlyLogDebugIfContainsLabel, "debug filter")

s = s.EnsureDefaults()
require.NotEmpty(t, s.File, "file")
require.NotEmpty(t, s.Level, "level")
require.NotEmpty(t, s.Format, "format")
require.NotEmpty(t, s.SensitiveInfoHandling, "piialg")
Expand Down
112 changes: 37 additions & 75 deletions clog/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package clog
import (
"os"
"path/filepath"
"time"

"golang.org/x/exp/slices"

Expand Down Expand Up @@ -51,25 +50,20 @@ const (
// configuration
// ---------------------------------------------------

// Default location for writing log files.
var defaultLogFileDir = filepath.Join(os.Getenv("HOME"), "Library", "Logs")

// ResolvedLogFile is the first log file established by the caller.
// It gets eagerly populated on the first act of ensuring settings
// defaults, which normally occurs during the Init call.
//
// If Init gets called more than once, or different settings are
// ensured, it's possible to override this value by manually specifying
// the log file in the settings used for that action. But if no file
// is provided, the default will fall back to this resolved file first.
var ResolvedLogFile string

// Settings records the user's preferred logging settings.
type Settings struct {
// core settings
File string // what file to log to (alt: stderr, stdout)
Format logFormat // whether to format as text (console) or json (cloud)
Level logLevel // what level to log at
// the log file isn't exposed to end users because we
// want to ensure a default of StdErr until they call
// one of the file override hooks.
fileOverride string

// Format defines the output structure, standard design is
// as text (human-at-a-console) or json (automation).
Format logFormat
// Level determines the minimum logging level. Anything
// below this level (following standard semantics) will
// not get logged.
Level logLevel

// more fiddly bits
SensitiveInfoHandling sensitiveInfoHandlingAlgo // how to obscure pii
Expand All @@ -80,6 +74,31 @@ type Settings struct {
OnlyLogDebugIfContainsLabel []string
}

// LogToStdOut swaps the log output from Stderr to Stdout.
func (s Settings) LogToStdOut() Settings {
s.fileOverride = Stdout
return s
}

// LogToFile defines a system file to write all logs onto.
func (s Settings) LogToFile(pathToFile string) (Settings, error) {
if len(pathToFile) == 0 {
return s, clues.New("missing filepath for logging")

Check failure on line 86 in clog/settings.go

View workflow job for this annotation

GitHub Actions / build

undefined: clues
}

logdir := filepath.Dir(pathToFile)

err := os.MkdirAll(logdir, 0o755)
if err != nil {
return s, clues.Wrap(err, "ensuring log file dir exists").

Check failure on line 93 in clog/settings.go

View workflow job for this annotation

GitHub Actions / build

undefined: clues
With("log_dir", logdir)
}

s.fileOverride = pathToFile

return s, nil
}

// EnsureDefaults sets any non-populated settings to their default value.
// exported for testing without circular dependencies.
func (s Settings) EnsureDefaults() Settings {
Expand All @@ -100,66 +119,9 @@ func (s Settings) EnsureDefaults() Settings {
set.SensitiveInfoHandling = ShowSensitiveInfoInPlainText
}

if len(set.File) == 0 {
set.File = GetLogFileOrDefault("")
}

if len(ResolvedLogFile) == 0 {
ResolvedLogFile = set.File
}

return set
}

// Returns the default location for log file storage.
func defaultLogLocation() string {
return filepath.Join(
defaultLogFileDir,
"clog",
time.Now().UTC().Format("2006-01-02T15-04-05Z")+".log")
}

// GetLogFileOrDefault finds the log file in the users local system.
// Uses the env var declaration, if populated, else defaults to stderr.
// If this has already been called once before, uses the result of that
// prior call.
func GetLogFileOrDefault(useThisFile string) string {
if len(ResolvedLogFile) > 0 {
return ResolvedLogFile
}

// start by preferring the file given to us by the caller.
r := useThisFile

// if no file was provided, look for a configured location using
// the default ENV.
if len(r) == 0 {
r = os.Getenv(clogLogFileEnv)
}

// if no file was provided, fall back to the default file location.
if len(r) == 0 {
r = defaultLogLocation()
}

// direct to Stdout if provided '-'.
if r == "-" {
r = Stdout
}

// if outputting to a file, make sure we can access the file.
if r != Stdout && r != Stderr {
logdir := filepath.Dir(r)

err := os.MkdirAll(logdir, 0o755)
if err != nil {
return Stderr
}
}

return r
}

func setCluesSecretsHash(alg sensitiveInfoHandlingAlgo) {
switch alg {
case HashSensitiveInfo:
Expand Down
47 changes: 47 additions & 0 deletions clog/settings_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package clog

import (
"path/filepath"
"testing"

"github.com/alcionai/clues"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestSettings_LogToFile(t *testing.T) {
tempDir := t.TempDir()

table := []struct {
name string
input string
expectErr require.ErrorAssertionFunc
expectOverride string
}{
{
name: "empty",
input: "",
expectErr: require.Error,
expectOverride: "",
},
{
name: "doesn't exist",
input: filepath.Join(tempDir, "foo", "bar", "baz", "log.log"),
expectErr: require.NoError,
expectOverride: filepath.Join(tempDir, "foo", "bar", "baz", "log.log"),
},
{
name: "exists",
input: filepath.Join(tempDir, "log.log"),
expectErr: require.NoError,
expectOverride: filepath.Join(tempDir, "log.log"),
},
}
for _, test := range table {
t.Run(test.name, func(t *testing.T) {
set, err := Settings{}.LogToFile(test.input)
test.expectErr(t, err, clues.ToCore(err))
assert.Equal(t, test.expectOverride, set.fileOverride)
})
}
}

0 comments on commit e45ccce

Please sign in to comment.