From 5b06e32f330a37bc26ac959e2541c271a6966e82 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 20 Apr 2026 19:15:12 +0000 Subject: [PATCH] logtail: add Config.Disabled to suppress the startup banner NewLogger unconditionally writes a "logtail started" banner before it returns, which callers that later call Logger.SetEnabled(false) have no way to suppress: the banner is already buffered for upload by the time the caller gets the logger back. Add Config.Disabled so callers that know up front they want the logger to start disabled (e.g. Android's remote-logging opt-out) can seed the state before NewLogger's internal Write. The process- wide Disable kill switch still takes precedence; SetEnabled can still flip the state at runtime. Updates #13174 Updates tailscale/tailscale-android#695 Change-Id: Icc4fa88c198447cf0faa707264dac84e359fe52c Signed-off-by: Brad Fitzpatrick --- logtail/config.go | 8 +++++ logtail/logtail.go | 1 + logtail/logtail_test.go | 75 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/logtail/config.go b/logtail/config.go index c504047a3..0ee599905 100644 --- a/logtail/config.go +++ b/logtail/config.go @@ -64,4 +64,12 @@ type Config struct { // being included in the logs. The sequence number is incremented for each // log message sent, but is not persisted across process restarts. IncludeProcSequence bool + + // Disabled, if true, causes the returned [Logger] to start in the + // disabled state, dropping entries without buffering or uploading + // (equivalent to calling [Logger.SetEnabled] with false immediately). + // It applies before the internal startup banner is written, so no + // log entries are emitted until [Logger.SetEnabled] is called with + // true. The process-wide [Disable] kill switch still takes precedence. + Disabled bool } diff --git a/logtail/logtail.go b/logtail/logtail.go index 39a48ba79..a45f1bfe9 100644 --- a/logtail/logtail.go +++ b/logtail/logtail.go @@ -132,6 +132,7 @@ func NewLogger(cfg Config, logf tslogger.Logf) *Logger { } logger.SetSockstatsLabel(sockstats.LabelLogtailLogger) logger.compressLogs = cfg.CompressLogs + logger.disabled.Store(cfg.Disabled) ctx, cancel := context.WithCancel(context.Background()) logger.uploadCancel = cancel diff --git a/logtail/logtail_test.go b/logtail/logtail_test.go index eadbbc630..f1d0585f5 100644 --- a/logtail/logtail_test.go +++ b/logtail/logtail_test.go @@ -7,11 +7,16 @@ import ( "bytes" "context" "encoding/json" + "fmt" "io" + "net" "net/http" "net/http/httptest" + "os" "strings" + "sync" "testing" + "testing/synctest" "time" "github.com/go-json-experiment/json/jsontext" @@ -21,6 +26,23 @@ import ( "tailscale.com/util/must" ) +// TestMain installs a safety net that refuses non-localhost dials for any +// test in this package. Config.BaseURL defaults to https://log.tailscale.com +// and Config.HTTPC defaults to http.DefaultClient, so a test that forgets to +// override either can otherwise silently hit the real logtail server. +func TestMain(m *testing.M) { + tr := http.DefaultTransport.(*http.Transport) + orig := tr.DialContext + tr.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + host, _, err := net.SplitHostPort(addr) + if err == nil && (host == "127.0.0.1" || host == "::1" || host == "localhost") { + return orig(ctx, network, addr) + } + return nil, fmt.Errorf("logtail tests: refusing to dial non-localhost address %q; use httptest.Server or a custom Config.HTTPC", addr) + } + os.Exit(m.Run()) +} + func TestFastShutdown(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -321,6 +343,59 @@ func TestLoggerWriteResult(t *testing.T) { } } +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func TestNewLoggerDisabled(t *testing.T) { synctest.Test(t, synctestNewLoggerDisabled) } + +func synctestNewLoggerDisabled(t *testing.T) { + // When Config.Disabled is true, NewLogger must not emit the usual + // "logtail started" banner: the logger should start in the disabled + // state before the internal startup write, so nothing ever lands + // in the buffer for the upload goroutine to drain. + buf := NewMemoryBuffer(100) + + // Any HTTP attempt indicates the banner leaked into the buffer and + // the upload goroutine tried to ship it. Report it once (so the + // retry spin doesn't drown the log), then block on the request + // context so synctest.Wait sees a durable block and Shutdown's + // uploadCancel can unblock us cleanly. + var once sync.Once + httpc := &http.Client{ + Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) { + once.Do(func() { + t.Errorf("unexpected HTTP request while Disabled=true: %s", r.URL) + }) + <-r.Context().Done() + return nil, r.Context().Err() + }), + } + + logger := NewLogger(Config{ + BaseURL: "http://logtail.test.invalid", + HTTPC: httpc, + Bus: eventbustest.NewBus(t), + Buffer: buf, + Disabled: true, + }, t.Logf) + defer func() { + // Pass an already-cancelled context so Shutdown invokes + // uploadCancel immediately; otherwise on the regression path + // (Disabled=false) the upload goroutine stays in its retry + // loop and synctest.Test never returns. + ctx, cancel := context.WithCancel(context.Background()) + cancel() + logger.Shutdown(ctx) + }() + + synctest.Wait() + + if back, _ := buf.TryReadLine(); len(back) != 0 { + t.Errorf("Disabled logger buffered a startup entry: %q", back) + } +} + func TestLoggerSetEnabled(t *testing.T) { buf := NewMemoryBuffer(100) lg := &Logger{