From a01f9396926fa8cb64630011515b38761cfd0fb4 Mon Sep 17 00:00:00 2001 From: slash Date: Fri, 29 May 2026 18:55:21 +0530 Subject: [PATCH 1/9] feat(go-sdk): add configurable logger interface and integrate throughout SDK --- foreign/go/client/iggy_client.go | 24 +++++- foreign/go/client/tcp/tcp_core.go | 16 ++++ .../go/client/tcp/tcp_session_management.go | 1 + foreign/go/contracts/default_logger.go | 84 +++++++++++++++++++ foreign/go/contracts/logger.go | 35 ++++++++ foreign/go/internal/util/leader_aware.go | 23 +++-- 6 files changed, 168 insertions(+), 15 deletions(-) create mode 100644 foreign/go/contracts/default_logger.go create mode 100644 foreign/go/contracts/logger.go diff --git a/foreign/go/client/iggy_client.go b/foreign/go/client/iggy_client.go index dbc8294b51..89b9ed782a 100644 --- a/foreign/go/client/iggy_client.go +++ b/foreign/go/client/iggy_client.go @@ -20,7 +20,6 @@ package client import ( "context" "fmt" - "log" "sync" "sync/atomic" "time" @@ -32,6 +31,7 @@ import ( type Options struct { protocol iggcon.Protocol tcpOptions []tcp.Option + logger iggcon.Logger heartbeatInterval time.Duration } @@ -54,8 +54,17 @@ func WithTcp(tcpOpts ...tcp.Option) Option { } } +// WithLogger sets the logger for the Iggy client and its underlying transport. +// When no logger is provided, all internal log output is silently discarded. +func WithLogger(logger iggcon.Logger) Option { + return func(opts *Options) { + opts.logger = logger + } +} + type IggyClient struct { iggcon.Client + logger iggcon.Logger cancel context.CancelFunc wg sync.WaitGroup heartbeatInterval time.Duration @@ -72,15 +81,24 @@ func NewIggyClient(options ...Option) (iggcon.Client, error) { opt(&opts) } + logger := opts.logger + if logger == nil { + logger = iggcon.NewNoopLogger() + } + + // Prepend the logger option so transport inherits it. + tcpOpts := append([]tcp.Option{tcp.WithLogger(logger)}, opts.tcpOptions...) + var cli iggcon.Client switch opts.protocol { case iggcon.Tcp: - cli = tcp.NewIggyTcpClient(opts.tcpOptions...) + cli = tcp.NewIggyTcpClient(tcpOpts...) default: return nil, fmt.Errorf("unknown protocol type: %v", opts.protocol) } ic := &IggyClient{ Client: cli, + logger: logger, cancel: func() {}, heartbeatInterval: opts.heartbeatInterval, } @@ -110,7 +128,7 @@ func (ic *IggyClient) Connect(ctx context.Context) error { case <-ticker.C: pingCtx, pingCancel := context.WithTimeout(lifetimeCtx, ic.heartbeatInterval/2) if err := ic.Ping(pingCtx); err != nil { - log.Printf("[WARN] heartbeat failed: %v", err) + ic.logger.Warn("heartbeat failed: %v", err) } pingCancel() } diff --git a/foreign/go/client/tcp/tcp_core.go b/foreign/go/client/tcp/tcp_core.go index 0d9d922fe2..5709914358 100644 --- a/foreign/go/client/tcp/tcp_core.go +++ b/foreign/go/client/tcp/tcp_core.go @@ -51,6 +51,7 @@ type IggyTcpClient struct { conn net.Conn mtx sync.Mutex config config + logger iggcon.Logger MessageCompression iggcon.IggyMessageCompression leaderRedirectionState iggcon.LeaderRedirectionState clientAddress string @@ -71,6 +72,8 @@ type config struct { reconnection tcpClientReconnectionConfig // noDelay disable Nagle's algorithm for the TCP connection noDelay bool + // logger is the logger used for internal diagnostics + logger iggcon.Logger } func defaultTcpClientConfig() config { @@ -156,6 +159,13 @@ func WithServerAddress(address string) Option { } } +// WithLogger sets the logger for the TCP client. +func WithLogger(logger iggcon.Logger) Option { + return func(opts *Options) { + opts.config.logger = logger + } +} + // TLSOption is a functional option for configuring TLS settings. type TLSOption func(cfg *tlsConfig) @@ -203,8 +213,14 @@ func NewIggyTcpClient(options ...Option) *IggyTcpClient { } } + logger := opts.config.logger + if logger == nil { + logger = iggcon.NewNoopLogger() + } + return &IggyTcpClient{ config: opts.config, + logger: logger, clientAddress: "", conn: nil, state: iggcon.StateDisconnected, diff --git a/foreign/go/client/tcp/tcp_session_management.go b/foreign/go/client/tcp/tcp_session_management.go index 36bada6882..80047ef2a3 100644 --- a/foreign/go/client/tcp/tcp_session_management.go +++ b/foreign/go/client/tcp/tcp_session_management.go @@ -89,6 +89,7 @@ func (c *IggyTcpClient) HandleLeaderRedirection(ctx context.Context) (bool, erro c, currentAddress, iggcon.Tcp, + c.logger, ) if err != nil { return false, err diff --git a/foreign/go/contracts/default_logger.go b/foreign/go/contracts/default_logger.go new file mode 100644 index 0000000000..fa1b4921cd --- /dev/null +++ b/foreign/go/contracts/default_logger.go @@ -0,0 +1,84 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 iggcon + +import ( + "fmt" + "io" + "log/slog" + "os" +) + +type DefaultLogger struct { + logger *slog.Logger +} + +func NewDefaultLogger(level LogLevel, writer io.Writer) *DefaultLogger { + handler := slog.NewTextHandler(writer, &slog.HandlerOptions{ + Level: toSlogLevel(level), + }) + return &DefaultLogger{ + logger: slog.New(handler), + } +} + +func NewStderrLogger(level LogLevel) *DefaultLogger { + return NewDefaultLogger(level, os.Stderr) +} + +func (l *DefaultLogger) Debug(msg string, args ...any) { + l.logger.Debug(fmt.Sprintf(msg, args...)) +} + +func (l *DefaultLogger) Info(msg string, args ...any) { + l.logger.Info(fmt.Sprintf(msg, args...)) +} + +func (l *DefaultLogger) Warn(msg string, args ...any) { + l.logger.Warn(fmt.Sprintf(msg, args...)) +} + +func (l *DefaultLogger) Error(msg string, args ...any) { + l.logger.Error(fmt.Sprintf(msg, args...)) +} + +type NoopLogger struct{} + +func (*NoopLogger) Debug(string, ...any) {} +func (*NoopLogger) Info(string, ...any) {} +func (*NoopLogger) Warn(string, ...any) {} +func (*NoopLogger) Error(string, ...any) {} + +func NewNoopLogger() Logger { + return &NoopLogger{} +} + +func toSlogLevel(level LogLevel) slog.Level { + switch level { + case LogLevelDebug: + return slog.LevelDebug + case LogLevelInfo: + return slog.LevelInfo + case LogLevelWarn: + return slog.LevelWarn + case LogLevelError: + return slog.LevelError + default: + return slog.LevelError + 1 + } +} diff --git a/foreign/go/contracts/logger.go b/foreign/go/contracts/logger.go new file mode 100644 index 0000000000..dce0d32932 --- /dev/null +++ b/foreign/go/contracts/logger.go @@ -0,0 +1,35 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 iggcon + +type LogLevel int + +const ( + LogLevelDebug LogLevel = iota + LogLevelInfo + LogLevelWarn + LogLevelError + LogLevelSilent +) + +type Logger interface { + Debug(msg string, args ...any) + Info(msg string, args ...any) + Warn(msg string, args ...any) + Error(msg string, args ...any) +} diff --git a/foreign/go/internal/util/leader_aware.go b/foreign/go/internal/util/leader_aware.go index c9e7cae6c7..3f9ace63a8 100644 --- a/foreign/go/internal/util/leader_aware.go +++ b/foreign/go/internal/util/leader_aware.go @@ -21,7 +21,6 @@ import ( "context" "errors" "fmt" - "log" "net" "strconv" "strings" @@ -31,25 +30,25 @@ import ( // CheckAndRedirectToLeader queries the client for cluster metadata and returns // an address to redirect to (empty string means no redirection needed). -func CheckAndRedirectToLeader(ctx context.Context, c iggcon.Client, currentAddress string, transport iggcon.Protocol) (string, error) { - log.Println("Checking cluster metadata for leader detection") +func CheckAndRedirectToLeader(ctx context.Context, c iggcon.Client, currentAddress string, transport iggcon.Protocol, logger iggcon.Logger) (string, error) { + logger.Debug("Checking cluster metadata for leader detection") meta, err := c.GetClusterMetadata(ctx) if err != nil { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { return "", err } - log.Printf("Failed to get cluster metadata: %v, connection will continue on server node %s\n", err, currentAddress) + logger.Warn("Failed to get cluster metadata: %v, connection will continue on server node %s", err, currentAddress) return "", nil } - log.Printf("Got cluster metadata: %d nodes, cluster: %s\n", len(meta.Nodes), meta.Name) - return processClusterMetadata(meta, currentAddress, transport) + logger.Debug("Got cluster metadata: %d nodes, cluster: %s", len(meta.Nodes), meta.Name) + return processClusterMetadata(meta, currentAddress, transport, logger) } -func processClusterMetadata(metadata *iggcon.ClusterMetadata, currentAddress string, transport iggcon.Protocol) (string, error) { +func processClusterMetadata(metadata *iggcon.ClusterMetadata, currentAddress string, transport iggcon.Protocol, logger iggcon.Logger) (string, error) { if len(metadata.Nodes) == 1 { - log.Printf("Single-node cluster detected (%s), no leader redirection needed\n", metadata.Nodes[0].Name) + logger.Debug("Single-node cluster detected (%s), no leader redirection needed", metadata.Nodes[0].Name) return "", nil } @@ -63,7 +62,7 @@ func processClusterMetadata(metadata *iggcon.ClusterMetadata, currentAddress str } if leader == nil { - log.Printf("No active leader found in cluster metadata, connection will continue on server node %s\n", currentAddress) + logger.Warn("No active leader found in cluster metadata, connection will continue on server node %s", currentAddress) return "", nil } @@ -82,14 +81,14 @@ func processClusterMetadata(metadata *iggcon.ClusterMetadata, currentAddress str } leaderAddress := net.JoinHostPort(leader.IP, strconv.Itoa(int(leaderPort))) - log.Printf("Found leader node: %s at %s (using %s transport)\n", leader.Name, leaderAddress, transport) + logger.Info("Found leader node: %s at %s (using %s transport)", leader.Name, leaderAddress, transport) if !isSameAddress(currentAddress, leaderAddress) { - log.Printf("Current connection to %s is not the leader, will redirect to %s\n", currentAddress, leaderAddress) + logger.Info("Current connection to %s is not the leader, will redirect to %s", currentAddress, leaderAddress) return leaderAddress, nil } - log.Printf("Already connected to leader at %s\n", currentAddress) + logger.Debug("Already connected to leader at %s", currentAddress) return "", nil } From 6e2bd67c4bc7aaf01f5ec8aa161bf408d15757bb Mon Sep 17 00:00:00 2001 From: slash Date: Sun, 31 May 2026 08:00:02 +0530 Subject: [PATCH 2/9] fix(go-sdk) : remove duplicate logger field from config struct --- foreign/go/client/tcp/tcp_core.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/foreign/go/client/tcp/tcp_core.go b/foreign/go/client/tcp/tcp_core.go index 5709914358..a98eb07148 100644 --- a/foreign/go/client/tcp/tcp_core.go +++ b/foreign/go/client/tcp/tcp_core.go @@ -39,6 +39,7 @@ type Option func(config *Options) type Options struct { config config + logger iggcon.Logger } func GetDefaultOptions() Options { @@ -72,8 +73,6 @@ type config struct { reconnection tcpClientReconnectionConfig // noDelay disable Nagle's algorithm for the TCP connection noDelay bool - // logger is the logger used for internal diagnostics - logger iggcon.Logger } func defaultTcpClientConfig() config { @@ -162,7 +161,7 @@ func WithServerAddress(address string) Option { // WithLogger sets the logger for the TCP client. func WithLogger(logger iggcon.Logger) Option { return func(opts *Options) { - opts.config.logger = logger + opts.logger = logger } } @@ -213,7 +212,7 @@ func NewIggyTcpClient(options ...Option) *IggyTcpClient { } } - logger := opts.config.logger + logger := opts.logger if logger == nil { logger = iggcon.NewNoopLogger() } From b649bd2d2b9ccd108a3af6f140c567164e85b8df Mon Sep 17 00:00:00 2001 From: slash Date: Sun, 31 May 2026 09:04:26 +0530 Subject: [PATCH 3/9] refactor(go-sdk) : use slog directly --- foreign/go/client/iggy_client.go | 11 ++-- foreign/go/client/tcp/tcp_core.go | 9 +-- foreign/go/contracts/default_logger.go | 84 ------------------------ foreign/go/contracts/logger.go | 30 +++++---- foreign/go/internal/util/leader_aware.go | 42 +++++++++--- 5 files changed, 61 insertions(+), 115 deletions(-) delete mode 100644 foreign/go/contracts/default_logger.go diff --git a/foreign/go/client/iggy_client.go b/foreign/go/client/iggy_client.go index 89b9ed782a..b8c74414ca 100644 --- a/foreign/go/client/iggy_client.go +++ b/foreign/go/client/iggy_client.go @@ -20,6 +20,7 @@ package client import ( "context" "fmt" + "log/slog" "sync" "sync/atomic" "time" @@ -31,7 +32,7 @@ import ( type Options struct { protocol iggcon.Protocol tcpOptions []tcp.Option - logger iggcon.Logger + logger *slog.Logger heartbeatInterval time.Duration } @@ -56,7 +57,7 @@ func WithTcp(tcpOpts ...tcp.Option) Option { // WithLogger sets the logger for the Iggy client and its underlying transport. // When no logger is provided, all internal log output is silently discarded. -func WithLogger(logger iggcon.Logger) Option { +func WithLogger(logger *slog.Logger) Option { return func(opts *Options) { opts.logger = logger } @@ -64,7 +65,7 @@ func WithLogger(logger iggcon.Logger) Option { type IggyClient struct { iggcon.Client - logger iggcon.Logger + logger *slog.Logger cancel context.CancelFunc wg sync.WaitGroup heartbeatInterval time.Duration @@ -83,7 +84,7 @@ func NewIggyClient(options ...Option) (iggcon.Client, error) { logger := opts.logger if logger == nil { - logger = iggcon.NewNoopLogger() + logger = iggcon.NopLogger() } // Prepend the logger option so transport inherits it. @@ -128,7 +129,7 @@ func (ic *IggyClient) Connect(ctx context.Context) error { case <-ticker.C: pingCtx, pingCancel := context.WithTimeout(lifetimeCtx, ic.heartbeatInterval/2) if err := ic.Ping(pingCtx); err != nil { - ic.logger.Warn("heartbeat failed: %v", err) + ic.logger.Warn("heartbeat failed", "err", err) } pingCancel() } diff --git a/foreign/go/client/tcp/tcp_core.go b/foreign/go/client/tcp/tcp_core.go index a98eb07148..f2aa1f3fd0 100644 --- a/foreign/go/client/tcp/tcp_core.go +++ b/foreign/go/client/tcp/tcp_core.go @@ -23,6 +23,7 @@ import ( "crypto/x509" "encoding/binary" "fmt" + "log/slog" "net" "os" "strings" @@ -39,7 +40,7 @@ type Option func(config *Options) type Options struct { config config - logger iggcon.Logger + logger *slog.Logger } func GetDefaultOptions() Options { @@ -52,7 +53,7 @@ type IggyTcpClient struct { conn net.Conn mtx sync.Mutex config config - logger iggcon.Logger + logger *slog.Logger MessageCompression iggcon.IggyMessageCompression leaderRedirectionState iggcon.LeaderRedirectionState clientAddress string @@ -159,7 +160,7 @@ func WithServerAddress(address string) Option { } // WithLogger sets the logger for the TCP client. -func WithLogger(logger iggcon.Logger) Option { +func WithLogger(logger *slog.Logger) Option { return func(opts *Options) { opts.logger = logger } @@ -214,7 +215,7 @@ func NewIggyTcpClient(options ...Option) *IggyTcpClient { logger := opts.logger if logger == nil { - logger = iggcon.NewNoopLogger() + logger = iggcon.NopLogger() } return &IggyTcpClient{ diff --git a/foreign/go/contracts/default_logger.go b/foreign/go/contracts/default_logger.go deleted file mode 100644 index fa1b4921cd..0000000000 --- a/foreign/go/contracts/default_logger.go +++ /dev/null @@ -1,84 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you 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 iggcon - -import ( - "fmt" - "io" - "log/slog" - "os" -) - -type DefaultLogger struct { - logger *slog.Logger -} - -func NewDefaultLogger(level LogLevel, writer io.Writer) *DefaultLogger { - handler := slog.NewTextHandler(writer, &slog.HandlerOptions{ - Level: toSlogLevel(level), - }) - return &DefaultLogger{ - logger: slog.New(handler), - } -} - -func NewStderrLogger(level LogLevel) *DefaultLogger { - return NewDefaultLogger(level, os.Stderr) -} - -func (l *DefaultLogger) Debug(msg string, args ...any) { - l.logger.Debug(fmt.Sprintf(msg, args...)) -} - -func (l *DefaultLogger) Info(msg string, args ...any) { - l.logger.Info(fmt.Sprintf(msg, args...)) -} - -func (l *DefaultLogger) Warn(msg string, args ...any) { - l.logger.Warn(fmt.Sprintf(msg, args...)) -} - -func (l *DefaultLogger) Error(msg string, args ...any) { - l.logger.Error(fmt.Sprintf(msg, args...)) -} - -type NoopLogger struct{} - -func (*NoopLogger) Debug(string, ...any) {} -func (*NoopLogger) Info(string, ...any) {} -func (*NoopLogger) Warn(string, ...any) {} -func (*NoopLogger) Error(string, ...any) {} - -func NewNoopLogger() Logger { - return &NoopLogger{} -} - -func toSlogLevel(level LogLevel) slog.Level { - switch level { - case LogLevelDebug: - return slog.LevelDebug - case LogLevelInfo: - return slog.LevelInfo - case LogLevelWarn: - return slog.LevelWarn - case LogLevelError: - return slog.LevelError - default: - return slog.LevelError + 1 - } -} diff --git a/foreign/go/contracts/logger.go b/foreign/go/contracts/logger.go index dce0d32932..2e271ee6a9 100644 --- a/foreign/go/contracts/logger.go +++ b/foreign/go/contracts/logger.go @@ -17,19 +17,23 @@ package iggcon -type LogLevel int - -const ( - LogLevelDebug LogLevel = iota - LogLevelInfo - LogLevelWarn - LogLevelError - LogLevelSilent +import ( + "io" + "log/slog" + "os" ) -type Logger interface { - Debug(msg string, args ...any) - Info(msg string, args ...any) - Warn(msg string, args ...any) - Error(msg string, args ...any) +func NewLogger(level slog.Level, writer io.Writer) *slog.Logger { + handler := slog.NewTextHandler(writer, &slog.HandlerOptions{ + Level: level, + }) + return slog.New(handler) +} + +func NewStderrLogger(level slog.Level) *slog.Logger { + return NewLogger(level, os.Stderr) +} + +func NopLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(io.Discard, nil)) } diff --git a/foreign/go/internal/util/leader_aware.go b/foreign/go/internal/util/leader_aware.go index 3f9ace63a8..f99fec7250 100644 --- a/foreign/go/internal/util/leader_aware.go +++ b/foreign/go/internal/util/leader_aware.go @@ -21,6 +21,7 @@ import ( "context" "errors" "fmt" + "log/slog" "net" "strconv" "strings" @@ -30,7 +31,7 @@ import ( // CheckAndRedirectToLeader queries the client for cluster metadata and returns // an address to redirect to (empty string means no redirection needed). -func CheckAndRedirectToLeader(ctx context.Context, c iggcon.Client, currentAddress string, transport iggcon.Protocol, logger iggcon.Logger) (string, error) { +func CheckAndRedirectToLeader(ctx context.Context, c iggcon.Client, currentAddress string, transport iggcon.Protocol, logger *slog.Logger) (string, error) { logger.Debug("Checking cluster metadata for leader detection") meta, err := c.GetClusterMetadata(ctx) @@ -38,17 +39,28 @@ func CheckAndRedirectToLeader(ctx context.Context, c iggcon.Client, currentAddre if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { return "", err } - logger.Warn("Failed to get cluster metadata: %v, connection will continue on server node %s", err, currentAddress) + logger.Warn( + "Failed to get cluster metadata, connection will continue on server node", + "error", err, + "current_address", currentAddress, + ) return "", nil } - logger.Debug("Got cluster metadata: %d nodes, cluster: %s", len(meta.Nodes), meta.Name) + logger.Debug( + "Got cluster metadata", + "nodes", len(meta.Nodes), + "cluster", meta.Name, + ) return processClusterMetadata(meta, currentAddress, transport, logger) } -func processClusterMetadata(metadata *iggcon.ClusterMetadata, currentAddress string, transport iggcon.Protocol, logger iggcon.Logger) (string, error) { +func processClusterMetadata(metadata *iggcon.ClusterMetadata, currentAddress string, transport iggcon.Protocol, logger *slog.Logger) (string, error) { if len(metadata.Nodes) == 1 { - logger.Debug("Single-node cluster detected (%s), no leader redirection needed", metadata.Nodes[0].Name) + logger.Debug( + "Single-node cluster detected, no leader redirection needed", + "node", metadata.Nodes[0].Name, + ) return "", nil } @@ -62,7 +74,10 @@ func processClusterMetadata(metadata *iggcon.ClusterMetadata, currentAddress str } if leader == nil { - logger.Warn("No active leader found in cluster metadata, connection will continue on server node %s", currentAddress) + logger.Warn( + "No active leader found in cluster metadata, connection will continue on server node", + "current_address", currentAddress, + ) return "", nil } @@ -81,14 +96,23 @@ func processClusterMetadata(metadata *iggcon.ClusterMetadata, currentAddress str } leaderAddress := net.JoinHostPort(leader.IP, strconv.Itoa(int(leaderPort))) - logger.Info("Found leader node: %s at %s (using %s transport)", leader.Name, leaderAddress, transport) + logger.Info( + "Found leader node", + "leader", leader.Name, + "address", leaderAddress, + "transport", transport, + ) if !isSameAddress(currentAddress, leaderAddress) { - logger.Info("Current connection to %s is not the leader, will redirect to %s", currentAddress, leaderAddress) + logger.Info( + "Current connection is not the leader, redirecting", + "current_address", currentAddress, + "leader_address", leaderAddress, + ) return leaderAddress, nil } - logger.Debug("Already connected to leader at %s", currentAddress) + logger.Debug("Already connected to leader at", "current_address", currentAddress) return "", nil } From b03065790d7099b413b893341c98bc49256eb636 Mon Sep 17 00:00:00 2001 From: slash Date: Sun, 31 May 2026 09:25:57 +0530 Subject: [PATCH 4/9] test(go-sdk): add logger unit tests --- foreign/go/contracts/logger_test.go | 87 +++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 foreign/go/contracts/logger_test.go diff --git a/foreign/go/contracts/logger_test.go b/foreign/go/contracts/logger_test.go new file mode 100644 index 0000000000..59ecd75f68 --- /dev/null +++ b/foreign/go/contracts/logger_test.go @@ -0,0 +1,87 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 iggcon + +import ( + "bytes" + "context" + "log/slog" + "strings" + "testing" +) + +func TestNewLogger_WritesToProvidedWriter(t *testing.T) { + var buf bytes.Buffer + logger := NewLogger(slog.LevelInfo, &buf) + + logger.Info("hello world") + + if !strings.Contains(buf.String(), "hello world") { + t.Errorf("expected output to contain 'hello world', got: %s", buf.String()) + } +} + +func TestNewLogger_RespectsLogLevel(t *testing.T) { + tests := []struct { + name string + loggerLevel slog.Level + logAt slog.Level + expectOutput bool + }{ + {"suppresses below level", slog.LevelInfo, slog.LevelDebug, false}, + {"passes at level", slog.LevelInfo, slog.LevelInfo, true}, + {"passes above level", slog.LevelInfo, slog.LevelWarn, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + logger := NewLogger(tt.loggerLevel, &buf) + + logger.Log(context.Background(), tt.logAt, "test message") + + if (buf.Len() > 0) != tt.expectOutput { + t.Errorf("expectOutput=%v, got output: %q", tt.expectOutput, buf.String()) + } + }) + } +} + +func TestNewStderrLogger_RespectsLevel(t *testing.T) { + logger := NewStderrLogger(slog.LevelWarn) + + // Verify level filtering without capturing stderr + if logger.Enabled(context.Background(), slog.LevelInfo) { + t.Error("info should be disabled on a warn-level logger") + } + if !logger.Enabled(context.Background(), slog.LevelWarn) { + t.Error("warn should be enabled on a warn-level logger") + } +} + +func TestNopLogger_DoesNotPanic(t *testing.T) { + logger := NopLogger() + + if logger == nil { + t.Fatal("expected non-nil logger") + } + + logger.Debug("msg") + logger.Info("msg") + logger.Warn("msg") + logger.Error("msg") +} From eb85b1f0497dc9290d6c6ea6fbf098fbf14b1280 Mon Sep 17 00:00:00 2001 From: slash Date: Mon, 1 Jun 2026 18:52:39 +0530 Subject: [PATCH 5/9] fix(go) : harden logger helpers, guard nil path, and document transport/client split --- foreign/go/client/iggy_client.go | 10 ++++++++-- foreign/go/client/tcp/tcp_core.go | 6 +++++- foreign/go/contracts/logger.go | 11 ++++------- foreign/go/contracts/logger_test.go | 16 ++-------------- foreign/go/internal/util/leader_aware.go | 3 +++ 5 files changed, 22 insertions(+), 24 deletions(-) diff --git a/foreign/go/client/iggy_client.go b/foreign/go/client/iggy_client.go index b8c74414ca..206e836437 100644 --- a/foreign/go/client/iggy_client.go +++ b/foreign/go/client/iggy_client.go @@ -48,6 +48,8 @@ func GetDefaultOptions() Options { type Option func(*Options) // WithTcp sets the client protocol to TCP and applies custom TCP options. +// tcp.WithLogger here only affects the transport; use WithLogger for +// the client-level heartbeat. func WithTcp(tcpOpts ...tcp.Option) Option { return func(opts *Options) { opts.protocol = iggcon.Tcp @@ -56,6 +58,8 @@ func WithTcp(tcpOpts ...tcp.Option) Option { } // WithLogger sets the logger for the Iggy client and its underlying transport. +// This logger is used by the heartbeat and forwarded to the transport as a +// default. A tcp.WithLogger passed via WithTcp overrides only the transport. // When no logger is provided, all internal log output is silently discarded. func WithLogger(logger *slog.Logger) Option { return func(opts *Options) { @@ -87,7 +91,9 @@ func NewIggyClient(options ...Option) (iggcon.Client, error) { logger = iggcon.NopLogger() } - // Prepend the logger option so transport inherits it. + // Prepend the logger so the transport inherits it as a default. + // A tcp.WithLogger in opts.tcpOptions will override the transport's + // copy; the heartbeat keeps using logger above. tcpOpts := append([]tcp.Option{tcp.WithLogger(logger)}, opts.tcpOptions...) var cli iggcon.Client @@ -129,7 +135,7 @@ func (ic *IggyClient) Connect(ctx context.Context) error { case <-ticker.C: pingCtx, pingCancel := context.WithTimeout(lifetimeCtx, ic.heartbeatInterval/2) if err := ic.Ping(pingCtx); err != nil { - ic.logger.Warn("heartbeat failed", "err", err) + ic.logger.Warn("heartbeat failed", "error", err) } pingCancel() } diff --git a/foreign/go/client/tcp/tcp_core.go b/foreign/go/client/tcp/tcp_core.go index f2aa1f3fd0..a7595fb418 100644 --- a/foreign/go/client/tcp/tcp_core.go +++ b/foreign/go/client/tcp/tcp_core.go @@ -159,7 +159,11 @@ func WithServerAddress(address string) Option { } } -// WithLogger sets the logger for the TCP client. +// WithLogger sets the logger for the TCP transport layer. +// +// When the transport is created via client.NewIggyClient, prefer +// client.WithLogger to configure a single logger for client-level +// events (such as heartbeats) and transport logging. func WithLogger(logger *slog.Logger) Option { return func(opts *Options) { opts.logger = logger diff --git a/foreign/go/contracts/logger.go b/foreign/go/contracts/logger.go index 2e271ee6a9..bd974ea65c 100644 --- a/foreign/go/contracts/logger.go +++ b/foreign/go/contracts/logger.go @@ -20,20 +20,17 @@ package iggcon import ( "io" "log/slog" - "os" ) -func NewLogger(level slog.Level, writer io.Writer) *slog.Logger { +// NewLogger creates a text-format logger that writes to writer at the given minimum level. +func NewLogger(writer io.Writer, level slog.Level) *slog.Logger { handler := slog.NewTextHandler(writer, &slog.HandlerOptions{ Level: level, }) return slog.New(handler) } -func NewStderrLogger(level slog.Level) *slog.Logger { - return NewLogger(level, os.Stderr) -} - +// NopLogger returns a logger that silently discards all output. func NopLogger() *slog.Logger { - return slog.New(slog.NewTextHandler(io.Discard, nil)) + return slog.New(slog.DiscardHandler) } diff --git a/foreign/go/contracts/logger_test.go b/foreign/go/contracts/logger_test.go index 59ecd75f68..b85908357a 100644 --- a/foreign/go/contracts/logger_test.go +++ b/foreign/go/contracts/logger_test.go @@ -26,7 +26,7 @@ import ( func TestNewLogger_WritesToProvidedWriter(t *testing.T) { var buf bytes.Buffer - logger := NewLogger(slog.LevelInfo, &buf) + logger := NewLogger(&buf, slog.LevelInfo) logger.Info("hello world") @@ -50,7 +50,7 @@ func TestNewLogger_RespectsLogLevel(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var buf bytes.Buffer - logger := NewLogger(tt.loggerLevel, &buf) + logger := NewLogger(&buf, tt.loggerLevel) logger.Log(context.Background(), tt.logAt, "test message") @@ -61,18 +61,6 @@ func TestNewLogger_RespectsLogLevel(t *testing.T) { } } -func TestNewStderrLogger_RespectsLevel(t *testing.T) { - logger := NewStderrLogger(slog.LevelWarn) - - // Verify level filtering without capturing stderr - if logger.Enabled(context.Background(), slog.LevelInfo) { - t.Error("info should be disabled on a warn-level logger") - } - if !logger.Enabled(context.Background(), slog.LevelWarn) { - t.Error("warn should be enabled on a warn-level logger") - } -} - func TestNopLogger_DoesNotPanic(t *testing.T) { logger := NopLogger() diff --git a/foreign/go/internal/util/leader_aware.go b/foreign/go/internal/util/leader_aware.go index f99fec7250..af766e6491 100644 --- a/foreign/go/internal/util/leader_aware.go +++ b/foreign/go/internal/util/leader_aware.go @@ -32,6 +32,9 @@ import ( // CheckAndRedirectToLeader queries the client for cluster metadata and returns // an address to redirect to (empty string means no redirection needed). func CheckAndRedirectToLeader(ctx context.Context, c iggcon.Client, currentAddress string, transport iggcon.Protocol, logger *slog.Logger) (string, error) { + if logger == nil { + logger = iggcon.NopLogger() + } logger.Debug("Checking cluster metadata for leader detection") meta, err := c.GetClusterMetadata(ctx) From fe2473e78dd8e3a41b0e3e693508e6dc27a8c7d0 Mon Sep 17 00:00:00 2001 From: slash Date: Thu, 4 Jun 2026 15:09:34 +0530 Subject: [PATCH 6/9] refactor(go): unify logger configuration and remove helpers --- foreign/go/client/iggy_client.go | 21 ++----- foreign/go/client/iggy_client_test.go | 59 +++++++++++++++++++ foreign/go/client/tcp/tcp_core.go | 19 +----- foreign/go/client/tcp/tcp_core_test.go | 27 ++++++++- foreign/go/contracts/logger.go | 36 ------------ foreign/go/contracts/logger_test.go | 75 ------------------------ foreign/go/internal/util/leader_aware.go | 3 - 7 files changed, 90 insertions(+), 150 deletions(-) create mode 100644 foreign/go/client/iggy_client_test.go delete mode 100644 foreign/go/contracts/logger.go delete mode 100644 foreign/go/contracts/logger_test.go diff --git a/foreign/go/client/iggy_client.go b/foreign/go/client/iggy_client.go index 206e836437..605d09813c 100644 --- a/foreign/go/client/iggy_client.go +++ b/foreign/go/client/iggy_client.go @@ -41,6 +41,7 @@ func GetDefaultOptions() Options { return Options{ protocol: iggcon.Tcp, tcpOptions: nil, + logger: slog.New(slog.DiscardHandler), heartbeatInterval: 5 * time.Second, } } @@ -48,8 +49,6 @@ func GetDefaultOptions() Options { type Option func(*Options) // WithTcp sets the client protocol to TCP and applies custom TCP options. -// tcp.WithLogger here only affects the transport; use WithLogger for -// the client-level heartbeat. func WithTcp(tcpOpts ...tcp.Option) Option { return func(opts *Options) { opts.protocol = iggcon.Tcp @@ -59,8 +58,8 @@ func WithTcp(tcpOpts ...tcp.Option) Option { // WithLogger sets the logger for the Iggy client and its underlying transport. // This logger is used by the heartbeat and forwarded to the transport as a -// default. A tcp.WithLogger passed via WithTcp overrides only the transport. -// When no logger is provided, all internal log output is silently discarded. +// default. When no logger is provided, all internal log output is silently +// discarded. func WithLogger(logger *slog.Logger) Option { return func(opts *Options) { opts.logger = logger @@ -86,26 +85,16 @@ func NewIggyClient(options ...Option) (iggcon.Client, error) { opt(&opts) } - logger := opts.logger - if logger == nil { - logger = iggcon.NopLogger() - } - - // Prepend the logger so the transport inherits it as a default. - // A tcp.WithLogger in opts.tcpOptions will override the transport's - // copy; the heartbeat keeps using logger above. - tcpOpts := append([]tcp.Option{tcp.WithLogger(logger)}, opts.tcpOptions...) - var cli iggcon.Client switch opts.protocol { case iggcon.Tcp: - cli = tcp.NewIggyTcpClient(tcpOpts...) + cli = tcp.NewIggyTcpClient(opts.logger, opts.tcpOptions...) default: return nil, fmt.Errorf("unknown protocol type: %v", opts.protocol) } ic := &IggyClient{ Client: cli, - logger: logger, + logger: opts.logger, cancel: func() {}, heartbeatInterval: opts.heartbeatInterval, } diff --git a/foreign/go/client/iggy_client_test.go b/foreign/go/client/iggy_client_test.go new file mode 100644 index 0000000000..12fd88ead1 --- /dev/null +++ b/foreign/go/client/iggy_client_test.go @@ -0,0 +1,59 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 client + +import ( + "bytes" + "log/slog" + "strings" + "testing" +) + +func TestGetDefaultOptions_ProvidesLogger(t *testing.T) { + opts := GetDefaultOptions() + if opts.logger == nil { + t.Fatal("expected default options to include a non-nil logger") + } +} + +func TestWithLogger_SetsClientLogger(t *testing.T) { + var buf bytes.Buffer + logger := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{ + Level: slog.LevelDebug, + })) + + cli, err := NewIggyClient(WithLogger(logger)) + if err != nil { + t.Fatalf("unexpected error creating client: %v", err) + } + + ic, ok := cli.(*IggyClient) + if !ok { + t.Fatal("expected *IggyClient from NewIggyClient") + } + + ic.logger.Info("probe message", "key", "value") + + output := buf.String() + if !strings.Contains(output, "probe message") { + t.Errorf("expected logger output to contain 'probe message', got: %q", output) + } + if !strings.Contains(output, "key=value") { + t.Errorf("expected logger output to contain 'key=value', got: %q", output) + } +} diff --git a/foreign/go/client/tcp/tcp_core.go b/foreign/go/client/tcp/tcp_core.go index a7595fb418..67ab4020fd 100644 --- a/foreign/go/client/tcp/tcp_core.go +++ b/foreign/go/client/tcp/tcp_core.go @@ -40,7 +40,6 @@ type Option func(config *Options) type Options struct { config config - logger *slog.Logger } func GetDefaultOptions() Options { @@ -159,17 +158,6 @@ func WithServerAddress(address string) Option { } } -// WithLogger sets the logger for the TCP transport layer. -// -// When the transport is created via client.NewIggyClient, prefer -// client.WithLogger to configure a single logger for client-level -// events (such as heartbeats) and transport logging. -func WithLogger(logger *slog.Logger) Option { - return func(opts *Options) { - opts.logger = logger - } -} - // TLSOption is a functional option for configuring TLS settings. type TLSOption func(cfg *tlsConfig) @@ -209,7 +197,7 @@ func WithTLSValidateCertificate(validate bool) TLSOption { // NewIggyTcpClient creates a new Iggy TCP client with the given options. // warning: don't use this function directly, use iggycli.NewIggyClient with iggycli.WithTcp instead. -func NewIggyTcpClient(options ...Option) *IggyTcpClient { +func NewIggyTcpClient(logger *slog.Logger, options ...Option) *IggyTcpClient { opts := GetDefaultOptions() for _, opt := range options { if opt != nil { @@ -217,11 +205,6 @@ func NewIggyTcpClient(options ...Option) *IggyTcpClient { } } - logger := opts.logger - if logger == nil { - logger = iggcon.NopLogger() - } - return &IggyTcpClient{ config: opts.config, logger: logger, diff --git a/foreign/go/client/tcp/tcp_core_test.go b/foreign/go/client/tcp/tcp_core_test.go index 71256cb4b9..19ac7d992d 100644 --- a/foreign/go/client/tcp/tcp_core_test.go +++ b/foreign/go/client/tcp/tcp_core_test.go @@ -18,10 +18,13 @@ package tcp import ( + "bytes" "context" "encoding/binary" "errors" + "log/slog" "net" + "strings" "testing" "time" @@ -36,8 +39,9 @@ func newTestClient(t *testing.T) (*IggyTcpClient, net.Conn) { t.Helper() serverConn, clientConn := net.Pipe() c := &IggyTcpClient{ - conn: clientConn, - state: iggcon.StateConnected, + conn: clientConn, + state: iggcon.StateConnected, + logger: slog.New(slog.DiscardHandler), } t.Cleanup(func() { err := clientConn.Close() @@ -236,3 +240,22 @@ func TestSendAndFetchResponse_SuccessWithBody(t *testing.T) { t.Errorf("expected state %v, got %v", iggcon.StateConnected, c.state) } } + +func TestNewIggyTcpClient_StoresProvidedLogger(t *testing.T) { + var buf bytes.Buffer + logger := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{ + Level: slog.LevelDebug, + })) + + c := NewIggyTcpClient(logger) + + c.logger.Info("transport probe", "source", "tcp") + + output := buf.String() + if !strings.Contains(output, "transport probe") { + t.Errorf("expected logger output to contain 'transport probe', got: %q", output) + } + if !strings.Contains(output, "source=tcp") { + t.Errorf("expected logger output to contain 'source=tcp', got: %q", output) + } +} diff --git a/foreign/go/contracts/logger.go b/foreign/go/contracts/logger.go deleted file mode 100644 index bd974ea65c..0000000000 --- a/foreign/go/contracts/logger.go +++ /dev/null @@ -1,36 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you 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 iggcon - -import ( - "io" - "log/slog" -) - -// NewLogger creates a text-format logger that writes to writer at the given minimum level. -func NewLogger(writer io.Writer, level slog.Level) *slog.Logger { - handler := slog.NewTextHandler(writer, &slog.HandlerOptions{ - Level: level, - }) - return slog.New(handler) -} - -// NopLogger returns a logger that silently discards all output. -func NopLogger() *slog.Logger { - return slog.New(slog.DiscardHandler) -} diff --git a/foreign/go/contracts/logger_test.go b/foreign/go/contracts/logger_test.go deleted file mode 100644 index b85908357a..0000000000 --- a/foreign/go/contracts/logger_test.go +++ /dev/null @@ -1,75 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you 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 iggcon - -import ( - "bytes" - "context" - "log/slog" - "strings" - "testing" -) - -func TestNewLogger_WritesToProvidedWriter(t *testing.T) { - var buf bytes.Buffer - logger := NewLogger(&buf, slog.LevelInfo) - - logger.Info("hello world") - - if !strings.Contains(buf.String(), "hello world") { - t.Errorf("expected output to contain 'hello world', got: %s", buf.String()) - } -} - -func TestNewLogger_RespectsLogLevel(t *testing.T) { - tests := []struct { - name string - loggerLevel slog.Level - logAt slog.Level - expectOutput bool - }{ - {"suppresses below level", slog.LevelInfo, slog.LevelDebug, false}, - {"passes at level", slog.LevelInfo, slog.LevelInfo, true}, - {"passes above level", slog.LevelInfo, slog.LevelWarn, true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var buf bytes.Buffer - logger := NewLogger(&buf, tt.loggerLevel) - - logger.Log(context.Background(), tt.logAt, "test message") - - if (buf.Len() > 0) != tt.expectOutput { - t.Errorf("expectOutput=%v, got output: %q", tt.expectOutput, buf.String()) - } - }) - } -} - -func TestNopLogger_DoesNotPanic(t *testing.T) { - logger := NopLogger() - - if logger == nil { - t.Fatal("expected non-nil logger") - } - - logger.Debug("msg") - logger.Info("msg") - logger.Warn("msg") - logger.Error("msg") -} diff --git a/foreign/go/internal/util/leader_aware.go b/foreign/go/internal/util/leader_aware.go index af766e6491..f99fec7250 100644 --- a/foreign/go/internal/util/leader_aware.go +++ b/foreign/go/internal/util/leader_aware.go @@ -32,9 +32,6 @@ import ( // CheckAndRedirectToLeader queries the client for cluster metadata and returns // an address to redirect to (empty string means no redirection needed). func CheckAndRedirectToLeader(ctx context.Context, c iggcon.Client, currentAddress string, transport iggcon.Protocol, logger *slog.Logger) (string, error) { - if logger == nil { - logger = iggcon.NopLogger() - } logger.Debug("Checking cluster metadata for leader detection") meta, err := c.GetClusterMetadata(ctx) From 0ed22448a39aeb55cacc5b5c7e80ab30b6789c12 Mon Sep 17 00:00:00 2001 From: slash Date: Fri, 5 Jun 2026 10:57:59 +0530 Subject: [PATCH 7/9] fix(go): guard WithLogger against nil and clean up tests --- foreign/go/client/iggy_client.go | 3 +++ foreign/go/client/iggy_client_test.go | 17 +++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/foreign/go/client/iggy_client.go b/foreign/go/client/iggy_client.go index 605d09813c..ebaad67b94 100644 --- a/foreign/go/client/iggy_client.go +++ b/foreign/go/client/iggy_client.go @@ -62,6 +62,9 @@ func WithTcp(tcpOpts ...tcp.Option) Option { // discarded. func WithLogger(logger *slog.Logger) Option { return func(opts *Options) { + if logger == nil { + return + } opts.logger = logger } } diff --git a/foreign/go/client/iggy_client_test.go b/foreign/go/client/iggy_client_test.go index 12fd88ead1..9ca61a92f3 100644 --- a/foreign/go/client/iggy_client_test.go +++ b/foreign/go/client/iggy_client_test.go @@ -24,10 +24,14 @@ import ( "testing" ) -func TestGetDefaultOptions_ProvidesLogger(t *testing.T) { - opts := GetDefaultOptions() - if opts.logger == nil { - t.Fatal("expected default options to include a non-nil logger") +func TestWithLogger_Nil(t *testing.T) { + cli, err := NewIggyClient(WithLogger(nil)) + if err != nil { + t.Fatalf("unexpected error creating client: %v", err) + } + ic := cli.(*IggyClient) + if ic.logger.Handler() != slog.DiscardHandler { + t.Errorf("expected slog.DiscardHandler, get %v", ic.logger.Handler()) } } @@ -42,10 +46,7 @@ func TestWithLogger_SetsClientLogger(t *testing.T) { t.Fatalf("unexpected error creating client: %v", err) } - ic, ok := cli.(*IggyClient) - if !ok { - t.Fatal("expected *IggyClient from NewIggyClient") - } + ic := cli.(*IggyClient) ic.logger.Info("probe message", "key", "value") From 93497bd25c84a4347368498790d9359c61d6e1cb Mon Sep 17 00:00:00 2001 From: Gaurav Date: Fri, 5 Jun 2026 23:26:18 +0530 Subject: [PATCH 8/9] test(go-sdk): fix typo in logger test Co-authored-by: Chengxi Luo --- foreign/go/client/iggy_client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/foreign/go/client/iggy_client_test.go b/foreign/go/client/iggy_client_test.go index 9ca61a92f3..cd640b8823 100644 --- a/foreign/go/client/iggy_client_test.go +++ b/foreign/go/client/iggy_client_test.go @@ -31,7 +31,7 @@ func TestWithLogger_Nil(t *testing.T) { } ic := cli.(*IggyClient) if ic.logger.Handler() != slog.DiscardHandler { - t.Errorf("expected slog.DiscardHandler, get %v", ic.logger.Handler()) + t.Errorf("expected slog.DiscardHandler, got %v", ic.logger.Handler()) } } From 768ad8956e66cfe307c5c226ae3c62b7478d0883 Mon Sep 17 00:00:00 2001 From: slash Date: Tue, 9 Jun 2026 16:11:32 +0530 Subject: [PATCH 9/9] fix(go): address logger review feedback --- foreign/go/client/iggy_client.go | 5 ++--- foreign/go/client/tcp/tcp_core.go | 3 +++ foreign/go/internal/util/leader_aware.go | 6 +++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/foreign/go/client/iggy_client.go b/foreign/go/client/iggy_client.go index ebaad67b94..452eb58bdb 100644 --- a/foreign/go/client/iggy_client.go +++ b/foreign/go/client/iggy_client.go @@ -57,9 +57,8 @@ func WithTcp(tcpOpts ...tcp.Option) Option { } // WithLogger sets the logger for the Iggy client and its underlying transport. -// This logger is used by the heartbeat and forwarded to the transport as a -// default. When no logger is provided, all internal log output is silently -// discarded. +// This logger is used by the heartbeat and forwarded to the transport. +// When no logger is provided, all internal log output is silently discarded. func WithLogger(logger *slog.Logger) Option { return func(opts *Options) { if logger == nil { diff --git a/foreign/go/client/tcp/tcp_core.go b/foreign/go/client/tcp/tcp_core.go index 67ab4020fd..dcfc540a8e 100644 --- a/foreign/go/client/tcp/tcp_core.go +++ b/foreign/go/client/tcp/tcp_core.go @@ -198,6 +198,9 @@ func WithTLSValidateCertificate(validate bool) TLSOption { // NewIggyTcpClient creates a new Iggy TCP client with the given options. // warning: don't use this function directly, use iggycli.NewIggyClient with iggycli.WithTcp instead. func NewIggyTcpClient(logger *slog.Logger, options ...Option) *IggyTcpClient { + if logger == nil { + logger = slog.New(slog.DiscardHandler) + } opts := GetDefaultOptions() for _, opt := range options { if opt != nil { diff --git a/foreign/go/internal/util/leader_aware.go b/foreign/go/internal/util/leader_aware.go index f99fec7250..a7e58c3175 100644 --- a/foreign/go/internal/util/leader_aware.go +++ b/foreign/go/internal/util/leader_aware.go @@ -96,10 +96,10 @@ func processClusterMetadata(metadata *iggcon.ClusterMetadata, currentAddress str } leaderAddress := net.JoinHostPort(leader.IP, strconv.Itoa(int(leaderPort))) - logger.Info( + logger.Debug( "Found leader node", "leader", leader.Name, - "address", leaderAddress, + "leader_address", leaderAddress, "transport", transport, ) @@ -112,7 +112,7 @@ func processClusterMetadata(metadata *iggcon.ClusterMetadata, currentAddress str return leaderAddress, nil } - logger.Debug("Already connected to leader at", "current_address", currentAddress) + logger.Debug("Already connected to leader", "current_address", currentAddress) return "", nil }