Commit 40a41790 authored by Elliot Forbes's avatar Elliot Forbes 2️⃣
Browse files

feat: allows the configuration of the log level on the v2 log pkg

This is required to allow services to control what level they
should be logging out.

I've also taken the opportunity to update some of the tests to
make them cleaner and easier to read.
parent e2527359
Loading
Loading
Loading
Loading
Loading
+13 −7
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ type Config struct {
	Writer        io.Writer
	UseTextFormat bool
	Clock         TimeFunc
	LogLevel      slog.Level
}

// New - a handy wrapper that configures the slog.Logger in a consistent
@@ -36,6 +37,9 @@ func NewWithConfig(cfg *Config) *slog.Logger {
		UseTextFormat: false,
		// Clock intentially nil by default - uses record's original timestamp
	}
	// if we have a passed cfg struct, then
	// we want to update the config struct to ensure
	// we honour those settings.
	if cfg != nil {
		if cfg.Writer != nil {
			config.Writer = cfg.Writer
@@ -43,10 +47,10 @@ func NewWithConfig(cfg *Config) *slog.Logger {
		if cfg.UseTextFormat {
			config.UseTextFormat = cfg.UseTextFormat
		}

		if cfg.Clock != nil {
			config.Clock = cfg.Clock
		}
		config.LogLevel = cfg.LogLevel
	}

	var baseHandler slog.Handler
@@ -55,13 +59,13 @@ func NewWithConfig(cfg *Config) *slog.Logger {
		baseHandler = slog.NewTextHandler(
			config.Writer,
			defaultHandlerOpts(
				config.Clock,
				config,
			))
	default:
		baseHandler = slog.NewJSONHandler(
			config.Writer,
			defaultHandlerOpts(
				config.Clock,
				config,
			),
		)
	}
@@ -69,14 +73,16 @@ func NewWithConfig(cfg *Config) *slog.Logger {
	return slog.New(NewContextHandler(baseHandler))
}

func defaultHandlerOpts(clockFunc TimeFunc) *slog.HandlerOptions {
	opts := &slog.HandlerOptions{}
func defaultHandlerOpts(cfg *Config) *slog.HandlerOptions {
	opts := &slog.HandlerOptions{
		Level: cfg.LogLevel,
	}
	opts.ReplaceAttr = func(groups []string, a slog.Attr) slog.Attr {
		if a.Key == slog.TimeKey && len(groups) == 0 {
			if a.Value.Kind() == slog.KindTime {
				t := a.Value.Time()
				if clockFunc != nil {
					t = clockFunc()
				if cfg.Clock != nil {
					t = cfg.Clock()
				}
				a.Value = slog.AnyValue(t.UTC().Format(time.RFC3339))
			}
+75 −23
Original line number Diff line number Diff line
@@ -23,12 +23,8 @@ type logRecord struct {
}

func TestFileWriting(t *testing.T) {
	testFile := "test.out"
	ensureFileDoesNotExist(t, testFile)
	t.Cleanup(func() { os.Remove(testFile) })
	f, err := os.Create(testFile)
	assert.Nil(t, err)
	defer f.Close()
	f, tidyUp := createTestLogFile(t)
	defer tidyUp(t)

	clockFunc := func() time.Time { return time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC) }
	logger := NewWithConfig(&Config{
@@ -40,7 +36,7 @@ func TestFileWriting(t *testing.T) {
		logger.Info("hello")
		require.NoError(t, f.Sync())

		records := readLogFile(t, testFile)
		records := readLogFile(t, f.Name())
		require.Equal(t, logRecord{
			Msg:   "hello",
			Level: "INFO",
@@ -54,7 +50,7 @@ func TestFileWriting(t *testing.T) {
		logger.InfoContext(ctx, "canonical_log")
		require.NoError(t, f.Sync())

		records := readLogFile(t, testFile)
		records := readLogFile(t, f.Name())
		require.Equal(t, logRecord{
			Msg:   "canonical_log",
			Level: "INFO",
@@ -66,13 +62,8 @@ func TestFileWriting(t *testing.T) {
}

func TestDefaultClockUsesRecordTimestamp(t *testing.T) {
	testFile := "test_no_clock.out"
	ensureFileDoesNotExist(t, testFile)
	t.Cleanup(func() { os.Remove(testFile) })

	f, err := os.Create(testFile)
	require.NoError(t, err)
	defer f.Close()
	f, tidyUp := createTestLogFile(t)
	defer tidyUp(t)

	// No Clock provided - should use record's original timestamp
	logger := NewWithConfig(&Config{
@@ -84,7 +75,7 @@ func TestDefaultClockUsesRecordTimestamp(t *testing.T) {
	require.NoError(t, f.Sync())
	after := time.Now().UTC()

	records := readLogFile(t, testFile)
	records := readLogFile(t, f.Name())
	require.Len(t, records, 1)

	// Timestamp should be between before and after (no drift)
@@ -97,13 +88,6 @@ func TestDefaultClockUsesRecordTimestamp(t *testing.T) {
	require.Equal(t, time.UTC, records[0].Time.Location())
}

func ensureFileDoesNotExist(t *testing.T, path string) {
	t.Helper()
	if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
		t.Fatalf("failed to remove file %s: %v", path, err)
	}
}

func readLogFile(t *testing.T, filename string) []logRecord {
	t.Helper()

@@ -134,3 +118,71 @@ func TestNoopLogging(t *testing.T) {
	})
	logger.Info("oh no")
}

func TestLogLevelSetting(t *testing.T) {
	t.Run("can set error level and info wont log", func(t *testing.T) {
		f, tidyUp := createTestLogFile(t)
		defer tidyUp(t)

		logger := NewWithConfig(&Config{
			Writer:   f,
			LogLevel: slog.LevelError,
		})

		logger.Info("shouldn't log")
		logRecord := readTextFile(t, f.Name())
		assert.Empty(t, logRecord)
	})

	t.Run("setting to info level and it should log", func(t *testing.T) {
		f, tidyUp := createTestLogFile(t)
		defer tidyUp(t)

		logger := NewWithConfig(&Config{
			Writer:   f,
			LogLevel: slog.LevelInfo,
		})

		logger.Info("should log")
		logRecord := readTextFile(t, f.Name())
		assert.NotEmpty(t, logRecord)
		// tests that this is a JSON formatted string
		// this is a bit hacky, but it gives us enough signal
		assert.Contains(t, logRecord, `"msg":"should log"`)
	})

	t.Run("default method of not passing still works", func(t *testing.T) {
		f, tidyUp := createTestLogFile(t)
		defer tidyUp(t)

		logger := NewWithConfig(&Config{
			Writer: f,
		})

		logger.Info("should log")
		logRecord := readTextFile(t, f.Name())
		assert.NotEmpty(t, logRecord)
		// tests that this is a JSON formatted string
		// this is a bit hacky, but it gives us enough signal
		assert.Contains(t, logRecord, `"msg":"should log"`)
	})
}

func createTestLogFile(t *testing.T) (*os.File, func(t *testing.T)) {
	t.Helper()
	file, err := os.CreateTemp("", "log.*.out")
	assert.Nil(t, err)
	return file, func(t *testing.T) {
		t.Helper()
		file.Close()
		os.Remove(file.Name())
	}
}

func readTextFile(t *testing.T, filename string) string {
	t.Helper()

	data, err := os.ReadFile(filename)
	assert.Nil(t, err)
	return string(data)
}