From 02b5208b2dde61a732f0abf61ce57ef114797449 Mon Sep 17 00:00:00 2001 From: thijmv <59415467+thijmv@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:53:20 +0200 Subject: [PATCH] feat(agent): add secondary logger for high-severity diagnostics --- agent/cmd/cmd.go | 19 ++++++++++++++++++- agent/cmd/cmd_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ agent/cmd/config.go | 2 ++ utils/log/log.go | 8 +++++++- utils/log/log_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 utils/log/log_test.go diff --git a/agent/cmd/cmd.go b/agent/cmd/cmd.go index b887d6b45..cb9638d4c 100644 --- a/agent/cmd/cmd.go +++ b/agent/cmd/cmd.go @@ -39,6 +39,7 @@ import ( "github.com/uber/kraken/utils/log" "github.com/uber/kraken/utils/netutil" "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) // Flags defines agent CLI flags. @@ -141,7 +142,7 @@ func Run(flags *Flags, opts ...Option) { if overrides.logger != nil { log.SetGlobalLogger(overrides.logger.Sugar()) } else { - zlog := log.ConfigureLogger(config.ZapLogging) + zlog := log.ConfigureLogger(config.ZapLogging, withDiagnostics(config.Diagnostics)) defer func() { if err := zlog.Sync(); err != nil { fmt.Printf("Failed to sync logger: %s", err) @@ -288,6 +289,22 @@ func validateRequiredPorts(flags *Flags) { } } +// withDiagnostics tees the logger's logs to a second logger defined by its config. +func withDiagnostics(config log.Config) log.Option { + return func(logger *zap.Logger) *zap.Logger { + if config.Path == "" { + return logger + } + dlogger, err := log.New(config, nil) + if err != nil { + panic(err) + } + return logger.WithOptions(zap.WrapCore(func(c zapcore.Core) zapcore.Core { + return zapcore.NewTee(c, dlogger.Core()) + })) + } +} + // heartbeatTicker provides the minimal ticker contract required by heartbeat. type heartbeatTicker interface { Chan() <-chan time.Time diff --git a/agent/cmd/cmd_test.go b/agent/cmd/cmd_test.go index 9ae7d5c39..ad70f9dc0 100644 --- a/agent/cmd/cmd_test.go +++ b/agent/cmd/cmd_test.go @@ -14,7 +14,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/uber-go/tally" + "github.com/uber/kraken/utils/log" "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" ) func TestParseFlags(t *testing.T) { @@ -245,6 +248,45 @@ func TestValidateRequiredPorts(t *testing.T) { }) } } + +func TestWithDiagnostics(t *testing.T) { + t.Run("no-op when path is empty", func(t *testing.T) { + core, _ := observer.New(zapcore.DebugLevel) + base := zap.New(core) + + opt := withDiagnostics(log.Config{}) + logger := opt(base) + + require.Same(t, base, logger) + }) + + t.Run("tees logs to diagnostics file", func(t *testing.T) { + core, observed := observer.New(zapcore.DebugLevel) + base := zap.New(core) + + f := filepath.Join(t.TempDir(), "diagnostics.log") + config := log.Config{ + Path: f, + Level: zapcore.ErrorLevel, + Encoding: "json", + } + opt := withDiagnostics(config) + logger := opt(base) + + logger.Info("info message") + logger.Error("error message") + require.NoError(t, logger.Sync()) + + assert.Equal(t, 2, observed.Len()) + + data, err := os.ReadFile(f) + require.NoError(t, err) + content := string(data) + assert.Contains(t, content, "error message") + assert.NotContains(t, content, "info message") + }) +} + func TestHeartbeatWithTicker(t *testing.T) { scope := tally.NewTestScope("", nil) mockClock := clock.NewMock() diff --git a/agent/cmd/config.go b/agent/cmd/config.go index b767df341..615758766 100644 --- a/agent/cmd/config.go +++ b/agent/cmd/config.go @@ -26,6 +26,7 @@ import ( "github.com/uber/kraken/metrics" "github.com/uber/kraken/nginx" "github.com/uber/kraken/utils/httputil" + "github.com/uber/kraken/utils/log" "go.uber.org/zap" ) @@ -47,6 +48,7 @@ type Config struct { TLS httputil.TLSConfig `yaml:"tls"` AllowedCidrs []string `yaml:"allowed_cidrs"` ContainerRuntime containerruntime.Config `yaml:"container_runtime"` + Diagnostics log.Config `yaml:"diagnostics_log"` // Deprecated DockerDaemon dockerdaemon.Config `yaml:"docker_daemon"` diff --git a/utils/log/log.go b/utils/log/log.go index e31d1eafb..b296be9dd 100644 --- a/utils/log/log.go +++ b/utils/log/log.go @@ -37,8 +37,11 @@ func init() { ConfigureLogger(zapConfig) } +// Option defines an optional ConfigureLogger parameter. +type Option func(*zap.Logger) *zap.Logger + // ConfigureLogger configures a global zap logger instance. -func ConfigureLogger(zapConfig zap.Config) *zap.SugaredLogger { +func ConfigureLogger(zapConfig zap.Config, opts ...Option) *zap.SugaredLogger { logger, err := zapConfig.Build() if err != nil { panic(err) @@ -46,6 +49,9 @@ func ConfigureLogger(zapConfig zap.Config) *zap.SugaredLogger { // Skip this wrapper in a call stack. logger = logger.WithOptions(zap.AddCallerSkip(1)) + for _, opt := range opts { + logger = opt(logger) + } _default = logger.Sugar() return _default diff --git a/utils/log/log_test.go b/utils/log/log_test.go new file mode 100644 index 000000000..b715070e7 --- /dev/null +++ b/utils/log/log_test.go @@ -0,0 +1,38 @@ +// Copyright (c) 2016-2019 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package log + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/zap" +) + +func TestConfigureLoggerAppliesOption(t *testing.T) { + prevDefault := _default + defer func() { _default = prevDefault }() + + var received *zap.Logger + override := zap.NewNop() + opt := func(l *zap.Logger) *zap.Logger { + received = l + return override + } + + logger := ConfigureLogger(zap.NewProductionConfig(), opt) + + assert.NotNil(t, received) + assert.Equal(t, override.Sugar(), logger) +}