Pull request 60: 3 darwin syslog

Updates #3. Squashed commit of the following: commit d7f07af Author: Eugene Burkov <[email protected]> Date: Thu Jul 18 14:48:34 2024 +0300 agdcslog: imp code commit 4c4abd7 Author: Eugene Burkov <[email protected]> Date: Wed Jul 17 19:45:27 2024 +0300 agdcslog: fix panic commit 3b2c62a Author: Eugene Burkov <[email protected]> Date: Wed Jul 17 19:34:37 2024 +0300 agdcslog: imp code, docs commit 39a9e61 Author: Eugene Burkov <[email protected]> Date: Wed Jul 17 17:48:34 2024 +0300 all: implement piped logger on darwin
AdguardTeam · Jul 22, 2024 · a51b997 · a51b997
1 parent f259bcd
commit a51b997
Show file tree

Hide file tree

Showing 8 changed files with 348 additions and 73 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,12 +30,16 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 ### Fixed
 
+- The `syslog` log output on macOS ([#3]).
+
+  **NOTE:** The implementation is actually a workaround for a known [Go issue][go-59229], and uses the `/usr/bin/logger` utility. This approach is suboptimal and will be improved once the Go issue is resolved.
 - DNS proxy logs being written to `stderr` instead of `log.output` ([#1]).
 
 [#1]: https://github.com/AdguardTeam/AdGuardDNSClient/issues/1
 [#2]: https://github.com/AdguardTeam/AdGuardDNSClient/issues/2
 
 [go-1.22.5]: https://groups.google.com/g/golang-announce/c/gyb7aM1C9H4
+[go-59229]:  https://github.com/golang/go/issues/59229
 
 <!--
 NOTE: Add new changes ABOVE THIS COMMENT.

diff --git a/internal/agdcslog/agdcslog.go b/internal/agdcslog/agdcslog.go
@@ -40,33 +40,18 @@ func (h *bufferedTextHandler) reset() {
 	h.buffer.Reset()
 }
 
-// Logger is a platform-specific system Logger.
-type Logger interface {
-	Debug(msg string) (err error)
-	Info(msg string) (err error)
-	Warning(msg string) (err error)
-	Error(msg string) (err error)
-	Close() (err error)
-}
-
-// NewSystemLogger returns a platform-specific system logger that writes to
-// system log.  name is the service name.
-func NewSystemLogger(name string) (l Logger, err error) {
-	return newSystemLogger(name)
-}
-
-// SystemHandler is a [slog.Handler] that writes to system log.
-type SystemHandler struct {
-	logger      Logger
+// SyslogHandler is a [slog.Handler] that writes to system log.
+type SyslogHandler struct {
+	logger      SystemLogger
 	level       slog.Leveler
 	bufTextPool *syncutil.Pool[bufferedTextHandler]
 	attrs       []slog.Attr
 }
 
-// NewSystemHandler returns an initialized SystemHandler that writes to system
+// NewSyslogHandler returns an initialized SyslogHandler that writes to system
 // log.  opts must not be nil and contain Level.
-func NewSystemHandler(logger Logger, opts *slog.HandlerOptions) (h *SystemHandler) {
-	return &SystemHandler{
+func NewSyslogHandler(logger SystemLogger, opts *slog.HandlerOptions) (h *SyslogHandler) {
+	return &SyslogHandler{
 		logger: logger,
 		level:  opts.Level,
 		bufTextPool: syncutil.NewPool(func() (bufTextHdlr *bufferedTextHandler) {
@@ -77,15 +62,15 @@ func NewSystemHandler(logger Logger, opts *slog.HandlerOptions) (h *SystemHandle
 }
 
 // type check
-var _ slog.Handler = (*SystemHandler)(nil)
+var _ slog.Handler = (*SyslogHandler)(nil)
 
-// Enabled implements the [slog.Handler] interface for *SystemHandler.
-func (h *SystemHandler) Enabled(_ context.Context, level slog.Level) (enabled bool) {
+// Enabled implements the [slog.Handler] interface for *SyslogHandler.
+func (h *SyslogHandler) Enabled(_ context.Context, level slog.Level) (enabled bool) {
 	return level >= h.level.Level()
 }
 
-// Handle implements the [slog.Handler] interface for *SystemHandler.
-func (h *SystemHandler) Handle(ctx context.Context, rec slog.Record) (err error) {
+// Handle implements the [slog.Handler] interface for *SyslogHandler.
+func (h *SyslogHandler) Handle(ctx context.Context, rec slog.Record) (err error) {
 	bufTextHdlr := h.bufTextPool.Get()
 	defer h.bufTextPool.Put(bufTextHdlr)
 
@@ -126,22 +111,22 @@ func (h *SystemHandler) Handle(ctx context.Context, rec slog.Record) (err error)
 	return err
 }
 
-// WithAttrs implements the [slog.Handler] interface for *SystemHandler.
-func (h *SystemHandler) WithAttrs(attrs []slog.Attr) (handler slog.Handler) {
-	return &SystemHandler{
+// WithAttrs implements the [slog.Handler] interface for *SyslogHandler.
+func (h *SyslogHandler) WithAttrs(attrs []slog.Attr) (handler slog.Handler) {
+	return &SyslogHandler{
 		logger:      h.logger,
 		level:       h.level,
 		bufTextPool: h.bufTextPool,
 		attrs:       append(slices.Clip(h.attrs), attrs...),
 	}
 }
 
-// WithGroup implements the [slog.Handler] interface for *SystemHandler.
-func (h *SystemHandler) WithGroup(name string) (handler slog.Handler) {
+// WithGroup implements the [slog.Handler] interface for *SyslogHandler.
+func (h *SyslogHandler) WithGroup(name string) (handler slog.Handler) {
 	return h
 }
 
 // Close closes an underlying system logger.
-func (h *SystemHandler) Close() (err error) {
+func (h *SyslogHandler) Close() (err error) {
 	return h.logger.Close()
 }
diff --git a/internal/agdcslog/agdcslog_test.go b/internal/agdcslog/agdcslog_test.go
@@ -14,7 +14,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// mockLogger is a mock implementation of [agdcslog.Logger] interface for tests.
+// mockLogger is a mock implementation of [agdcslog.SystemLogger] interface for tests.
 type mockLogger struct {
 	onDebug   func(msg string) (err error)
 	onInfo    func(msg string) (err error)
@@ -43,34 +43,34 @@ func NewMockLogger() (l *mockLogger) {
 }
 
 // type check
-var _ agdcslog.Logger = (*mockLogger)(nil)
+var _ agdcslog.SystemLogger = (*mockLogger)(nil)
 
-// Debug implements [agdcslog.Logger] interface for *mockLogger.
+// Debug implements [agdcslog.SystemLogger] interface for *mockLogger.
 func (l *mockLogger) Debug(msg string) (err error) {
 	return l.onDebug(msg)
 }
 
-// Info implements [agdcslog.Logger] interface for *mockLogger.
+// Info implements [agdcslog.SystemLogger] interface for *mockLogger.
 func (l *mockLogger) Info(msg string) (err error) {
 	return l.onInfo(msg)
 }
 
-// Warning implements [agdcslog.Logger] interface for *mockLogger.
+// Warning implements [agdcslog.SystemLogger] interface for *mockLogger.
 func (l *mockLogger) Warning(msg string) (err error) {
 	return l.onWarning(msg)
 }
 
-// Error implements [agdcslog.Logger] interface for *mockLogger.
+// Error implements [agdcslog.SystemLogger] interface for *mockLogger.
 func (l *mockLogger) Error(msg string) (err error) {
 	return l.onError(msg)
 }
 
-// Close implements [agdcslog.Logger] interface for *mockLogger.
+// Close implements [agdcslog.SystemLogger] interface for *mockLogger.
 func (l *mockLogger) Close() (err error) {
 	return l.onClose()
 }
 
-func TestSystemHandler_Handle(t *testing.T) {
+func TestSyslogHandler_Handle(t *testing.T) {
 	var (
 		mu     = sync.Mutex{}
 		output = &bytes.Buffer{}
@@ -91,7 +91,7 @@ func TestSystemHandler_Handle(t *testing.T) {
 	l.onError = outputWrite
 	l.onDebug = outputWrite
 
-	handler := agdcslog.NewSystemHandler(l, &slog.HandlerOptions{
+	handler := agdcslog.NewSyslogHandler(l, &slog.HandlerOptions{
 		Level: slog.LevelDebug,
 	})
 
@@ -153,7 +153,7 @@ func TestSystemHandler_Handle(t *testing.T) {
 	}
 }
 
-func TestSystemHandler_Handle_race(t *testing.T) {
+func TestSyslogHandler_Handle_race(t *testing.T) {
 	var (
 		mu     = sync.Mutex{}
 		output = &bytes.Buffer{}
@@ -169,7 +169,7 @@ func TestSystemHandler_Handle_race(t *testing.T) {
 		return nil
 	}
 
-	h := agdcslog.NewSystemHandler(l, &slog.HandlerOptions{
+	h := agdcslog.NewSyslogHandler(l, &slog.HandlerOptions{
 		Level: slog.LevelDebug,
 	})
 
@@ -182,11 +182,11 @@ func TestSystemHandler_Handle_race(t *testing.T) {
 	for i := 0; i < numGoroutine; i++ {
 		wg.Add(1)
 
-		go func(i int) {
+		go func() {
 			defer wg.Done()
 
 			logger.Info("test message", "attr", "abc")
-		}(i)
+		}()
 	}
 
 	wg.Wait()
@@ -203,11 +203,11 @@ func TestSystemHandler_Handle_race(t *testing.T) {
 // errSink is a sink for benchmark results.
 var errSink error
 
-func BenchmarkSystemHandler_Handle(b *testing.B) {
+func BenchmarkSyslogHandler_Handle(b *testing.B) {
 	l := NewMockLogger()
 	l.onInfo = func(_ string) (_ error) { return nil }
 
-	h := agdcslog.NewSystemHandler(l, &slog.HandlerOptions{
+	h := agdcslog.NewSyslogHandler(l, &slog.HandlerOptions{
 		Level: slog.LevelDebug,
 	})
 
@@ -226,10 +226,9 @@ func BenchmarkSystemHandler_Handle(b *testing.B) {
 
 	require.NoError(b, errSink)
 
-	// Most recent result, on a ThinkPad P15s with a Intel Core i7-10510U CPU:
-	//	goos: linux
-	//	goarch: amd64
-	//	pkg: github.com/AdguardTeam/AdGuardDNSClient/internal/agdcslog
-	//	cpu: Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
-	//	BenchmarkSystemHandler_Handle-8   	 2595381	       448.2 ns/op	      64 B/op	       1 allocs/op
+	// goos: darwin
+	// goarch: amd64
+	// pkg: github.com/AdguardTeam/AdGuardDNSClient/internal/agdcslog
+	// cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
+	// BenchmarkSyslogHandler_Handle-12		2365461		501.1 ns/op		64 B/op		1 allocs/op
 }
diff --git a/internal/agdcslog/syslog.go b/internal/agdcslog/syslog.go
@@ -0,0 +1,29 @@
+package agdcslog
+
+import "io"
+
+// SystemLogger is a platform-specific system logger.
+//
+// TODO(e.burkov):  Consider moving to golibs.
+type SystemLogger interface {
+	// Debug logs a message at debug level.
+	Debug(msg string) (err error)
+
+	// Info logs a message at info level.
+	Info(msg string) (err error)
+
+	// Warning logs a message at warning level.
+	Warning(msg string) (err error)
+
+	// Error logs a message at error level.
+	Error(msg string) (err error)
+
+	// Close detaches from the system logger.
+	io.Closer
+}
+
+// NewSystemLogger returns a platform-specific system logger.  name is the
+// name of service.
+func NewSystemLogger(name string) (l SystemLogger, err error) {
+	return newSystemLogger(name)
+}