From d97ee121791b2dad411e566eab8942ee5a785bb1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 21 Dec 2020 09:03:39 -0800 Subject: [PATCH] logtail, logpolicy: remove an unidiomatic use of an interface --- logpolicy/logpolicy.go | 4 +- logtail/example/logtail/logtail.go | 2 +- logtail/logtail.go | 76 +++++++++++++----------------- logtail/logtail_test.go | 6 +-- 4 files changed, 40 insertions(+), 48 deletions(-) diff --git a/logpolicy/logpolicy.go b/logpolicy/logpolicy.go index fba389e15..6e6150c88 100644 --- a/logpolicy/logpolicy.go +++ b/logpolicy/logpolicy.go @@ -49,7 +49,7 @@ type Config struct { // Policy is a logger and its public ID. type Policy struct { // Logtail is the logger. - Logtail logtail.Logger + Logtail *logtail.Logger // PublicID is the logger's instance identifier. PublicID logtail.PublicID } @@ -391,7 +391,7 @@ func New(collection string) *Policy { if filchBuf != nil { c.Buffer = filchBuf } - lw := logtail.Log(c, log.Printf) + lw := logtail.NewLogger(c, log.Printf) log.SetFlags(0) // other logflags are set on console, not here log.SetOutput(lw) diff --git a/logtail/example/logtail/logtail.go b/logtail/example/logtail/logtail.go index b2b9d92a3..46ae9426b 100644 --- a/logtail/example/logtail/logtail.go +++ b/logtail/example/logtail/logtail.go @@ -31,7 +31,7 @@ func main() { log.Fatalf("logtail: bad -privateid: %v", err) } - logger := logtail.Log(logtail.Config{ + logger := logtail.NewLogger(logtail.Config{ Collection: *collection, PrivateID: id, }, log.Printf) diff --git a/logtail/logtail.go b/logtail/logtail.go index b9c94e98f..8e16e71bd 100644 --- a/logtail/logtail.go +++ b/logtail/logtail.go @@ -25,34 +25,6 @@ // Config.BaseURL isn't provided. const DefaultHost = "log.tailscale.io" -type Logger interface { - // Write logs an encoded JSON blob. - // - // If the []byte passed to Write is not an encoded JSON blob, - // then contents is fit into a JSON blob and written. - // - // This is intended as an interface for the stdlib "log" package. - Write([]byte) (int, error) - - // Flush uploads all logs to the server. - // It blocks until complete or there is an unrecoverable error. - Flush() error - - // Shutdown gracefully shuts down the logger while completing any - // remaining uploads. - // - // It will block, continuing to try and upload unless the passed - // context object interrupts it by being done. - // If the shutdown is interrupted, an error is returned. - Shutdown(context.Context) error - - // Close shuts down this logger object, the background log uploader - // process, and any associated goroutines. - // - // DEPRECATED: use Shutdown - Close() -} - type Encoder interface { EncodeAll(src, dst []byte) []byte Close() error @@ -75,7 +47,7 @@ type Config struct { DrainLogs <-chan struct{} } -func Log(cfg Config, logf tslogger.Logf) Logger { +func NewLogger(cfg Config, logf tslogger.Logf) *Logger { if cfg.BaseURL == "" { cfg.BaseURL = "https://" + DefaultHost } @@ -95,7 +67,7 @@ func Log(cfg Config, logf tslogger.Logf) Logger { } cfg.Buffer = NewMemoryBuffer(pendingSize) } - l := &logger{ + l := &Logger{ stderr: cfg.Stderr, httpc: cfg.HTTPC, url: cfg.BaseURL + "/c/" + cfg.Collection + "/" + cfg.PrivateID.String(), @@ -123,7 +95,9 @@ func Log(cfg Config, logf tslogger.Logf) Logger { return l } -type logger struct { +// Logger writes logs, splitting them as configured between local +// logging facilities and uploading to a log server. +type Logger struct { stderr io.Writer httpc *http.Client url string @@ -142,7 +116,13 @@ type logger struct { shutdownDone chan struct{} // closd when shutdown complete } -func (l *logger) Shutdown(ctx context.Context) error { +// Shutdown gracefully shuts down the logger while completing any +// remaining uploads. +// +// It will block, continuing to try and upload unless the passed +// context object interrupts it by being done. +// If the shutdown is interrupted, an error is returned. +func (l *Logger) Shutdown(ctx context.Context) error { done := make(chan struct{}) go func() { select { @@ -164,7 +144,11 @@ func (l *logger) Shutdown(ctx context.Context) error { return nil } -func (l *logger) Close() { +// Close shuts down this logger object, the background log uploader +// process, and any associated goroutines. +// +// Deprecated: use Shutdown +func (l *Logger) Close() { l.Shutdown(context.Background()) } @@ -175,7 +159,7 @@ func (l *logger) Close() { // // If the caller provides a DrainLogs channel, then unblock-drain-on-Write // is disabled, and it is up to the caller to trigger unblock the drain. -func (l *logger) drainBlock() (shuttingDown bool) { +func (l *Logger) drainBlock() (shuttingDown bool) { if l.drainLogs == nil { select { case <-l.shutdownStart: @@ -194,7 +178,7 @@ func (l *logger) drainBlock() (shuttingDown bool) { // drainPending drains and encodes a batch of logs from the buffer for upload. // If no logs are available, drainPending blocks until logs are available. -func (l *logger) drainPending() (res []byte) { +func (l *Logger) drainPending() (res []byte) { buf := new(bytes.Buffer) entries := 0 @@ -255,7 +239,7 @@ func (l *logger) drainPending() (res []byte) { } // This is the goroutine that repeatedly uploads logs in the background. -func (l *logger) uploading(ctx context.Context) { +func (l *Logger) uploading(ctx context.Context) { defer close(l.shutdownDone) for { @@ -300,7 +284,7 @@ func (l *logger) uploading(ctx context.Context) { // upload uploads body to the log server. // origlen indicates the pre-compression body length. // origlen of -1 indicates that the body is not compressed. -func (l *logger) upload(ctx context.Context, body []byte, origlen int) (uploaded bool, err error) { +func (l *Logger) upload(ctx context.Context, body []byte, origlen int) (uploaded bool, err error) { req, err := http.NewRequest("POST", l.url, bytes.NewReader(body)) if err != nil { // I know of no conditions under which this could fail. @@ -347,11 +331,13 @@ func (l *logger) upload(ctx context.Context, body []byte, origlen int) (uploaded return true, nil } -func (l *logger) Flush() error { +// Flush uploads all logs to the server. +// It blocks until complete or there is an unrecoverable error. +func (l *Logger) Flush() error { return nil } -func (l *logger) send(jsonBlob []byte) (int, error) { +func (l *Logger) send(jsonBlob []byte) (int, error) { n, err := l.buffer.Write(jsonBlob) if l.drainLogs == nil { select { @@ -364,7 +350,7 @@ func (l *logger) send(jsonBlob []byte) (int, error) { // TODO: instead of allocating, this should probably just append // directly into the output log buffer. -func (l *logger) encodeText(buf []byte, skipClientTime bool) []byte { +func (l *Logger) encodeText(buf []byte, skipClientTime bool) []byte { now := l.timeNow() // Factor in JSON encoding overhead to try to only do one alloc @@ -420,7 +406,7 @@ func (l *logger) encodeText(buf []byte, skipClientTime bool) []byte { return b } -func (l *logger) encode(buf []byte) []byte { +func (l *Logger) encode(buf []byte) []byte { if buf[0] != '{' { return l.encodeText(buf, l.skipClientTime) // text fast-path } @@ -461,7 +447,13 @@ func (l *logger) encode(buf []byte) []byte { return b } -func (l *logger) Write(buf []byte) (int, error) { +// Write logs an encoded JSON blob. +// +// If the []byte passed to Write is not an encoded JSON blob, +// then contents is fit into a JSON blob and written. +// +// This is intended as an interface for the stdlib "log" package. +func (l *Logger) Write(buf []byte) (int, error) { if len(buf) == 0 { return 0, nil } diff --git a/logtail/logtail_test.go b/logtail/logtail_test.go index 76f91127b..5f435a8cd 100644 --- a/logtail/logtail_test.go +++ b/logtail/logtail_test.go @@ -14,7 +14,7 @@ func TestFastShutdown(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - l := Log(Config{ + l := NewLogger(Config{ BaseURL: "http://localhost:1234", }, t.Logf) l.Shutdown(ctx) @@ -23,7 +23,7 @@ func TestFastShutdown(t *testing.T) { var sink []byte func TestLoggerEncodeTextAllocs(t *testing.T) { - lg := &logger{timeNow: time.Now} + lg := &Logger{timeNow: time.Now} inBuf := []byte("some text to encode") n := testing.AllocsPerRun(1000, func() { sink = lg.encodeText(inBuf, false) @@ -34,7 +34,7 @@ func TestLoggerEncodeTextAllocs(t *testing.T) { } func TestLoggerWriteLength(t *testing.T) { - lg := &logger{ + lg := &Logger{ timeNow: time.Now, buffer: NewMemoryBuffer(1024), }